Bug 1466343 - fix NS_MsgStripRE(). r=mkmelin a=jorgk
authorJorg K <jorgk@jorgk.com>
Sun, 03 Jun 2018 23:29:12 +0200
changeset 31577 7318bc0281a5a8e1d339f64286d7e66bcd2da4b3
parent 31576 580ea0b9e5e1e982de74fe76146080d3a435143f
child 31578 6badc1f1ac53aa58e7ab1d60be43cef77f76510c
push id21
push usermozilla@hocat.ca
push dateFri, 13 Jul 2018 19:54:19 +0000
treeherdercomm-esr60@3c593339cb1d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin, jorgk
bugs1466343
Bug 1466343 - fix NS_MsgStripRE(). r=mkmelin a=jorgk
mailnews/base/util/nsMsgUtils.cpp
mailnews/base/util/nsMsgUtils.h
mailnews/local/src/nsParseMailbox.cpp
mailnews/news/src/nsNNTPNewsgroupList.cpp
--- a/mailnews/base/util/nsMsgUtils.cpp
+++ b/mailnews/base/util/nsMsgUtils.cpp
@@ -615,143 +615,116 @@ nsresult NS_MsgCreatePathStringFromFolde
       endSlashPos = oldPath.Length();
 
     if (startSlashPos >= endSlashPos)
       break;
   }
   return NS_CopyUnicodeToNative(path, aPathCString);
 }
 
-bool NS_MsgStripRE(const char **stringP, uint32_t *lengthP, char **modifiedSubject)
+bool NS_MsgStripRE(const nsCString& subject, nsCString& modifiedSubject)
 {
-  const char *s, *s_end;
-  uint32_t L;
   bool result = false;
-  NS_ASSERTION(stringP, "bad null param");
-  if (!stringP) return false;
 
-  // get localizedRe pref
+  // Get localizedRe pref.
   nsresult rv;
   nsString utf16LocalizedRe;
   NS_GetLocalizedUnicharPreferenceWithDefault(nullptr,
                                               "mailnews.localizedRe",
                                               EmptyString(),
                                               utf16LocalizedRe);
   NS_ConvertUTF16toUTF8 localizedRe(utf16LocalizedRe);
 
-  // hardcoded "Re" so that no one can configure Mozilla standards incompatible
+  // Hardcoded "Re" so that no one can configure Mozilla standards incompatible.
   nsAutoCString checkString("Re,RE,re,rE");
   if (!localizedRe.IsEmpty()) {
     checkString.Append(',');
     checkString.Append(localizedRe);
   }
 
-  // decode the string
+  // Decode the string.
   nsCString decodedString;
   nsCOMPtr<nsIMimeConverter> mimeConverter;
-  // we cannot strip "Re:" for MIME encoded subject without modifying the original
-  if (modifiedSubject && strstr(*stringP, "=?"))
+  // We cannot strip "Re:" for RFC2047-encoded subject without modifying the original.
+  if (subject.Find("=?") != kNotFound)
   {
     mimeConverter = do_GetService(NS_MIME_CONVERTER_CONTRACTID, &rv);
     if (NS_SUCCEEDED(rv))
-      rv = mimeConverter->DecodeMimeHeaderToUTF8(nsDependentCString(*stringP),
+      rv = mimeConverter->DecodeMimeHeaderToUTF8(subject,
         nullptr, false, true, decodedString);
   }
 
-  s = !decodedString.IsEmpty() ? decodedString.get() : *stringP;
-  L = lengthP ? *lengthP : strlen(s);
-
-  s_end = s + L;
+  const char *s, *s_end;
+  if (decodedString.IsEmpty()) {
+    s = subject.BeginReading();
+    s_end = s + subject.Length();
+  } else {
+    s = decodedString.BeginReading();
+    s_end = s + decodedString.Length();
+  }
 
- AGAIN:
-
+AGAIN:
   while (s < s_end && IS_SPACE(*s))
-  s++;
+    s++;
 
   const char *tokPtr = checkString.get();
   while (*tokPtr)
   {
-    //tokenize the comma separated list
+    // Tokenize the comma separated list.
     size_t tokenLength = 0;
     while (*tokPtr && *tokPtr != ',') {
       tokenLength++;
       tokPtr++;
     }
-    //check if the beginning of s is the actual token
+    // Check if the beginning of s is the actual token.
     if (tokenLength && !strncmp(s, tokPtr - tokenLength, tokenLength))
     {
       if (s[tokenLength] == ':')
       {
         s = s + tokenLength + 1; /* Skip over "Re:" */
-        result = true;        /* Yes, we stripped it. */
+        result = true;           /* Yes, we stripped it. */
         goto AGAIN;              /* Skip whitespace and try again. */
       }
       else if (s[tokenLength] == '[' || s[tokenLength] == '(')
       {
         const char *s2 = s + tokenLength + 1; /* Skip over "Re[" */
 
-        /* Skip forward over digits after the "[". */
+        // Skip forward over digits after the "[".
         while (s2 < (s_end - 2) && isdigit((unsigned char)*s2))
           s2++;
 
-        /* Now ensure that the following thing is "]:"
-           Only if it is do we alter `s'. */
+        // Now ensure that the following thing is "]:".
+        // Only if it is do we alter `s`.
         if ((s2[0] == ']' || s2[0] == ')') && s2[1] == ':')
         {
           s = s2 + 2;       /* Skip over "]:" */
-          result = true; /* Yes, we stripped it. */
+          result = true;    /* Yes, we stripped it. */
           goto AGAIN;       /* Skip whitespace and try again. */
         }
       }
     }
     if (*tokPtr)
       tokPtr++;
   }
 
-  if (!decodedString.IsEmpty())
-  {
-    // encode the string back if any modification is made
-    if (s != decodedString.get())
-    {
-      // extract between "=?" and "?"
-      // e.g. =?ISO-2022-JP?
-      const char *p1 = strstr(*stringP, "=?");
-      if (p1)
-      {
-        p1 += sizeof("=?")-1;         // skip "=?"
-        const char *p2 = strchr(p1, '?');   // then search for '?'
-        if (p2)
-        {
-          char charset[nsIMimeConverter::MAX_CHARSET_NAME_LENGTH] = "";
-          if (nsIMimeConverter::MAX_CHARSET_NAME_LENGTH >= (p2 - p1))
-            strncpy(charset, p1, p2 - p1);
-          nsAutoCString encodedString;
-          rv = mimeConverter->EncodeMimePartIIStr_UTF8(nsDependentCString(s),
-            false, "UTF-8", sizeof("Subject:"),
-            nsIMimeConverter::MIME_ENCODED_WORD_SIZE, encodedString);
-          if (NS_SUCCEEDED(rv))
-          {
-            *modifiedSubject = PL_strdup(encodedString.get());
-            return result;
-          }
-        }
-      }
-    }
-    else
-      s = *stringP;   // no modification, set the original encoded string
+  // If we didn't strip anything, we can return here.
+  if (!result)
+    return false;
+
+  if (decodedString.IsEmpty()) {
+    // We didn't decode anything, so just return a new string.
+    modifiedSubject.Assign(s);
+    return true;
   }
 
-
-  /* Decrease length by difference between current ptr and original ptr.
-   Then store the current ptr back into the caller. */
-  if (lengthP)
-    *lengthP -= (s - (*stringP));
-  *stringP = s;
-
-  return result;
+  // We decoded the string, so we need to encode it again. We always encode in UTF-8.
+  mimeConverter->EncodeMimePartIIStr_UTF8(nsDependentCString(s),
+    false, "UTF-8", sizeof("Subject:"),
+    nsIMimeConverter::MIME_ENCODED_WORD_SIZE, modifiedSubject);
+  return true;
 }
 
 /*  Very similar to strdup except it free's too
  */
 char * NS_MsgSACopy (char **destination, const char *source)
 {
   if(*destination)
   {
--- a/mailnews/base/util/nsMsgUtils.h
+++ b/mailnews/base/util/nsMsgUtils.h
@@ -86,25 +86,19 @@ NS_MsgCreatePathStringFromFolderURI(cons
 /**
  * Given a string and a length, removes any "Re:" strings from the front.
  * It also deals with that dumbass "Re[2]:" thing that some losing mailers do.
  *
  * If mailnews.localizedRe is set, it will also remove localized "Re:" strings.
  *
  * @return true if it made a change (in which case the caller should look to
  *         modifiedSubject for the result) and false otherwise (in which
- *         case the caller should look at stringp/length for the result)
- *
- * @note In the case of a true return value, the string is not altered:
- *       the pointer to its head is merely advanced, and the length
- *       correspondingly decreased.
- *
- * @note This API is insane and should be fixed.
+ *         case the caller should look at subject for the result)
  */
-NS_MSG_BASE bool NS_MsgStripRE(const char **stringP, uint32_t *lengthP, char **modifiedSubject=nullptr);
+NS_MSG_BASE bool NS_MsgStripRE(const nsCString& subject, nsCString& modifiedSubject);
 
 NS_MSG_BASE char * NS_MsgSACopy(char **destination, const char *source);
 
 NS_MSG_BASE char * NS_MsgSACat(char **destination, const char *source);
 
 NS_MSG_BASE nsresult NS_MsgEscapeEncodeURLPath(const nsAString& aStr,
                                                nsCString& aResult);
 
--- a/mailnews/local/src/nsParseMailbox.cpp
+++ b/mailnews/local/src/nsParseMailbox.cpp
@@ -1211,74 +1211,45 @@ nsresult nsParseMailMessageState::ParseE
 
   /* #### short-circuit const */
   ((char *) m_envelope_from.value) [m_envelope_from.length] = 0;
   ((char *) m_envelope_date.value) [m_envelope_date.length] = 0;
 
   return NS_OK;
 }
 
-#ifdef WE_CONDENSE_MIME_STRINGS
-static char *
-msg_condense_mime2_string(char *sourceStr)
-{
-  char *returnVal = strdup(sourceStr);
-  if (!returnVal)
-    return nullptr;
-
-  MIME_StripContinuations(returnVal);
-
-  return returnVal;
-}
-#endif // WE_CONDENSE_MIME_STRINGS
-
 nsresult nsParseMailMessageState::InternSubject (struct message_header *header)
 {
-  char *key;
-  uint32_t L;
-
   if (!header || header->length == 0)
   {
     m_newMsgHdr->SetSubject("");
     return NS_OK;
   }
 
-  key = (char *) header->value;  /* #### const evilness */
-
-  L = header->length;
-
+  const char *key = header->value;
 
   uint32_t flags;
   (void)m_newMsgHdr->GetFlags(&flags);
   /* strip "Re: " */
   /**
         We trust the X-Mozilla-Status line to be the smartest in almost
         all things.  One exception, however, is the HAS_RE flag.  Since
          we just parsed the subject header anyway, we expect that parsing
          to be smartest.  (After all, what if someone just went in and
         edited the subject line by hand?)
      */
   nsCString modifiedSubject;
-  if (NS_MsgStripRE((const char **) &key, &L, getter_Copies(modifiedSubject)))
+  if (NS_MsgStripRE(nsDependentCString(key), modifiedSubject))
     flags |= nsMsgMessageFlags::HasRe;
   else
     flags &= ~nsMsgMessageFlags::HasRe;
   m_newMsgHdr->SetFlags(flags); // this *does not* update the mozilla-status header in the local folder
 
-  //  if (!*key) return 0; /* To catch a subject of "Re:" */
-
   // Condense the subject text into as few MIME-2 encoded words as possible.
-#ifdef WE_CONDENSE_MIME_STRINGS
-  char *condensedKey = msg_condense_mime2_string(modifiedSubject.IsEmpty() ? key : modifiedSubject.get());
-#else
-  char *condensedKey = nullptr;
-#endif
-  m_newMsgHdr->SetSubject(condensedKey ? condensedKey :
-  (modifiedSubject.IsEmpty() ? key : modifiedSubject.get()));
-  PR_FREEIF(condensedKey);
+  m_newMsgHdr->SetSubject(modifiedSubject.IsEmpty() ? key : modifiedSubject.get());
 
   return NS_OK;
 }
 
 // we've reached the end of the envelope, and need to turn all our accumulated message_headers
 // into a single nsIMsgDBHdr to store in a database.
 nsresult nsParseMailMessageState::FinalizeHeaders()
 {
--- a/mailnews/news/src/nsNNTPNewsgroupList.cpp
+++ b/mailnews/news/src/nsNNTPNewsgroupList.cpp
@@ -528,23 +528,22 @@ nsNNTPNewsgroupList::ParseLine(char *lin
 
   NS_ASSERTION(newMsgHdr, "CreateNewHdr didn't fail, but it returned a null newMsgHdr");
   if (!newMsgHdr)
     return NS_ERROR_NULL_POINTER;
 
   GET_TOKEN (); /* subject */
   if (line) {
     const char *subject = line;  /* #### const evilness */
-    uint32_t subjectLen = strlen(line);
 
     uint32_t flags = 0;
     // ### should call IsHeaderRead here...
     /* strip "Re: " */
     nsCString modifiedSubject;
-    if (NS_MsgStripRE(&subject, &subjectLen, getter_Copies(modifiedSubject)))
+    if (NS_MsgStripRE(nsDependentCString(subject), modifiedSubject))
       (void) newMsgHdr->OrFlags(nsMsgMessageFlags::HasRe, &flags);
 
     // this will make sure read flags agree with newsrc
     if (! (flags & nsMsgMessageFlags::Read))
       rv = newMsgHdr->OrFlags(nsMsgMessageFlags::New, &flags);
 
     rv = newMsgHdr->SetSubject(modifiedSubject.IsEmpty() ? subject : modifiedSubject.get());
 
@@ -1048,23 +1047,22 @@ nsNNTPNewsgroupList::AddHeader(const cha
     PRTime date;
     PRStatus status = PR_ParseTimeString (value, false, &date);
     if (PR_SUCCESS == status)
       rv = m_newMsgHdr->SetDate(date);
   }
   else if (PL_strcmp(header, "subject") == 0)
   {
     const char *subject = value;
-    uint32_t subjectLen = strlen(value);
 
     uint32_t flags = 0;
     // ### should call IsHeaderRead here...
     /* strip "Re: " */
     nsCString modifiedSubject;
-    if (NS_MsgStripRE(&subject, &subjectLen, getter_Copies(modifiedSubject)))
+    if (NS_MsgStripRE(nsDependentCString(subject), modifiedSubject))
       // this will make sure read flags agree with newsrc
      (void) m_newMsgHdr->OrFlags(nsMsgMessageFlags::HasRe, &flags);
 
     if (! (flags & nsMsgMessageFlags::Read))
       rv = m_newMsgHdr->OrFlags(nsMsgMessageFlags::New, &flags);
 
     rv = m_newMsgHdr->SetSubject(modifiedSubject.IsEmpty() ? subject :
       modifiedSubject.get());