Bug 1572864 - Use common function to remove query part of URL in nsMsgCompose::IsEmbeddedObjectSafe(). r=aceman a=jorgk
authorJorg K <jorgk@jorgk.com>
Mon, 19 Aug 2019 22:08:15 +0200
changeset 36183 343f09115e69ba55ea5fded11339f8c79dbbdfdd
parent 36182 15cdc4e6dcb7e0bd5014cd1ee77e061062b55d7c
child 36184 60cb0003b0ce316627e6b4c3b31e280e84f38c44
push id392
push userclokep@gmail.com
push dateMon, 02 Sep 2019 20:17:19 +0000
reviewersaceman, jorgk
bugs1572864
Bug 1572864 - Use common function to remove query part of URL in nsMsgCompose::IsEmbeddedObjectSafe(). r=aceman a=jorgk * * * Bug 1572864 - Follow-up: Remove query from base URL as well. r=aceman * * * Bug 1572864 - Follow-up: Add comment. r=aceman
mailnews/base/util/nsMsgUtils.cpp
mailnews/base/util/nsMsgUtils.h
mailnews/compose/src/nsMsgCompose.cpp
mailnews/imap/src/nsImapProtocol.cpp
mailnews/imap/src/nsImapUrl.cpp
mailnews/jsaccount/src/JaUrl.cpp
mailnews/local/src/nsMailboxUrl.cpp
--- a/mailnews/base/util/nsMsgUtils.cpp
+++ b/mailnews/base/util/nsMsgUtils.cpp
@@ -1959,8 +1959,25 @@ nsCString MsgExtractQueryPart(const nsAC
     // Nothing follows, so return from where the query qualifier started.
     queryPart.Assign(Substring(spec, queryIndex));
   } else {
     // Return the substring that represents the query qualifier.
     queryPart.Assign(Substring(spec, queryIndex, queryEnd - queryIndex));
   }
   return queryPart;
 }
+
+// Helper function to remove query part from URL spec or path.
+void MsgRemoveQueryPart(nsCString &aSpec) {
+  // Sadly the query part can have different forms, these were seen
+  // "in the wild", even with two ?:
+  // /;section=2?part=1.2&filename=A01.JPG
+  // ?section=2?part=1.2&filename=A01.JPG&type=image/jpeg&filename=A01.JPG
+  // ?header=quotebody/;section=2.2?part=1.2.2&filename=lijbmghmkilicioj.png
+  // ?part=1.2&type=image/jpeg&filename=IMG_C0030.jpg
+  // ?header=quotebody&part=1.2&filename=lijbmghmkilicioj.png
+
+  // Truncate path at the first of /; or ?
+  int32_t ind = aSpec.FindChar('?');
+  if (ind != kNotFound) aSpec.SetLength(ind);
+  ind = aSpec.Find("/;");
+  if (ind != kNotFound) aSpec.SetLength(ind);
+}
--- a/mailnews/base/util/nsMsgUtils.h
+++ b/mailnews/base/util/nsMsgUtils.h
@@ -390,16 +390,20 @@ NS_MSG_BASE nsMsgKey msgKeyFromInt(uint6
 
 NS_MSG_BASE uint32_t msgKeyToInt(nsMsgKey aMsgKey);
 
 /**
  * Helper function to extract query part from URL spec.
  */
 nsCString MsgExtractQueryPart(const nsACString &spec,
                               const char *queryToExtract);
+/**
+ * Helper function to remove query part from URL spec or path.
+ */
+void MsgRemoveQueryPart(nsCString &aSpec);
 
 /**
  * Helper macro for defining getter/setters. Ported from nsISupportsObsolete.h
  */
 #define NS_IMPL_GETSET(clazz, attr, type, member) \
   NS_IMETHODIMP clazz::Get##attr(type *result) {  \
     NS_ENSURE_ARG_POINTER(result);                \
     *result = member;                             \
--- a/mailnews/compose/src/nsMsgCompose.cpp
+++ b/mailnews/compose/src/nsMsgCompose.cpp
@@ -260,21 +260,32 @@ bool nsMsgCompose::IsEmbeddedObjectSafe(
         rv = uri->GetAsciiHost(host);
         // mailbox url don't have a host therefore don't be too strict.
         if (NS_SUCCEEDED(rv) &&
             (host.IsEmpty() || originalHost ||
              host.Equals(originalHost, nsCaseInsensitiveCStringComparator()))) {
           nsAutoCString path;
           rv = uri->GetPathQueryRef(path);
           if (NS_SUCCEEDED(rv)) {
-            const char *query = strrchr(path.get(), '?');
-            if (query && PL_strncasecmp(path.get(), originalPath,
-                                        query - path.get()) == 0)
-              return true;  // This object is a part of the original message, we
-                            // can send it safely.
+            nsAutoCString orgPath(originalPath);
+            MsgRemoveQueryPart(orgPath);
+            MsgRemoveQueryPart(path);
+            // mailbox: and JS Account URLs have a message number in
+            // the query part of "path query ref". We removed this so
+            // we're not comparing down to the message but down to the folder.
+            // Code in the frontend (in the "error" event listener in
+            // MsgComposeCommands.js that deals with unblocking images) will
+            // prompt if a part of another message is referenced.
+            // A saved message opened for reply or forwarding has a
+            // mailbox: URL.
+            // imap: URLs don't have the message number in the query, so we do
+            // compare it here.
+            // news: URLs use group and key in the query, but it's OK to compare
+            // without them.
+            return path.Equals(orgPath, nsCaseInsensitiveCStringComparator());
           }
         }
       }
     }
   }
 
   return false;
 }
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -8876,20 +8876,17 @@ nsresult nsImapMockChannel::OpenCacheEnt
     }
   }
   nsCString filenameQuery = MsgExtractQueryPart(path, "&filename=");
   MOZ_LOG(IMAPCache, LogLevel::Debug,
           ("OpenCacheEntry: part = |%s|, filename = |%s|", partQuery.get(),
            filenameQuery.get()));
 
   // Truncate path at either /; or ?
-  int32_t ind = path.FindChar('?');
-  if (ind != kNotFound) path.SetLength(ind);
-  ind = path.Find("/;");
-  if (ind != kNotFound) path.SetLength(ind);
+  MsgRemoveQueryPart(path);
 
   nsCOMPtr<nsIURI> newUri;
   if (partQuery.IsEmpty()) {
     // Not looking for a part. That's the easy part.
     rv = NS_MutateURI(m_url).SetPathQueryRef(path).Finalize(newUri);
     NS_ENSURE_SUCCESS(rv, rv);
     MOZ_LOG(IMAPCache, LogLevel::Debug,
             ("OpenCacheEntry(): Call AsyncOpenURI() on entire message"));
--- a/mailnews/imap/src/nsImapUrl.cpp
+++ b/mailnews/imap/src/nsImapUrl.cpp
@@ -1021,21 +1021,17 @@ NS_IMETHODIMP nsImapUrl::GetNormalizedSp
   // Normalized spec: imap://user@domain@server:port/fetch>UID>folder>nn
   nsCOMPtr<nsIMsgMailNewsUrl> mailnewsURL;
   QueryInterface(NS_GET_IID(nsIMsgMailNewsUrl), getter_AddRefs(mailnewsURL));
 
   nsAutoCString spec;
   mailnewsURL->GetSpecIgnoringRef(spec);
 
   // Strip any query part beginning with ? or /;
-  int32_t ind = spec.Find("/;");
-  if (ind != kNotFound) spec.SetLength(ind);
-
-  ind = spec.FindChar('?');
-  if (ind != kNotFound) spec.SetLength(ind);
+  MsgRemoveQueryPart(spec);
 
   aPrincipalSpec.Assign(spec);
   return NS_OK;
 }
 
 NS_IMETHODIMP nsImapUrl::SetUri(const char *aURI) {
   mURI = aURI;
   return NS_OK;
--- a/mailnews/jsaccount/src/JaUrl.cpp
+++ b/mailnews/jsaccount/src/JaUrl.cpp
@@ -103,21 +103,17 @@ NS_IMETHODIMP JaBaseCppUrl::GetNormalize
   QueryInterface(NS_GET_IID(nsIMsgMailNewsUrl), getter_AddRefs(mailnewsURL));
 
   nsAutoCString spec;
   mailnewsURL->GetSpecIgnoringRef(spec);
 
   nsCString queryPart = MsgExtractQueryPart(spec, "number=");
 
   // Strip any query part beginning with ? or /;
-  int32_t ind = spec.Find("/;");
-  if (ind != kNotFound) spec.SetLength(ind);
-
-  ind = spec.FindChar('?');
-  if (ind != kNotFound) spec.SetLength(ind);
+  MsgRemoveQueryPart(spec);
 
   if (!queryPart.IsEmpty()) spec += NS_LITERAL_CSTRING("?") + queryPart;
 
   aPrincipalSpec.Assign(spec);
   return NS_OK;
 }
 
 NS_IMETHODIMP JaBaseCppUrl::GetMessageHeader(nsIMsgDBHdr **aMessageHeader) {
--- a/mailnews/local/src/nsMailboxUrl.cpp
+++ b/mailnews/local/src/nsMailboxUrl.cpp
@@ -121,21 +121,17 @@ NS_IMETHODIMP nsMailboxUrl::GetNormalize
   // mailbox: URLs contain a lot of query parts. We want need a normalized form:
   // mailbox:///path/to/folder?number=nn.
   // We also need to translate the second form
   // mailbox://user@domain@server/folder?number=nn.
 
   char *messageKey = extractAttributeValue(spec.get(), "number=");
 
   // Strip any query part beginning with ? or /;
-  int32_t ind = spec.Find("/;");
-  if (ind != kNotFound) spec.SetLength(ind);
-
-  ind = spec.FindChar('?');
-  if (ind != kNotFound) spec.SetLength(ind);
+  MsgRemoveQueryPart(spec);
 
   // Check for format lacking absolute path.
   if (spec.Find("///") == kNotFound) {
     nsCString folderPath;
     nsresult rv = nsLocalURI2Path(kMailboxRootURI, spec.get(), folderPath);
     if (NS_SUCCEEDED(rv)) {
       nsAutoCString buf;
       MsgEscapeURL(