From bc3c2931e6340093547547023f5b965691d2d78f Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Tue, 10 Feb 2015 16:11:24 -0800 Subject: [PATCH] Bug 1126240 - Correctly encode APK paths in SearchEngineManager. r=margaret 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 | 6 +---- mobile/android/base/db/LocalBrowserDB.java | 11 +++----- mobile/android/base/favicons/Favicons.java | 5 +--- mobile/android/base/tests/testJarReader.java | 7 +++++ mobile/android/base/util/GeckoJarReader.java | 21 +++++++++++++++ .../search/providers/SearchEngineManager.java | 27 +++++++------------ 6 files changed, 42 insertions(+), 35 deletions(-) diff --git a/mobile/android/base/BrowserLocaleManager.java b/mobile/android/base/BrowserLocaleManager.java index 045e01d28399..86087fdc78fe 100644 --- a/mobile/android/base/BrowserLocaleManager.java +++ b/mobile/android/base/BrowserLocaleManager.java @@ -398,11 +398,7 @@ public class BrowserLocaleManager implements LocaleManager { */ public static Collection 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) { diff --git a/mobile/android/base/db/LocalBrowserDB.java b/mobile/android/base/db/LocalBrowserDB.java index 4ab2f7493026..0b6ff3addff7 100644 --- a/mobile/android/base/db/LocalBrowserDB.java +++ b/mobile/android/base/db/LocalBrowserDB.java @@ -468,18 +468,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); - - 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); } diff --git a/mobile/android/base/favicons/Favicons.java b/mobile/android/base/favicons/Favicons.java index 3c467032f57c..8ff6ca342456 100644 --- a/mobile/android/base/favicons/Favicons.java +++ b/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 "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) { diff --git a/mobile/android/base/tests/testJarReader.java b/mobile/android/base/tests/testJarReader.java index 478b099a2f6c..0b712057cd95 100644 --- a/mobile/android/base/tests/testJarReader.java +++ b/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."); + mAsserter.ok(s.toLowerCase().contains("%2f"), "Path characters aren't escaped."); + } + public void testJarReader() { String appPath = getActivity().getApplication().getPackageResourcePath(); mAsserter.isnot(appPath, null, "getPackageResourcePath is non-null"); diff --git a/mobile/android/base/util/GeckoJarReader.java b/mobile/android/base/util/GeckoJarReader.java index e10063f4a01e..828d7cf7a432 100644 --- a/mobile/android/base/util/GeckoJarReader.java +++ b/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,22 @@ 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. + */ + 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; + } } diff --git a/mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java b/mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java index 62e2e619fba0..465c78dfa61a 100644 --- a/mobile/android/search/java/org/mozilla/search/providers/SearchEngineManager.java +++ b/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) { - 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) {