Handle shares of content URIs without permission grants gracefully - #14958
Handle shares of content URIs without permission grants gracefully#14958tuncoglu wants to merge 2 commits into
Conversation
When an app shares a content URI without FLAG_GRANT_READ_URI_PERMISSION (e.g. AOSP Contacts sharing a vCard), Signal fails to access it. The failure was silent: SecurityExceptions from resolve() were either swallowed into a generic failure or surfaced as RxJava errors, and the share activity just finished without any user feedback. Now all content-resolver reads are wrapped, SecurityExceptions are distinguished from other failures, and the share activity surfaces a toast explaining the access problem before finishing.
- Any SecurityException while resolving a multi-share item (mime lookup or stream open) now fails the whole share with ACCESS_DENIED, instead of silently sharing a subset of the user's selection. - Mime lookups that are denied are classified as ACCESS_DENIED rather than UNKNOWN when no shareable media remains. - Extracts resolveMedia() returning a sealed MediaResult; removes the mutable accessBlocked flag. - ShareError moved to its own file; share error toast shown once across activity recreation; added ShareViewModelTest.
| }.filterNotNull() | ||
| // Same as above: if URI access was denied for any item, fail the whole share so the user | ||
| // learns why instead of silently sharing a subset of their selection. | ||
| if (mediaResults.any { it is MediaResult.AccessDenied }) { |
There was a problem hiding this comment.
Now fails the whole multi-share with ACCESS_DENIED if any selected item is denied, even when other items resolve fine. Previously accessBlocked was only consulted when the media list was empty, which silently shared a subset — the same silent-loss problem this PR exists to fix.
Non-permission failures (IOException) keep their per-item skip semantics, and an all-skipped multi-share still surfaces UNKNOWN. The check intentionally runs before take(maxAttachmentCount): if a URI the user selected is denied, we surface the error rather than treating the denial as part of truncation. Happy to scope it to the first N items if preferred.
|
|
||
| // If any URI's provider denied the metadata query, the share cannot be completed: fail loudly | ||
| // rather than silently dropping the user's selection. | ||
| if (mimeTypeResolutions.any { it.second.accessDenied }) { |
There was a problem hiding this comment.
Denied mime lookups are now classified as ACCESS_DENIED rather than UNKNOWN. Previously getMimeType() swallowed the SecurityException and returned UNKNOWN, so a multi-share where every provider rejected getType() collapsed to Failure(UNKNOWN) — the permission toast never appeared. Single-share is unchanged: it prefers the intent's mime type as a fallback and only classifies at openInputStream().
| * Whether the current failure has already been surfaced to the user. Prevents the share error | ||
| * toast from being shown again when the activity is recreated while the state is still failed. | ||
| */ | ||
| fun hasShareErrorBeenShown(): Boolean = shareErrorShown |
There was a problem hiding this comment.
RxStore.stateFlowable replays the latest state on subscription, so an activity recreated while the failure state is current (e.g. rotation) would re-show the toast. The flag lives on the ViewModel because that survives configuration changes — the activity instance does not.
1 participant
Related issue: #14617
Problem
When an app shares a
content://URI withoutFLAG_GRANT_READ_URI_PERMISSION(e.g. AOSP/GrapheneOS Contacts sharing a vCard, see #14617), Signal cannot read the URI unless it holds the provider's own permission (e.g.READ_CONTACTS).The failure mode was bad:
ShareRepository.getSize()/getFileName()/getMimeType()callContentResolver.query()/getType()with no exception handling, so aSecurityExceptionpropagated as an RxJava error instead of a share failure.ShareActivityjust calledfinish()— the user got a share sheet that flashes and closes with zero feedback.What changed
ShareRepository — all
ContentResolverreads are wrapped:query()(size/file name),getType()(mime type), andopenInputStream()now catchSecurityException/IOException.SecurityExceptionproducesResolvedShareData.Failure(ShareError.ACCESS_DENIED); other failures produceShareError.UNKNOWN.UX — the silent
finish()is replaced with a toast before finishing:ACCESS_DENIED→ "Signal couldn't access the shared content. The sending app didn't grant the necessary permission."UNKNOWN→ "Signal couldn't load the shared content."Tests — new
ShareRepositoryTest(Robolectric) covering:SecurityExceptionfrom the resolver →Failure(ACCESS_DENIED)(the vCard scenario from Sharing a contact via Android share menu results in 'invalid file' on GrapheneOS #14617)IOException→Failure(UNKNOWN)ExternalUriwith correct mime typeNotes
This is a defensive/UX fix on Signal's side. When the sending app doesn't grant URI access and Signal lacks the provider permission, the share physically cannot be read (platform limitation) — the complementary sender-side fix is GrapheneOS/platform_packages_apps_Contacts#25. With this change, users at least learn why instead of seeing a silent flash, and other apps sharing the same way don't produce confusing silent failures.
Multi-share behavior: if any selected item's provider denies URI access (at
getType()oropenInputStream()), the whole share fails with theACCESS_DENIEDtoast instead of silently sharing the remaining subset.Partially addresses #14617: this makes the failure graceful and explainable, but the contact still cannot be shared until the sending app grants URI access. Related: #12324, #6520.