Merge "Pass caller information in cancelBugreport." am: 21d36ec4f5
Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1552882 MUST ONLY BE SUBMITTED BY AUTOMERGER Change-Id: Iec3425e57af20906ba0459ba96b06fccedfb8954
This commit is contained in:
@@ -26,7 +26,6 @@ import android.annotation.SystemApi;
|
||||
import android.annotation.SystemService;
|
||||
import android.app.ActivityManager;
|
||||
import android.content.Context;
|
||||
import android.os.Handler;
|
||||
import android.util.Log;
|
||||
import android.widget.Toast;
|
||||
|
||||
@@ -189,13 +188,18 @@ public final class BugreportManager {
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Cancels a currently running bugreport.
|
||||
/**
|
||||
* Cancels the currently running bugreport.
|
||||
*
|
||||
* <p>Apps are only able to cancel their own bugreports. App A cannot cancel a bugreport started
|
||||
* by app B.
|
||||
*
|
||||
* @throws SecurityException if trying to cancel another app's bugreport in progress
|
||||
*/
|
||||
@RequiresPermission(android.Manifest.permission.DUMP)
|
||||
public void cancelBugreport() {
|
||||
try {
|
||||
mBinder.cancelBugreport();
|
||||
mBinder.cancelBugreport(-1 /* callingUid */, mContext.getOpPackageName());
|
||||
} catch (RemoteException e) {
|
||||
throw e.rethrowFromSystemServer();
|
||||
}
|
||||
|
||||
@@ -23,7 +23,10 @@ import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
import android.Manifest;
|
||||
import android.content.BroadcastReceiver;
|
||||
import android.content.Context;
|
||||
import android.content.Intent;
|
||||
import android.content.IntentFilter;
|
||||
import android.os.BugreportManager;
|
||||
import android.os.BugreportManager.BugreportCallback;
|
||||
import android.os.BugreportParams;
|
||||
@@ -31,7 +34,9 @@ import android.os.FileUtils;
|
||||
import android.os.Handler;
|
||||
import android.os.HandlerThread;
|
||||
import android.os.ParcelFileDescriptor;
|
||||
import android.os.Process;
|
||||
import android.os.StrictMode;
|
||||
import android.text.TextUtils;
|
||||
import android.util.Log;
|
||||
|
||||
import androidx.annotation.NonNull;
|
||||
@@ -53,10 +58,11 @@ import org.junit.runner.RunWith;
|
||||
import org.junit.runners.JUnit4;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.Executor;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
|
||||
/**
|
||||
* Tests for BugreportManager API.
|
||||
*/
|
||||
@@ -67,8 +73,16 @@ public class BugreportManagerTest {
|
||||
|
||||
private static final String TAG = "BugreportManagerTest";
|
||||
private static final long BUGREPORT_TIMEOUT_MS = TimeUnit.MINUTES.toMillis(10);
|
||||
private static final long DUMPSTATE_STARTUP_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10);
|
||||
private static final long UIAUTOMATOR_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(10);
|
||||
|
||||
// Sent by Shell when its bugreport finishes (contains final bugreport/screenshot file name
|
||||
// associated with the bugreport).
|
||||
private static final String INTENT_BUGREPORT_FINISHED =
|
||||
"com.android.internal.intent.action.BUGREPORT_FINISHED";
|
||||
private static final String EXTRA_BUGREPORT = "android.intent.extra.BUGREPORT";
|
||||
private static final String EXTRA_SCREENSHOT = "android.intent.extra.SCREENSHOT";
|
||||
|
||||
private Handler mHandler;
|
||||
private Executor mExecutor;
|
||||
private BugreportManager mBrm;
|
||||
@@ -211,6 +225,48 @@ public class BugreportManagerTest {
|
||||
assertFdsAreClosed(mBugreportFd, mScreenshotFd);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void cancelBugreport_noReportStarted() throws Exception {
|
||||
// Without the native DumpstateService running, we don't get a SecurityException.
|
||||
mBrm.cancelBugreport();
|
||||
}
|
||||
|
||||
@LargeTest
|
||||
@Test
|
||||
public void cancelBugreport_fromDifferentUid() throws Exception {
|
||||
assertThat(Process.myUid()).isNotEqualTo(Process.SHELL_UID);
|
||||
|
||||
// Start a bugreport through ActivityManager's shell command - this starts a BR from the
|
||||
// shell UID rather than our own.
|
||||
BugreportBroadcastReceiver br = new BugreportBroadcastReceiver();
|
||||
InstrumentationRegistry.getContext()
|
||||
.registerReceiver(br, new IntentFilter(INTENT_BUGREPORT_FINISHED));
|
||||
UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
|
||||
.executeShellCommand("am bug-report");
|
||||
|
||||
// The command triggers the report through a broadcast, so wait until dumpstate actually
|
||||
// starts up, which may take a bit.
|
||||
waitTillDumpstateRunningOrTimeout();
|
||||
|
||||
try {
|
||||
mBrm.cancelBugreport();
|
||||
fail("Expected cancelBugreport to throw SecurityException when report started by "
|
||||
+ "different UID");
|
||||
} catch (SecurityException expected) {
|
||||
} finally {
|
||||
// Do this in the finally block so that even if this test case fails, we don't break
|
||||
// other test cases unexpectedly due to the still-running shell report.
|
||||
try {
|
||||
// The shell's BR is still running and should complete successfully.
|
||||
br.waitForBugreportFinished();
|
||||
} finally {
|
||||
// The latch may fail for a number of reasons but we still need to unregister the
|
||||
// BroadcastReceiver.
|
||||
InstrumentationRegistry.getContext().unregisterReceiver(br);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void insufficientPermissions_throwsException() throws Exception {
|
||||
dropPermissions();
|
||||
@@ -346,6 +402,28 @@ public class BugreportManagerTest {
|
||||
.adoptShellPermissionIdentity(Manifest.permission.DUMP);
|
||||
}
|
||||
|
||||
private static boolean isDumpstateRunning() {
|
||||
String[] output;
|
||||
try {
|
||||
output =
|
||||
UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
|
||||
.executeShellCommand("ps -A -o NAME | grep dumpstate")
|
||||
.trim()
|
||||
.split("\n");
|
||||
} catch (IOException e) {
|
||||
Log.w(TAG, "Failed to check if dumpstate is running", e);
|
||||
return false;
|
||||
}
|
||||
for (String line : output) {
|
||||
// Check for an exact match since there may be other things that contain "dumpstate" as
|
||||
// a substring (e.g. the dumpstate HAL).
|
||||
if (TextUtils.equals("dumpstate", line)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private static void assertFdIsClosed(ParcelFileDescriptor pfd) {
|
||||
try {
|
||||
int fd = pfd.getFd();
|
||||
@@ -364,18 +442,25 @@ public class BugreportManagerTest {
|
||||
return System.currentTimeMillis();
|
||||
}
|
||||
|
||||
private static boolean shouldTimeout(long startTimeMs) {
|
||||
return now() - startTimeMs >= BUGREPORT_TIMEOUT_MS;
|
||||
private static void waitTillDumpstateRunningOrTimeout() throws Exception {
|
||||
long startTimeMs = now();
|
||||
while (!isDumpstateRunning()) {
|
||||
Thread.sleep(500 /* .5s */);
|
||||
if (now() - startTimeMs >= DUMPSTATE_STARTUP_TIMEOUT_MS) {
|
||||
break;
|
||||
}
|
||||
Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms for dumpstate to start");
|
||||
}
|
||||
}
|
||||
|
||||
private static void waitTillDoneOrTimeout(BugreportCallbackImpl callback) throws Exception {
|
||||
long startTimeMs = now();
|
||||
while (!callback.isDone()) {
|
||||
Thread.sleep(1000 /* 1s */);
|
||||
if (shouldTimeout(startTimeMs)) {
|
||||
if (now() - startTimeMs >= BUGREPORT_TIMEOUT_MS) {
|
||||
break;
|
||||
}
|
||||
Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms");
|
||||
Log.d(TAG, "Waited " + (now() - startTimeMs) + "ms for bugreport to finish");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -450,6 +535,36 @@ public class BugreportManagerTest {
|
||||
assertTrue(device.wait(Until.gone(consentTitleObj), UIAUTOMATOR_TIMEOUT_MS));
|
||||
}
|
||||
|
||||
private class BugreportBroadcastReceiver extends BroadcastReceiver {
|
||||
Intent mBugreportFinishedIntent = null;
|
||||
final CountDownLatch mLatch;
|
||||
|
||||
BugreportBroadcastReceiver() {
|
||||
mLatch = new CountDownLatch(1);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onReceive(Context context, Intent intent) {
|
||||
setBugreportFinishedIntent(intent);
|
||||
mLatch.countDown();
|
||||
}
|
||||
|
||||
private void setBugreportFinishedIntent(Intent intent) {
|
||||
mBugreportFinishedIntent = intent;
|
||||
}
|
||||
|
||||
public Intent getBugreportFinishedIntent() {
|
||||
return mBugreportFinishedIntent;
|
||||
}
|
||||
|
||||
public void waitForBugreportFinished() throws Exception {
|
||||
if (!mLatch.await(BUGREPORT_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
|
||||
throw new Exception("Failed to receive BUGREPORT_FINISHED in "
|
||||
+ BUGREPORT_TIMEOUT_MS + " ms.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A rule to change strict mode vm policy temporarily till test method finished.
|
||||
*
|
||||
|
||||
@@ -94,9 +94,12 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
|
||||
|
||||
@Override
|
||||
@RequiresPermission(android.Manifest.permission.DUMP)
|
||||
public void cancelBugreport() {
|
||||
public void cancelBugreport(int callingUidUnused, String callingPackage) {
|
||||
mContext.enforceCallingOrSelfPermission(android.Manifest.permission.DUMP,
|
||||
"cancelBugreport");
|
||||
int callingUid = Binder.getCallingUid();
|
||||
mAppOps.checkPackage(callingUid, callingPackage);
|
||||
|
||||
synchronized (mLock) {
|
||||
IDumpstate ds = getDumpstateBinderServiceLocked();
|
||||
if (ds == null) {
|
||||
@@ -104,7 +107,11 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
ds.cancelBugreport();
|
||||
// Note: this may throw SecurityException back out to the caller if they aren't
|
||||
// allowed to cancel the report, in which case we should NOT be setting ctl.stop,
|
||||
// since that would unintentionally kill some other app's bugreport, which we
|
||||
// specifically disallow.
|
||||
ds.cancelBugreport(callingUid, callingPackage);
|
||||
} catch (RemoteException e) {
|
||||
Slog.e(TAG, "RemoteException in cancelBugreport", e);
|
||||
}
|
||||
@@ -182,7 +189,7 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub {
|
||||
// lifecycle correctly. If we don't subsequent callers will get
|
||||
// BUGREPORT_ERROR_ANOTHER_REPORT_IN_PROGRESS error.
|
||||
// Note that listener will be notified by the death recipient below.
|
||||
cancelBugreport();
|
||||
cancelBugreport(callingUid, callingPackage);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user