Bug 705431: make sure a MIME part with content-disposition: attachment is displayed as an attachment nonetheless even though it doesn't have a filename r=squib,a=Standard8
authorJonathan Protzenko <jonathan.protzenko@gmail.com>
Fri, 20 Jan 2012 23:21:48 +0100
changeset 10171 66d01c6ae24489154dc693e5d18290a4edf985e0
parent 10170 6fa3d8b42adeb4f861f7a3bcb6efb43a1433a6d8
child 10172 b6fad7040c8eedebf0bb07303fab39435193300c
push idunknown
push userunknown
push dateunknown
reviewerssquib, Standard8
bugs705431
Bug 705431: make sure a MIME part with content-disposition: attachment is displayed as an attachment nonetheless even though it doesn't have a filename r=squib,a=Standard8
mailnews/compose/public/nsIMsgSend.idl
mailnews/db/gloda/modules/datamodel.js
mailnews/db/gloda/test/unit/test_mime_emitter.js
mailnews/mime/src/mimemoz2.cpp
--- a/mailnews/compose/public/nsIMsgSend.idl
+++ b/mailnews/compose/public/nsIMsgSend.idl
@@ -173,36 +173,41 @@ class nsMsgAttachmentData : public nsIMs
 {
 public:
   NS_DECL_NSIMSGATTACHMENTDATA
   NS_DECL_ISUPPORTS
 
   nsMsgAttachmentData();
   ~nsMsgAttachmentData();
 
-  nsCOMPtr<nsIURI> m_url; // The URL to attach.
+  nsCOMPtr<nsIURI> m_url;   // The URL to attach.
 
-  nsCString m_desiredType; // The type to which this document should be
-                          // converted.  Legal values are NULL, TEXT_PLAIN
-                          // and APPLICATION_POSTSCRIPT (which are macros
-                          // defined in net.h); other values are ignored.
+  nsCString m_desiredType;  // The type to which this document should be
+                            // converted.  Legal values are NULL, TEXT_PLAIN
+                            // and APPLICATION_POSTSCRIPT (which are macros
+                            // defined in net.h); other values are ignored.
+
+  nsCString m_realType;     // The type of the URL if known, otherwise NULL. For example, if 
+                            // you were attaching a temp file which was known to contain HTML data, 
+                            // you would pass in TEXT_HTML as the real_type, to override whatever type 
+                            // the name of the tmp file might otherwise indicate.
 
-  nsCString m_realType;        // The type of the URL if known, otherwise NULL. For example, if 
-                          // you were attaching a temp file which was known to contain HTML data, 
-                          // you would pass in TEXT_HTML as the real_type, to override whatever type 
-                          // the name of the tmp file might otherwise indicate.
+  nsCString m_realEncoding; // Goes along with real_type
 
-  nsCString m_realEncoding;    // Goes along with real_type 
+  nsCString m_realName;     // The original name of this document, which will eventually show up in the 
+                            // Content-Disposition header. For example, if you had copied a document to a 
+                            // tmp file, this would be the original, human-readable name of the document.
 
-  nsCString m_realName;        // The original name of this document, which will eventually show up in the 
-                          // Content-Disposition header. For example, if you had copied a document to a 
-                          // tmp file, this would be the original, human-readable name of the document.
+  nsCString m_description;  // If you put a string here, it will show up as the Content-Description header.  
+                            // This can be any explanatory text; it's not a file name.             
 
-  nsCString m_description;      // If you put a string here, it will show up as the Content-Description header.  
-                          // This can be any explanatory text; it's not a file name.             
+  nsCString m_disposition;  // The Content-Disposition header (if any). a
+                            // nsMsgAttachmentData can very well have
+                            // Content-Disposition: inline value, instead of
+                            // "attachment".
 
   // Mac-specific data that should show up as optional parameters
   // to the content-type header.
   nsCString m_xMacType;
   nsCString m_xMacCreator;
 
   PRInt32 m_size;                  // The size of the attachment. May be 0.
   bool    m_isExternalAttachment;  // Flag for determining if the attachment is external
--- a/mailnews/db/gloda/modules/datamodel.js
+++ b/mailnews/db/gloda/modules/datamodel.js
@@ -880,17 +880,17 @@ GlodaAttachment.prototype = {
       if (!uri)
         throw new Error("The message doesn't exist anymore, unable to rebuild attachment URL");
       let neckoURL = {};
       let msgService = getMessenger().messageServiceFromURI(uri);
       msgService.GetUrlForUri(uri, neckoURL, null);
       let url = neckoURL.value.spec;
       let hasParamAlready = url.match(/\?[a-z]+=[^\/]+$/);
       let sep = hasParamAlready ? "&" : "?";
-      return url+sep+"part="+this._part+"&filename="+this._name;
+      return url+sep+"part="+this._part+"&filename="+encodeURIComponent(this._name);
     }
   },
   get isExternal() { return this._isExternal; },
 
   toString: function gloda_attachment_toString() {
     return "attachment: " + this._name + ":" + this._contentType;
   },
 
--- a/mailnews/db/gloda/test/unit/test_mime_emitter.js
+++ b/mailnews/db/gloda/test/unit/test_mime_emitter.js
@@ -82,16 +82,24 @@ var partEnriched = new SyntheticPartLeaf
   "<bold><italic>I am not a popular format! sad woo :(</italic></bold>",
   {
     contentType: "text/enriched"
   }
 );
 var partAlternative = new SyntheticPartMultiAlternative([partText, partHtml]);
 var partMailingListFooter = new SyntheticPartLeaf("I am an annoying footer!");
 
+// We need to make sure a part that has content-disposition: attachment, even
+// though it doesn't have any filename, still is treated as an attachment.
+var tachNoFilename = {
+  body: "I like Bordeaux wine",
+  contentType: "text/plain",
+  disposition: "attachment",
+};
+
 // This is an external attachment, i.e. a mime part that basically says "go find
 // the attachment on disk, assuming it still exists, here's the path to the file
 // on disk". It turns out feed enclosures are presented in the exact same way,
 // so this covers this case as well.
 var tachExternal = {
   body:
     'You deleted an attachment from this message. The original MIME headers for the attachment were:\n'+
     'Content-Type: image/png;\n'+
@@ -478,16 +486,19 @@ var partTachNestedMessages = [
     }),
   msgGen.makeMessage({
       attachments: [tachImage, tachApplication]
     }),
 ];
 
 var attMessagesParams = [
   {
+    attachments: [tachNoFilename],
+  },
+  {
     attachments: [tachExternal],
   },
   {
     name: 'attached rfc822',
     bodyPart: new SyntheticPartMultiMixed([partAlternative,
                                            partTachNestedMessages[0]]),
   },
   {
@@ -500,16 +511,20 @@ var attMessagesParams = [
     bodyPart: new SyntheticPartMultiMixed([partAlternative,
                                            partTachApplication,
                                            partTachNestedMessages[2]]),
   },
 ];
 
 var expectedAttachmentsInfo = [
   {
+    allAttachmentsContentTypes: ["text/plain"],
+    allUserAttachmentsContentTypes: ["text/plain"],
+  },
+  {
     allAttachmentsContentTypes: ["image/png"],
     allUserAttachmentsContentTypes: ["image/png"],
   },
   {
     allAttachmentsContentTypes: [],
     allUserAttachmentsContentTypes: ["message/rfc822"],
     firstAttachmentName: "S\u00e3o Paulo.eml",
   },
--- a/mailnews/mime/src/mimemoz2.cpp
+++ b/mailnews/mime/src/mimemoz2.cpp
@@ -370,16 +370,17 @@ GenerateAttachmentData(MimeObject *objec
     return NS_OK;
 
   nsMsgAttachmentData *tmp = &(aAttachData[attIndex++]);
 
   tmp->m_realType = object->content_type;
   tmp->m_realEncoding = object->encoding;
   tmp->m_isExternalAttachment = isExternalAttachment;
   tmp->m_size = attSize;
+  tmp->m_disposition.Adopt(MimeHeaders_get(object->headers, HEADER_CONTENT_DISPOSITION, true, false));
 
   char *part_addr = mime_imap_part_address(object);
   tmp->m_isDownloaded = !part_addr;
   PR_FREEIF(part_addr);
 
   PRInt32 i;
   char *charset = nsnull;
   char *disp = MimeHeaders_get(object->headers, HEADER_CONTENT_DISPOSITION, PR_FALSE, PR_FALSE);
@@ -664,18 +665,25 @@ NotifyEmittersOfAttachmentList(MimeDispl
   PRInt32     i = 0;
   nsMsgAttachmentData  *tmp = data;
 
   if (!tmp)
     return;
 
   while (tmp->m_url)
   {
-    if (tmp->m_realName.IsEmpty() || (!tmp->m_hasFilename &&
-        (opt->html_as_p != 4 || opt->metadata_only)))
+    // The code below implements the following logic:
+    // - always display if Content-Disposition: attachment
+    // - IF there's no name at all (we don't know what to do with it then)!,
+    //   OR if the attachment doesn't have a "provided name" (it's a Part 1.2
+    //   thingy, then) and we're not asking for all body parts or only
+    //   interested in metadata, THEN skip
+    if (!tmp->m_disposition.Equals("attachment") &&
+        (tmp->m_realName.IsEmpty() || (!tmp->m_hasFilename &&
+        (opt->html_as_p != 4 || opt->metadata_only))))
     {
       ++i;
       ++tmp;
       continue;
     }
 
     nsCAutoString spec;
     if (tmp->m_url)