From 32d4ec8115a97b84e6dae068647cddd1ac468074 Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Wed, 19 Apr 2023 19:06:26 +0000 Subject: Allow refinement of more non-filter-equals fields. This change allows callers' "refinement activities" to override the source intent's ClipData and given 'extras' entries. Note neither of these fields are considered as factors in `Intent.filterEquals()`, so allowing callers to refine their values doesn't violate the documented guardrails on refinement. It also makes sense to support, as otherwise refinement wouldn't be able to modify the "payload" share content at all, which (by the docs for refinement) is clearly something we'd *intended* to support. Test: CTS & atest. Upcoming CL adds new CTS coverage for payload refinements (already verified against this CL). Bug: 262808892 Change-Id: I7fb344fadc04f5ebed83f70f3b16ba9c1f28391c --- .../intentresolver/chooser/DisplayResolveInfo.java | 7 ++-- .../chooser/ImmutableTargetInfo.java | 4 +- .../chooser/SelectableTargetInfo.java | 6 +-- .../android/intentresolver/chooser/TargetInfo.java | 45 ++++++++++++++++++++++ 4 files changed, 54 insertions(+), 8 deletions(-) (limited to 'java') diff --git a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java index 29be6dc6..09cf319f 100644 --- a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java +++ b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java @@ -184,9 +184,10 @@ public class DisplayResolveInfo implements TargetInfo { return null; } - Intent merged = new Intent(matchingBase); - merged.fillIn(proposedRefinement, 0); - return new DisplayResolveInfo(this, merged, mPresentationGetter); + return new DisplayResolveInfo( + this, + TargetInfo.mergeRefinementIntoMatchingBaseIntent(matchingBase, proposedRefinement), + mPresentationGetter); } @Override diff --git a/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java b/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java index 2d9683e1..10d4415a 100644 --- a/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/ImmutableTargetInfo.java @@ -427,8 +427,8 @@ public final class ImmutableTargetInfo implements TargetInfo { return null; } - Intent merged = new Intent(matchingBase); - merged.fillIn(proposedRefinement, 0); + Intent merged = TargetInfo.mergeRefinementIntoMatchingBaseIntent( + matchingBase, proposedRefinement); return toBuilder().setBaseIntentToSend(merged).build(); } diff --git a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java index 74c19e67..5766db0e 100644 --- a/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/SelectableTargetInfo.java @@ -279,9 +279,9 @@ public final class SelectableTargetInfo extends ChooserTargetInfo { return null; } - Intent merged = new Intent(matchingBase); - merged.fillIn(proposedRefinement, 0); - return new SelectableTargetInfo(this, merged); + return new SelectableTargetInfo( + this, + TargetInfo.mergeRefinementIntoMatchingBaseIntent(matchingBase, proposedRefinement)); } @Override diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java index 2f48704c..9d793994 100644 --- a/java/src/com/android/intentresolver/chooser/TargetInfo.java +++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java @@ -454,4 +454,49 @@ public interface TargetInfo { intent.fixUris(currentUserId); } } + + /** + * Derive a "complete" intent from a proposed `refinement` intent by merging it into a matching + * `base` intent, without modifying the filter-equality properties of the `base` intent, while + * still allowing the `refinement` to replace Share "payload" fields. + * Note! Callers are responsible for ensuring that the `base` is a suitable match for the given + * `refinement`, such that the two can be merged without modifying filter-equality properties. + */ + static Intent mergeRefinementIntoMatchingBaseIntent(Intent base, Intent refinement) { + Intent mergedIntent = new Intent(base); + + /* Copy over any fields from the `refinement` that weren't already specified by the `base`, + * along with the refined ClipData (if present, even if that overwrites data given in the + * `base` intent). + * + * Refinement may have modified the payload content stored in the ClipData; such changes + * are permitted in refinement since ClipData isn't a factor in the determination of + * `Intent.filterEquals()` (which must be preserved as an invariant of refinement). */ + mergedIntent.fillIn(refinement, Intent.FILL_IN_CLIP_DATA); + + /* Refinement may also modify payload content held in the 'extras' representation, as again + * those attributes aren't a factor in determining filter-equality. There is no `FILL_IN_*` + * flag that would allow the refinement to overwrite existing keys in the `base` extras, so + * here we have to implement the logic ourselves. + * + * Note this still doesn't imply that the refined intent is the final authority on extras; + * in particular, `SelectableTargetInfo.mActivityStarter` uses `Intent.putExtras(Bundle)` to + * merge in the `mChooserTargetIntentExtras` (i.e., the `EXTRA_SHORTCUT_ID`), which will + * overwrite any existing value. + * + * TODO: formalize the precedence and make sure extras are set in the appropriate stages, + * instead of relying on maintainers to know that (e.g.) authoritative changes belong in the + * `TargetActivityStarter`. Otherwise, any extras-based data that Sharesheet adds internally + * might be susceptible to "spoofing" from the refinement activity. */ + mergedIntent.putExtras(refinement); // Re-merge extras to favor refinement. + + // TODO(b/279067078): consider how to populate the "merged" ClipData. The `base` + // already has non-null ClipData due to the implicit migration in Intent, so if the + // refinement modified any of the payload extras, they *must* also provide a modified + // ClipData, or else the updated "extras" payload will be inconsistent with the + // pre-refinement ClipData when they're merged together. We may be able to do better, + // but there are complicated tradeoffs. + + return mergedIntent; + } } -- cgit v1.2.3-59-g8ed1b