commit ad9b3c0d704dceab75c2b3f6246740c78d8f7c04
Author: Jan Henning <[email protected]>
Date: Sun May 13 00:07:48 2018 +0200
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
--HG--
extra : rebase_source : a690b3097fdb03591f25f05a944c9ca3c05ddd04
---
.../org/mozilla/gecko/notifications/NotificationHelper.java | 7 +++++++
.../src/main/java/org/mozilla/gecko/GeckoAppShell.java | 10 +++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git
a/mobile/android/base/java/org/mozilla/gecko/notifications/NotificationHelper.java
b/mobile/android/base/java/org/mozilla/gecko/notifications/NotificationHelper.java
index 35366609da49..713e9f9d4b3c 100644
---
a/mobile/android/base/java/org/mozilla/gecko/notifications/NotificationHelper.java
+++
b/mobile/android/base/java/org/mozilla/gecko/notifications/NotificationHelper.java
@@ -32,6 +32,7 @@ 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.util.SimpleArrayMap;
import android.util.Log;
@@ -295,8 +296,14 @@ public final class NotificationHelper implements
BundleEventListener {
// 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);
diff --git
a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
index 34ba3315f295..3d21d7bb2f56 100644
---
a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+++
b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
@@ -86,6 +86,7 @@ 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;
@@ -956,7 +957,14 @@ public class GeckoAppShell
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")
_______________________________________________
tor-commits mailing list
[email protected]
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
