Skip to content

Commit 66e18e6

Browse files
Miranda KephartAndroid Build Coastguard Worker
authored andcommitted
[DO NOT MERGE] Update quickshare intent rather than recreating
Currently, we extract the quickshare intent and re-wrap it as a new PendingIntent once we get the screenshot URI. This is insecure as it leads to executing the original with SysUI's permissions, which the app may not have. This change switches to using Intent.fillin to add the URI, keeping the original PendingIntent and original permission set. Bug: 278720336 Fix: 278720336 Test: manual (to test successful quickshare), atest SaveImageInBackgroundTaskTest (to verify original pending intent unchanged) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:589ce3909c6e9e30f073df86e7de4503854a032a) Merged-In: Icad3d5f939fcfb894e2038948954bc2735dbe326 Change-Id: Icad3d5f939fcfb894e2038948954bc2735dbe326
1 parent 835e7a9 commit 66e18e6

5 files changed

Lines changed: 356 additions & 53 deletions

File tree

packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java

Lines changed: 66 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,12 @@ protected Void doInBackground(Void... paramsUnused) {
141141
// Since Quick Share target recommendation does not rely on image URL, it is
142142
// queried and surfaced before image compress/export. Action intent would not be
143143
// used, because it does not contain image URL.
144-
queryQuickShareAction(image, user);
144+
Notification.Action quickShare =
145+
queryQuickShareAction(mScreenshotId, image, user, null);
146+
if (quickShare != null) {
147+
mQuickShareData.quickShareAction = quickShare;
148+
mParams.mQuickShareActionsReadyListener.onActionsReady(mQuickShareData);
149+
}
145150
}
146151

147152
// Call synchronously here since already on a background thread.
@@ -180,9 +185,10 @@ protected Void doInBackground(Void... paramsUnused) {
180185
smartActionsEnabled);
181186
mImageData.deleteAction = createDeleteAction(mContext, mContext.getResources(), uri,
182187
smartActionsEnabled);
183-
mImageData.quickShareAction = createQuickShareAction(mContext,
184-
mQuickShareData.quickShareAction, uri);
185-
mImageData.subject = getSubjectString();
188+
mImageData.quickShareAction = createQuickShareAction(
189+
mQuickShareData.quickShareAction, mScreenshotId, uri, mImageTime, image,
190+
user);
191+
mImageData.subject = getSubjectString(mImageTime);
186192

187193
mParams.mActionsReadyListener.onActionsReady(mImageData);
188194
if (DEBUG_CALLBACK) {
@@ -257,7 +263,7 @@ Supplier<ActionTransition> createShareAction(Context context, Resources r, Uri u
257263
new String[]{ClipDescription.MIMETYPE_TEXT_PLAIN}),
258264
new ClipData.Item(uri));
259265
sharingIntent.setClipData(clipdata);
260-
sharingIntent.putExtra(Intent.EXTRA_SUBJECT, getSubjectString());
266+
sharingIntent.putExtra(Intent.EXTRA_SUBJECT, getSubjectString(mImageTime));
261267
sharingIntent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
262268
.addFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
263269

@@ -423,60 +429,73 @@ private static void addIntentExtras(String screenshotId, Intent intent, String a
423429
}
424430

425431
/**
426-
* Populate image uri into intent of Quick Share action.
432+
* Wrap the quickshare intent and populate the fillin intent with the URI
427433
*/
428434
@VisibleForTesting
429-
private Notification.Action createQuickShareAction(Context context, Notification.Action action,
430-
Uri uri) {
431-
if (action == null) {
435+
Notification.Action createQuickShareAction(
436+
Notification.Action quickShare, String screenshotId, Uri uri, long imageTime,
437+
Bitmap image, UserHandle user) {
438+
if (quickShare == null) {
432439
return null;
440+
} else if (quickShare.actionIntent.isImmutable()) {
441+
Notification.Action quickShareWithUri =
442+
queryQuickShareAction(screenshotId, image, user, uri);
443+
if (quickShareWithUri == null
444+
|| !quickShareWithUri.title.toString().contentEquals(quickShare.title)) {
445+
return null;
446+
}
447+
quickShare = quickShareWithUri;
433448
}
434-
// Populate image URI into Quick Share chip intent
435-
Intent sharingIntent = action.actionIntent.getIntent();
436-
sharingIntent.setType("image/png");
437-
sharingIntent.putExtra(Intent.EXTRA_STREAM, uri);
438-
String subjectDate = DateFormat.getDateTimeInstance().format(new Date(mImageTime));
439-
String subject = String.format(SCREENSHOT_SHARE_SUBJECT_TEMPLATE, subjectDate);
440-
sharingIntent.putExtra(Intent.EXTRA_SUBJECT, subject);
441-
// Include URI in ClipData also, so that grantPermission picks it up.
442-
// We don't use setData here because some apps interpret this as "to:".
443-
ClipData clipdata = new ClipData(new ClipDescription("content",
444-
new String[]{"image/png"}),
445-
new ClipData.Item(uri));
446-
sharingIntent.setClipData(clipdata);
447-
sharingIntent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
448-
PendingIntent updatedPendingIntent = PendingIntent.getActivity(
449-
context, 0, sharingIntent,
450-
PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE);
451-
452-
// Proxy smart actions through {@link SmartActionsReceiver} for logging smart actions.
453-
Bundle extras = action.getExtras();
449+
450+
Intent wrappedIntent = new Intent(mContext, SmartActionsReceiver.class)
451+
.putExtra(ScreenshotController.EXTRA_ACTION_INTENT, quickShare.actionIntent)
452+
.putExtra(ScreenshotController.EXTRA_ACTION_INTENT_FILLIN,
453+
createFillInIntent(uri, imageTime))
454+
.addFlags(Intent.FLAG_RECEIVER_FOREGROUND);
455+
Bundle extras = quickShare.getExtras();
454456
String actionType = extras.getString(
455457
ScreenshotNotificationSmartActionsProvider.ACTION_TYPE,
456458
ScreenshotNotificationSmartActionsProvider.DEFAULT_ACTION_TYPE);
457-
Intent intent = new Intent(context, SmartActionsReceiver.class)
458-
.putExtra(ScreenshotController.EXTRA_ACTION_INTENT, updatedPendingIntent)
459-
.addFlags(Intent.FLAG_RECEIVER_FOREGROUND);
460459
// We only query for quick share actions when smart actions are enabled, so we can assert
461460
// that it's true here.
462-
addIntentExtras(mScreenshotId, intent, actionType, true /* smartActionsEnabled */);
463-
PendingIntent broadcastIntent = PendingIntent.getBroadcast(context,
464-
mRandom.nextInt(),
465-
intent,
466-
PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE);
467-
return new Notification.Action.Builder(action.getIcon(), action.title,
468-
broadcastIntent).setContextual(true).addExtras(extras).build();
461+
addIntentExtras(screenshotId, wrappedIntent, actionType, true /* smartActionsEnabled */);
462+
PendingIntent broadcastIntent =
463+
PendingIntent.getBroadcast(mContext, mRandom.nextInt(), wrappedIntent,
464+
PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE);
465+
return new Notification.Action.Builder(quickShare.getIcon(), quickShare.title,
466+
broadcastIntent)
467+
.setContextual(true)
468+
.addExtras(extras)
469+
.build();
470+
}
471+
472+
private Intent createFillInIntent(Uri uri, long imageTime) {
473+
Intent fillIn = new Intent();
474+
fillIn.setType("image/png");
475+
fillIn.putExtra(Intent.EXTRA_STREAM, uri);
476+
fillIn.putExtra(Intent.EXTRA_SUBJECT, getSubjectString(imageTime));
477+
// Include URI in ClipData also, so that grantPermission picks it up.
478+
// We don't use setData here because some apps interpret this as "to:".
479+
ClipData clipData = new ClipData(
480+
new ClipDescription("content", new String[]{"image/png"}),
481+
new ClipData.Item(uri));
482+
fillIn.setClipData(clipData);
483+
fillIn.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
484+
return fillIn;
469485
}
470486

471487
/**
472488
* Query and surface Quick Share chip if it is available. Action intent would not be used,
473489
* because it does not contain image URL which would be populated in {@link
474-
* #createQuickShareAction(Context, Notification.Action, Uri)}
490+
* #createQuickShareAction(Notification.Action, String, Uri, long, Bitmap, UserHandle)}
475491
*/
476-
private void queryQuickShareAction(Bitmap image, UserHandle user) {
492+
493+
@VisibleForTesting
494+
Notification.Action queryQuickShareAction(
495+
String screenshotId, Bitmap image, UserHandle user, Uri uri) {
477496
CompletableFuture<List<Notification.Action>> quickShareActionsFuture =
478497
mScreenshotSmartActions.getSmartActionsFuture(
479-
mScreenshotId, null, image, mSmartActionsProvider,
498+
screenshotId, uri, image, mSmartActionsProvider,
480499
ScreenshotSmartActionType.QUICK_SHARE_ACTION,
481500
true /* smartActionsEnabled */, user);
482501
int timeoutMs = DeviceConfig.getInt(
@@ -485,17 +504,17 @@ private void queryQuickShareAction(Bitmap image, UserHandle user) {
485504
500);
486505
List<Notification.Action> quickShareActions =
487506
mScreenshotSmartActions.getSmartActions(
488-
mScreenshotId, quickShareActionsFuture, timeoutMs,
507+
screenshotId, quickShareActionsFuture, timeoutMs,
489508
mSmartActionsProvider,
490509
ScreenshotSmartActionType.QUICK_SHARE_ACTION);
491510
if (!quickShareActions.isEmpty()) {
492-
mQuickShareData.quickShareAction = quickShareActions.get(0);
493-
mParams.mQuickShareActionsReadyListener.onActionsReady(mQuickShareData);
511+
return quickShareActions.get(0);
494512
}
513+
return null;
495514
}
496515

497-
private String getSubjectString() {
498-
String subjectDate = DateFormat.getDateTimeInstance().format(new Date(mImageTime));
516+
private static String getSubjectString(long imageTime) {
517+
String subjectDate = DateFormat.getDateTimeInstance().format(new Date(imageTime));
499518
return String.format(SCREENSHOT_SHARE_SUBJECT_TEMPLATE, subjectDate);
500519
}
501520
}

packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ interface TransitionDestination {
246246
static final String EXTRA_SMART_ACTIONS_ENABLED = "android:smart_actions_enabled";
247247
static final String EXTRA_OVERRIDE_TRANSITION = "android:screenshot_override_transition";
248248
static final String EXTRA_ACTION_INTENT = "android:screenshot_action_intent";
249+
static final String EXTRA_ACTION_INTENT_FILLIN = "android:screenshot_action_intent_fillin";
249250

250251
static final String SCREENSHOT_URI_ID = "android:screenshot_uri_id";
251252
static final String EXTRA_CANCEL_NOTIFICATION = "android:screenshot_cancel_notification";

packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotSmartActions.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import android.os.UserHandle;
3131
import android.util.Log;
3232

33-
import com.android.internal.annotations.VisibleForTesting;
3433
import com.android.systemui.dagger.SysUISingleton;
3534
import com.android.systemui.shared.system.ActivityManagerWrapper;
3635

@@ -61,7 +60,6 @@ public ScreenshotSmartActions(
6160
screenshotNotificationSmartActionsProviderProvider;
6261
}
6362

64-
@VisibleForTesting
6563
CompletableFuture<List<Notification.Action>> getSmartActionsFuture(
6664
String screenshotId, Uri screenshotUri, Bitmap image,
6765
ScreenshotNotificationSmartActionsProvider smartActionsProvider,
@@ -83,7 +81,7 @@ CompletableFuture<List<Notification.Action>> getSmartActionsFuture(
8381
if (image.getConfig() != Bitmap.Config.HARDWARE) {
8482
if (DEBUG_ACTIONS) {
8583
Log.d(TAG, String.format("Bitmap expected: Hardware, Bitmap found: %s. "
86-
+ "Returning empty list.", image.getConfig()));
84+
+ "Returning empty list.", image.getConfig()));
8785
}
8886
return CompletableFuture.completedFuture(Collections.emptyList());
8987
}
@@ -112,7 +110,6 @@ CompletableFuture<List<Notification.Action>> getSmartActionsFuture(
112110
return smartActionsFuture;
113111
}
114112

115-
@VisibleForTesting
116113
List<Notification.Action> getSmartActions(String screenshotId,
117114
CompletableFuture<List<Notification.Action>> smartActionsFuture, int timeoutMs,
118115
ScreenshotNotificationSmartActionsProvider smartActionsProvider,

packages/SystemUI/src/com/android/systemui/screenshot/SmartActionsReceiver.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.android.systemui.screenshot.LogConfig.DEBUG_ACTIONS;
2020
import static com.android.systemui.screenshot.ScreenshotController.EXTRA_ACTION_INTENT;
21+
import static com.android.systemui.screenshot.ScreenshotController.EXTRA_ACTION_INTENT_FILLIN;
2122
import static com.android.systemui.screenshot.ScreenshotController.EXTRA_ACTION_TYPE;
2223
import static com.android.systemui.screenshot.ScreenshotController.EXTRA_ID;
2324

@@ -46,15 +47,17 @@ public class SmartActionsReceiver extends BroadcastReceiver {
4647

4748
@Override
4849
public void onReceive(Context context, Intent intent) {
49-
PendingIntent pendingIntent = intent.getParcelableExtra(EXTRA_ACTION_INTENT);
50+
PendingIntent pendingIntent =
51+
intent.getParcelableExtra(EXTRA_ACTION_INTENT, PendingIntent.class);
52+
Intent fillIn = intent.getParcelableExtra(EXTRA_ACTION_INTENT_FILLIN, Intent.class);
5053
String actionType = intent.getStringExtra(EXTRA_ACTION_TYPE);
5154
if (DEBUG_ACTIONS) {
5255
Log.d(TAG, "Executing smart action [" + actionType + "]:" + pendingIntent.getIntent());
5356
}
5457
ActivityOptions opts = ActivityOptions.makeBasic();
5558

5659
try {
57-
pendingIntent.send(context, 0, null, null, null, null, opts.toBundle());
60+
pendingIntent.send(context, 0, fillIn, null, null, null, opts.toBundle());
5861
} catch (PendingIntent.CanceledException e) {
5962
Log.e(TAG, "Pending intent canceled", e);
6063
}

0 commit comments

Comments
 (0)