Bug 1551183: Part 2 - Support enumerating directories in extensions in nested JARs. r=aswan a=jcristau
☠☠ backed out by 3b4af55e6edc ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Wed, 22 May 2019 14:34:07 -0700
changeset 536733 76077274869a12283e79ab53491fff41264efda8
parent 536732 ffa52c63ff472d43c6b81170a32dc98b05cc7dde
child 536734 259f5bcec259ed020c3612b4278c2ffa184ec77f
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, jcristau
bugs1551183
milestone68.0
Bug 1551183: Part 2 - Support enumerating directories in extensions in nested JARs. r=aswan a=jcristau On Android, omni.ja is bundled inside an API, and therefore loaded as a nested JAR. That means that its resource URIs resolve to something resembling "jar:jar:...!/omni.ja!/...". Our current enumeration code assumes no nesting of jar: URIs, and therefore can't handle this. This patch changes our enumeration helpers to accept URIs rather than nsIFile instances, and to correctly handle resolving the ZipReaders for those nested JARs. It also moves the path filter generation into the native helper to make things easier for other callers which may need this behavior. Differential Revision: https://phabricator.services.mozilla.com/D32224
toolkit/components/extensions/Extension.jsm
toolkit/mozapps/extensions/AddonManagerStartup.cpp
toolkit/mozapps/extensions/amIAddonManagerStartup.idl
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -423,38 +423,32 @@ class ExtensionData {
         }
       }
       iter.close();
 
       return results;
     }
 
     let uri = this.rootURI.QueryInterface(Ci.nsIJARURI);
-    let file = uri.JARFile.QueryInterface(Ci.nsIFileURL).file;
 
-    // Normalize the directory path.
-    path = `${uri.JAREntry}/${path}`;
-    path = path.replace(/\/\/+/g, "/").replace(/^\/|\/$/g, "") + "/";
-    if (path === "/") {
-      path = "";
-    }
-
-    // Escape pattern metacharacters.
-    let pattern = path.replace(/[[\]()?*~|$\\]/g, "\\$&") + "*";
+    // Append the sub-directory path to the base JAR URI and normalize the
+    // result.
+    let entry = `${uri.JAREntry}/${path}/`.replace(/\/\/+/g, "/").replace(/^\//, "");
+    uri = Services.io.newURI(`jar:${uri.JARFile.spec}!/${entry}`);
 
     let results = [];
-    for (let name of aomStartup.enumerateZipFile(file, pattern)) {
-      if (!name.startsWith(path)) {
+    for (let name of aomStartup.enumerateJARSubtree(uri)) {
+      if (!name.startsWith(entry)) {
         throw new Error("Unexpected ZipReader entry");
       }
 
       // The enumerator returns the full path of all entries.
       // Trim off the leading path, and filter out entries from
       // subdirectories.
-      name = name.slice(path.length);
+      name = name.slice(entry.length);
       if (name && !/\/./.test(name)) {
         results.push({
           name: name.replace("/", ""),
           isDir: name.endsWith("/"),
         });
       }
     }
 
--- a/toolkit/mozapps/extensions/AddonManagerStartup.cpp
+++ b/toolkit/mozapps/extensions/AddonManagerStartup.cpp
@@ -71,16 +71,54 @@ nsIFile* AddonManagerStartup::ProfileDir
   }
 
   return mProfileDir;
 }
 
 NS_IMPL_ISUPPORTS(AddonManagerStartup, amIAddonManagerStartup, nsIObserver)
 
 /*****************************************************************************
+ * URI utils
+ *****************************************************************************/
+
+static nsresult ParseJARURI(nsIJARURI* uri, nsIURI** jarFile,
+                            nsCString& entry) {
+  MOZ_TRY(uri->GetJARFile(jarFile));
+  MOZ_TRY(uri->GetJAREntry(entry));
+
+  // The entry portion of a jar: URI is required to begin with a '/', but for
+  // nested JAR URIs, the leading / of the outer entry is currently stripped.
+  // This is a bug which should be fixed in the JAR URI code, but...
+  if (entry.IsEmpty() || entry[0] != '/') {
+    entry.Insert('/', 0);
+  }
+  return NS_OK;
+}
+
+static nsresult ParseJARURI(nsIURI* uri, nsIURI** jarFile, nsCString& entry) {
+  nsresult rv;
+  nsCOMPtr<nsIJARURI> jarURI = do_QueryInterface(uri, &rv);
+  MOZ_TRY(rv);
+
+  return ParseJARURI(jarURI, jarFile, entry);
+}
+
+static Result<nsCOMPtr<nsIFile>, nsresult> GetFile(nsIURI* uri) {
+  nsresult rv;
+  nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(uri, &rv);
+  MOZ_TRY(rv);
+
+  nsCOMPtr<nsIFile> file;
+  MOZ_TRY(fileURL->GetFile(getter_AddRefs(file)));
+  MOZ_ASSERT(file);
+
+  return std::move(file);
+}
+
+/*****************************************************************************
  * File utils
  *****************************************************************************/
 
 static already_AddRefed<nsIFile> CloneAndAppend(nsIFile* aFile,
                                                 const char* name) {
   nsCOMPtr<nsIFile> file;
   aFile->Clone(getter_AddRefs(file));
   file->AppendNative(nsDependentCString(name));
@@ -205,29 +243,21 @@ static Result<FileLocation, nsresult> Ge
   FileLocation location;
 
   nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(uri);
   nsCOMPtr<nsIFile> file;
   if (fileURL) {
     MOZ_TRY(fileURL->GetFile(getter_AddRefs(file)));
     location.Init(file);
   } else {
-    nsCOMPtr<nsIJARURI> jarURI = do_QueryInterface(uri);
-    NS_ENSURE_TRUE(jarURI, Err(NS_ERROR_INVALID_ARG));
-
     nsCOMPtr<nsIURI> fileURI;
-    MOZ_TRY(jarURI->GetJARFile(getter_AddRefs(fileURI)));
+    nsCString entry;
+    MOZ_TRY(ParseJARURI(uri, getter_AddRefs(fileURI), entry));
 
-    fileURL = do_QueryInterface(fileURI);
-    NS_ENSURE_TRUE(fileURL, Err(NS_ERROR_INVALID_ARG));
-
-    MOZ_TRY(fileURL->GetFile(getter_AddRefs(file)));
-
-    nsCString entry;
-    MOZ_TRY(jarURI->GetJAREntry(entry));
+    MOZ_TRY_VAR(file, GetFile(fileURI));
 
     location.Init(file, entry.get());
   }
 
   return std::move(location);
 }
 
 /*****************************************************************************
@@ -557,51 +587,83 @@ nsresult AddonManagerStartup::DecodeBlob
   NS_ENSURE_TRUE(ok, NS_ERROR_OUT_OF_MEMORY);
 
   ErrorResult rv;
   holder.Read(cx, result, rv);
   return rv.StealNSResult();
   ;
 }
 
-nsresult AddonManagerStartup::EnumerateZipFile(nsIFile* file,
-                                               const nsACString& pattern,
-                                               uint32_t* countOut,
-                                               char16_t*** entriesOut) {
-  NS_ENSURE_ARG_POINTER(file);
-  NS_ENSURE_ARG_POINTER(countOut);
-  NS_ENSURE_ARG_POINTER(entriesOut);
-
-  nsCOMPtr<nsIZipReaderCache> zipCache;
-  MOZ_TRY_VAR(zipCache, GetJarCache());
-
-  nsCOMPtr<nsIZipReader> zip;
-  MOZ_TRY(zipCache->GetZip(file, getter_AddRefs(zip)));
-
+static nsresult EnumerateZip(nsIZipReader* zip, const nsACString& pattern,
+                             nsTArray<nsString>& results) {
   nsCOMPtr<nsIUTF8StringEnumerator> entries;
   MOZ_TRY(zip->FindEntries(pattern, getter_AddRefs(entries)));
 
-  nsTArray<nsString> results;
   bool hasMore;
   while (NS_SUCCEEDED(entries->HasMore(&hasMore)) && hasMore) {
     nsAutoCString name;
     MOZ_TRY(entries->GetNext(name));
 
     results.AppendElement(NS_ConvertUTF8toUTF16(name));
   }
 
-  auto strResults = MakeUnique<char16_t*[]>(results.Length());
-  for (uint32_t i = 0; i < results.Length(); i++) {
-    strResults[i] = ToNewUnicode(results[i]);
+  return NS_OK;
+}
+
+nsresult AddonManagerStartup::EnumerateJAR(nsIURI* uri,
+                                           const nsACString& pattern,
+                                           nsTArray<nsString>& results) {
+  nsCOMPtr<nsIZipReaderCache> zipCache;
+  MOZ_TRY_VAR(zipCache, GetJarCache());
+
+  nsCOMPtr<nsIZipReader> zip;
+  nsCOMPtr<nsIFile> file;
+  if (nsCOMPtr<nsIJARURI> jarURI = do_QueryInterface(uri)) {
+    nsCOMPtr<nsIURI> fileURI;
+    nsCString entry;
+    MOZ_TRY(ParseJARURI(jarURI, getter_AddRefs(fileURI), entry));
+
+    MOZ_TRY_VAR(file, GetFile(fileURI));
+    MOZ_TRY(zipCache->GetInnerZip(file, Substring(entry, 1),
+                                  getter_AddRefs(zip)));
+  } else {
+    MOZ_TRY_VAR(file, GetFile(uri));
+    MOZ_TRY(zipCache->GetZip(file, getter_AddRefs(zip)));
   }
+  MOZ_ASSERT(zip);
 
-  *countOut = results.Length();
-  *entriesOut = strResults.release();
+  return EnumerateZip(zip, pattern, results);
+}
+
+nsresult AddonManagerStartup::EnumerateJARSubtree(nsIURI* uri,
+                                                  nsTArray<nsString>& results) {
+  nsCOMPtr<nsIURI> fileURI;
+  nsCString entry;
+  MOZ_TRY(ParseJARURI(uri, getter_AddRefs(fileURI), entry));
+
+  // Mangle the path into a pattern to match all child entries by escaping any
+  // existing pattern matching metacharacters it contains and appending "/*".
+  NS_NAMED_LITERAL_CSTRING(metaChars, "[]()?*~|$\\");
 
-  return NS_OK;
+  nsCString pattern;
+  pattern.SetCapacity(entry.Length());
+
+  // The first character of the entry name is "/", which we want to skip.
+  for (auto chr : MakeSpan(Substring(entry, 1))) {
+    if (metaChars.FindChar(chr) >= 0) {
+      pattern.Append('\\');
+    }
+    pattern.Append(chr);
+  }
+  if (!pattern.IsEmpty() && !StringEndsWith(pattern, NS_LITERAL_CSTRING("/"))) {
+    pattern.Append('/');
+  }
+  pattern.Append('*');
+
+  return EnumerateJAR(fileURI, pattern, results);
 }
 
 nsresult AddonManagerStartup::InitializeURLPreloader() {
   MOZ_RELEASE_ASSERT(xpc::IsInAutomation());
 
   URLPreloader::ReInitialize();
 
   return NS_OK;
--- a/toolkit/mozapps/extensions/amIAddonManagerStartup.idl
+++ b/toolkit/mozapps/extensions/amIAddonManagerStartup.idl
@@ -40,30 +40,42 @@ interface amIAddonManagerStartup : nsISu
 
   [implicit_jscontext]
   jsval encodeBlob(in jsval value);
 
   [implicit_jscontext]
   jsval decodeBlob(in jsval value);
 
   /**
-   * Enumerates over all entries in the given zip file matching the given
-   * pattern, and returns an array of their paths.
+   * Enumerates over all entries in the JAR file at the given URI, and returns
+   * an array of entry paths which match the given pattern. The URI may be
+   * either a file: URL pointing directly to a zip file, or a jar: URI
+   * pointing to a zip file nested within another zip file. Only one level of
+   * nesting is supported.
    *
    * This should be used in preference to manually opening or retrieving a
    * ZipReader from the zip cache, since the former causes main thread IO and
    * the latter can lead to file locking issues due to unpredictable GC behavior
    * keeping the cached ZipReader alive after the cache is flushed.
    *
-   * @param file The zip file to enumerate.
+   * @param uri The URI of the zip file to enumerate.
    * @param pattern The pattern to match, as passed to nsIZipReader.findEntries.
    */
-  void enumerateZipFile(in nsIFile file, in AUTF8String pattern,
-                        [optional] out unsigned long count,
-                        [retval, array, size_is(count)] out wstring entries);
+  Array<AString> enumerateJAR(in nsIURI uri, in AUTF8String pattern);
+
+  /**
+   * Similar to |enumerateJAR| above, but accepts the URI of a directory
+   * within a JAR file, and returns a list of all entries below it.
+   *
+   * The given URI must be a jar: URI, and its JAR file must point either to a
+   * file: URI, or to a singly-nested JAR within another JAR file (i.e.,
+   * "jar:file:///thing.jar!/" or "jar:jar:file:///thing.jar!/stuff.jar!/").
+   * Multiple levels of nesting are not supported.
+   */
+  Array<AString> enumerateJARSubtree(in nsIURI uri);
 
   /**
    * Initializes the URL Preloader.
    *
    * NOT FOR USE OUTSIDE OF UNIT TESTS.
    */
   void initializeURLPreloader();