[Android] Fail gracefully when accessing exported=false providers

[Android] Fail gracefully when accessing exported=false providers

BUG=382367
Committed: https://crrev.com/0898f973fe11613d431264f9b1bf07ca802d2055
Cr-Commit-Position: refs/heads/master@{#293137}

[Miguel Garcia, 2014-08-13 09:53:46]

Hi Chris, 

 Can you have a look at this one liner CL? I think catching SecurityException
here is justified but I'd like someone from security to confirm it's ok.

A quick explanation. Some third party apps will declare in the manifest that
they can handle ACTION_GET_CONTENT so they will pop up in the intent picker when
selecting a file. However unless they don't declare the resulting selection as
android:exported:true trying to access the result of the intent will hit a
Security exception.

[Miguel Garcia, 2014-08-15 14:42:47]

Adding Aurimas as well for general review.
Friendly ping Chris :)

If you were to be ok with the one line fix would you mind clicking the commit
box as well?

[aurimas (slooooooooow), 2014-08-15 17:44:56]

lgtm

https://chromiumcodereview.appspot.com/466983004/diff/20001/ui/android/java/s...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://chromiumcodereview.appspot.com/466983004/diff/20001/ui/android/java/s...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:243: } catch
(SecurityException e) {
Which calls can throw SecurityException? Do we have to wrap that much of a
function in try-catch?

[palmer, 2014-08-19 20:43:06]

LGTM with nits and a preference to wrap the minimum amount of code in try/catch.

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:243: } catch
(SecurityException e) {
> Which calls can throw SecurityException? Do we have to wrap that much of a
> function in try-catch?

I concur; let's wrap the minimum amount of code in the try/catch.

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:245: // to handle
the ACTION_GET_CONTENT intent but then declares themselves
Typo: "declare"

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:248: // In those
cases trying to access the contents raises a security exception
Nit: Wrap/fill the paragraph.

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:250: // See
crbug/382367 for details.
Nit: crbug.com

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:251:
Log.w(LOG_TAG, "Unable to extract results from the content provider");
Does this really need to be here? Probably nobody will see it?

[Miguel Garcia, 2014-09-03 14:16:00]

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
File ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java (right):

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:243: } catch
(SecurityException e) {
On 2014/08/19 20:43:05, Chromium Palmer wrote:
> > Which calls can throw SecurityException? Do we have to wrap that much of a
> > function in try-catch?
> 
> I concur; let's wrap the minimum amount of code in the try/catch.

Done.

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:245: // to handle
the ACTION_GET_CONTENT intent but then declares themselves
On 2014/08/19 20:43:05, Chromium Palmer wrote:
> Typo: "declare"

Done.

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:248: // In those
cases trying to access the contents raises a security exception
On 2014/08/19 20:43:05, Chromium Palmer wrote:
> Nit: Wrap/fill the paragraph.

Done.

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:250: // See
crbug/382367 for details.
On 2014/08/19 20:43:05, Chromium Palmer wrote:
> Nit: http://crbug.com

Done.

https://codereview.chromium.org/466983004/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java:251:
Log.w(LOG_TAG, "Unable to extract results from the content provider");
On 2014/08/19 20:43:05, Chromium Palmer wrote:
> Does this really need to be here? Probably nobody will see it?

It is useful information in case someone wonders why their uploads from apps
with this problem are not working.

[Miguel Garcia, 2014-09-03 14:16:22]

The CQ bit was checked by miguelg@chromium.org

[commit-bot: I haz the power, 2014-09-03 14:17:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miguelg@chromium.org/466983004/60001

[commit-bot: I haz the power, 2014-09-03 15:17:15]

Committed patchset #4 (id:60001) as f74115a63260242312d422ecb68c11248bd22086

[commit-bot: I haz the power, 2014-09-10 03:25:46]

Patchset 4 (id:??) landed as
https://crrev.com/0898f973fe11613d431264f9b1bf07ca802d2055
Cr-Commit-Position: refs/heads/master@{#293137}