Skip to content

[image_picker] Fix crash due to SecurityException#4004

Merged
auto-submit[bot] merged 3 commits intoflutter:mainfrom
JeroenWeener:image-picker/100025
May 19, 2023
Merged

[image_picker] Fix crash due to SecurityException#4004
auto-submit[bot] merged 3 commits intoflutter:mainfrom
JeroenWeener:image-picker/100025

Conversation

@JeroenWeener
Copy link
Contributor

@JeroenWeener JeroenWeener commented May 16, 2023

Issue flutter/flutter#100025 mentions crashing of the image picker plugin due to a SecurityException. As research into the issue did not yield reproduction steps, we decided to surround the breaking method call with a try/catch block for now (see discussion in the issue). This PR implements just that. Instead of crashing on a SecurityException, the plugin will now return an image path of null.

This PR fixes flutter/flutter#100025.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@JeroenWeener JeroenWeener requested a review from gmackall as a code owner May 16, 2023 08:30
@stuartmorgan-g stuartmorgan-g requested a review from tarrinneal May 16, 2023 20:14
tarrinneal
tarrinneal approved these changes May 16, 2023
View reviewed changes
Copy link
Contributor

@tarrinneal tarrinneal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with nit

...ker/image_picker_android/android/src/main/java/io/flutter/plugins/imagepicker/FileUtils.java Outdated
Copy link
Contributor

@tarrinneal tarrinneal May 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the markdown formatting here won't do anything useful (as far as I know), I would just omit the [this issue] and the parens

gmackall
gmackall approved these changes May 16, 2023
View reviewed changes
Copy link
Member

@gmackall gmackall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (minus outstanding previous nit pointed out by @tarrinneal), thanks for the fix!

@JeroenWeener JeroenWeener force-pushed the image-picker/100025 branch from 6dd1794 to c701668 Compare May 17, 2023 08:48
@tarrinneal
Copy link
Contributor

tarrinneal commented May 17, 2023

@JeroenWeener if you can merge main, we can land this today.

@JeroenWeener JeroenWeener force-pushed the image-picker/100025 branch from c701668 to 44afbb4 Compare May 19, 2023 07:27
@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label May 19, 2023
@auto-submit auto-submit bot merged commit e84b49c into flutter:main May 19, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 22, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2023
@engine-flutter-autoroll
e84b49c [image_picker] Fix crash due to SecurityException (flutter/…
3c4f4ee 
…packages#4004)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 22, 2023
@engine-flutter-autoroll
Roll Packages from 1e214d7 to 83959fb (9 revisions) (#127322)
5b0615c 
flutter/packages@1e214d7...83959fb

2023-05-22 JeroenWeener@users.noreply.github.com [in_app_purchases] Fix mismatching method signature strings (flutter/packages#4040)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 engine-flutter-autoroll@skia.org Roll Flutter from 077d644 to ab57304 (18 revisions) (flutter/packages#4051)
2023-05-19 engine-flutter-autoroll@skia.org Roll Flutter from 5ae6438 to 077d644 (23 revisions) (flutter/packages#4043)
2023-05-19 stuartmorgan@google.com [local_auth] Migrate iOS to Pigeon (flutter/packages#3974)
2023-05-19 hamadyalghanim@gmail.com [go_router] fix context extension for replaceNamed (flutter/packages#3927)
2023-05-19 JeroenWeener@users.noreply.github.com [image_picker] Fix crash due to `SecurityException` (flutter/packages#4004)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
sybrands-place pushed a commit to sybrands-place/packages that referenced this pull request May 23, 2023
Merge branch 'main' into webview_patchedv5
a3ba9c3 
* main: (104 commits)
  [various] Remove unnecessary null checks (flutter#4060)
  [ci] Add a legacy Android build-all test (flutter#4005)
  Roll Flutter from ab57304 to 3437189 (5 revisions) (flutter#4062)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [in_app_purchases] Fix mismatching method signature strings (flutter#4040)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  [go_router] Nested stateful navigation with ShellRoute (flutter#2650)
  Roll Flutter from 077d644 to ab57304 (18 revisions) (flutter#4051)
  Roll Flutter from 5ae6438 to 077d644 (23 revisions) (flutter#4043)
  [local_auth] Migrate iOS to Pigeon (flutter#3974)
  [go_router] fix context extension for replaceNamed (flutter#3927)
  [image_picker] Fix crash due to `SecurityException` (flutter#4004)
  Roll Flutter from d0d1feb to 5ae6438 (42 revisions) (flutter#4038)
  [ci] Lower iOS LUCI timeouts (flutter#4035)
  [ci] Increase Android sharding (flutter#4029)
  [flutter_plugin_android_lifecycle] Fix lints (flutter#4030)
  [rfw] Fix a typo in the API documentation (flutter#4023)
  ...
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
@engine-flutter-autoroll
Roll Packages from 1e214d7 to 83959fb (9 revisions) (flutter#127322)
8f54016 
flutter/packages@1e214d7...83959fb

2023-05-22 JeroenWeener@users.noreply.github.com [in_app_purchases] Fix mismatching method signature strings (flutter/packages#4040)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 engine-flutter-autoroll@skia.org Roll Flutter from 077d644 to ab57304 (18 revisions) (flutter/packages#4051)
2023-05-19 engine-flutter-autoroll@skia.org Roll Flutter from 5ae6438 to 077d644 (23 revisions) (flutter/packages#4043)
2023-05-19 stuartmorgan@google.com [local_auth] Migrate iOS to Pigeon (flutter/packages#3974)
2023-05-19 hamadyalghanim@gmail.com [go_router] fix context extension for replaceNamed (flutter/packages#3927)
2023-05-19 JeroenWeener@users.noreply.github.com [image_picker] Fix crash due to `SecurityException` (flutter/packages#4004)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This was referenced Jul 7, 2023
Merged
Closed
Merged
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
@JeroenWeener @nploi
[image_picker] Fix crash due to SecurityException (flutter#4004)
29827aa 
Issue flutter/flutter#100025 mentions crashing of the image picker plugin due to a `SecurityException`. As research into the issue did not yield reproduction steps, we decided to surround the breaking method call with a `try/catch` block for now (see discussion in the issue). This PR implements just that. Instead of crashing on a `SecurityException`, the plugin will now return an image path of `null`.

This PR fixes flutter/flutter#100025.
@reidbaker reidbaker mentioned this pull request Jul 26, 2023
Merged
11 tasks
@reidbaker reidbaker mentioned this pull request Aug 4, 2023
Merged
11 tasks
@gmackall gmackall mentioned this pull request Nov 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmackall gmackall gmackall approved these changes

@tarrinneal tarrinneal tarrinneal approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App p: image_picker platform-android

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[image_picker] [Android] java.lang.SecurityException io.flutter.plugins.imagepicker.FileUtils.getPathFromUri

3 participants

@JeroenWeener @tarrinneal @gmackall