Bug 1259040 - use Message-ID as basis for maildir file names, and add ".eml" extension. r=jorgk DONTBUILD
authorBen Campbell <benc@thunderbird.net>
Fri, 23 Nov 2018 19:41:53 +1300
changeset 34165 277f7d1fd5527cb886549c76870aeaf68d265ebb
parent 34164 6c36a1eb832351416d0a9e6ff513ce9f1745aa43
child 34166 77f1eb52e15571ac991adb90c0862701e7238bc1
push id389
push userclokep@gmail.com
push dateMon, 18 Mar 2019 19:01:53 +0000
reviewersjorgk
bugs1259040
Bug 1259040 - use Message-ID as basis for maildir file names, and add ".eml" extension. r=jorgk DONTBUILD
mailnews/base/util/converterWorker.js
mailnews/base/util/mailstoreConverter.jsm
mailnews/local/src/nsMsgMaildirStore.cpp
--- a/mailnews/base/util/converterWorker.js
+++ b/mailnews/base/util/converterWorker.js
@@ -211,17 +211,17 @@ self.addEventListener("message", functio
             while (resultLast !== null) {
               lastPos[lastPos.length] = resultLast.index;
               resultLast = regexpLast.exec(text);
             }
 
             // Create a maildir message file in 'dest/destFile/cur/'
             // and open it for writing.
             targetFile = OS.File.open(OS.Path.join(dest, destFile, "cur",
-              name.toString()), {write: true, create: true}, {});
+              name.toString() + ".eml"), {write: true, create: true}, {});
 
             // Extract the text in 'text' between 'lastPos[0]' ie the
             // index of the first "From - " match and the end of 'text'.
             targetFile.write(encoder.encode(text.substring(lastPos[0],
               text.length)));
             targetFile.close();
 
             // Send a message indicating that a message was copied.
@@ -250,17 +250,17 @@ self.addEventListener("message", functio
 
         if (msgPos.length > 1) {
           // More than one "From - " match is found.
           noOfBytes = constNoOfBytes;
           for (let i = 0; i < msgPos.length - 1; i++) {
             // Create and open a new file in 'dest/destFile/cur'
             // to hold the next mail.
             targetFile = OS.File.open(OS.Path.join(dest, destFile, "cur",
-              name.toString()), {write: true, create: true}, {});
+              name.toString() + ".eml"), {write: true, create: true}, {});
             // Extract the text lying between consecutive indices, encode
             // it and write it.
             targetFile.write(encoder.encode(text.substring(msgPos[i],
               msgPos[i + 1])));
             targetFile.close();
 
             // Send a message indicating that a mail was copied.
             // This indicates progress for a pop account if the no. of msgs
--- a/mailnews/base/util/mailstoreConverter.jsm
+++ b/mailnews/base/util/mailstoreConverter.jsm
@@ -211,34 +211,35 @@ function convertMailStoreTo(aMailstoreCo
         aMailstoreContractId,
         tmpDir.path,
         aServer.type
       ];
 
       if (content.isDirectory()) {
         if (content.leafName.substr(-4) != ".sbd" && content.leafName.substr(-8) != ".mozmsgs") {
           // Assume it's a maildir, and grab the list of messages.
-          // Array to hold unsorted list of maildir msg filenames.
-          let dataArrayUnsorted = [];
-          // "cur" directory inside the maildir msg folder.
+          // We sort the messages by timestamp to order the resulting
+          // mbox. Not ideal, but best we can do without parsing headers.
+
+          // XXX TODO: should use OS.File APIs for this, and probably
+          // just do it all in the worker. See:
+          // https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/OSFile.jsm/OS.File.DirectoryIterator_for_workers#Example_Sorting_files_by_last_modification_date
+          let msgFiles = [];
           let cur = FileUtils.File(OS.Path.join(content.path,"cur"));
-          // Msg files inside "cur" directory.
           let msgs = cur.directoryEntries;
-
           while (msgs.hasMoreElements()) {
-            // Add filenames as integers into 'dataArrayUnsorted'.
-            // TODO: this'll break if maildir scheme changes! (eg .eml extension)
-            let msg = msgs.getNext()
-                          .QueryInterface(Ci.nsIFile);
-            dataArrayUnsorted.push(parseInt(msg.leafName));
+            let msg = msgs.getNext().QueryInterface(Ci.nsIFile);
+            msgFiles.push(msg);
           }
-          dataArrayUnsorted.sort()
-          // Add the maildir msg filenames into 'dataArray' in a sorted order.
-          for (let elem of dataArrayUnsorted) {
-            dataArray.push(elem.toString());
+          msgFiles.sort(function compare(a, b) {
+            return a.lastModifiedTime - b.lastModifiedTime;
+          });
+          // Build the list of msg files to send to the worker.
+          for (let elem of msgFiles) {
+            dataArray.push(elem.leafName.toString());
           }
         }
       }
 
       // Set up the worker.
       let converterWorker = new ChromeWorker(
         "resource:///modules/converterWorker.js");
       gConverterWorkerArray.push(converterWorker);
--- a/mailnews/local/src/nsMsgMaildirStore.cpp
+++ b/mailnews/local/src/nsMsgMaildirStore.cpp
@@ -34,16 +34,45 @@
 #include "nsIMailboxUrl.h"
 #include "nsIMsgMailNewsUrl.h"
 #include "nsIMsgFilterPlugin.h"
 #include "nsLocalUndoTxn.h"
 #include "nsIMessenger.h"
 
 static mozilla::LazyLogModule MailDirLog("MailDirStore");
 
+// Helper function to produce a safe filename from a Message-ID value.
+// We'll percent-encode anything not in this set: [-+.%=@_0-9a-zA-Z]
+// This is an overly-picky set, but should:
+//  - leave most sane Message-IDs unchanged
+//  - be safe on windows (the pickiest case)
+//  - avoid chars that can trip up shell scripts (spaces, semicolons etc)
+// If input contains malicious binary (or multibyte chars) it'll be
+// safely encoded as individual bytes.
+static void percentEncode(nsACString const& in, nsACString& out)
+{
+  const char* end = in.EndReading();
+  const char* cur;
+  // We know the output will be at least as long as the input.
+  out.SetLength(0);
+  out.SetCapacity(in.Length());
+  for (cur = in.BeginReading(); cur != end; ++cur) {
+    const char c = *cur;
+    bool whitelisted = (c >= '0' && c <= '9') ||
+      (c >= 'A' && c <= 'Z') ||
+      (c >= 'a' && c <= 'z') ||
+      c == '-' || c == '+' || c == '.' ||c == '%' || c == '=' | c == '@' | c == '_';
+    if (whitelisted) {
+      out.Append(c);
+    } else {
+      out.AppendPrintf("%%%02x", (const unsigned char)c);
+    }
+  }
+}
+
 nsMsgMaildirStore::nsMsgMaildirStore()
 {
 }
 
 nsMsgMaildirStore::~nsMsgMaildirStore()
 {
 }
 
@@ -601,37 +630,44 @@ nsMsgMaildirStore::GetNewMsgOutputStream
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (!*aNewMsgHdr)
   {
     rv = db->CreateNewHdr(nsMsgKey_None, aNewMsgHdr);
     NS_ENSURE_SUCCESS(rv, rv);
 
   }
+  // With maildir, messages have whole file to themselves.
   (*aNewMsgHdr)->SetMessageOffset(0);
-  // path to the message download folder
+
+  // We're going to save the new message into the maildir 'tmp' folder.
+  // When the message is completed, it can be moved to 'cur'.
   nsCOMPtr<nsIFile> newFile;
   rv = aFolder->GetFilePath(getter_AddRefs(newFile));
   NS_ENSURE_SUCCESS(rv, rv);
   newFile->Append(NS_LITERAL_STRING("tmp"));
 
   // let's check if the folder exists
+  // XXX TODO: kill this and make sure maildir creation includes cur/tmp
   bool exists;
   newFile->Exists(&exists);
   if (!exists) {
     MOZ_LOG(MailDirLog, mozilla::LogLevel::Info,
            ("GetNewMsgOutputStream - tmp subfolder does not exist!!\n"));
     rv = newFile->Create(nsIFile::DIRECTORY_TYPE, 0755);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  // generate new file name
+  // Generate the 'tmp' file name based on timestamp.
+  // (We'll use the Message-ID as the basis for the final filename,
+  // but we don't have headers at this point).
   nsAutoCString newName;
   newName.AppendInt(static_cast<int64_t>(PR_Now()));
   newFile->AppendNative(newName);
+
   // CreateUnique, in case we get more than one message per millisecond :-)
   rv = newFile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600);
   NS_ENSURE_SUCCESS(rv, rv);
   newFile->GetNativeLeafName(newName);
   // save the file name in the message header - otherwise no way to retrieve it
   (*aNewMsgHdr)->SetStringProperty("storeToken", newName.get());
   return MsgNewBufferedFileOutputStream(aResult, newFile,
                                         PR_WRONLY | PR_CREATE_FILE, 00600);
@@ -676,72 +712,118 @@ nsMsgMaildirStore::FinishNewMessage(nsIO
 
   nsCOMPtr<nsIFile> folderPath;
   nsCOMPtr<nsIMsgFolder> folder;
   nsresult rv = aNewHdr->GetFolder(getter_AddRefs(folder));
   NS_ENSURE_SUCCESS(rv, rv);
   rv = folder->GetFilePath(getter_AddRefs(folderPath));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // file path is stored in message header property
-  nsAutoCString fileName;
-  aNewHdr->GetStringProperty("storeToken", getter_Copies(fileName));
-  if (fileName.IsEmpty())
+  // tmp filename is stored in "storeToken".
+  // By now we'll have the Message-ID, which we'll base the final filename on.
+  nsAutoCString tmpName;
+  aNewHdr->GetStringProperty("storeToken", getter_Copies(tmpName));
+  if (tmpName.IsEmpty())
   {
     NS_ERROR("FinishNewMessage - no storeToken in msg hdr!!\n");
     return NS_ERROR_FAILURE;
   }
 
   // path to the new destination
-  nsCOMPtr<nsIFile> toPath;
-  folderPath->Clone(getter_AddRefs(toPath));
-  toPath->Append(NS_LITERAL_STRING("cur"));
+  nsCOMPtr<nsIFile> curPath;
+  folderPath->Clone(getter_AddRefs(curPath));
+  curPath->Append(NS_LITERAL_STRING("cur"));
 
   // let's check if the folder exists
+  // XXX TODO: kill this and make sure maildir creation includes cur/tmp
   bool exists;
-  toPath->Exists(&exists);
+  curPath->Exists(&exists);
   if (!exists)
   {
-    rv = toPath->Create(nsIFile::DIRECTORY_TYPE, 0755);
+    rv = curPath->Create(nsIFile::DIRECTORY_TYPE, 0755);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // path to the downloaded message
   nsCOMPtr<nsIFile> fromPath;
   folderPath->Clone(getter_AddRefs(fromPath));
   fromPath->Append(NS_LITERAL_STRING("tmp"));
-  fromPath->AppendNative(fileName);
+  fromPath->AppendNative(tmpName);
 
-  // let's check if the tmp file exists
+  // Check that the message is still in tmp.
+  // XXX TODO: revisit this. I think it's needed because the
+  // pairing rules for:
+  // GetNewMsgOutputStream(), FinishNewMessage(),
+  // MoveNewlyDownloadedMessage() and DiscardNewMessage()
+  // are not well defined.
+  // If they are sorted out, this code can be removed.
   fromPath->Exists(&exists);
   if (!exists)
   {
     // Perhaps the message has already moved. See bug 1028372 to fix this.
-    toPath->AppendNative(fileName);
-    toPath->Exists(&exists);
+    nsCOMPtr<nsIFile> existingPath;
+    curPath->Clone(getter_AddRefs(existingPath));
+    existingPath->AppendNative(tmpName);
+    existingPath->Exists(&exists);
     if (exists) // then there is nothing to do
       return NS_OK;
 
     NS_ERROR("FinishNewMessage - oops! file does not exist!");
     return NS_ERROR_FILE_TARGET_DOES_NOT_EXIST;
   }
 
-  nsCOMPtr<nsIFile> existingPath;
-  toPath->Clone(getter_AddRefs(existingPath));
-  existingPath->AppendNative(fileName);
-  existingPath->Exists(&exists);
+  nsCString msgID;
+  aNewHdr->GetMessageId(getter_Copies(msgID));
 
-  if (exists) {
-    rv = existingPath->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600);
-    NS_ENSURE_SUCCESS(rv, rv);
-    existingPath->GetNativeLeafName(fileName);
-    aNewHdr->SetStringProperty("storeToken", fileName.get());
+  nsCString baseName;
+  // For missing or suspiciously-short Message-IDs, use a timestamp
+  // instead.
+  // This also avoids some special filenames we can't use in windows (CON,
+  // AUX, NUL, LPT1 etc...). With an extension (eg "LPT4.txt") they're all
+  // below 9 chars.
+  if (msgID.Length() < 9) {
+    baseName.AppendInt(static_cast<int64_t>(PR_Now()));
+  } else {
+    percentEncode(msgID, baseName);
+    // No length limit on Message-Id header, but lets clip our filenames
+    // well below any MAX_PATH limits.
+    if (baseName.Length() > (128 - 4)) {
+      baseName.SetLength(128 - 4);   // (4 for ".eml")
+    }
   }
 
-  return fromPath->MoveToNative(toPath, fileName);
+  nsCOMPtr<nsIFile> toPath;
+  curPath->Clone(getter_AddRefs(toPath));
+  nsCString toName(baseName);
+  toName.Append(".eml");
+  toPath->AppendNative(toName);
+
+  // Using CreateUnique in case we have duplicate Message-Ids
+  rv = toPath->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600);
+  if (NS_FAILED(rv)) {
+    // NS_ERROR_FILE_TOO_BIG means CreateUnique() bailed out at 10000 attempts.
+    if (rv != NS_ERROR_FILE_TOO_BIG) {
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+    // As a last resort, fall back to using timestamp as filename.
+    toName.SetLength(0);
+    toName.AppendInt(static_cast<int64_t>(PR_Now()));
+    toName.Append(".eml");
+    toPath->SetNativeLeafName(toName);
+    rv = toPath->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  // Move into place (using whatever name CreateUnique() settled upon).
+  toPath->GetNativeLeafName(toName);
+  rv = fromPath->MoveToNative(curPath, toName);
+  NS_ENSURE_SUCCESS(rv, rv);
+  // Update the db to reflect the final filename.
+  aNewHdr->SetStringProperty("storeToken", toName.get());
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMsgMaildirStore::MoveNewlyDownloadedMessage(nsIMsgDBHdr *aHdr,
                                               nsIMsgFolder *aDestFolder,
                                               bool *aResult)
 {
   NS_ENSURE_ARG_POINTER(aHdr);
@@ -781,16 +863,17 @@ nsMsgMaildirStore::MoveNewlyDownloadedMe
 
   // move to the "cur" subfolder
   nsCOMPtr<nsIFile> toPath;
   aDestFolder->GetFilePath(getter_AddRefs(folderPath));
   folderPath->Clone(getter_AddRefs(toPath));
   toPath->Append(NS_LITERAL_STRING("cur"));
 
   // let's check if the folder exists
+  // XXX TODO: kill this and make sure maildir creation includes cur/tmp
   toPath->Exists(&exists);
   if (!exists)
   {
     rv = toPath->Create(nsIFile::DIRECTORY_TYPE, 0755);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   nsCOMPtr<nsIMsgDatabase> destMailDB;
@@ -907,16 +990,17 @@ nsMsgMaildirStore::GetMsgInputStream(nsI
     MOZ_LOG(MailDirLog, mozilla::LogLevel::Info,
            ("GetMsgInputStream - empty storeToken!!\n"));
     return NS_ERROR_FAILURE;
   }
 
   path->Append(NS_LITERAL_STRING("cur"));
 
   // let's check if the folder exists
+  // XXX TODO: kill this and make sure maildir creation includes cur/tmp
   bool exists;
   path->Exists(&exists);
   if (!exists) {
     MOZ_LOG(MailDirLog, mozilla::LogLevel::Info,
            ("GetMsgInputStream - oops! cur subfolder does not exist!\n"));
     rv = path->Create(nsIFile::DIRECTORY_TYPE, 0755);
     NS_ENSURE_SUCCESS(rv, rv);
   }