Bug 1255071 - Don't use base64 encoding for message/rfc822 attachments. r=mkmelin a=rkent
authorJorg K
Mon, 14 Mar 2016 11:37:24 +0100
changeset 26822 f19c1ba0792add3d74062aa55c538405861e2814
parent 26821 0c4109ac2b8a472cbfb58ea375756a4533fcf3fb
child 26823 151317bf199543f86283ac6261d9088820bb8f0a
push id1850
push userclokep@gmail.com
push dateWed, 08 Mar 2017 19:29:12 +0000
treeherdercomm-esr52@028df196b2d9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin, rkent
bugs1255071
Bug 1255071 - Don't use base64 encoding for message/rfc822 attachments. r=mkmelin a=rkent
mail/test/mozmill/composition/long-html-line.eml
mail/test/mozmill/composition/test-forward-rfc822-attach.js
mailnews/compose/src/nsMsgAttachmentHandler.cpp
mailnews/compose/src/nsMsgSend.cpp
mailnews/compose/src/nsMsgSend.h
mailnews/compose/test/unit/test_longLines.js
new file mode 100644
--- /dev/null
+++ b/mail/test/mozmill/composition/long-html-line.eml
@@ -0,0 +1,16 @@
+From: "Long line writer" <email@longline.invalid>
+To: <user@example.com>
+Subject: Long HTML line
+Date: Wed, 10 Feb 2016 16:45:36 -0600
+MIME-Version: 1.0
+Content-Type: text/html;
+	charset="us-ascii"
+Content-Transfer-Encoding: 7bit
+
+<!DOCTYPE html>
+<html>
+<body>
+
+We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. We like writing long lines. This is 998 long.
+
+ </body>
new file mode 100644
--- /dev/null
+++ b/mail/test/mozmill/composition/test-forward-rfc822-attach.js
@@ -0,0 +1,101 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * Tests that attached messages (message/rfc822) are correctly sent.
+ * It's easiest to test the forward case.
+ */
+
+// mozmake SOLO_TEST=composition/test-forward-rfc822-attach.js mozmill-one
+
+var MODULE_NAME = "test-forward-rfc822-attach";
+
+var RELATIVE_ROOT = "../shared-modules";
+var MODULE_REQUIRES = ["folder-display-helpers", "compose-helpers", "window-helpers"];
+
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource:///modules/mailServices.js");
+var os = {};
+Cu.import('resource://mozmill/stdlib/os.js', os);
+
+var draftsFolder;
+
+function setupModule(module) {
+  for (let lib of MODULE_REQUIRES) {
+    collector.getModule(lib).installInto(module);
+  }
+
+  if (!MailServices.accounts
+                   .localFoldersServer
+                   .rootFolder
+                   .containsChildNamed("Drafts")) {
+    create_folder("Drafts", [Ci.nsMsgFolderFlags.Drafts]);
+  }
+  draftsFolder = MailServices.accounts
+                             .localFoldersServer
+                             .rootFolder
+                             .getChildNamed("Drafts");
+}
+
+/**
+ * Helper to get the full message content.
+ *
+ * @param aMsgHdr: nsIMsgDBHdr object whose text body will be read
+ * @return string with full message source
+ */
+function getMsgSource(aMsgHdr) {
+  let msgFolder = aMsgHdr.folder;
+  let msgUri = msgFolder.getUriForMsg(aMsgHdr);
+
+  let messenger = Cc["@mozilla.org/messenger;1"]
+                    .createInstance(Ci.nsIMessenger);
+  let streamListener = Cc["@mozilla.org/network/sync-stream-listener;1"]
+                         .createInstance(Ci.nsISyncStreamListener);
+  messenger.messageServiceFromURI(msgUri).streamMessage(msgUri,
+                                                        streamListener,
+                                                        null,
+                                                        null,
+                                                        false,
+                                                        "",
+                                                        false);
+  let sis = Cc["@mozilla.org/scriptableinputstream;1"]
+              .createInstance(Ci.nsIScriptableInputStream);
+  sis.init(streamListener.inputStream);
+  const MAX_MESSAGE_LENGTH = 65536;
+  let content = sis.read(MAX_MESSAGE_LENGTH);
+  sis.close();
+  return content;
+}
+
+function forwardDirect(aFilePath) {
+  let file = os.getFileForPath(os.abspath(aFilePath,
+                               os.getFileForPath(__file__)));
+  let msgc = open_message_from_file(file);
+
+  let cwc = open_compose_with_forward_as_attachments(msgc);
+
+  // Ctrl+S saves as draft.
+  cwc.keypress(null, "s", {shiftKey: false, accelKey: true});
+
+  close_compose_window(cwc);
+  close_window(msgc);
+
+  be_in_folder(draftsFolder);
+  let draftMsg = select_click_row(0);
+
+  let draftMsgContent = getMsgSource(draftMsg);
+
+  if (!draftMsgContent.includes("We like writing long lines.")) {
+    assert_true(false, "Failed to find expected text");
+  }
+
+  press_delete(mc); // clean up the created draft
+}
+
+function test_forwarding_long_html_line_as_attachment() {
+  forwardDirect("./long-html-line.eml");
+}
+
+function teardownModule() {
+}
--- a/mailnews/compose/src/nsMsgAttachmentHandler.cpp
+++ b/mailnews/compose/src/nsMsgAttachmentHandler.cpp
@@ -389,17 +389,17 @@ nsMsgAttachmentHandler::PickEncoding(con
       if (mCompFields->GetForceMsgEncoding())
         force_p = true;
     } else if (mime_delivery_state) {
       if (((nsMsgComposeAndSend *)mime_delivery_state)->mCompFields->GetForceMsgEncoding()) {
         force_p = true;
       }
     }
 
-    if (force_p || (m_max_column > 900)) {
+    if (force_p || (m_max_column > LINELENGTH_ENCODING_THRESHOLD)) {
       encode_p = true;
     } else if (UseQuotedPrintable() && m_unprintable_count) {
       encode_p = true;
     } else if (m_null_count) {
       // If there are nulls, we must always encode, because sendmail will
       // blow up.
       encode_p = true;
     } else {
@@ -449,17 +449,23 @@ nsMsgAttachmentHandler::PickEncoding(con
 
   // Always base64 binary data.
   if (needsB64)
     m_encoding = ENCODING_BASE64;
 
   // According to RFC 821 we must always have lines shorter than 998 bytes.
   // To encode "long lines" use a CTE that will transmit shorter lines.
   // Switch to base64 if we are not already using "quoted printable".
-  if (m_max_column > 900 && !isUsingQP)
+
+  // We don't do this for message/rfc822 attachments, since we can't
+  // change the original Content-Transfer-Encoding of the message we're
+  // attaching. We rely on the original message complying with RFC 821,
+  // if it doesn't we won't either. Not ideal.
+  if (!m_type.LowerCaseEqualsLiteral(MESSAGE_RFC822) &&
+      m_max_column > LINELENGTH_ENCODING_THRESHOLD && !isUsingQP)
     m_encoding = ENCODING_BASE64;
 
   // Now that we've picked an encoding, initialize the filter.
   NS_ASSERTION(!m_encoder, "not-null m_encoder");
   if (m_encoding.LowerCaseEqualsLiteral(ENCODING_BASE64))
   {
     m_encoder = MimeEncoder::GetBase64Encoder(mime_encoder_output_fn,
       mime_delivery_state);
--- a/mailnews/compose/src/nsMsgSend.cpp
+++ b/mailnews/compose/src/nsMsgSend.cpp
@@ -623,17 +623,17 @@ nsMsgComposeAndSend::GatherMimeAttachmen
         cur_column = 0;
       } else if (*c != '\r') {
         cur_column++;
       }
     }
     // Check one last time for the last line.
     if (cur_column > max_column)
       max_column = cur_column;
-    if (max_column > 900 && !isUsingQP) {
+    if (max_column > LINELENGTH_ENCODING_THRESHOLD && !isUsingQP) {
       // To encode "long lines" use a CTE that will transmit shorter lines.
       // Switch to base64 if we are not already using "quoted printable".
       PR_FREEIF(m_attachment1_encoding);
       m_attachment1_encoding = PL_strdup (ENCODING_BASE64);
     }
   }
   PR_FREEIF (m_attachment1_body);
 
@@ -1040,17 +1040,17 @@ nsMsgComposeAndSend::PreProcessPart(nsMs
   if (ma->m_bogus_attachment)
     return 0;
 
   // If at this point we *still* don't have a content-type, then
   // we're never going to get one.
   if (ma->m_type.IsEmpty())
     ma->m_type = UNKNOWN_CONTENT_TYPE;
 
-  ma->PickEncoding (mCompFields->GetCharacterSet(), this);
+  ma->PickEncoding(mCompFields->GetCharacterSet(), this);
   ma->PickCharset();
 
   part = new nsMsgSendPart(this);
   if (!part)
     return 0;
   status = toppart->AddChild(part);
   // Remember the part number if it has a node index.
   if (ma->mNodeIndex != -1)
@@ -2368,17 +2368,17 @@ nsMsgComposeAndSend::HackAttachments(nsI
       /* If the attachment has an encoding, and it's not one of
       the "null" encodings, then keep it. */
       if (!m_attachments[i]->m_encoding.IsEmpty() &&
           !m_attachments[i]->m_encoding.LowerCaseEqualsLiteral(ENCODING_7BIT) &&
           !m_attachments[i]->m_encoding.LowerCaseEqualsLiteral(ENCODING_8BIT) &&
           !m_attachments[i]->m_encoding.LowerCaseEqualsLiteral(ENCODING_BINARY))
         m_attachments[i]->m_already_encoded_p = true;
 
-            if (m_attachments[i]->mURL)
+      if (m_attachments[i]->mURL)
         msg_pick_real_name(m_attachments[i], nullptr, mCompFields->GetCharacterSet());
     }
   }
 
   // First, handle the multipart related attachments if any...
   //
   int32_t mailbox_count = 0, news_count = 0;
   int32_t multipartRelatedCount = GetMultipartRelatedCount();
--- a/mailnews/compose/src/nsMsgSend.h
+++ b/mailnews/compose/src/nsMsgSend.h
@@ -45,17 +45,18 @@
   as source or postscript, it will be run through the appropriate converter
   (which will result in a document of type text/plain.)
 
   An attachment will be encoded if:
 
    - it is of a non-text type (in which case we will use base64); or
    - The "use QP" option has been selected and high-bit characters exist; or
    - any NULLs exist in the document; or
-   - any line is longer than 900 bytes.
+   - any line is longer than 990 bytes (see LINELENGTH_ENCODING_THRESHOLD below)
+     and it is not of type message/rfc822.
 
    - If we are encoding, and more than 10% of the document consists of
      non-ASCII characters, then we always use base64 instead of QP.
 
   We eschew quoted-printable in favor of base64 for documents which are likely
   to always be binary (images, sound) because, on the off chance that a GIF
   file (for example) might contain primarily bytes in the ASCII range, using
   the quoted-printable representation might cause corruption due to the
@@ -135,16 +136,19 @@
 #include "nsIMsgOperationListener.h"
 
 //
 // Some necessary defines...
 //
 #define TEN_K                 10240
 #define MIME_BUFFER_SIZE      4096 // must be greater than 1000
                                    // SMTP (RFC821) limit
+// Maximum number of bytes we allow in a line before we force
+// encoding to base64 if not already QR-encoded or of type message/rfc822.
+#define LINELENGTH_ENCODING_THRESHOLD 990
 
 //
 // Utilities for string handling
 //
 #define PUSH_STRING(S) \
  do { PL_strcpy (buffer_tail, S); buffer_tail += PL_strlen (S); } while(0)
 #define PUSH_STRINGN(S,N) \
  do { memcpy(buffer_tail, (S), (N)); buffer_tail += (N); } while(0)
--- a/mailnews/compose/test/unit/test_longLines.js
+++ b/mailnews/compose/test/unit/test_longLines.js
@@ -77,17 +77,17 @@ function checkMessageHeaders(msgData, ex
 // two bytes c3a1 in UTF-8.
 let longMultibyteLine = "\u00E1".repeat(600);
 
 // And here a line with a Korean character, encoded as three bytes
 // ec9588 in UTF-8.
 let longMultibyteLineCJK = "안".repeat(400);
 
 // And some Japanese.
-let longMultibyteLineJapanese = "語".repeat(400);
+let longMultibyteLineJapanese = "語".repeat(450);
 
 function* testBodyWithLongLine() {
   let newline;
   // Windows uses CR+LF, the other platforms just LF.
   // Note: Services.appinfo.OS returns "XPCShell" in the test, so we
   // use this hacky condition to separate Windows from the others.
   if ("@mozilla.org/windows-registry-key;1" in Components.classes) {
     newline = "\r\n";