[file_selector_android] Surface file-copy failures to Dart instead of crashing#12107
[file_selector_android] Surface file-copy failures to Dart instead of crashing#12107tonytonycoder11 wants to merge 2 commits into
Conversation
… crashing Return null from FileSelectorApiImpl.toFileResponse when a file cannot be resolved to a path, so the existing error path surfaces a PlatformException to Dart instead of building a FileResponse with a null path (which crashes the non-null Pigeon field). Fixes flutter/flutter#159568
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request fixes a crash in the Android file selector implementation when a selected file cannot be copied to a readable location (such as during a SecurityException). It updates FileSelectorApiImpl to return null when uriPath is null, allowing the failure to be surfaced to Dart, and updates the corresponding tests to verify this behavior. The review feedback suggests using primitive boolean[] arrays instead of boxed Boolean[] arrays in the tests to prevent potential NullPointerExceptions during unboxing in assertions.
Avoids a NullPointerException on unboxing if a callback is never invoked; a primitive boolean[] defaults to false, so a missing callback surfaces as a clear assertion failure instead.
file_selector_androidcrashes when a selected file can't be copied to a readable location.FileSelectorApiImpl.toFileResponsecatchesIOException/SecurityExceptionfromFileUtils.getPathFromCopyOfFileFromUriand sets the path tonull, but then still builds aFileResponsewith that nullpath. Sincepathis non-null in the Pigeon-generatedFileResponse, this throws — aNullPointerExceptionsince the move to the Kotlin Pigeon generator (the issue reported it as anIllegalStateExceptionon the older Java generator) — so the app crashes instead of surfacing an error.When no path can be produced,
toFileResponsenow returnsnull. The callers already handle that by completing the result with an error (the samecompleteWithError("Failed to read file: …")path they use elsewhere when a file can't be read), so the failure now reaches Dart as aPlatformExceptioninstead of crashing the app.I kept this scoped to the crash. The issue also suggests moving these branches onto the typed
FileSelectorNativeExceptionmechanism (like #8184); that would need a Pigeon regeneration and a Dart-facing API change, so I left it out here — happy to do it as a follow-up if that's the preferred direction.The existing test that documented the crash (marked "Remove when fixed") now asserts the failure is surfaced, plus a matching test for the single-file
openFilepath.Fixes flutter/flutter#159568
Pre-Review Checklist
[shared_preferences]///).