Bug #73738 --> fix a race condition in news. Fix crash trying to display inline images in news articles created by lipr0n.

Implement Save and Open attachment for news articles again from the message pane.
sr=bienvenu

mailnews/mime/emitters/src/Makefile.in

@@ -28,7 +28,7 @@ include $(DEPTH)/config/autoconf.mk
 
 MODULE		= mimeemitter
 LIBRARY_NAME	= emitterutil_s
-REQUIRES	= xpcom string mime msgbase msgbaseutil pref necko intl locale mailnews msgdb addrbook mimetype
+REQUIRES	= xpcom string mime msgbase msgbaseutil pref necko intl locale mailnews msgdb addrbook mimetype msgnews
 
 EXPORTS		= \
 		nsEmitterUtils.h \

mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp

@@ -36,6 +36,9 @@
 #include "nsMimeTypes.h"
 #include "prtime.h"
 
+// hack: include this to fix opening news attachments.
+#include "nsINntpUrl.h"
+
 #include "nsIMimeConverter.h"
 #include "nsMsgMimeCID.h"
 #include "nsDateTimeFormatCID.h"
@@ -311,7 +314,15 @@ nsMimeHtmlDisplayEmitter::StartAttachment(const char *name, const char *contentT
 
     nsCOMPtr<nsIMsgMessageUrl> msgurl (do_QueryInterface(mURL, &rv));
     if (NS_SUCCEEDED(rv))
+    {
+      // HACK: news urls require us to use the originalSpec. everyone else uses GetURI
+      // to get the RDF resource which describes the message.
+      nsCOMPtr<nsINntpUrl> nntpUrl (do_QueryInterface(mURL, &rv));
+      if (NS_SUCCEEDED(rv) && nntpUrl)
+        rv = msgurl->GetOriginalSpec(getter_Copies(uriString));
+      else
         rv = msgurl->GetUri(getter_Copies(uriString));
+    }
 
     // we need to convert the attachment name from UTF-8 to unicode before
     // we emit it...

mailnews/news/public/nsINntpUrl.idl

@@ -47,11 +47,12 @@ interface nsINntpUrl : nsISupports {
   attribute boolean getOldMessages;
 
   const nsNewsAction ActionGetNewNews = 0;
-  const nsNewsAction ActionDisplayArticle = 1;
+  const nsNewsAction ActionFetchArticle = 1;
   const nsNewsAction ActionSaveMessageToDisk = 2;
   const nsNewsAction ActionCancelArticle = 3;
   const nsNewsAction ActionPostArticle = 4;
   const nsNewsAction ActionSearch = 5;
   const nsNewsAction ActionUpdateCounts = 6;
   const nsNewsAction ActionListGroups = 7;
+  const nsNewsAction ActionFetchPart = 8;
 };

mailnews/news/src/nsNNTPProtocol.cpp

@@ -87,6 +87,7 @@
 
 #include "nsINntpService.h"
 #include "nntpCore.h"
+#include "nsIStreamConverterService.h"
 
 #undef GetPort  // XXX Windows!
 #undef SetPort  // XXX Windows!
@@ -152,6 +153,7 @@ char * NET_SACat (char **destination, const char *source);
 
 }
 
+static NS_DEFINE_CID(kIStreamConverterServiceCID, NS_STREAMCONVERTERSERVICE_CID);
 static NS_DEFINE_CID(kCHeaderParserCID, NS_MSGHEADERPARSER_CID);
 static NS_DEFINE_CID(kNNTPArticleListCID, NS_NNTPARTICLELIST_CID);
 static NS_DEFINE_CID(kCMsgMailSessionCID, NS_MSGMAILSESSION_CID);
@@ -651,6 +653,10 @@ NS_IMETHODIMP nsNNTPProtocol::LoadNewsUrl(nsIURI * aURL, nsISupports * aConsumer
   // don't reuse an existing channel listener
   m_channelListener = nsnull;
   m_channelListener = do_QueryInterface(aConsumer);
+  nsCOMPtr<nsINntpUrl> newsUrl (do_QueryInterface(aURL));
+  newsUrl->GetNewsAction(&m_newsAction);
+
+  SetupPartExtractor(m_channelListener);
   return LoadUrl(aURL, aConsumer);
 }
 
@@ -742,6 +748,26 @@ nsNntpCacheStreamListener::OnDataAvailable(nsIRequest *request, nsISupports * aC
     return mListener->OnDataAvailable(ourRequest, aCtxt, aInStream, aSourceOffset, aCount);
 }
 
+nsresult nsNNTPProtocol::SetupPartExtractor(nsIStreamListener * aConsumer)
+{
+  if (m_newsAction == nsINntpUrl::ActionFetchPart)
+  {
+    nsCOMPtr<nsIStreamConverterService> converter = do_GetService(kIStreamConverterServiceCID);
+    if (converter && aConsumer)
+    {
+      nsCOMPtr<nsIStreamListener> newConsumer;
+      nsIChannel * channel;
+      QueryInterface(NS_GET_IID(nsIChannel), (void **) &channel);
+      converter->AsyncConvertData(NS_LITERAL_STRING("message/rfc822").get(), NS_LITERAL_STRING("*/*").get(),
+           aConsumer, channel, getter_AddRefs(newConsumer));
+      if (newConsumer)
+        m_channelListener = newConsumer;
+      NS_IF_RELEASE(channel);
+    }
+  }
+
+  return NS_OK;
+}
 
 NS_IMETHODIMP nsNNTPProtocol::AsyncOpen(nsIStreamListener *listener, nsISupports *ctxt)
 {
@@ -754,13 +780,15 @@ NS_IMETHODIMP nsNNTPProtocol::AsyncOpen(nsIStreamListener *listener, nsISupports
   m_runningURL->GetNewsAction(&m_newsAction);
   // first, check if this is a message load that should come from either
   // the memory cache or the local msg cache.
-  if (mailnewsUrl && m_newsAction == nsINntpUrl::ActionDisplayArticle)
+  if (mailnewsUrl && (m_newsAction == nsINntpUrl::ActionFetchArticle || m_newsAction == nsINntpUrl::ActionFetchPart))
   {
     nsCOMPtr<nsICachedNetData>  cacheEntry;
     PRUint32 contentLength = 0;
     PRBool partialFlag = PR_FALSE;
     PRBool msgIsInLocalCache;
     
+    SetupPartExtractor(m_channelListener);
+
     mailnewsUrl->GetMsgIsInLocalCache(&msgIsInLocalCache);
     if (msgIsInLocalCache)
     {
@@ -878,6 +906,7 @@ nsresult nsNNTPProtocol::LoadUrl(nsIURI * aURL, nsISupports * aConsumer)
   m_runningURL = do_QueryInterface(aURL, &rv);
   if (NS_FAILED(rv)) return rv;
   m_runningURL->GetNewsAction(&m_newsAction);
+
   nsCOMPtr<nsIMsgMailNewsUrl> mailnewsUrl = do_QueryInterface(m_runningURL);
 
   m_connectionBusy = PR_TRUE;
@@ -5164,7 +5193,6 @@ nsresult nsNNTPProtocol::ProcessProtocolState(nsIURI * url, nsIInputStream * inp
 				m_nextState = NEWS_FREE;
 				break;
             case NEWS_FREE:
-                m_connectionBusy = PR_FALSE;
                 mailnewsurl->SetUrlState(PR_FALSE, NS_OK);
                 m_lastActiveTimeStamp = PR_Now(); // remmeber when we last used this connection.
                 return CleanupAfterRunningUrl();
@@ -5235,7 +5263,6 @@ nsresult nsNNTPProtocol::CleanupAfterRunningUrl()
   nsresult rv = NS_OK;
   PR_LOG(NNTP,PR_LOG_ALWAYS,("CleanupAfterRunningUrl()"));
 
-  m_connectionBusy = PR_FALSE;
   // send StopRequest notification after we've cleaned up the protocol
   // because it can synchronously causes a new url to get run in the
   // protocol - truly evil, but we're stuck at the moment.
@@ -5267,8 +5294,6 @@ nsresult nsNNTPProtocol::CleanupAfterRunningUrl()
   PR_FREEIF(m_cancelID);  
   m_cancelID = nsnull;
   
-  if (!m_connectionBusy)
-  {
   mDisplayInputStream = nsnull;
   mDisplayOutputStream = nsnull;
   mProgressEventSink = nsnull;
@@ -5278,7 +5303,10 @@ nsresult nsNNTPProtocol::CleanupAfterRunningUrl()
   m_channelListener = nsnull;
   m_loadGroup = nsnull;
   mCallbacks = nsnull;
-  }
+
+  // don't mark ourselves as not busy until we are done cleaning up the connection. it should be the
+  // last thing we do.
+  m_connectionBusy = PR_FALSE;
  
   return NS_OK;
 }

mailnews/news/src/nsNNTPProtocol.h

@@ -170,6 +170,7 @@ public:
   nsresult LoadUrl(nsIURI * aURL, nsISupports * aConsumer);
 
 private:
+  nsresult SetupPartExtractor(nsIStreamListener * aConsumer);
 	// over-rides from nsMsgProtocol
 	virtual nsresult ProcessProtocolState(nsIURI * url, nsIInputStream * inputStream, 
 									      PRUint32 sourceOffset, PRUint32 length);

mailnews/news/src/nsNntpService.cpp

@@ -86,12 +86,13 @@ nsNntpService::~nsNntpService()
 NS_IMPL_THREADSAFE_ADDREF(nsNntpService);
 NS_IMPL_THREADSAFE_RELEASE(nsNntpService);
 
-NS_IMPL_QUERY_INTERFACE6(nsNntpService,
+NS_IMPL_QUERY_INTERFACE7(nsNntpService,
                          nsINntpService,
                          nsIMsgMessageService,
                          nsIProtocolHandler,
                          nsIMsgProtocolInfo,
                          nsICmdLineHandler,
+                         nsIMsgMessageFetchPartService,
 						 nsIContentHandler)
 
 ////////////////////////////////////////////////////////////////////////////////////////
@@ -225,8 +226,12 @@ nsNntpService::DisplayMessage(const char* aMessageURI, nsISupports * aDisplayCon
   if (mPrintingOperation)
     uri.Append("?header=print");
 
+  nsNewsAction action = nsINntpUrl::ActionFetchArticle;
+  if (mOpenAttachmentOperation)
+    action = nsINntpUrl::ActionFetchPart;
+
   nsCOMPtr<nsIURI> url;
-  rv = ConstructNntpUrl(uri.get(), aUrlListener, aMsgWindow, aMessageURI, nsINntpUrl::ActionDisplayArticle, getter_AddRefs(url));
+  rv = ConstructNntpUrl(uri.get(), aUrlListener, aMsgWindow, aMessageURI, action, getter_AddRefs(url));
   NS_ENSURE_SUCCESS(rv,rv);
 
   if (NS_SUCCEEDED(rv))
@@ -269,21 +274,23 @@ nsNntpService::DisplayMessage(const char* aMessageURI, nsISupports * aDisplayCon
     // run the url in the webshell in order to display it. If it isn't a docshell then just
     // run the news url like we would any other news url. 
 	  nsCOMPtr<nsIDocShell> docShell(do_QueryInterface(aDisplayConsumer, &rv));
-	  if (NS_SUCCEEDED(rv) && docShell) {
+	  if (NS_SUCCEEDED(rv) && docShell) 
+    {
 		nsCOMPtr<nsIDocShellLoadInfo> loadInfo;
 		// DIRTY LITTLE HACK --> if we are opening an attachment we want the docshell to
         // treat this load as if it were a user click event. Then the dispatching stuff will be much
         // happier.
-        if (mOpenAttachmentOperation) {
+      if (mOpenAttachmentOperation) 
+      {
 			docShell->CreateLoadInfo(getter_AddRefs(loadInfo));
 			loadInfo->SetLoadType(nsIDocShellLoadInfo::loadLink);
 		}
+	    
 	    rv = docShell->LoadURI(url, loadInfo, nsIWebNavigation::LOAD_FLAGS_NONE);
 	  }
-	  else {
+	  else 
 		rv = RunNewsUrl(url, aMsgWindow, aDisplayConsumer);
 	  }
-  }
 
   if (aURL) {
 	  *aURL = url;
@@ -315,7 +322,7 @@ nsNntpService::FetchMessage(nsIMsgFolder *folder, nsMsgKey key, nsIMsgWindow *aM
   NS_ENSURE_SUCCESS(rv,rv);
 
   nsCOMPtr<nsIURI> url;
-  rv = ConstructNntpUrl((const char *)messageIdURL, aUrlListener, aMsgWindow, originalMessageUri.get(), nsINntpUrl::ActionDisplayArticle, getter_AddRefs(url));
+  rv = ConstructNntpUrl((const char *)messageIdURL, aUrlListener, aMsgWindow, originalMessageUri.get(), nsINntpUrl::ActionFetchArticle, getter_AddRefs(url));
   NS_ENSURE_SUCCESS(rv,rv);
 
   rv = RunNewsUrl(url, aMsgWindow, aConsumer);
@@ -330,6 +337,18 @@ nsNntpService::FetchMessage(nsIMsgFolder *folder, nsMsgKey key, nsIMsgWindow *aM
   return rv;
 }
 
+NS_IMETHODIMP nsNntpService::FetchMimePart(nsIURI *aURI, const char *aMessageURI, nsISupports *aDisplayConsumer, nsIMsgWindow *aMsgWindow, nsIUrlListener *aUrlListener, nsIURI **aURL)
+{
+  nsCOMPtr<nsIMsgMailNewsUrl> msgUrl (do_QueryInterface(aURI));
+  msgUrl->SetMsgWindow(aMsgWindow);
+
+  // set up the url listener
+	if (aUrlListener)
+		msgUrl->RegisterListener(aUrlListener);
+ 
+  return RunNewsUrl(msgUrl, aMsgWindow, aDisplayConsumer);
+}
+
 NS_IMETHODIMP nsNntpService::OpenAttachment(const char *aContentType, 
                                             const char *aFileName,
                                             const char *aUrl, 
@@ -338,19 +357,33 @@ NS_IMETHODIMP nsNntpService::OpenAttachment(const char *aContentType,
                                             nsIMsgWindow *aMsgWindow, 
                                             nsIUrlListener *aUrlListener)
 {
-  nsCAutoString partMsgUrl(aMessageUri);
 
-  // try to extract the specific part number out from the url string
+  nsCOMPtr<nsIURI> url;
-  partMsgUrl += "?";
+  nsresult rv = NS_OK;
-  const char *part = PL_strstr(aUrl, "part=");
+  NewURI(aUrl, nsnull, getter_AddRefs(url));
-  partMsgUrl += part;
+
-  partMsgUrl += "&type=";
+  if (NS_SUCCEEDED(rv) && url)
-  partMsgUrl += aContentType;
+  {
-  mOpenAttachmentOperation = PR_TRUE;
+    nsCOMPtr<nsIMsgMailNewsUrl> msgUrl (do_QueryInterface(url));
-  nsresult rv = DisplayMessage(partMsgUrl, aDisplayConsumer,
+    msgUrl->SetMsgWindow(aMsgWindow);
-                      aMsgWindow, aUrlListener, nsnull, nsnull);
+    msgUrl->SetFileName(aFileName);
-  mOpenAttachmentOperation = PR_FALSE;
+
-  return rv;
+    // set up the url listener
+	  if (aUrlListener)
+	  	msgUrl->RegisterListener(aUrlListener);
+
+	  nsCOMPtr<nsIDocShell> docShell(do_QueryInterface(aDisplayConsumer, &rv));
+	  if (NS_SUCCEEDED(rv) && docShell) 
+    {
+		  nsCOMPtr<nsIDocShellLoadInfo> loadInfo;
+			docShell->CreateLoadInfo(getter_AddRefs(loadInfo));
+			loadInfo->SetLoadType(nsIDocShellLoadInfo::loadLink);
+	    return docShell->LoadURI(url, loadInfo, nsIWebNavigation::LOAD_FLAGS_NONE);
+    }
+    else
+      return RunNewsUrl(url, aMsgWindow, aDisplayConsumer);
+	}
+  return NS_OK;
 }
 
 NS_IMETHODIMP nsNntpService::GetUrlForUri(const char *aMessageURI, nsIURI **aURL, nsIMsgWindow *aMsgWindow) 
@@ -375,7 +408,7 @@ NS_IMETHODIMP nsNntpService::GetUrlForUri(const char *aMessageURI, nsIURI **aURL
   NS_ENSURE_SUCCESS(rv,rv);
 
   // this is only called by view message source
-  rv = ConstructNntpUrl(messageIdURL.get(), nsnull, aMsgWindow, aMessageURI, nsINntpUrl::ActionDisplayArticle, aURL);
+  rv = ConstructNntpUrl(messageIdURL.get(), nsnull, aMsgWindow, aMessageURI, nsINntpUrl::ActionFetchArticle, aURL);
   NS_ENSURE_SUCCESS(rv,rv);
   if (folder && *aURL)
   {
@@ -837,14 +870,12 @@ nsNntpService::ConstructNntpUrl(const char *urlString, nsIUrlListener *aUrlListe
   nsCOMPtr <nsINntpUrl> nntpUrl = do_CreateInstance(NS_NNTPURL_CONTRACTID,&rv);
   NS_ENSURE_SUCCESS(rv,rv);
   
-  rv = nntpUrl->SetNewsAction(action);
-  NS_ENSURE_SUCCESS(rv,rv);
-
   nsCOMPtr <nsIMsgMailNewsUrl> mailnewsurl = do_QueryInterface(nntpUrl);
   mailnewsurl->SetMsgWindow(aMsgWindow);
   nsCOMPtr <nsIMsgMessageUrl> msgUrl = do_QueryInterface(nntpUrl);
   msgUrl->SetUri(urlString);
   mailnewsurl->SetSpec(urlString);
+  nntpUrl->SetNewsAction(action);
   
   if (originalMessageUri) {
     // we'll use this later in nsNNTPProtocol::ParseURL()
@@ -1166,10 +1197,6 @@ NS_IMETHODIMP nsNntpService::NewURI(const char *aSpec, nsIURI *aBaseURI, nsIURI
     NS_ENSURE_SUCCESS(rv,rv);
     if (!nntpUrl) return NS_ERROR_FAILURE;
 
-    // XXX is this always the case?
-	rv = nntpUrl->SetNewsAction(nsINntpUrl::ActionDisplayArticle);
-    NS_ENSURE_SUCCESS(rv,rv);
-
 	nntpUrl->QueryInterface(NS_GET_IID(nsIURI), (void **) _retval);
 
 	(*_retval)->SetSpec(aSpec);

mailnews/news/src/nsNntpService.h

@@ -42,6 +42,7 @@ class nsIUrlListener;
 
 class nsNntpService : public nsINntpService,
                       public nsIMsgMessageService,
+                      public nsIMsgMessageFetchPartService,
                       public nsIProtocolHandler,
                       public nsIMsgProtocolInfo,
                       public nsICmdLineHandler,
@@ -56,6 +57,7 @@ public:
   NS_DECL_NSIMSGPROTOCOLINFO
   NS_DECL_NSICMDLINEHANDLER
   NS_DECL_NSICONTENTHANDLER
+  NS_DECL_NSIMSGMESSAGEFETCHPARTSERVICE
   
   // nsNntpService
   nsNntpService();

mailnews/news/src/nsNntpUrl.cpp

@@ -79,6 +79,26 @@ NS_INTERFACE_MAP_END_INHERITING(nsMsgMailNewsUrl)
 ////////////////////////////////////////////////////////////////////////////////////
 // Begin nsINntpUrl specific support
 ////////////////////////////////////////////////////////////////////////////////////
+NS_IMETHODIMP nsNntpUrl::SetSpec(const char * aSpec)
+{
+	nsresult rv = nsMsgMailNewsUrl::SetSpec(aSpec);
+	if (NS_SUCCEEDED(rv))
+		ParseUrl(aSpec);
+	return rv;
+}
+
+nsresult nsNntpUrl::ParseUrl(const char * aSpec)
+{
+  char * partString = PL_strcasestr(aSpec, "?part=");
+  if (partString)
+    m_newsAction = nsINntpUrl::ActionFetchPart;
+  else
+    m_newsAction = nsINntpUrl::ActionFetchArticle;
+
+  // XXX to do: add more smart detection code for setting the action type based on the contents of the url
+  // string.
+  return NS_OK;
+}
 
 NS_IMETHODIMP nsNntpUrl::SetGetOldMessages(PRBool aGetOldMessages)
 {
@@ -229,7 +249,7 @@ NS_IMETHODIMP nsNntpUrl::IsUrlType(PRUint32 type, PRBool *isType)
 	switch(type)
 	{
 		case nsIMsgMailNewsUrl::eDisplay:
-			*isType = (m_newsAction == nsINntpUrl::ActionDisplayArticle);
+			*isType = (m_newsAction == nsINntpUrl::ActionFetchArticle);
 			break;
 		default:
 			*isType = PR_FALSE;

mailnews/news/src/nsNntpUrl.h

@@ -36,6 +36,9 @@ public:
   NS_DECL_NSIMSGMESSAGEURL
   NS_DECL_NSIMSGI18NURL
 
+  // nsIURI over-ride...
+	NS_IMETHOD SetSpec(const char * aSpec);
+
 	NS_IMETHOD IsUrlType(PRUint32 type, PRBool *isType);
 
   // nsNntpUrl
@@ -45,6 +48,7 @@ public:
   NS_DECL_ISUPPORTS_INHERITED
 
 protected:  
+  virtual nsresult ParseUrl(const char * aSpec);
 	virtual const char * GetUserName() { return nsnull; }
   nsINNTPNewsgroupPost *m_newsgroupPost;
 	nsNewsAction m_newsAction; // the action this url represents...parse mailbox, display messages, etc.