Bug 1126240 - Correctly encode APK paths in SearchEngineManager. r=margaret, a=sledru
authorRichard Newman <rnewman@mozilla.com>
Tue, 10 Feb 2015 16:11:24 -0800
changeset 243772 5d83c055e2a9
parent 243771 58fa5b70d329
child 243773 bbb250fc79b7
child 243775 e88b1974bc53
push id4470
push userryanvm@gmail.com
push date2015-02-13 20:35 +0000
treeherdermozilla-beta@5d83c055e2a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmargaret, sledru
bugs1126240
milestone36.0
Bug 1126240 - Correctly encode APK paths in SearchEngineManager. r=margaret, a=sledru This is the approach we already take everywhere else we make a jar:jar: URI. I've unified those places into GeckoJarReader, cleaned up imports, fixed a typo, and wrote a trivial test for this case. I made a few utility methods static to facilitate testing and future refactoring.
mobile/android/base/BrowserLocaleManager.java
mobile/android/base/db/LocalBrowserDB.java
mobile/android/base/favicons/Favicons.java
mobile/android/base/tests/testJarReader.java
mobile/android/base/util/GeckoJarReader.java
mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java
--- a/mobile/android/base/BrowserLocaleManager.java
+++ b/mobile/android/base/BrowserLocaleManager.java
@@ -451,21 +451,17 @@ public class BrowserLocaleManager implem
      *              "fr", "ga-IE", "hu", "id", "it", "ja", "ko",
      *              "lt", "lv", "nb-NO", "nl", "pl", "pt-BR",
      *              "pt-PT", "ro", "ru", "sk", "sl", "sv-SE", "th",
      *              "tr", "uk", "zh-CN", "zh-TW", "en-US"]}
      * </code>
      */
     public static Collection<String> getPackagedLocaleTags(final Context context) {
         final String resPath = "res/multilocale.json";
-        final String apkPath = context.getPackageResourcePath();
-
-        final String jarURL = "jar:jar:" + new File(apkPath).toURI() + "!/" +
-                              AppConstants.OMNIJAR_NAME + "!/" +
-                              resPath;
+        final String jarURL = GeckoJarReader.getJarURL(context, resPath);
 
         final String contents = GeckoJarReader.getText(jarURL);
         if (contents == null) {
             // GeckoJarReader logs and swallows exceptions.
             return null;
         }
 
         try {
--- a/mobile/android/base/db/LocalBrowserDB.java
+++ b/mobile/android/base/db/LocalBrowserDB.java
@@ -430,28 +430,23 @@ public class LocalBrowserDB {
     }
 
     /**
      * Load a favicon from the omnijar.
      * @return A ConsumedInputStream containing the bytes loaded from omnijar. This must be a format
      *         compatible with the favicon decoder (most probably a PNG or ICO file).
      */
     private static ConsumedInputStream getDefaultFaviconFromPath(Context context, String name) {
-        int faviconId = getFaviconId(name);
+        final int faviconId = getFaviconId(name);
         if (faviconId == FAVICON_ID_NOT_FOUND) {
             return null;
         }
 
-        String path = context.getString(faviconId);
-
-        String apkPath = context.getPackageResourcePath();
-        File apkFile = new File(apkPath);
-        String bitmapPath = "jar:jar:" + apkFile.toURI() + "!/" + AppConstants.OMNIJAR_NAME + "!/" + path;
-
-        InputStream iStream = GeckoJarReader.getStream(bitmapPath);
+        final String bitmapPath = GeckoJarReader.getJarURL(context, context.getString(faviconId));
+        final InputStream iStream = GeckoJarReader.getStream(bitmapPath);
 
         return IOUtils.readFully(iStream, DEFAULT_FAVICON_BUFFER_SIZE);
     }
 
     private static ConsumedInputStream getDefaultFaviconFromDrawable(Context context, String name) {
         int faviconId = getFaviconId(name);
         if (faviconId == FAVICON_ID_NOT_FOUND) {
             return null;
--- a/mobile/android/base/favicons/Favicons.java
+++ b/mobile/android/base/favicons/Favicons.java
@@ -492,20 +492,17 @@ public class Favicons {
         putFaviconsInMemCache(BUILT_IN_SEARCH_URL, searchIcons.iterator(), true);
     }
 
     /**
      * Compute a string like:
      * "jar:jar:file:///data/app/org.mozilla.firefox-1.apk!/assets/omni.ja!/chrome/chrome/content/branding/favicon64.png"
      */
     private static String getBrandingBitmapPath(Context context, String name) {
-        final String apkPath = context.getPackageResourcePath();
-        return "jar:jar:" + new File(apkPath).toURI() + "!/" +
-               AppConstants.OMNIJAR_NAME + "!/" +
-               "chrome/chrome/content/branding/" + name;
+        return GeckoJarReader.getJarURL(context, "chrome/chrome/content/branding/" + name);
     }
 
     private static Bitmap loadBrandingBitmap(Context context, String name) {
         Bitmap b = GeckoJarReader.getBitmap(context.getResources(),
                                             getBrandingBitmapPath(context, name));
         if (b == null) {
             throw new IllegalStateException("Bitmap " + name + " missing from JAR!");
         }
--- a/mobile/android/base/tests/testJarReader.java
+++ b/mobile/android/base/tests/testJarReader.java
@@ -5,16 +5,23 @@ import java.io.InputStream;
 import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.util.GeckoJarReader;
 
 /**
  * A basic jar reader test. Tests reading a png from fennec's apk, as well
  * as loading some invalid jar urls.
  */
 public class testJarReader extends BaseTest {
+    public void testGetJarURL() {
+        // Invalid characters are escaped.
+        final String s = GeckoJarReader.computeJarURI("some[1].apk", "something/else");
+        mAsserter.ok(!s.contains("["), "Illegal characters are escaped away.", null);
+        mAsserter.ok(!s.toLowerCase().contains("%2f"), "Path characters aren't escaped.", null);
+    }
+
     public void testJarReader() {
         String appPath = getActivity().getApplication().getPackageResourcePath();
         mAsserter.isnot(appPath, null, "getPackageResourcePath is non-null");
 
         // Test reading a file from a jar url that looks correct.
         String url = "jar:file://" + appPath + "!/" + AppConstants.OMNIJAR_NAME;
         InputStream stream = GeckoJarReader.getStream("jar:" + url + "!/chrome/chrome/content/branding/favicon32.png");
         mAsserter.isnot(stream, null, "JarReader returned non-null for valid file in valid jar");
--- a/mobile/android/base/util/GeckoJarReader.java
+++ b/mobile/android/base/util/GeckoJarReader.java
@@ -1,23 +1,26 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.util;
 
+import android.content.Context;
+import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.mozglue.NativeZip;
 
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.drawable.BitmapDrawable;
 import android.util.Log;
 import org.mozilla.gecko.mozglue.RobocopTarget;
 
 import java.io.BufferedReader;
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Stack;
 
 /* Reads out of a multiple level deep jar file such as
@@ -166,9 +169,28 @@ public final class GeckoJarReader {
             String subStr = url.substring(4, jarEnd);
             results.push(url.substring(jarEnd+2)); // remove the !/ characters
             return parseUrl(subStr, results);
         } else {
             results.push(url);
             return results;
         }
     }
+
+    public static String getJarURL(Context context, String pathInsideJAR) {
+        // We need to encode the package resource path, because it might contain illegal characters. For example:
+        //   /mnt/asec2/[2]org.mozilla.fennec-1/pkg.apk
+        // The round-trip through a URI does this for us.
+        final String resourcePath = context.getPackageResourcePath();
+        return computeJarURI(resourcePath, pathInsideJAR);
+    }
+
+    /**
+     * Encodes its resource path correctly.
+     */
+    @RobocopTarget
+    public static String computeJarURI(String resourcePath, String pathInsideJAR) {
+        final String resURI = new File(resourcePath).toURI().toString();
+
+        // TODO: do we need to encode the file path, too?
+        return "jar:jar:" + resURI + "!/" + AppConstants.OMNIJAR_NAME + "!/" + pathInsideJAR;
+    }
 }
--- a/mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java
+++ b/mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java
@@ -3,47 +3,42 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.search.providers;
 
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.text.TextUtils;
 import android.util.Log;
-
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.BrowserLocaleManager;
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.GeckoSharedPrefs;
 import org.mozilla.gecko.R;
+import org.mozilla.gecko.distribution.Distribution;
 import org.mozilla.gecko.util.FileUtils;
 import org.mozilla.gecko.util.GeckoJarReader;
 import org.mozilla.gecko.util.HardwareUtils;
 import org.mozilla.gecko.util.RawResource;
 import org.mozilla.gecko.util.ThreadUtils;
-import org.mozilla.gecko.distribution.Distribution;
-import org.mozilla.search.Constants;
 import org.xmlpull.v1.XmlPullParserException;
 
 import java.io.BufferedInputStream;
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.io.UnsupportedEncodingException;
 import java.net.HttpURLConnection;
 import java.net.URL;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Locale;
 
 public class SearchEngineManager implements SharedPreferences.OnSharedPreferenceChangeListener {
     private static final String LOG_TAG = "GeckoSearchEngineManager";
 
     // Gecko pref that defines the name of the default search engine.
     private static final String PREF_GECKO_DEFAULT_ENGINE = "browser.search.defaultenginename";
 
@@ -527,17 +522,17 @@ public class SearchEngineManager impleme
         for (int i = 0; i < files.length; i++) {
             try {
                 final FileInputStream fis = new FileInputStream(files[i]);
                 final SearchEngine engine = createEngineFromInputStream(null, fis);
                 if (engine != null && engine.getName().equals(name)) {
                     return engine;
                 }
             } catch (IOException e) {
-                Log.e(LOG_TAG, "Error creating earch engine from name: " + name, e);
+                Log.e(LOG_TAG, "Error creating search engine from name: " + name, e);
             }
         }
         return null;
     }
 
     /**
      * Creates a SearchEngine instance from an InputStream.
      *
@@ -568,50 +563,50 @@ public class SearchEngineManager impleme
      * @param fileName name of the file to read.
      * @return InputStream for file.
      */
     private InputStream getInputStreamFromSearchPluginsJar(String fileName) {
         final Locale locale = Locale.getDefault();
 
         // First, try a file path for the full locale.
         final String languageTag = BrowserLocaleManager.getLanguageTag(locale);
-        String url = getSearchPluginsJarURL(languageTag, fileName);
+        String url = getSearchPluginsJarURL(context, languageTag, fileName);
 
         InputStream in = GeckoJarReader.getStream(url);
         if (in != null) {
             return in;
         }
 
         // If that doesn't work, try a file path for just the language.
         final String language = BrowserLocaleManager.getLanguage(locale);
         if (!languageTag.equals(language)) {
-            url = getSearchPluginsJarURL(language, fileName);
+            url = getSearchPluginsJarURL(context, language, fileName);
             in = GeckoJarReader.getStream(url);
             if (in != null) {
                 return in;
             }
         }
 
         // Finally, fall back to default locale defined in chrome registry.
-        url = getSearchPluginsJarURL(getFallbackLocale(), fileName);
+        url = getSearchPluginsJarURL(context, getFallbackLocale(), fileName);
         return GeckoJarReader.getStream(url);
     }
 
     /**
      * Finds a fallback locale in the Gecko chrome registry. If a locale is declared
      * here, we should be guaranteed to find a searchplugins directory for it.
      *
      * This method should only be accessed from the background thread.
      */
     private String getFallbackLocale() {
         if (fallbackLocale != null) {
             return fallbackLocale;
         }
 
-        final InputStream in = GeckoJarReader.getStream(getJarURL("!/chrome/chrome.manifest"));
+        final InputStream in = GeckoJarReader.getStream(GeckoJarReader.getJarURL(context, "chrome/chrome.manifest"));
         final BufferedReader br = getBufferedReader(in);
 
         try {
             String line;
             while ((line = br.readLine()) != null) {
                 // We're looking for a line like "locale global en-US en-US/locale/en-US/global/"
                 // https://developer.mozilla.org/en/docs/Chrome_Registration#locale
                 if (line.startsWith("locale global ")) {
@@ -633,23 +628,19 @@ public class SearchEngineManager impleme
 
     /**
      * Gets the jar URL for a file in the searchplugins directory.
      *
      * @param locale String representing the Gecko locale (e.g. "en-US").
      * @param fileName The name of the file to read.
      * @return URL for jar file.
      */
-    private String getSearchPluginsJarURL(String locale, String fileName) {
-        final String path = "!/chrome/" + locale + "/locale/" + locale + "/browser/searchplugins/" + fileName;
-        return getJarURL(path);
-    }
-
-    private String getJarURL(String path) {
-        return "jar:jar:file://" + context.getPackageResourcePath() + "!/" + AppConstants.OMNIJAR_NAME + path;
+    private static String getSearchPluginsJarURL(Context context, String locale, String fileName) {
+        final String path = "chrome/" + locale + "/locale/" + locale + "/browser/searchplugins/" + fileName;
+        return GeckoJarReader.getJarURL(context, path);
     }
 
     private BufferedReader getBufferedReader(InputStream in) {
         try {
             return new BufferedReader(new InputStreamReader(in, "UTF-8"));
         } catch (UnsupportedEncodingException e) {
             // Cannot happen.
             return null;