Bug 226890 - Thunderbird doesn't handle news URIs properly, part 4: Move the parsing code to nsNntpUrl. r=bienvenu, sr=Standard8
authorJoshua Cranmer <Pidgeot18@gmail.com>
Mon, 15 Nov 2010 18:43:43 -0500
changeset 7696 15f14ac7e2dd243d58ea3b78f314e2cb9af21fb3
parent 7695 3a9190fb95e1d5f7298cedf05a36fe54df03b147
child 7697 36381d5e01efc8cc51d868796f8908468aefba47
push idunknown
push userunknown
push dateunknown
reviewersbienvenu, Standard8
bugs226890
Bug 226890 - Thunderbird doesn't handle news URIs properly, part 4: Move the parsing code to nsNntpUrl. r=bienvenu, sr=Standard8
mailnews/news/public/nsINntpUrl.idl
mailnews/news/src/nsNNTPProtocol.cpp
mailnews/news/src/nsNNTPProtocol.h
mailnews/news/src/nsNntpUrl.cpp
mailnews/news/src/nsNntpUrl.h
--- a/mailnews/news/public/nsINntpUrl.idl
+++ b/mailnews/news/public/nsINntpUrl.idl
@@ -62,17 +62,17 @@ typedef long nsNewsAction;
  *   corresponding [s]news or nntp URL, and set the original one in
  *   nsIMsgMessageUrl::uri and ::originalSpec.
  * - A URI that results in an ActionUnknown will not be run.
  * - Cancel URIs require the original spec to also be set, so it can find both
  *   the message ID and the group/key combination.
  * * Some actions require either a group or a message id. Since actions can be
  *   set after the fact, these conditions are not verified.
  */
-[scriptable, uuid(b8bb6567-7433-4a52-beae-20e8122deb78)]
+[scriptable, uuid(d2ac7147-63c4-4f3f-b949-552381bc5241)]
 interface nsINntpUrl : nsISupports {
   /// For ActionPostArticle URIs, the message to be posted.
   attribute nsINNTPNewsgroupPost messageToPost;
 
   /**
    * The action that this URL will take when run.
    *
    * Most actions can be automatically determined from the URL spec as follows:
@@ -83,16 +83,22 @@ interface nsINntpUrl : nsISupports {
    *
    * 3. A non-empty group is found (ActionGetNewNews or ActionListGroups).
    */
   attribute nsNewsAction newsAction;
 
   /// For ActionGetNewNews URIs, whether or not to get older messages.
   attribute boolean getOldMessages;
 
+  /// The group portion of the URI, if one is present
+  readonly attribute ACString group;
+
+  /// The message ID portion of the URI, if one is present
+  readonly attribute ACString messageID;
+
   /// The action of this news URI could not be determined
   const nsNewsAction ActionUnknown = 0;
   /// Fetch the contents of an article
   const nsNewsAction ActionFetchArticle = 1;
   /// Fetch the part of an article (requires ?part=)
   const nsNewsAction ActionFetchPart = 2;
   /// Save the contents of an article to disk
   const nsNewsAction ActionSaveMessageToDisk = 3;
--- a/mailnews/news/src/nsNNTPProtocol.cpp
+++ b/mailnews/news/src/nsNNTPProtocol.cpp
@@ -314,17 +314,16 @@ nsNNTPProtocol::nsNNTPProtocol(nsIURI * 
   m_responseText = nsnull;
   m_dataBuf = nsnull;
 
   m_cancelFromHdr = nsnull;
   m_cancelNewsgroups = nsnull;
   m_cancelDistribution = nsnull;
   m_cancelID = nsnull;
 
-  m_messageID = nsnull;
   m_key = nsMsgKey_None;
 
   m_searchData = nsnull;
 
   mBytesReceived = 0;
   mBytesReceivedSinceLastStatusUpdate = 0;
   m_startTime = PR_Now();
 
@@ -361,17 +360,16 @@ nsNNTPProtocol::~nsNNTPProtocol()
 void nsNNTPProtocol::Cleanup()  //free char* member variables
 {
   PR_FREEIF(m_responseText);
   PR_FREEIF(m_dataBuf);
   PR_FREEIF(m_cancelFromHdr);
   PR_FREEIF(m_cancelNewsgroups);
   PR_FREEIF(m_cancelDistribution);
   PR_FREEIF(m_cancelID);
-  PR_FREEIF(m_messageID);
 }
 
 NS_IMETHODIMP nsNNTPProtocol::Initialize(nsIURI * aURL, nsIMsgWindow *aMsgWindow)
 {
   nsresult rv = NS_OK;
 
   if (aMsgWindow) {
     m_msgWindow = aMsgWindow;
@@ -427,25 +425,23 @@ NS_IMETHODIMP nsNNTPProtocol::Initialize
 
   NS_PRECONDITION(m_url , "invalid URL passed into NNTP Protocol");
 
   m_runningURL = do_QueryInterface(m_url, &rv);
   if (NS_FAILED(rv)) return rv;
   SetIsBusy(PR_TRUE);
 
   nsCString group;
-  nsCString commandSpecificData;
-  PR_FREEIF(m_messageID);
 
   // Initialize m_newsAction before possible use in ParseURL method
   m_runningURL->GetNewsAction(&m_newsAction);
   
   // parse url to get the msg folder and check if the message is in the folder's
   // local cache before opening a new socket and trying to download the message
-  rv = ParseURL(m_url, group, &m_messageID, commandSpecificData);
+  rv = ParseURL(m_url, group, m_messageID);
 
   if (NS_SUCCEEDED(rv) && m_runningURL)
   {
     nsCOMPtr<nsIMsgMailNewsUrl> mailnewsUrl = do_QueryInterface(m_runningURL);
     if (mailnewsUrl)
     {
       if (aMsgWindow)
         mailnewsUrl->SetMsgWindow(aMsgWindow);
@@ -531,18 +527,16 @@ NS_IMETHODIMP nsNNTPProtocol::Initialize
 
   m_firstArticle = 0;
   m_lastArticle = 0;
   m_firstPossibleArticle = 0;
   m_lastPossibleArticle = 0;
   m_numArticlesLoaded = 0;
   m_numArticlesWanted = 0;
 
-  PR_FREEIF(m_messageID);
-
   m_key = nsMsgKey_None;
 
   m_articleNumber = 0;
   m_originalContentLength = 0;
   m_cancelID = nsnull;
   m_cancelFromHdr = nsnull;
   m_cancelNewsgroups = nsnull;
   m_cancelDistribution = nsnull;
@@ -767,20 +761,18 @@ nsresult nsNNTPProtocol::ReadFromMemCach
 
   if (NS_SUCCEEDED(rv))
   {
     nsCOMPtr<nsIInputStreamPump> pump;
     rv = NS_NewInputStreamPump(getter_AddRefs(pump), cacheStream);
     if (NS_FAILED(rv)) return rv;
 
     nsCString group;
-    nsCString commandSpecificData;
     // do this to get m_key set, so that marking the message read will work.
-    PR_FREEIF(m_messageID);
-    rv = ParseURL(m_url, group, &m_messageID, commandSpecificData);
+    rv = ParseURL(m_url, group, m_messageID);
 
     nsNntpCacheStreamListener * cacheListener = new nsNntpCacheStreamListener();
     if (!cacheListener)
        return NS_ERROR_OUT_OF_MEMORY;
 
     NS_ADDREF(cacheListener);
 
     SetLoadGroup(m_loadGroup);
@@ -1004,190 +996,159 @@ NS_IMETHODIMP nsNNTPProtocol::AsyncOpen(
   return nsMsgProtocol::AsyncOpen(listener, ctxt);
 }
 
 nsresult nsNNTPProtocol::LoadUrl(nsIURI * aURL, nsISupports * aConsumer)
 {
   NS_ENSURE_ARG_POINTER(aURL);
 
   nsCString group;
-  nsCString commandSpecificData;
   m_ContentType.Truncate();
   nsresult rv = NS_OK;
 
   m_runningURL = do_QueryInterface(aURL, &rv);
   if (NS_FAILED(rv)) return rv;
   m_runningURL->GetNewsAction(&m_newsAction);
 
   SetIsBusy(PR_TRUE);
 
-  PR_FREEIF(m_messageID);
-  m_messageID = nsnull;
-
-  rv = ParseURL(aURL, group, &m_messageID, commandSpecificData);
+  rv = ParseURL(aURL, group, m_messageID);
   NS_ASSERTION(NS_SUCCEEDED(rv),"failed to parse news url");
   //if (NS_FAILED(rv)) return rv;
   // XXX group returned from ParseURL is assumed to be in UTF-8
   NS_ASSERTION(MsgIsUTF8(group), "newsgroup name is not in UTF-8");
-
-  PR_LOG(NNTP,PR_LOG_ALWAYS,("(%p) m_messageID = %s", this, m_messageID ? m_messageID :"(null)"));
+  NS_ASSERTION(m_nntpServer, "Parsing must result in an m_nntpServer");
+
+  PR_LOG(NNTP,PR_LOG_ALWAYS,("(%p) m_messageID = %s", this, m_messageID.get()));
   PR_LOG(NNTP,PR_LOG_ALWAYS,("(%p) group = %s", this, group.get()));
-  PR_LOG(NNTP,PR_LOG_ALWAYS,("(%p) commandSpecificData = %s", this, !commandSpecificData.IsEmpty() ? commandSpecificData.get():"(null)"));
   PR_LOG(NNTP,PR_LOG_ALWAYS,("(%p) m_key = %d",this,m_key));
 
-  /* We are posting a user-written message
-     if and only if this message has a message to post
-     Cancel messages are created later with a ?cancel URL
-  */
-  nsCOMPtr <nsINNTPNewsgroupPost> message;
-  rv = m_runningURL->GetMessageToPost(getter_AddRefs(message));
-  if (NS_SUCCEEDED(rv) && message)
+  if (m_newsAction == nsINntpUrl::ActionFetchArticle ||
+      m_newsAction == nsINntpUrl::ActionFetchPart ||
+      m_newsAction == nsINntpUrl::ActionSaveMessageToDisk)
+    m_typeWanted = ARTICLE_WANTED;
+  else if (m_newsAction == nsINntpUrl::ActionCancelArticle)
+    m_typeWanted = CANCEL_WANTED;
+  else if (m_newsAction == nsINntpUrl::ActionPostArticle)
   {
     m_typeWanted = NEWS_POST;
-    NS_MsgSACopy(&m_messageID, "");
+    m_messageID = "";
+  }
+  else if (m_newsAction == nsINntpUrl::ActionListIds)
+  {
+    m_typeWanted = IDS_WANTED;
+    rv = m_nntpServer->FindGroup(group, getter_AddRefs(m_newsFolder));
+  }
+  else if (m_newsAction == nsINntpUrl::ActionSearch)
+  {
+    m_typeWanted = SEARCH_WANTED;
+
+    // Get the search data
+    nsCString commandSpecificData, unescapedCommandSpecificData;
+    nsCOMPtr<nsIURL> url = do_QueryInterface(m_runningURL);
+    rv = url->GetQuery(commandSpecificData);
+    NS_ENSURE_SUCCESS(rv, rv);
+    MsgUnescapeString(commandSpecificData, 0, unescapedCommandSpecificData);
+    m_searchData = ToNewCString(unescapedCommandSpecificData);
+
+    rv = m_nntpServer->FindGroup(group, getter_AddRefs(m_newsFolder));
+    if (!m_newsFolder)
+      goto FAIL;
+  }
+  else if (m_newsAction == nsINntpUrl::ActionGetNewNews)
+  {
+    PRBool containsGroup = PR_TRUE;
+    rv = m_nntpServer->ContainsNewsgroup(group, &containsGroup);
+    if (NS_FAILED(rv))
+      goto FAIL;
+
+    if (!containsGroup)
+    {
+      // We have the name of a newsgroup which we're not subscribed to,
+      // the next step is to ask the user whether we should subscribe to it.
+      nsCOMPtr<nsIPrompt> dialog;
+
+      if (m_msgWindow)
+        m_msgWindow->GetPromptDialog(getter_AddRefs(dialog));
+
+      if (!dialog)
+      {
+         nsCOMPtr<nsIWindowWatcher> wwatch(do_GetService(NS_WINDOWWATCHER_CONTRACTID));
+         wwatch->GetNewPrompter(nsnull, getter_AddRefs(dialog));
+      }
+
+      nsString statusString, confirmText;
+      nsCOMPtr<nsIStringBundle> bundle;
+      nsCOMPtr<nsIStringBundleService> bundleService = do_GetService(NS_STRINGBUNDLE_CONTRACTID);
+
+      // to handle non-ASCII newsgroup names, we store them internally
+      // as escaped. decode and unescape the newsgroup name so we'll
+      // display a proper name.
+
+      nsAutoString unescapedName;
+      rv = NS_MsgDecodeUnescapeURLPath(group, unescapedName);
+      NS_ENSURE_SUCCESS(rv,rv);
+
+      bundleService->CreateBundle(NEWS_MSGS_URL, getter_AddRefs(bundle));
+      const PRUnichar *formatStrings[1] = { unescapedName.get() };
+
+      rv = bundle->FormatStringFromName(
+        NS_LITERAL_STRING("autoSubscribeText").get(), formatStrings, 1,
+        getter_Copies(confirmText));
+      NS_ENSURE_SUCCESS(rv,rv);
+
+      PRBool confirmResult = PR_FALSE;
+      rv = dialog->Confirm(nsnull, confirmText.get(), &confirmResult);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      if (confirmResult)
+      {
+         rv = m_nntpServer->SubscribeToNewsgroup(group);
+         containsGroup = PR_TRUE;
+      }
+      else
+      {
+        // XXX FIX ME
+        // the way news is current written, we've already opened the socket
+        // and initialized the connection.
+        //
+        // until that is fixed, when the user cancels an autosubscribe, we've got to close it and clean up after ourselves
+        //
+        // see bug http://bugzilla.mozilla.org/show_bug.cgi?id=108293
+        // another problem, autosubscribe urls are ending up as cache entries
+        // because the default action on nntp urls is ActionFetchArticle
+        //
+        // see bug http://bugzilla.mozilla.org/show_bug.cgi?id=108294
+        if (m_runningURL)
+          FinishMemCacheEntry(PR_FALSE); // cleanup mem cache entry
+
+        return CloseConnection();
+      }
+    }
+
+    // If we have a group (since before, or just subscribed), set the m_newsFolder.
+    if (containsGroup)
+    {
+      rv = m_nntpServer->FindGroup(group, getter_AddRefs(m_newsFolder));
+      if (!m_newsFolder)
+        goto FAIL;
+    }
+    m_typeWanted = GROUP_WANTED;
   }
   else if (m_newsAction == nsINntpUrl::ActionListGroups)
     m_typeWanted = LIST_WANTED;
-  else if (m_newsAction == nsINntpUrl::ActionCancelArticle)
-    m_typeWanted = CANCEL_WANTED;
+  else if (m_newsAction == nsINntpUrl::ActionListNewGroups)
+    m_typeWanted = NEW_GROUPS;
+  else if (!m_messageID.IsEmpty() || m_key != nsMsgKey_None)
+    m_typeWanted = ARTICLE_WANTED;
   else
-    if (m_messageID || (m_key != nsMsgKey_None))
-    {
-    /*
-    news-message://HOST/GROUP#key
-    news://HOST/MESSAGE_ID
-
-      not sure about these:
-
-        news:MESSAGE_ID
-        news:/GROUP/MESSAGE_ID (useless)
-        news://HOST/GROUP/MESSAGE_ID (useless)
-      */
-      m_typeWanted = ARTICLE_WANTED;
-    }
-    else if (!commandSpecificData.IsEmpty())
-    {
-      if (PL_strstr (commandSpecificData.get(), "?newgroups"))
-      /* news://HOST/?newsgroups
-      news:/GROUP/?newsgroups (useless)
-      news:?newsgroups (default host)
-       */
-             m_typeWanted = NEW_GROUPS;
-      else
-      {
-        if (PL_strstr(commandSpecificData.get(), "?list-ids"))
-        {
-          m_typeWanted = IDS_WANTED;
-
-          rv = m_nntpServer->FindGroup(group, getter_AddRefs(m_newsFolder));
-          if (!m_newsFolder) goto FAIL;
-        }
-        else
-        {
-          m_typeWanted = SEARCH_WANTED;
-          nsCString unescapedCommandSpecificData;
-          MsgUnescapeString(commandSpecificData, 0, unescapedCommandSpecificData);
-          m_searchData = ToNewCString(unescapedCommandSpecificData);
-
-          rv = m_nntpServer->FindGroup(group, getter_AddRefs(m_newsFolder));
-          if (!m_newsFolder) goto FAIL;
-        }
-      }
-    }
-    else if (!group.IsEmpty())
-    {
-    /* news:GROUP
-    news:/GROUP
-    news://HOST/GROUP
-      */
-        if (m_nntpServer)
-        {
-          PRBool containsGroup = PR_TRUE;
-          rv = m_nntpServer->ContainsNewsgroup(group, &containsGroup);
-          if (NS_FAILED(rv)) goto FAIL;
-
-          if (!containsGroup)
-          {
-            // We have the name of a newsgroup which we're not subscribed to,
-            // the next step is to ask the user whether we should subscribe to it.
-
-            nsCOMPtr<nsIPrompt> dialog;
-
-            if (m_msgWindow)
-              m_msgWindow->GetPromptDialog(getter_AddRefs(dialog));
-
-            if (!dialog)
-            {
-              nsCOMPtr<nsIWindowWatcher> wwatch(do_GetService(NS_WINDOWWATCHER_CONTRACTID));
-              wwatch->GetNewPrompter(nsnull, getter_AddRefs(dialog));
-            }
-
-            nsString statusString, confirmText;
-            nsCOMPtr<nsIStringBundle> bundle;
-            nsCOMPtr<nsIStringBundleService> bundleService = do_GetService(NS_STRINGBUNDLE_CONTRACTID);
-
-            // to handle non-ASCII newsgroup names, we store them internally
-            // as escaped. decode and unescape the newsgroup name so we'll
-            // display a proper name.
-
-            nsAutoString unescapedName;
-            rv = NS_MsgDecodeUnescapeURLPath(group, unescapedName);
-            NS_ENSURE_SUCCESS(rv,rv);
-
-            bundleService->CreateBundle(NEWS_MSGS_URL, getter_AddRefs(bundle));
-            const PRUnichar *formatStrings[1] = { unescapedName.get() };
-
-            rv = bundle->FormatStringFromName(NS_LITERAL_STRING("autoSubscribeText").get(),
-              formatStrings, 1,
-              getter_Copies(confirmText));
-            NS_ENSURE_SUCCESS(rv,rv);
-
-            PRBool confirmResult = PR_FALSE;
-            rv = dialog->Confirm(nsnull, confirmText.get(), &confirmResult);
-            NS_ENSURE_SUCCESS(rv, rv);
-
-            if (confirmResult)
-            {
-              rv = m_nntpServer->SubscribeToNewsgroup(group);
-              containsGroup = PR_TRUE;
-            }
-            else {
-              // XXX FIX ME
-              // the way news is current written, we've already opened the socket
-              // and initialized the connection.
-              //
-              // until that is fixed, when the user cancels an autosubscribe, we've got to close it and clean up after ourselves
-              //
-              // see bug http://bugzilla.mozilla.org/show_bug.cgi?id=108293
-              // another problem, autosubscribe urls are ending up as cache entries
-              // because the default action on nntp urls is ActionFetchArticle
-              //
-              // see bug http://bugzilla.mozilla.org/show_bug.cgi?id=108294
-              if (m_runningURL)
-                FinishMemCacheEntry(PR_FALSE); // cleanup mem cache entry
-
-              return CloseConnection();
-            }
-          }
-
-          // If we have a group (since before, or just subscribed), set the m_newsFolder.
-          if (containsGroup)
-          {
-            rv = m_nntpServer->FindGroup(group, getter_AddRefs(m_newsFolder));
-            if (!m_newsFolder) goto FAIL;
-          }
-        }
-        m_typeWanted = GROUP_WANTED;
-    }
-    else
-    {
-      // news: or news://HOST
-      NS_ASSERTION(0, "Should not get here!");
-      rv = NS_ERROR_FAILURE;
-    }
+  {
+    NS_NOTREACHED("Unknown news action");
+    rv = NS_ERROR_FAILURE;
+  }
 
     // if this connection comes from the cache, we need to initialize the
     // load group here, by generating the start request notification. nsMsgProtocol::OnStartRequest
     // ignores the first parameter (which is supposed to be the channel) so we'll pass in null.
     if (m_fromCache)
       nsMsgProtocol::OnStartRequest(nsnull, aURL);
 
       /* At this point, we're all done parsing the URL, and know exactly
@@ -1224,33 +1185,32 @@ FAIL:
     // Make sure that we have the information we need to be able to run the
     // URLs
     NS_ASSERTION(m_nntpServer, "Parsing must result in an m_nntpServer");
     if (m_typeWanted == ARTICLE_WANTED)
     {
       if (m_key != nsMsgKey_None)
         NS_ASSERTION(m_newsFolder, "ARTICLE_WANTED needs m_newsFolder w/ key");
       else
-        NS_ASSERTION(m_messageID, "ARTICLE_WANTED needs m_messageID w/o key");
+        NS_ASSERTION(!m_messageID.IsEmpty(), "ARTICLE_WANTED needs m_messageID w/o key");
     }
     else if (m_typeWanted == CANCEL_WANTED)
     {
-      NS_ASSERTION(m_messageID, "CANCEL_WANTED needs m_messageID");
+      NS_ASSERTION(!m_messageID.IsEmpty(), "CANCEL_WANTED needs m_messageID");
       NS_ASSERTION(m_newsFolder, "CANCEL_WANTED needs m_newsFolder");
       NS_ASSERTION(m_key != nsMsgKey_None, "CANCEL_WANTED needs m_key");
     }
     else if (m_typeWanted == GROUP_WANTED)
       NS_ASSERTION(m_newsFolder, "GROUP_WANTED needs m_newsFolder");
     else if (m_typeWanted == SEARCH_WANTED)
       NS_ASSERTION(m_searchData, "SEARCH_WANTED needs m_searchData");
     else if (m_typeWanted == IDS_WANTED)
       NS_ASSERTION(m_newsFolder, "IDS_WANTED needs m_newsFolder");
 
     return rv;
-
 }
 
 void nsNNTPProtocol::FinishMemCacheEntry(PRBool valid)
 {
   nsCOMPtr <nsICacheEntryDescriptor> memCacheEntry;
   nsCOMPtr<nsIMsgMailNewsUrl> mailnewsurl = do_QueryInterface(m_runningURL);
   if (mailnewsurl)
     mailnewsurl->GetMemCacheEntry(getter_AddRefs(memCacheEntry));
@@ -1285,68 +1245,20 @@ NS_IMETHODIMP nsNNTPProtocol::OnStopRequ
 }
 
 NS_IMETHODIMP nsNNTPProtocol::Cancel(nsresult status)  // handle stop button
 {
     m_nextState = NNTP_ERROR;
     return nsMsgProtocol::Cancel(NS_BINDING_ABORTED);
 }
 
-/*
-   FIX THIS COMMENT, this is how 4.x worked.  6.x is different.
-
-   The main news load init routine, and URL parser.
-   The syntax of news URLs is:
-
-   To list all hosts:
-     news:
-
-   To list all groups on a host, or to post a new message:
-     news://HOST
-
-   To list some articles in a group:
-     news:GROUP
-     news:/GROUP
-     news://HOST/GROUP
-
-   To list a particular range of articles in a group:
-     news:GROUP/N1-N2
-     news:/GROUP/N1-N2
-     news://HOST/GROUP/N1-N2
-
-   To retrieve an article:
-     news:MESSAGE-ID                (message-id must contain "@")
-
-    To cancel an article:
-     news:MESSAGE-ID?cancel
-
-   (Note that message IDs may contain / before the @:)
-
-     news:SOME/ID@HOST?headers=all
-     news:/SOME/ID@HOST?headers=all
-     news:/SOME?ID@HOST?headers=all
-        are the same as
-     news://HOST/SOME/ID@HOST?headers=all
-     news://HOST//SOME/ID@HOST?headers=all
-     news://HOST//SOME?ID@HOST?headers=all
-        bug: if the ID is <//xxx@host> we will parse this correctly:
-          news://non-default-host///xxx@host
-        but will mis-parse it if it's on the default host:
-          news://xxx@host
-        whoever had the idea to leave the <> off the IDs should be flogged.
-    So, we'll make sure we quote / in message IDs as %2F.
- */
 nsresult
-nsNNTPProtocol::ParseURL(nsIURI *aURL, nsCString &aGroup, char **aMessageID,
-                         nsACString &aCommandSpecificData)
+nsNNTPProtocol::ParseURL(nsIURI *aURL, nsCString &aGroup, nsCString &aMessageID)
 {
     NS_ENSURE_ARG_POINTER(aURL);
-    NS_ENSURE_ARG_POINTER(aMessageID);
-
-  nsCAutoString message_id;
 
     PR_LOG(NNTP,PR_LOG_ALWAYS,("(%p) ParseURL",this));
 
     nsresult rv;
     nsCOMPtr <nsIMsgFolder> folder;
     nsCOMPtr <nsINntpService> nntpService = do_GetService(NS_NNTPSERVICE_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv,rv);
 
@@ -1375,107 +1287,62 @@ nsNNTPProtocol::ParseURL(nsIURI *aURL, n
         }
     }
     else {
         // clear this, we'll set it later.
         m_newsFolder = nsnull;
         m_currentGroup.Truncate();
     }
 
-  // get the file path part and store it as the group...
-  nsCAutoString fullPath;
-  rv = aURL->GetPath(fullPath);
-  NS_ENSURE_SUCCESS(rv,rv);
-
-  PR_LOG(NNTP,PR_LOG_ALWAYS,("(%p) fullPath = %s",this, fullPath.get()));
-
-  if (fullPath.First() == '/')
-    aGroup = Substring(fullPath, 1);
-  else
-    aGroup = fullPath;
-
-  // more to do here, but for now, this works.
-  // only escape if we are doing a search
-  nsCString unescapedGroup;
-  MsgUnescapeString(aGroup, 0, unescapedGroup);
-  if (m_newsAction == nsINntpUrl::ActionSearch)
-    aGroup = unescapedGroup;
-  else if (aGroup.Find("@") != -1 || aGroup.Find("%40") != -1)
-  {
-    message_id = unescapedGroup;
-    aGroup.Truncate();
-  }
-
-  /* At this point, the search data is attached to `message_id' (if there is
-     one) or `aGroup' (otherwise.)
-     Separate the search data from what it is clinging to, being careful to
-     interpret the "?" only if it comes after the "@" in an ID, since the
-     syntax of message IDs is tricky.  (There may be a ? in the random-
-     characters part of the ID (before @), but not in the host part (after @).)
-   */
-   if (message_id.Length())
-   {
-     PRInt32 atIndex = message_id.Find("@");
-     PRInt32 index = message_id.Find("?", atIndex);
-     if (index >= 0)
-     {
-       aCommandSpecificData = Substring(message_id, index);
-       message_id.SetLength(index);
-     }
-   }
-   else if (aGroup.Length())
-   {
-     PRInt32 index = aGroup.Find("?");
-     if (index >= 0)
-     {
-       aCommandSpecificData = Substring(aGroup, index);
-       aGroup.SetLength(index);
-     }
-   }
-
-  NS_ASSERTION(!message_id.Length() || message_id != aGroup, "something not null");
-  if (message_id.Length() > 0)
-    *aMessageID = NS_strdup(message_id.get());
+  // Load the values from the URL for parsing.
+  rv = m_runningURL->GetGroup(aGroup);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = m_runningURL->GetMessageID(aMessageID);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  NS_ASSERTION(aMessageID.IsEmpty() || aMessageID != aGroup, "something not null");
 
   // If we are cancelling, we've got our message id, m_key, and m_newsFolder.
   // Bail out now to prevent messing those up.
   if (m_newsAction == nsINntpUrl::ActionCancelArticle)
     return NS_OK;
 
   nsCAutoString serverURI;
 
-  if (*aMessageID) {
+  if (!aMessageID.IsEmpty())
+  {
     // if this is a message id, use the pre path (the server) for the folder uri.
     rv = aURL->GetPrePath(serverURI);
     NS_ENSURE_SUCCESS(rv,rv);
   }
-  else if (aGroup.Length()) {
+  else if (!aGroup.IsEmpty())
+  {
     if (aGroup.Find("*") >= 0) {
       rv = aURL->GetPrePath(serverURI);
       NS_ENSURE_SUCCESS(rv,rv);
     }
     else {
       // let if fall through.  we'll set m_newsFolder later.
     }
   }
 
   if (!serverURI.IsEmpty()) {
     // if we get here, we, we are either doing:
     // news://host/message-id or news://host/*
     // (but not news://host/message-id?cancel)
     // for authentication, we set m_newsFolder to be the server's folder.
     // while we are here, we set m_nntpServer.
 
-    if (*aMessageID)
+    if (!aMessageID.IsEmpty())
     { // for news://host/message-id decompose complete url
       nsCAutoString urlSpec;
       m_url->GetAsciiSpec(urlSpec);
       rv = nntpService->DecomposeNewsURI(urlSpec.get(), getter_AddRefs(folder), &m_key);
     }
-    if ((!*aMessageID) || NS_FAILED(rv))
+    if (aMessageID.IsEmpty() || NS_FAILED(rv))
     { // for news://host/* decompose only server uri
       rv = nntpService->DecomposeNewsURI(serverURI.get(), getter_AddRefs(folder), &m_key);
       NS_ENSURE_SUCCESS(rv,rv);
     }
 
 
     if (m_key != nsMsgKey_None)
     {
@@ -2110,30 +1977,30 @@ PRInt32 nsNNTPProtocol::SendFirstNNTPCom
   }
   else if (m_typeWanted == IDS_WANTED)
   {
     m_nextState = NNTP_LIST_GROUP;
     return 0;
   }
   else  /* article or cancel */
   {
-    NS_ASSERTION(m_messageID, "No message ID, bailing!");
-    if (!m_messageID) return -1;
+    NS_ASSERTION(!m_messageID.IsEmpty(), "No message ID, bailing!");
+    if (m_messageID.IsEmpty()) return -1;
 
     if (m_typeWanted == CANCEL_WANTED)
       NS_MsgSACopy(&command, "HEAD ");
     else {
       NS_ASSERTION(m_typeWanted == ARTICLE_WANTED, "not cancel, and not article");
       NS_MsgSACopy(&command, "ARTICLE ");
     }
 
-    if (*m_messageID != '<')
+    if (m_messageID[0] != '<')
       NS_MsgSACat(&command,"<");
 
-    NS_MsgSACat(&command, m_messageID);
+    NS_MsgSACat(&command, m_messageID.get());
 
     if (PL_strchr(command+8, '>')==0)
       NS_MsgSACat(&command,">");
   }
 
     NS_MsgSACat(&command, CRLF);
   nsCOMPtr<nsIMsgMailNewsUrl> mailnewsurl = do_QueryInterface(m_runningURL);
   if (mailnewsurl)
@@ -3606,21 +3473,16 @@ PRInt32 nsNNTPProtocol::ReadHeaders()
 */
 
 PRInt32 nsNNTPProtocol::ReadNewsgroupResponse()
 {
   if (m_responseCode == MK_NNTP_RESPONSE_ARTICLE_HEAD)
   {     /* Head follows - parse it:*/
     m_nextState = NNTP_READ_GROUP_BODY;
 
-    if(m_messageID)
-      *m_messageID = '\0';
-
-    m_key = nsMsgKey_None;
-
     return 0;
   }
   else
   {
     m_newsgroupList->HEADFailed(m_articleNumber);
     m_nextState = NNTP_READ_GROUP;
     return(0);
   }
--- a/mailnews/news/src/nsNNTPProtocol.h
+++ b/mailnews/news/src/nsNNTPProtocol.h
@@ -246,17 +246,17 @@ private:
   char     *m_cancelDistribution;
   char     *m_cancelID;
   PRInt32    m_cancelStatus;
 
   // variable for ReadNewsList
   PRInt32   m_readNewsListCount;
 
   // Per news article state information. (article number, author, subject, id, etc
-  char   *m_messageID;
+  nsCString m_messageID;
   PRInt32   m_articleNumber;   /* current article number */
   char   *m_searchData;
 
   PRInt32   m_originalContentLength; /* the content length at the time of calling graph progress */
 
   nsCOMPtr<nsIStringBundle> m_stringBundle;
 
   nsCOMPtr<nsINntpIncomingServer> m_nntpServer;
@@ -456,17 +456,17 @@ private:
   PRInt32 Search();
   PRInt32 SearchResponse();
   PRInt32 SearchResults(nsIInputStream *inputStream, PRUint32 length);
 
   //////////////////////////////////////////////////////////////////////////////
   // End of Protocol Methods
   //////////////////////////////////////////////////////////////////////////////
 
-  nsresult ParseURL(nsIURI * aURL, nsCString &aGroup, char ** aMessageID, nsACString &aCommandSpecificData);
+  nsresult ParseURL(nsIURI *aURL, nsCString &aGroup, nsCString &aMessageID);
 
   void SetProgressBarPercent(PRUint32 aProgress, PRUint32 aProgressMax);
   nsresult SetProgressStatus(const PRUnichar *aMessage);
   nsresult InitializeNewsFolderFromUri(const char *uri);
   void TimerCallback();
 
   void HandleAuthenticationFailure();
   nsCOMPtr <nsIInputStream> mInputStream;
--- a/mailnews/news/src/nsNntpUrl.cpp
+++ b/mailnews/news/src/nsNntpUrl.cpp
@@ -44,16 +44,17 @@
 #include "nsIURL.h"
 #include "nsNntpUrl.h"
 
 #include "nsStringGlue.h"
 #include "prmem.h"
 #include "plstr.h"
 #include "prprf.h"
 #include "nsNewsUtils.h"
+#include "nsMsgUtils.h"
 #include "nsISupportsObsolete.h"
 
 #include "nntpCore.h"
 
 #include "nsCOMPtr.h"
 #include "nsIMsgDatabase.h"
 #include "nsMsgDBCID.h"
 #include "nsMsgNewsCID.h"
@@ -70,17 +71,16 @@ nsNntpUrl::nsNntpUrl()
   m_addDummyEnvelope = PR_FALSE;
   m_canonicalLineEnding = PR_FALSE;
   m_filePath = nsnull;
   m_getOldMessages = PR_FALSE;
 }
 
 nsNntpUrl::~nsNntpUrl()
 {
-  NS_IF_RELEASE(m_newsgroupPost);
 }
 
 NS_IMPL_ADDREF_INHERITED(nsNntpUrl, nsMsgMailNewsUrl)
 NS_IMPL_RELEASE_INHERITED(nsNntpUrl, nsMsgMailNewsUrl)
 
 NS_INTERFACE_MAP_BEGIN(nsNntpUrl)
    NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsINntpUrl)
    NS_INTERFACE_MAP_ENTRY(nsINntpUrl)
@@ -109,83 +109,115 @@ NS_INTERFACE_MAP_END_INHERITING(nsMsgMai
  * canonicalization.
  */
 
 NS_IMETHODIMP nsNntpUrl::SetSpec(const nsACString &aSpec)
 {
   nsresult rv = nsMsgMailNewsUrl::SetSpec(aSpec);
   NS_ENSURE_SUCCESS(rv,rv);
 
+  nsCAutoString scheme;
+  rv = GetScheme(scheme);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (scheme.EqualsLiteral("news") || scheme.EqualsLiteral("snews"))
+    rv = ParseNewsURL();
+  NS_ENSURE_SUCCESS(rv, rv);
+
   rv = DetermineNewsAction();
   NS_ENSURE_SUCCESS(rv,rv);
   return rv;
 }
 
+nsresult nsNntpUrl::ParseNewsURL()
+{
+  // The path here is the group/msgid portion
+  nsCAutoString path;
+  nsresult rv = GetFilePath(path);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // If the path is empty, we have no authority component
+  if (path.IsEmpty())
+  {
+    rv = GetSpec(path);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  // Drop the potential beginning from the path
+  if (path.Length() && path[0] == '/')
+    path = Substring(path, 1);
+
+  // The presence of an `@' is a sign we have a msgid
+  if (path.Find("@") != -1 || path.Find("%40") != -1)
+    MsgUnescapeString(path, 0, m_messageID);
+  else
+    MsgUnescapeString(path, 0, m_group);
+
+  return NS_OK;
+}
+
 nsresult nsNntpUrl::DetermineNewsAction()
 {
   nsCAutoString path;
   nsresult rv = nsMsgMailNewsUrl::GetPath(path);
   NS_ENSURE_SUCCESS(rv,rv);
 
-  if (!strcmp(path.get(),"/*")) {
-    // news://news.mozilla.org/*
-    // get all newsgroups on the server, for subscribe
-    m_newsAction = nsINntpUrl::ActionListGroups;
+  nsCAutoString query;
+  rv = GetQuery(query);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (query.EqualsLiteral("cancel"))
+  {
+    m_newsAction = nsINntpUrl::ActionCancelArticle;
+    return NS_OK;
+  }
+  if (query.EqualsLiteral("list-ids"))
+  {
+    m_newsAction = nsINntpUrl::ActionListIds;
     return NS_OK;
   }
-
-  if (!strcmp(path.get(),"/")) {
-    // could be news:netscape.public.mozilla.mail-news or news://news.mozilla.org
-    // news:netscape.public.mozilla.mail-news gets turned into news://netscape.public.mozilla.mail-news/ by nsStandardURL
-    // news://news.mozilla.org gets turned in to news://news.mozilla.org/ by nsStandardURL
-    // news://news.mozilla.org is nsINntpUrl::ActionUpdateCounts
-    // (which is "update the unread counts for all groups that we're subscribed to on news.mozilla.org)
-    // news:netscape.public.mozilla.mail-news is nsINntpUrl::AutoSubscribe
-    //
-    // also in here for, news:3B98D201.3020100@cs.com
-    // and when posting, and during message display GetCodeBasePrinciple() and nsMimeNewURI()
-    //
-    // set it as unknown (so we won't try to check the cache for it
-    // we'll figure out the action later, or it will be set on the url by the caller.
-    m_newsAction = nsINntpUrl::ActionUnknown;
+  if (query.EqualsLiteral("newgroups"))
+  {
+    m_newsAction = nsINntpUrl::ActionListNewGroups;
     return NS_OK;
   }
-
-  if (PL_strcasestr(path.get(), "?part=") || PL_strcasestr(path.get(), "&part=")) {
+  if (StringBeginsWith(query, NS_LITERAL_CSTRING("search")))
+  {
+    m_newsAction = nsINntpUrl::ActionSearch;
+    return NS_OK;
+  }
+  if (StringBeginsWith(query, NS_LITERAL_CSTRING("part=")) ||
+      query.Find("&part=") > 0)
+  {
     // news://news.mozilla.org:119/3B98D201.3020100%40cs.com?part=1
     // news://news.mozilla.org:119/b58dme%24aia2%40ripley.netscape.com?header=print&part=1.2&type=image/jpeg&filename=Pole.jpg
     m_newsAction = nsINntpUrl::ActionFetchPart;
     return NS_OK;
   }
 
-  if (PL_strcasestr(path.get(), "?cancel")) {
-    // news://news.mozilla.org/3C06C0E8.5090107@sspitzer.org?cancel
-    m_newsAction = nsINntpUrl::ActionCancelArticle;
-    return NS_OK;
-  }
-
-  if (PL_strcasestr(path.get(), "?list-ids")) {
-    // news://news.mozilla.org/netscape.test?list-ids
-    // remove all cancelled articles from netscape.test
-    m_newsAction = nsINntpUrl::ActionListIds;
-    return NS_OK;
-  }
-
-  if (strchr(path.get(), '@') || strstr(path.get(),"%40")) {
-    // news://news.mozilla.org/3B98D201.3020100@cs.com
-    // news://news.mozilla.org/3B98D201.3020100%40cs.com
+  if (!m_messageID.IsEmpty())
+  {
     m_newsAction = nsINntpUrl::ActionFetchArticle;
     return NS_OK;
   }
 
-  // news://news.mozilla.org/netscape.test could be
-  // get new news for netscape.test (nsINntpUrl::ActionGetNewNews)
-  // or subscribe to netscape.test (nsINntpUrl::AutoSubscribe)
-  // set it as unknown (so we won't try to check the cache for it
-  // we'll figure out the action later, or it will be set on the url by the caller.
+  if (m_group.Find("*") >= 0)
+  {
+    // If the group is a wildmat, list groups instead of grabbing a group.
+    m_newsAction = nsINntpUrl::ActionListGroups;
+    return NS_OK;
+  }
+  if (!m_group.IsEmpty())
+  {
+    m_newsAction = nsINntpUrl::ActionGetNewNews;
+    return NS_OK;
+  }
+
+  // At this point, we have a URI that contains neither a query, a group, nor a
+  // message ID. Ergo, we don't know what it is.
   m_newsAction = nsINntpUrl::ActionUnknown;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsNntpUrl::SetGetOldMessages(PRBool aGetOldMessages)
 {
   m_getOldMessages = aGetOldMessages;
   return NS_OK;
@@ -207,16 +239,28 @@ NS_IMETHODIMP nsNntpUrl::GetNewsAction(n
 
 
 NS_IMETHODIMP nsNntpUrl::SetNewsAction(nsNewsAction aNewsAction)
 {
   m_newsAction = aNewsAction;
   return NS_OK;
 }
 
+NS_IMETHODIMP nsNntpUrl::GetGroup(nsACString &group)
+{
+  group = m_group;
+  return NS_OK;
+}
+
+NS_IMETHODIMP nsNntpUrl::GetMessageID(nsACString &messageID)
+{
+  messageID = m_messageID;
+  return NS_OK;
+}
+
 NS_IMETHODIMP nsNntpUrl::SetUri(const char * aURI)
 {
   mURI = aURI;
   return NS_OK;
 }
 
 // from nsIMsgMessageUrl
 NS_IMETHODIMP nsNntpUrl::GetUri(char ** aURI)
@@ -255,32 +299,27 @@ NS_IMETHODIMP nsNntpUrl::GetMessageFile(
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // End nsINntpUrl specific support
 ////////////////////////////////////////////////////////////////////////////////
 
 nsresult nsNntpUrl::SetMessageToPost(nsINNTPNewsgroupPost *post)
 {
-    NS_LOCK_INSTANCE();
-    NS_IF_RELEASE(m_newsgroupPost);
-    m_newsgroupPost=post;
-    if (m_newsgroupPost) NS_ADDREF(m_newsgroupPost);
-    NS_UNLOCK_INSTANCE();
-    return NS_OK;
+  m_newsgroupPost = post;
+  if (post)
+    SetNewsAction(nsINntpUrl::ActionPostArticle);
+  return NS_OK;
 }
 
 nsresult nsNntpUrl::GetMessageToPost(nsINNTPNewsgroupPost **aPost)
 {
-    NS_LOCK_INSTANCE();
-    if (!aPost) return NS_ERROR_NULL_POINTER;
-    *aPost = m_newsgroupPost;
-    if (*aPost) NS_ADDREF(*aPost);
-    NS_UNLOCK_INSTANCE();
-    return NS_OK;
+  NS_ENSURE_ARG_POINTER(aPost);
+  NS_IF_ADDREF(*aPost = m_newsgroupPost);
+  return NS_OK;
 }
 
 NS_IMETHODIMP nsNntpUrl::GetMessageHeader(nsIMsgDBHdr ** aMsgHdr)
 {
   nsresult rv;
 
   nsCOMPtr <nsINntpService> nntpService = do_GetService(NS_NNTPSERVICE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv,rv);
--- a/mailnews/news/src/nsNntpUrl.h
+++ b/mailnews/news/src/nsNntpUrl.h
@@ -62,27 +62,31 @@ public:
   // nsNntpUrl
   nsNntpUrl();
   virtual ~nsNntpUrl();
 
   NS_DECL_ISUPPORTS_INHERITED
 
 private:
   nsresult DetermineNewsAction();
+  nsresult ParseNewsURL();
 
-  nsINNTPNewsgroupPost *m_newsgroupPost;
+  nsCOMPtr<nsINNTPNewsgroupPost> m_newsgroupPost;
   nsNewsAction m_newsAction; // the action this url represents...parse mailbox, display messages, etc.
 
   nsCString mURI; // the RDF URI associated with this url.
   nsCString mCharsetOverride; // used by nsIMsgI18NUrl...
 
   nsCString mOriginalSpec;
   nsCOMPtr <nsILocalFile>  m_filePath;
 
   // used by save message to disk
   nsCOMPtr<nsIFile> m_messageFile;
 
   PRPackedBool  m_addDummyEnvelope;
   PRPackedBool  m_canonicalLineEnding;
   PRPackedBool  m_getOldMessages;
+
+  nsCString m_group;
+  nsCString m_messageID;
 };
 
 #endif // nsNntpUrl_h__