catch SecurityException on media URI access in the share flow - #143
Open
munzzyy wants to merge 1 commit into
Open
catch SecurityException on media URI access in the share flow#143munzzyy wants to merge 1 commit into
munzzyy wants to merge 1 commit into
Conversation
…hare flow ContentResolver.openInputStream() throws SecurityException, not FileNotFoundException, when the app no longer has access to a content:// URI - e.g. a MediaStore URI shared in from another app whose temporary read grant already expired. The share screen only caught FileNotFoundException around these calls, so an expired grant crashed the app instead of failing gracefully like every other already-handled read error. Catch SecurityException the same way FileNotFoundException is already handled in checkProof/proofExists, generateProof, and generateMultiProof - treat it as can't-read-this-media and let the UI fall back to its existing no-proof/error state instead of dying. Fixes guardianproject#137
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #137.
The crash report's
java.lang.SecurityException: org.witness.proofmode has no access to content://media/external/video/media/1000011572comes fromContentResolver.openInputStream(). That method documentsSecurityExceptionseparately fromFileNotFoundException, thrown when the caller lacks a valid grant for the URI. Acontent://media/...URI like this one is exactly what you get fromIntent.EXTRA_STREAMon an incomingACTION_SEND, and its read grant is temporary, tied to the sending app's intent rather than persisted. If that grant is gone by the time ProofMode tries to read it,openInputStreamthrowsSecurityException.ShareProofActivityonly catchesFileNotFoundExceptionaround theseopenInputStreamcalls, incheckProof()/proofExists()(runs automatically fromonResume()the moment the share screen opens),generateProof(), andgenerateMultiProof(). SinceSecurityExceptionis aRuntimeException, not anIOException, none of those catch blocks touch it, so it escapes the coroutine and crashes the app. That matches a crash that fires right on opening the share screen, before the user does anything.Fix: catch
SecurityExceptionalongside the existingFileNotFoundExceptionin those three spots, with the same fallback that's already there for a missing file. Fail this one media item gracefully instead of taking the app down.What I checked:
proofExists()'sopenInputStreamcall insidecheckProof()'s coroutine, matching the IO-dispatcher frames beneath it.checkProof()runs unconditionally inonResume()for bothACTION_SENDandACTION_SEND_MULTIPLE.openInputStream's documentedSecurityExceptionbehavior.kotlincto confirm it's valid Kotlin. Order doesn't matter here sinceFileNotFoundExceptionandSecurityExceptionaren't related types.:appGradle build here.plugin-location-protocoldepends on the generated:fluttermodule, which needs the Flutter SDK to scaffoldflutter-location-protocol/.android, not installed in this environment and unrelated to this change. If you can run./gradlew :app:compileDebugKotlinlocally that would be good extra confirmation.