Bug 1054252 - wrong values in From and Recipient columns in message list due to improper RFC 2047 decoding / parsing order r=kent
authorRadics Péter <mitchnull@gmail.com>
Sat, 18 Oct 2014 09:33:00 -0400
changeset 16753 0b9f01eb00e7c925f6c7961325d5b90b7b0bfb88
parent 16752 e074346e8a12e698ca2bcf5e2dc40714c8e8fe02
child 16754 ba812ce4e6f4adcc7099be4dcff2b4179d82da36
push id1274
push usermbanner@mozilla.com
push dateMon, 12 Jan 2015 19:54:49 +0000
treeherdercomm-esr52@3d3ef8b6c3b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskent
bugs1054252
Bug 1054252 - wrong values in From and Recipient columns in message list due to improper RFC 2047 decoding / parsing order r=kent
mailnews/base/public/nsIMsgHdr.idl
mailnews/base/src/nsMsgDBView.cpp
mailnews/base/test/unit/test_nsMsgDBView_headerValues.js
mailnews/base/test/unit/xpcshell.ini
mailnews/db/msgdb/public/nsMsgDatabase.h
mailnews/db/msgdb/src/nsMsgDatabase.cpp
mailnews/db/msgdb/src/nsMsgHdr.cpp
--- a/mailnews/base/public/nsIMsgHdr.idl
+++ b/mailnews/base/public/nsIMsgHdr.idl
@@ -5,17 +5,17 @@
 
 #include "nsISupports.idl"
 
 #include "MailNewsTypes2.idl"
 
 interface nsIMsgFolder;
 interface nsIUTF8StringEnumerator;
 
-[scriptable, uuid(ec1053a9-1a3d-4572-8f47-c103f2c014fb)]
+[scriptable, uuid(3c11ddbe-c805-40c5-b9c9-d065fad5d0be)]
 interface nsIMsgDBHdr : nsISupports
 {
     /* general property routines - I think this can retrieve any
        header in the db */
     AString getProperty(in string propertyName);
     void setProperty(in string propertyName, in AString propertyStr);
     void setStringProperty(in string propertyName, in string propertyValue);
     string getStringProperty(in string propertyName);
@@ -81,16 +81,23 @@ interface nsIMsgDBHdr : nsISupports
     void getAuthorCollationKey(out unsigned long aCount,
                               [array, size_is(aCount)] out octet aKey);
     void getSubjectCollationKey(out unsigned long aCount,
                                 [array, size_is(aCount)] out octet aKey);
     void getRecipientsCollationKey(out unsigned long aCount,
                                   [array, size_is(aCount)] out octet aKey);
 
     attribute string Charset;
+    /**
+     * Returns the effective character set for the message (@ref Charset).
+     * If there is no specific set defined for the message or the
+     * characterSetOverride option is turned on for the folder it will return
+     * the effective character set of the folder instead.
+     */
+    readonly attribute ACString effectiveCharset;
     attribute nsMsgLabelValue label;
     attribute string accountKey;
     readonly attribute nsIMsgFolder folder;
 
     /// Enumerator for names of all database properties in the header.
     readonly attribute nsIUTF8StringEnumerator propertyEnumerator;
 };
 /* *******************************************************************************/
--- a/mailnews/base/src/nsMsgDBView.cpp
+++ b/mailnews/base/src/nsMsgDBView.cpp
@@ -389,22 +389,26 @@ nsresult nsMsgDBView::FetchAuthor(nsIMsg
     GetCachedName(unparsedAuthor, currentDisplayNameVersion, cachedDisplayName);
     if (!cachedDisplayName.IsEmpty())
     {
       CopyUTF8toUTF16(cachedDisplayName, aSenderString);
       return NS_OK;
     }
   }
 
-  nsString author;
-  nsresult rv = aHdr->GetMime2DecodedAuthor(author);
+  nsCString author;
+  nsresult rv = aHdr->GetAuthor(getter_Copies(author));
+
+  nsCString headerCharset;
+  aHdr->GetEffectiveCharset(headerCharset);
 
   nsCString emailAddress;
   nsString name;
-  ExtractFirstAddress(DecodedHeader(author), name, emailAddress);
+  ExtractFirstAddress(EncodedHeader(author, headerCharset.get()), name,
+    emailAddress);
 
   if (showCondensedAddresses)
     GetDisplayNameInAddressBook(emailAddress, aSenderString);
 
   if (aSenderString.IsEmpty())
   {
     // we can't use the display name in the card,
     // use the name contained in the header or email address.
@@ -449,17 +453,16 @@ nsresult nsMsgDBView::FetchAccount(nsIMs
     server->GetPrettyName(aAccount);
   else
     CopyASCIItoUTF16(accountKey, aAccount);
   return NS_OK;
 }
 
 nsresult nsMsgDBView::FetchRecipients(nsIMsgDBHdr * aHdr, nsAString &aRecipientsString)
 {
-  nsString unparsedRecipients;
   nsCString recipients;
   int32_t currentDisplayNameVersion = 0;
   bool showCondensedAddresses = false;
   nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID));
 
   prefs->GetIntPref("mail.displayname.version", &currentDisplayNameVersion);
   prefs->GetBoolPref("mail.showCondensedAddresses", &showCondensedAddresses);
 
@@ -475,21 +478,26 @@ nsresult nsMsgDBView::FetchRecipients(ns
     // was changed after cache.
     if (!cachedRecipients.IsEmpty())
     {
       CopyUTF8toUTF16(cachedRecipients, aRecipientsString);
       return NS_OK;
     }
   }
 
-  nsresult rv = aHdr->GetMime2DecodedRecipients(unparsedRecipients);
+  nsCString unparsedRecipients;
+  nsresult rv = aHdr->GetRecipients(getter_Copies(unparsedRecipients));
+
+  nsCString headerCharset;
+  aHdr->GetEffectiveCharset(headerCharset);
+
   nsTArray<nsString> names;
   nsTArray<nsCString> emails;
-  ExtractAllAddresses(DecodedHeader(unparsedRecipients), names,
-    UTF16ArrayAdapter<>(emails));
+  ExtractAllAddresses(EncodedHeader(unparsedRecipients, headerCharset.get()),
+    names, UTF16ArrayAdapter<>(emails));
 
   uint32_t numAddresses = names.Length();
 
   nsCOMPtr<nsISimpleEnumerator> enumerator;
   nsCOMPtr<nsIAbManager>
     abManager(do_GetService("@mozilla.org/abmanager;1", &rv));
   NS_ENSURE_SUCCESS(rv, NS_OK);
 
new file mode 100644
--- /dev/null
+++ b/mailnews/base/test/unit/test_nsMsgDBView_headerValues.js
@@ -0,0 +1,71 @@
+/* Test that nsMsgDBView properly reports the values of messages in the display.
+ */
+load("../../../resources/logHelper.js");
+load("../../../resources/asyncTestUtils.js");
+load("../../../resources/abSetup.js");
+
+load("../../../resources/messageGenerator.js");
+load("../../../resources/messageModifier.js");
+load("../../../resources/messageInjection.js");
+
+var ViewType = Components.interfaces.nsMsgViewType;
+var SortType = Components.interfaces.nsMsgViewSortType;
+var SortOrder = Components.interfaces.nsMsgViewSortOrder;
+
+// This is an array of the actual test data. Each test datum is an array of two
+// elements: the first element is the argument into a simple message generator,
+// and the second element is a map of column names to expected values when
+// requesting the cell text for a given column name.
+let tests = [
+  [{from: "John Doe <db@tinderbox.invalid>"}, {sender: "John Doe"}],
+  [{from: "\"Doe, John\" <db@tinderbox.invalid>"}, {sender: "Doe, John"}],
+  [{from: "John Doe <db@tinderbox.invalid>, Sally Ann <db@null.invalid>"},
+    {sender: "John Doe"}],
+  [{from: "=?UTF-8?Q?David_H=C3=A5s=C3=A4ther?= <db@null.invalid>"},
+    {sender: "David Håsäther"}],
+  [{from: "=?UTF-8?Q?H=C3=A5s=C3=A4ther=2C_David?= <db@null.invalid>"},
+    {sender: "Håsäther, David"}],
+  [{from: "John Doe \xF5  <db@null.invalid>",
+     clobberHeaders: { "Content-type" : "text/plain; charset=ISO-8859-1" }},
+	{sender: "John Doe õ"}],
+  [{from: "John Doe \xF5 <db@null.invalid>",
+     clobberHeaders: { "Content-type" : "text/plain; charset=ISO-8859-2" }},
+	{sender: "John Doe ő"}],
+  [{from: "=?UTF-8?Q?H=C3=A5s=C3=A4ther=2C_David?= <db@null.invalid>",
+     clobberHeaders: { "Content-type" : "text/plain; charset=ISO-8859-2" }},
+    {sender: "Håsäther, David"}],
+];
+
+function real_test() {
+  // Add the messages to the folder
+  let msgGenerator = new MessageGenerator();
+  let genMessages = tests.map(data => msgGenerator.makeMessage(data[0]));
+  let folder = make_empty_folder();
+  yield add_sets_to_folder(folder, [new SyntheticMessageSet(genMessages)]);
+
+  // Make the DB view
+  let dbviewContractId = "@mozilla.org/messenger/msgdbview;1?type=threaded";
+  let dbView = Components.classes[dbviewContractId]
+                         .createInstance(Components.interfaces.nsIMsgDBView);
+  dbView.init(null, null, null);
+  var outCount = {};
+  dbView.open(folder, SortType.byDate, SortOrder.ascending, 0, outCount);
+
+  // Did we add all the messages properly?
+  let treeView = dbView.QueryInterface(Components.interfaces.nsITreeView);
+  do_check_eq(treeView.rowCount, tests.length);
+
+  // For each test, make sure that the display is correct.
+  tests.forEach(function (data, i) {
+    do_print("Checking data for " + uneval(data));
+    let expected = data[1];
+    for (let column in expected) {
+      do_check_eq(dbView.cellTextForColumn(i, column), expected[column]);
+    }
+  });
+}
+
+function run_test() {
+  configure_message_injection({mode: "local"});
+  async_run_tests([real_test]);
+}
--- a/mailnews/base/test/unit/xpcshell.ini
+++ b/mailnews/base/test/unit/xpcshell.ini
@@ -44,16 +44,17 @@ support-files = nodelist_test.xml data/*
 skip-if = os != 'mac'
 
 [test_nsIMsgFolder.js]
 [test_nsIMsgFolderListener.js]
 [test_nsIMsgFolderListenerLocal.js]
 [test_nsIMsgTagService.js]
 [test_nsMailDirProvider.js]
 [test_nsMsgDBView.js]
+[test_nsMsgDBView_headerValues.js]
 [test_nsMsgMailSession_Alerts.js]
 [test_nsMsgMailSession_Listeners.js]
 [test_nsMsgTraitService.js]
 [test_postPluginFilters.js]
 [test_quarantineFilterMove.js]
 [test_retention.js]
 [test_search.js]
 [test_searchAddressInAb.js]
--- a/mailnews/db/msgdb/public/nsMsgDatabase.h
+++ b/mailnews/db/msgdb/public/nsMsgDatabase.h
@@ -189,16 +189,18 @@ public:
   nsresult RowCellColumnToUInt32(nsIMdbRow *row, mdb_token columnToken, uint32_t *uint32Result, uint32_t defaultValue = 0);
   nsresult RowCellColumnToUInt32(nsIMdbRow *row, mdb_token columnToken, uint32_t &uint32Result, uint32_t defaultValue = 0);
   nsresult RowCellColumnToUInt64(nsIMdbRow *row, mdb_token columnToken, uint64_t *uint64Result, uint64_t defaultValue = 0);
   nsresult RowCellColumnToMime2DecodedString(nsIMdbRow *row, mdb_token columnToken, nsAString &resultStr);
   nsresult RowCellColumnToCollationKey(nsIMdbRow *row, mdb_token columnToken, uint8_t **result, uint32_t *len);
   nsresult RowCellColumnToConstCharPtr(nsIMdbRow *row, mdb_token columnToken, const char **ptr);
   nsresult RowCellColumnToAddressCollationKey(nsIMdbRow *row, mdb_token colToken, uint8_t **result, uint32_t *len);
 
+  nsresult GetEffectiveCharset(nsIMdbRow *row, nsACString &resultCharset);
+
   // these methods take the property name as a string, not a token.
   // they should be used when the properties aren't accessed a lot
   nsresult        GetProperty(nsIMdbRow *row, const char *propertyName, char **result);
   nsresult        SetProperty(nsIMdbRow *row, const char *propertyName, const char *propertyVal);
   nsresult        GetPropertyAsNSString(nsIMdbRow *row, const char *propertyName, nsAString &result);
   nsresult        SetPropertyFromNSString(nsIMdbRow *row, const char *propertyName, const nsAString &propertyVal);
   nsresult        GetUint32Property(nsIMdbRow *row, const char *propertyName, uint32_t *result, uint32_t defaultValue = 0);
   nsresult        SetUint32Property(nsIMdbRow *row, const char *propertyName, uint32_t propertyVal);
--- a/mailnews/db/msgdb/src/nsMsgDatabase.cpp
+++ b/mailnews/db/msgdb/src/nsMsgDatabase.cpp
@@ -3591,39 +3591,46 @@ nsIMimeConverter *nsMsgDatabase::GetMime
   if (!m_mimeConverter)
   {
     // apply mime decode
     m_mimeConverter = do_GetService(NS_MIME_CONVERTER_CONTRACTID);
   }
   return m_mimeConverter;
 }
 
+nsresult nsMsgDatabase::GetEffectiveCharset(nsIMdbRow *row, nsACString &resultCharset)
+{
+  resultCharset.Truncate();
+  bool characterSetOverride;
+  m_dbFolderInfo->GetCharacterSetOverride(&characterSetOverride);
+  nsresult rv = RowCellColumnToCharPtr(row, m_messageCharSetColumnToken, getter_Copies(resultCharset));
+  if (NS_FAILED(rv) || resultCharset.IsEmpty() ||
+      resultCharset.Equals("us-ascii") || characterSetOverride)
+  {
+    rv = m_dbFolderInfo->GetEffectiveCharacterSet(resultCharset);
+  }
+  return rv;
+}
+
 nsresult nsMsgDatabase::RowCellColumnToMime2DecodedString(nsIMdbRow *row, mdb_token columnToken, nsAString &resultStr)
 {
   nsresult err = NS_OK;
   const char *nakedString = nullptr;
   err = RowCellColumnToConstCharPtr(row, columnToken, &nakedString);
   if (NS_SUCCEEDED(err) && nakedString && strlen(nakedString))
   {
     GetMimeConverter();
     if (m_mimeConverter)
     {
       nsAutoString decodedStr;
       nsCString charSet;
-      bool characterSetOverride;
-      m_dbFolderInfo->GetCharacterSetOverride(&characterSetOverride);
-      err = RowCellColumnToCharPtr(row, m_messageCharSetColumnToken, getter_Copies(charSet));
-      if (NS_FAILED(err) || charSet.IsEmpty() || charSet.Equals("us-ascii") ||
-          characterSetOverride)
-      {
-        m_dbFolderInfo->GetEffectiveCharacterSet(charSet);
-      }
+      GetEffectiveCharset(row, charSet);
 
       err = m_mimeConverter->DecodeMimeHeader(nakedString, charSet.get(),
-        characterSetOverride, true, resultStr);
+        false, true, resultStr);
     }
   }
   return err;
 }
 
 nsresult nsMsgDatabase::RowCellColumnToAddressCollationKey(nsIMdbRow *row, mdb_token colToken, uint8_t **result, uint32_t *len)
 {
   nsString sender;
@@ -3677,27 +3684,20 @@ nsresult nsMsgDatabase::RowCellColumnToC
     nakedString = "";
   if (NS_SUCCEEDED(err))
   {
     GetMimeConverter();
     if (m_mimeConverter)
     {
       nsCString decodedStr;
       nsCString charSet;
-      bool characterSetOverride;
-      m_dbFolderInfo->GetCharacterSetOverride(&characterSetOverride);
-      err = RowCellColumnToCharPtr(row, m_messageCharSetColumnToken, getter_Copies(charSet));
-      if (NS_FAILED(err) || charSet.IsEmpty() || charSet.Equals("us-ascii") ||
-          characterSetOverride)
-      {
-        m_dbFolderInfo->GetEffectiveCharacterSet(charSet);
-      }
+      GetEffectiveCharset(row, charSet);
 
       err = m_mimeConverter->DecodeMimeHeaderToUTF8(
-        nsDependentCString(nakedString), charSet.get(), characterSetOverride,
+        nsDependentCString(nakedString), charSet.get(), false,
         true, decodedStr);
       if (NS_SUCCEEDED(err))
         err = CreateCollationKey(NS_ConvertUTF8toUTF16(decodedStr), len, result);
     }
   }
   return err;
 }
 
--- a/mailnews/db/msgdb/src/nsMsgHdr.cpp
+++ b/mailnews/db/msgdb/src/nsMsgHdr.cpp
@@ -621,16 +621,21 @@ NS_IMETHODIMP nsMsgHdr::GetCharset(char 
   return m_mdb->RowCellColumnToCharPtr(GetMDBRow(), m_mdb->m_messageCharSetColumnToken, aCharset);
 }
 
 NS_IMETHODIMP nsMsgHdr::SetCharset(const char *aCharset)
 {
   return SetStringColumn(aCharset, m_mdb->m_messageCharSetColumnToken);
 }
 
+NS_IMETHODIMP nsMsgHdr::GetEffectiveCharset(nsACString &resultCharset)
+{
+  return m_mdb->GetEffectiveCharset(m_mdbRow, resultCharset);
+}
+
 NS_IMETHODIMP nsMsgHdr::SetThreadParent(nsMsgKey inKey)
 {
   m_threadParent = inKey;
   if (inKey == m_messageKey)
     NS_ASSERTION(false, "can't be your own parent");
   SetUInt32Column(m_threadParent, m_mdb->m_threadParentColumnToken);
   m_initedValues |= THREAD_PARENT_INITED;
   return NS_OK;