Bug 1126240 - Correctly encode APK paths in SearchEngineManager. r=margaret

This is the approach we already take everywhere else we make a 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

@@ -398,11 +398,7 @@ public class BrowserLocaleManager implements LocaleManager {
      */
     public static Collection<String> getPackagedLocaleTags(final Context context) {
         final String resPath = "res/multilocale.json";
-        final String apkPath = context.getPackageResourcePath();
+        final String jarURL = GeckoJarReader.getJarURL(context, resPath);
-
-        final String jarURL = "jar:jar:" + new File(apkPath).toURI() + "!/" +
-                              AppConstants.OMNIJAR_NAME + "!/" +
-                              resPath;
 
         final String contents = GeckoJarReader.getText(jarURL);
         if (contents == null) {

mobile/android/base/db/LocalBrowserDB.java

@@ -473,18 +473,13 @@ public class LocalBrowserDB implements BrowserDB {
      *         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);
+        final String bitmapPath = GeckoJarReader.getJarURL(context, context.getString(faviconId));
-
+        final InputStream iStream = GeckoJarReader.getStream(bitmapPath);
-        String apkPath = context.getPackageResourcePath();
-        File apkFile = new File(apkPath);
-        String bitmapPath = "jar:jar:" + apkFile.toURI() + "!/" + AppConstants.OMNIJAR_NAME + "!/" + path;
-
-        InputStream iStream = GeckoJarReader.getStream(bitmapPath);
 
         return IOUtils.readFully(iStream, DEFAULT_FAVICON_BUFFER_SIZE);
     }

mobile/android/base/favicons/Favicons.java

@@ -498,10 +498,7 @@ public class Favicons {
      * "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 GeckoJarReader.getJarURL(context, "chrome/chrome/content/branding/" + name);
-        return "jar:jar:" + new File(apkPath).toURI() + "!/" +
-               AppConstants.OMNIJAR_NAME + "!/" +
-               "chrome/chrome/content/branding/" + name;
     }
 
     private static Bitmap loadBrandingBitmap(Context context, String name) {

mobile/android/base/tests/testJarReader.java

@@ -14,6 +14,13 @@ import org.mozilla.gecko.util.GeckoJarReader;
  * 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");

mobile/android/base/util/GeckoJarReader.java

@@ -4,6 +4,8 @@
 
 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;
@@ -13,6 +15,7 @@ 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;
@@ -171,4 +174,23 @@ public final class GeckoJarReader {
             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;
+    }
 }

mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java

@@ -8,7 +8,6 @@ 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;
@@ -16,20 +15,18 @@ import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.GeckoSharedPrefs;
 import org.mozilla.gecko.Locales;
 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;
@@ -37,8 +34,6 @@ 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 {
@@ -532,7 +527,7 @@ public class SearchEngineManager implements SharedPreferences.OnSharedPreference
                     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;
@@ -573,7 +568,7 @@ public class SearchEngineManager implements SharedPreferences.OnSharedPreference
 
         // First, try a file path for the full locale.
         final String languageTag = Locales.getLanguageTag(locale);
-        String url = getSearchPluginsJarURL(languageTag, fileName);
+        String url = getSearchPluginsJarURL(context, languageTag, fileName);
 
         InputStream in = GeckoJarReader.getStream(url);
         if (in != null) {
@@ -583,7 +578,7 @@ public class SearchEngineManager implements SharedPreferences.OnSharedPreference
         // If that doesn't work, try a file path for just the language.
         final String language = Locales.getLanguage(locale);
         if (!languageTag.equals(language)) {
-            url = getSearchPluginsJarURL(language, fileName);
+            url = getSearchPluginsJarURL(context, language, fileName);
             in = GeckoJarReader.getStream(url);
             if (in != null) {
                 return in;
@@ -591,7 +586,7 @@ public class SearchEngineManager implements SharedPreferences.OnSharedPreference
         }
 
         // Finally, fall back to default locale defined in chrome registry.
-        url = getSearchPluginsJarURL(getFallbackLocale(), fileName);
+        url = getSearchPluginsJarURL(context, getFallbackLocale(), fileName);
         return GeckoJarReader.getStream(url);
     }
 
@@ -606,7 +601,7 @@ public class SearchEngineManager implements SharedPreferences.OnSharedPreference
             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 {
@@ -638,13 +633,9 @@ public class SearchEngineManager implements SharedPreferences.OnSharedPreference
      * @param fileName The name of the file to read.
      * @return URL for jar file.
      */
-    private String getSearchPluginsJarURL(String locale, String fileName) {
+    private static String getSearchPluginsJarURL(Context context, String locale, String fileName) {
-        final String path = "!/chrome/" + locale + "/locale/" + locale + "/browser/searchplugins/" + fileName;
+        final String path = "chrome/" + locale + "/locale/" + locale + "/browser/searchplugins/" + fileName;
-        return getJarURL(path);
+        return GeckoJarReader.getJarURL(context, path);
-    }
-
-    private String getJarURL(String path) {
-        return "jar:jar:file://" + context.getPackageResourcePath() + "!/" + AppConstants.OMNIJAR_NAME + path;
     }
 
     private BufferedReader getBufferedReader(InputStream in) {