Bug 1450449 - Part 5: Disable file:// URI checks for downloaded files and launching files from Gecko. r?jchen
This is a case where I disagree with Google's stance about content:// URIs.
They're perfect for granting access to files that might not even be present on
the file system, e.g. virtual files generated on the spot or retrieved from some
database, a cloud storage provider's app granting access to files stored in the
cloud, etc., as well as for being able to selectively grant access to files
conceptually "owned" by a certain app, especially files within the app's private
internal storage.
However when considering files that don't actually "belong" to any specific app
in particular and that are already being stored in a publicly accessible (modulo
the READ_EXTERNAL_STORAGE permission, respectively the user granting access
through the Storage Access Framework) directory somewhere within the external
storage, they also have a number of drawbacks:
- While in practice a number of FileProviders will "leak" the true file system
path through the content:// URI they generate, the problem remains that
there's no way to know for sure whether two content:// URIs received from
different apps are in fact referring to the same file or not.
In case of our downloads for example, content:// URIs all referring to the
same file could in principle be generated
* by Firefox itself
* by the system Downloads app
* by the system file browser app
* by any other third-party file browser or similar app that the user might
have installed
which e.g. will needlessly clutter up any LRU lists other apps might keep.
- content:// URIs obviously depend on the generating app still being installed.
So even if we fixed
bug 1280184, so that uninstalling Firefox would no longer
remove the user's downloads, all content:// URIs generated by Firefox re-
ferring to those files would become invalid anyway.
- Even if the actual file is already sitting in a public directory, when
accessing it through the content:// URI the receiving app still needs to
explicitly persist the permissions granted for that URI, and there are some
signs that you can only persist permissions for a limited number of files. For
file:// URIs on the other hand the only limitation on the number of file://
URIS you can remember is the available storage space for storing those URIs,
i.e. for practical purposes more or less unlimited.
- content:// URIs only grant access to a specific file. If we (or possibly an
add-on) started implementing saving of websites as on desktop (i.e. HTML +
associated support files instead of a PDF "copy"), then receiving apps
couldn't properly open those additional support files (images, style sheets,
etc.) when getting a content:// URI to the main HTML file
(see https://issuetracker.google.com/issues/77406791).
Since we do store downloads in the public Downloads folder on the external
storage by default and I believe that conceptually, those files belong to the
user and not Firefox specifically, I propose to continue launching downloaded
files directly through their file:// URI.
To that end, we temporarily disable the corresponding StrictMode restrictions
when required and restore them afterwards.
MozReview-Commit-ID: LuIYIA5FSGf
--- a/mobile/android/base/java/org/mozilla/gecko/notifications/NotificationHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/notifications/NotificationHelper.java
@@ -27,16 +27,17 @@ import android.app.Activity;
import android.app.PendingIntent;
import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.content.pm.ResolveInfo;
import android.graphics.Bitmap;
import android.net.Uri;
+import android.os.StrictMode;
import android.support.v4.app.NotificationCompat;
import android.support.v4.util.SimpleArrayMap;
import android.util.Log;
public final class NotificationHelper implements BundleEventListener {
public static final String HELPER_BROADCAST_ACTION = AppConstants.ANDROID_PACKAGE_NAME + ".helperBroadcastAction";
public static final String NOTIFICATION_ID = "NotificationHelper_ID";
@@ -289,18 +290,24 @@ public final class NotificationHelper im
}
}
// Bug 1320889 - Tapping the notification for a downloaded file shouldn't open firefox.
// If the gecko event is for download completion, we create another intent with different
// scheme to prevent Fennec from popping up.
final Intent viewFileIntent = createIntentIfDownloadCompleted(message);
if (builder != null && viewFileIntent != null && mContext != null) {
+ // Bug 1450449 - Downloaded files already are already in a public directory and aren't
+ // really owned exclusively by Firefox, so there's no real benefit to using
+ // content:// URIs here.
+ StrictMode.VmPolicy prevPolicy = StrictMode.getVmPolicy();
+ StrictMode.setVmPolicy(StrictMode.VmPolicy.LAX);
final PendingIntent pIntent = PendingIntent.getActivity(
mContext, 0, viewFileIntent, PendingIntent.FLAG_UPDATE_CURRENT);
+ StrictMode.setVmPolicy(prevPolicy);
builder.setAutoCancel(true);
builder.setContentIntent(pIntent);
} else {
final PendingIntent pi = buildNotificationPendingIntent(message, CLICK_EVENT);
final PendingIntent deletePendingIntent = buildNotificationPendingIntent(
message, CLEARED_EVENT);
builder.setContentIntent(pi);
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
@@ -77,16 +77,17 @@ import android.net.ConnectivityManager;
import android.net.NetworkInfo;
import android.net.Uri;
import android.os.Build;
import android.os.Bundle;
import android.os.Environment;
import android.os.Looper;
import android.os.ParcelFileDescriptor;
import android.os.PowerManager;
+import android.os.StrictMode;
import android.os.SystemClock;
import android.os.Vibrator;
import android.provider.Settings;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v4.util.SimpleArrayMap;
import android.telephony.TelephonyManager;
import android.text.TextUtils;
@@ -943,17 +944,24 @@ public class GeckoAppShell
String packageName,
String className,
String action,
String title) {
final GeckoInterface geckoInterface = getGeckoInterface();
if (geckoInterface == null) {
return false;
}
- return geckoInterface.openUriExternal(targetURI, mimeType, packageName, className, action, title);
+ // Bug 1450449 - Downloaded files already are already in a public directory and aren't
+ // really owned exclusively by Firefox, so there's no real benefit to using
+ // content:// URIs here.
+ StrictMode.VmPolicy prevPolicy = StrictMode.getVmPolicy();
+ StrictMode.setVmPolicy(StrictMode.VmPolicy.LAX);
+ boolean success = geckoInterface.openUriExternal(targetURI, mimeType, packageName, className, action, title);
+ StrictMode.setVmPolicy(prevPolicy);
+ return success;
}
@WrapForJNI(dispatchTo = "gecko")
private static native void notifyAlertListener(String name, String topic, String cookie);
/**
* Called by the NotificationListener to notify Gecko that a notification has been
* shown.