From b9d54474adb78243ac426b9c9b562997c92ff078 Mon Sep 17 00:00:00 2001 From: Richard Uhler Date: Tue, 22 Jan 2019 12:50:08 +0000 Subject: [PATCH 1/3] Assign a rollbackId to all rollbacks. To make it easier to specify what rollback to perform when a rollback is executed. Bug: 112431924 Test: atest RollbackTest Change-Id: I860ad443848341fbb99169a05b084fa797c5e08c --- api/system-current.txt | 1 + .../content/rollback/RollbackInfo.java | 19 ++++++- .../android/server/rollback/RollbackData.java | 8 ++- .../rollback/RollbackManagerServiceImpl.java | 52 ++++++++++++++----- .../server/rollback/RollbackStore.java | 19 ++++--- 5 files changed, 77 insertions(+), 22 deletions(-) diff --git a/api/system-current.txt b/api/system-current.txt index cdd2d804844b8..2bd06bdcb2efa 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -1688,6 +1688,7 @@ package android.content.rollback { public final class RollbackInfo implements android.os.Parcelable { method public int describeContents(); + method public int getRollbackId(); method public void writeToParcel(android.os.Parcel, int); field public static final android.os.Parcelable.Creator CREATOR; field public final android.content.rollback.PackageRollbackInfo targetPackage; diff --git a/core/java/android/content/rollback/RollbackInfo.java b/core/java/android/content/rollback/RollbackInfo.java index 66df4fea3f23b..0803a7c1d6519 100644 --- a/core/java/android/content/rollback/RollbackInfo.java +++ b/core/java/android/content/rollback/RollbackInfo.java @@ -29,6 +29,11 @@ import android.os.Parcelable; @SystemApi public final class RollbackInfo implements Parcelable { + /** + * A unique identifier for the rollback. + */ + private final int mRollbackId; + /** * The package that needs to be rolled back. */ @@ -40,12 +45,21 @@ public final class RollbackInfo implements Parcelable { // staged installs is supported. /** @hide */ - public RollbackInfo(PackageRollbackInfo targetPackage) { + public RollbackInfo(int rollbackId, PackageRollbackInfo targetPackage) { + this.mRollbackId = rollbackId; this.targetPackage = targetPackage; } private RollbackInfo(Parcel in) { - this.targetPackage = PackageRollbackInfo.CREATOR.createFromParcel(in); + mRollbackId = in.readInt(); + targetPackage = PackageRollbackInfo.CREATOR.createFromParcel(in); + } + + /** + * Returns a unique identifier for this rollback. + */ + public int getRollbackId() { + return mRollbackId; } @Override @@ -55,6 +69,7 @@ public final class RollbackInfo implements Parcelable { @Override public void writeToParcel(Parcel out, int flags) { + out.writeInt(mRollbackId); targetPackage.writeToParcel(out, flags); } diff --git a/services/core/java/com/android/server/rollback/RollbackData.java b/services/core/java/com/android/server/rollback/RollbackData.java index f0589aac8239b..4015c4f7b3915 100644 --- a/services/core/java/com/android/server/rollback/RollbackData.java +++ b/services/core/java/com/android/server/rollback/RollbackData.java @@ -28,6 +28,11 @@ import java.util.List; * packages. */ class RollbackData { + /** + * A unique identifier for this rollback. + */ + public final int rollbackId; + /** * The per-package rollback information. */ @@ -44,7 +49,8 @@ class RollbackData { */ public Instant timestamp; - RollbackData(File backupDir) { + RollbackData(int rollbackId, File backupDir) { + this.rollbackId = rollbackId; this.backupDir = backupDir; } } diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 085b4afac0e22..0b0f1dca98d38 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -46,6 +46,7 @@ import android.os.ParcelFileDescriptor; import android.os.Process; import android.os.storage.StorageManager; import android.util.Log; +import android.util.SparseBooleanArray; import com.android.internal.annotations.GuardedBy; import com.android.server.LocalServices; @@ -55,14 +56,16 @@ import com.android.server.pm.PackageManagerServiceUtils; import java.io.File; import java.io.IOException; +import java.security.SecureRandom; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; -import java.util.Map; import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.Random; import java.util.Set; import java.util.function.Consumer; @@ -82,6 +85,13 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // mLock is held when they are called. private final Object mLock = new Object(); + // Used for generating rollback IDs. + private final Random mRandom = new SecureRandom(); + + // Set of allocated rollback ids + @GuardedBy("mLock") + private final SparseBooleanArray mAllocatedRollbackIds = new SparseBooleanArray(); + // Package rollback data for rollback-enabled installs that have not yet // been committed. Maps from sessionId to rollback data. @GuardedBy("mLock") @@ -212,7 +222,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { if (info.packageName.equals(packageName)) { // TODO: Once the RollbackInfo API supports info about // dependant packages, add that info here. - return new RollbackInfo(info); + return new RollbackInfo(data.rollbackId, info); } } return null; @@ -282,15 +292,9 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return; } - // Verify the latest rollback matches the version requested. - // TODO: Check dependant packages too once RollbackInfo includes that - // information. - for (PackageRollbackInfo info : data.packages) { - if (info.packageName.equals(targetPackageName) - && !rollback.targetPackage.higherVersion.equals(info.higherVersion)) { - sendFailure(statusReceiver, "Rollback is out of date."); - return; - } + if (data.rollbackId != rollback.getRollbackId()) { + sendFailure(statusReceiver, "Rollback for package is out of date"); + return; } // Verify the RollbackData is up to date with what's installed on @@ -469,7 +473,15 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { @GuardedBy("mLock") private void loadAllRollbackDataLocked() { mAvailableRollbacks = mRollbackStore.loadAvailableRollbacks(); + for (RollbackData data : mAvailableRollbacks) { + mAllocatedRollbackIds.put(data.rollbackId, true); + } + mRecentlyExecutedRollbacks = mRollbackStore.loadRecentlyExecutedRollbacks(); + for (RollbackInfo info : mRecentlyExecutedRollbacks) { + mAllocatedRollbackIds.put(info.getRollbackId(), true); + } + scheduleExpiration(0); } @@ -732,7 +744,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { mChildSessions.put(childSessionId, parentSessionId); data = mPendingRollbacks.get(parentSessionId); if (data == null) { - data = mRollbackStore.createAvailableRollback(); + int rollbackId = allocateRollbackIdLocked(); + data = mRollbackStore.createAvailableRollback(rollbackId); mPendingRollbacks.put(parentSessionId, data); } data.packages.add(info); @@ -912,4 +925,19 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } return null; } + + @GuardedBy("mLock") + private int allocateRollbackIdLocked() throws IOException { + int n = 0; + int rollbackId; + do { + rollbackId = mRandom.nextInt(Integer.MAX_VALUE - 1) + 1; + if (!mAllocatedRollbackIds.get(rollbackId, false)) { + mAllocatedRollbackIds.put(rollbackId, true); + return rollbackId; + } + } while (n++ < 32); + + throw new IOException("Failed to allocate rollback ID"); + } } diff --git a/services/core/java/com/android/server/rollback/RollbackStore.java b/services/core/java/com/android/server/rollback/RollbackStore.java index eb06bb24c24c5..52208597af2b5 100644 --- a/services/core/java/com/android/server/rollback/RollbackStore.java +++ b/services/core/java/com/android/server/rollback/RollbackStore.java @@ -29,7 +29,6 @@ import org.json.JSONObject; import java.io.File; import java.io.IOException; import java.io.PrintWriter; -import java.nio.file.Files; import java.time.Instant; import java.time.format.DateTimeParseException; import java.util.ArrayList; @@ -58,7 +57,7 @@ class RollbackStore { // base.apk // recently_executed.json // - // * XXX, YYY are random strings from Files.createTempDirectory + // * XXX, YYY are the rollbackIds for the corresponding rollbacks. // * rollback.json contains all relevant metadata for the rollback. This // file is not written until the rollback is made available. // @@ -113,13 +112,14 @@ class RollbackStore { JSONArray array = object.getJSONArray("recentlyExecuted"); for (int i = 0; i < array.length(); ++i) { JSONObject element = array.getJSONObject(i); + int rollbackId = element.getInt("rollbackId"); String packageName = element.getString("packageName"); long higherVersionCode = element.getLong("higherVersionCode"); long lowerVersionCode = element.getLong("lowerVersionCode"); PackageRollbackInfo target = new PackageRollbackInfo(packageName, new PackageRollbackInfo.PackageVersion(higherVersionCode), new PackageRollbackInfo.PackageVersion(lowerVersionCode)); - RollbackInfo rollback = new RollbackInfo(target); + RollbackInfo rollback = new RollbackInfo(rollbackId, target); recentlyExecutedRollbacks.add(rollback); } } catch (IOException | JSONException e) { @@ -135,9 +135,9 @@ class RollbackStore { /** * Creates a new RollbackData instance with backupDir assigned. */ - RollbackData createAvailableRollback() throws IOException { - File backupDir = Files.createTempDirectory(mAvailableRollbacksDir.toPath(), null).toFile(); - return new RollbackData(backupDir); + RollbackData createAvailableRollback(int rollbackId) throws IOException { + File backupDir = new File(mAvailableRollbacksDir, Integer.toString(rollbackId)); + return new RollbackData(rollbackId, backupDir); } /** @@ -162,6 +162,7 @@ class RollbackStore { infoJson.put("lowerVersionCode", info.lowerVersion.versionCode); packagesJson.put(infoJson); } + dataJson.put("rollbackId", data.rollbackId); dataJson.put("packages", packagesJson); dataJson.put("timestamp", data.timestamp.toString()); @@ -195,6 +196,7 @@ class RollbackStore { for (int i = 0; i < recentlyExecutedRollbacks.size(); ++i) { RollbackInfo rollback = recentlyExecutedRollbacks.get(i); JSONObject element = new JSONObject(); + element.put("rollbackId", rollback.getRollbackId()); element.put("packageName", rollback.targetPackage.packageName); element.put("higherVersionCode", rollback.targetPackage.higherVersion.versionCode); element.put("lowerVersionCode", rollback.targetPackage.lowerVersion.versionCode); @@ -216,10 +218,13 @@ class RollbackStore { */ private RollbackData loadRollbackData(File backupDir) throws IOException { try { - RollbackData data = new RollbackData(backupDir); File rollbackJsonFile = new File(backupDir, "rollback.json"); JSONObject dataJson = new JSONObject( IoUtils.readFileAsString(rollbackJsonFile.getAbsolutePath())); + + int rollbackId = dataJson.getInt("rollbackId"); + RollbackData data = new RollbackData(rollbackId, backupDir); + JSONArray packagesJson = dataJson.getJSONArray("packages"); for (int i = 0; i < packagesJson.length(); ++i) { JSONObject infoJson = packagesJson.getJSONObject(i); From a7e9b2db4bc2c3cf146c8d8ba2348792ef316e50 Mon Sep 17 00:00:00 2001 From: Richard Uhler Date: Tue, 22 Jan 2019 17:20:58 +0000 Subject: [PATCH 2/3] Use VersionedPackage in PackageRollbackInfo. Rather than defining a new custom PackageRollbackInfo.PackageVersion type. Also clean up PackageRollbackInfo API by replacing public fields with methods and picking better names. Bug: 112431924 Test: atest RollbackTest Change-Id: I68a91e07b8745df9c5ecb22fdccbfcd76385814a --- api/system-current.txt | 12 +-- .../content/rollback/PackageRollbackInfo.java | 80 +++++++------------ .../rollback/RollbackManagerServiceImpl.java | 48 ++++++----- .../server/rollback/RollbackStore.java | 29 ++++--- .../android/tests/rollback/RollbackTest.java | 80 ++++++++----------- 5 files changed, 109 insertions(+), 140 deletions(-) diff --git a/api/system-current.txt b/api/system-current.txt index 2bd06bdcb2efa..2cdad1bdd36f6 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -1672,18 +1672,12 @@ package android.content.pm.permission { package android.content.rollback { public final class PackageRollbackInfo implements android.os.Parcelable { - ctor public PackageRollbackInfo(String, android.content.rollback.PackageRollbackInfo.PackageVersion, android.content.rollback.PackageRollbackInfo.PackageVersion); method public int describeContents(); + method public String getPackageName(); + method public android.content.pm.VersionedPackage getVersionRolledBackFrom(); + method public android.content.pm.VersionedPackage getVersionRolledBackTo(); method public void writeToParcel(android.os.Parcel, int); field public static final android.os.Parcelable.Creator CREATOR; - field public final android.content.rollback.PackageRollbackInfo.PackageVersion higherVersion; - field public final android.content.rollback.PackageRollbackInfo.PackageVersion lowerVersion; - field public final String packageName; - } - - public static class PackageRollbackInfo.PackageVersion { - ctor public PackageRollbackInfo.PackageVersion(long); - field public final long versionCode; } public final class RollbackInfo implements android.os.Parcelable { diff --git a/core/java/android/content/rollback/PackageRollbackInfo.java b/core/java/android/content/rollback/PackageRollbackInfo.java index 204002426d173..4644a83de4623 100644 --- a/core/java/android/content/rollback/PackageRollbackInfo.java +++ b/core/java/android/content/rollback/PackageRollbackInfo.java @@ -17,11 +17,10 @@ package android.content.rollback; import android.annotation.SystemApi; +import android.content.pm.VersionedPackage; import android.os.Parcel; import android.os.Parcelable; -import java.util.Objects; - /** * Information about a rollback available for a particular package. * @@ -29,59 +28,41 @@ import java.util.Objects; */ @SystemApi public final class PackageRollbackInfo implements Parcelable { - /** - * The name of a package being rolled back. - */ - public final String packageName; + + private final VersionedPackage mVersionRolledBackFrom; + private final VersionedPackage mVersionRolledBackTo; /** - * The version the package was rolled back from. + * Returns the name of the package to roll back from. */ - public final PackageVersion higherVersion; - - /** - * The version the package was rolled back to. - */ - public final PackageVersion lowerVersion; - - /** - * Represents a version of a package. - */ - public static class PackageVersion { - public final long versionCode; - - // TODO(b/120200473): Include apk sha or some other way to distinguish - // between two different apks with the same version code. - public PackageVersion(long versionCode) { - this.versionCode = versionCode; - } - - @Override - public boolean equals(Object other) { - if (other instanceof PackageVersion) { - PackageVersion otherVersion = (PackageVersion) other; - return versionCode == otherVersion.versionCode; - } - return false; - } - - @Override - public int hashCode() { - return Objects.hash(versionCode); - } + public String getPackageName() { + return mVersionRolledBackFrom.getPackageName(); } - public PackageRollbackInfo(String packageName, - PackageVersion higherVersion, PackageVersion lowerVersion) { - this.packageName = packageName; - this.higherVersion = higherVersion; - this.lowerVersion = lowerVersion; + /** + * Returns the version of the package rolled back from. + */ + public VersionedPackage getVersionRolledBackFrom() { + return mVersionRolledBackFrom; + } + + /** + * Returns the version of the package rolled back to. + */ + public VersionedPackage getVersionRolledBackTo() { + return mVersionRolledBackTo; + } + + /** @hide */ + public PackageRollbackInfo(VersionedPackage packageRolledBackFrom, + VersionedPackage packageRolledBackTo) { + this.mVersionRolledBackFrom = packageRolledBackFrom; + this.mVersionRolledBackTo = packageRolledBackTo; } private PackageRollbackInfo(Parcel in) { - this.packageName = in.readString(); - this.higherVersion = new PackageVersion(in.readLong()); - this.lowerVersion = new PackageVersion(in.readLong()); + this.mVersionRolledBackFrom = VersionedPackage.CREATOR.createFromParcel(in); + this.mVersionRolledBackTo = VersionedPackage.CREATOR.createFromParcel(in); } @Override @@ -91,9 +72,8 @@ public final class PackageRollbackInfo implements Parcelable { @Override public void writeToParcel(Parcel out, int flags) { - out.writeString(packageName); - out.writeLong(higherVersion.versionCode); - out.writeLong(lowerVersion.versionCode); + mVersionRolledBackFrom.writeToParcel(out, flags); + mVersionRolledBackTo.writeToParcel(out, flags); } public static final Parcelable.Creator CREATOR = diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 0b0f1dca98d38..2b76d00481820 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -32,6 +32,7 @@ import android.content.pm.PackageManagerInternal; import android.content.pm.PackageParser; import android.content.pm.ParceledListSlice; import android.content.pm.StringParceledListSlice; +import android.content.pm.VersionedPackage; import android.content.rollback.IRollbackManager; import android.content.rollback.PackageRollbackInfo; import android.content.rollback.RollbackInfo; @@ -219,7 +220,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // it's out of date or not, so no need to check package versions here. for (PackageRollbackInfo info : data.packages) { - if (info.packageName.equals(packageName)) { + if (info.getPackageName().equals(packageName)) { // TODO: Once the RollbackInfo API supports info about // dependant packages, add that info here. return new RollbackInfo(data.rollbackId, info); @@ -240,7 +241,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { for (int i = 0; i < mAvailableRollbacks.size(); ++i) { RollbackData data = mAvailableRollbacks.get(i); for (PackageRollbackInfo info : data.packages) { - packageNames.add(info.packageName); + packageNames.add(info.getPackageName()); } } } @@ -282,7 +283,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { */ private void executeRollbackInternal(RollbackInfo rollback, String callerPackageName, IntentSender statusReceiver) { - String targetPackageName = rollback.targetPackage.packageName; + String targetPackageName = rollback.targetPackage.getPackageName(); Log.i(TAG, "Initiating rollback of " + targetPackageName); // Get the latest RollbackData for the target package. @@ -306,15 +307,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // Figure out how to ensure we don't commit the rollback if // roll forward happens at the same time. for (PackageRollbackInfo info : data.packages) { - PackageRollbackInfo.PackageVersion installedVersion = - getInstalledPackageVersion(info.packageName); + VersionedPackage installedVersion = getInstalledPackageVersion(info.getPackageName()); if (installedVersion == null) { // TODO: Test this case sendFailure(statusReceiver, "Package to roll back is not installed"); return; } - if (!info.higherVersion.equals(installedVersion)) { + if (!packageVersionsEqual(info.getVersionRolledBackFrom(), installedVersion)) { // TODO: Test this case sendFailure(statusReceiver, "Package version to roll back not installed."); return; @@ -357,7 +357,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // TODO: Will it always be called "base.apk"? What about splits? // What about apex? - File packageDir = new File(data.backupDir, info.packageName); + File packageDir = new File(data.backupDir, info.getPackageName()); File baseApk = new File(packageDir, "base.apk"); try (ParcelFileDescriptor fd = ParcelFileDescriptor.open(baseApk, ParcelFileDescriptor.MODE_READ_ONLY)) { @@ -431,7 +431,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { while (iter.hasNext()) { RollbackData data = iter.next(); for (PackageRollbackInfo info : data.packages) { - if (info.packageName.equals(packageName)) { + if (info.getPackageName().equals(packageName)) { iter.remove(); mRollbackStore.deleteAvailableRollback(data); break; @@ -493,8 +493,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { private void onPackageReplaced(String packageName) { // TODO: Could this end up incorrectly deleting a rollback for a // package that is about to be installed? - PackageRollbackInfo.PackageVersion installedVersion = - getInstalledPackageVersion(packageName); + VersionedPackage installedVersion = getInstalledPackageVersion(packageName); synchronized (mLock) { ensureRollbackDataLoadedLocked(); @@ -502,8 +501,10 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { while (iter.hasNext()) { RollbackData data = iter.next(); for (PackageRollbackInfo info : data.packages) { - if (info.packageName.equals(packageName) - && !info.higherVersion.equals(installedVersion)) { + if (info.getPackageName().equals(packageName) + && !packageVersionsEqual( + info.getVersionRolledBackFrom(), + installedVersion)) { iter.remove(); mRollbackStore.deleteAvailableRollback(data); break; @@ -526,7 +527,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { boolean changed = false; while (iter.hasNext()) { RollbackInfo rollback = iter.next(); - if (packageName.equals(rollback.targetPackage.packageName)) { + if (packageName.equals(rollback.targetPackage.getPackageName())) { iter.remove(); changed = true; } @@ -701,8 +702,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return false; } - PackageRollbackInfo.PackageVersion newVersion = - new PackageRollbackInfo.PackageVersion(newPackage.versionCode); + VersionedPackage newVersion = new VersionedPackage(packageName, newPackage.versionCode); // Get information about the currently installed package. PackageManagerInternal pm = LocalServices.getService(PackageManagerInternal.class); @@ -713,8 +713,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { Log.e(TAG, packageName + " is not installed"); return false; } - PackageRollbackInfo.PackageVersion installedVersion = - new PackageRollbackInfo.PackageVersion(installedPackage.getLongVersionCode()); + VersionedPackage installedVersion = new VersionedPackage(packageName, + installedPackage.getLongVersionCode()); for (int user : installedUsers) { final int storageFlags; @@ -735,8 +735,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } } - PackageRollbackInfo info = new PackageRollbackInfo( - packageName, newVersion, installedVersion); + PackageRollbackInfo info = new PackageRollbackInfo(newVersion, installedVersion); RollbackData data; try { @@ -832,7 +831,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { * Gets the version of the package currently installed. * Returns null if the package is not currently installed. */ - private PackageRollbackInfo.PackageVersion getInstalledPackageVersion(String packageName) { + private VersionedPackage getInstalledPackageVersion(String packageName) { PackageManager pm = mContext.getPackageManager(); PackageInfo pkgInfo = null; try { @@ -841,7 +840,12 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return null; } - return new PackageRollbackInfo.PackageVersion(pkgInfo.getLongVersionCode()); + return new VersionedPackage(packageName, pkgInfo.getLongVersionCode()); + } + + private boolean packageVersionsEqual(VersionedPackage a, VersionedPackage b) { + return a.getPackageName().equals(b.getPackageName()) + && a.getLongVersionCode() == b.getLongVersionCode(); } private class SessionCallback extends PackageInstaller.SessionCallback { @@ -917,7 +921,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { for (int i = 0; i < mAvailableRollbacks.size(); ++i) { RollbackData data = mAvailableRollbacks.get(i); for (PackageRollbackInfo info : data.packages) { - if (info.packageName.equals(packageName)) { + if (info.getPackageName().equals(packageName)) { return data; } } diff --git a/services/core/java/com/android/server/rollback/RollbackStore.java b/services/core/java/com/android/server/rollback/RollbackStore.java index 52208597af2b5..7738be9d32bbc 100644 --- a/services/core/java/com/android/server/rollback/RollbackStore.java +++ b/services/core/java/com/android/server/rollback/RollbackStore.java @@ -16,6 +16,7 @@ package com.android.server.rollback; +import android.content.pm.VersionedPackage; import android.content.rollback.PackageRollbackInfo; import android.content.rollback.RollbackInfo; import android.util.Log; @@ -116,9 +117,9 @@ class RollbackStore { String packageName = element.getString("packageName"); long higherVersionCode = element.getLong("higherVersionCode"); long lowerVersionCode = element.getLong("lowerVersionCode"); - PackageRollbackInfo target = new PackageRollbackInfo(packageName, - new PackageRollbackInfo.PackageVersion(higherVersionCode), - new PackageRollbackInfo.PackageVersion(lowerVersionCode)); + PackageRollbackInfo target = new PackageRollbackInfo( + new VersionedPackage(packageName, higherVersionCode), + new VersionedPackage(packageName, lowerVersionCode)); RollbackInfo rollback = new RollbackInfo(rollbackId, target); recentlyExecutedRollbacks.add(rollback); } @@ -157,9 +158,11 @@ class RollbackStore { JSONArray packagesJson = new JSONArray(); for (PackageRollbackInfo info : data.packages) { JSONObject infoJson = new JSONObject(); - infoJson.put("packageName", info.packageName); - infoJson.put("higherVersionCode", info.higherVersion.versionCode); - infoJson.put("lowerVersionCode", info.lowerVersion.versionCode); + infoJson.put("packageName", info.getPackageName()); + infoJson.put("higherVersionCode", + info.getVersionRolledBackFrom().getLongVersionCode()); + infoJson.put("lowerVersionCode", + info.getVersionRolledBackTo().getVersionCode()); packagesJson.put(infoJson); } dataJson.put("rollbackId", data.rollbackId); @@ -197,9 +200,11 @@ class RollbackStore { RollbackInfo rollback = recentlyExecutedRollbacks.get(i); JSONObject element = new JSONObject(); element.put("rollbackId", rollback.getRollbackId()); - element.put("packageName", rollback.targetPackage.packageName); - element.put("higherVersionCode", rollback.targetPackage.higherVersion.versionCode); - element.put("lowerVersionCode", rollback.targetPackage.lowerVersion.versionCode); + element.put("packageName", rollback.targetPackage.getPackageName()); + element.put("higherVersionCode", + rollback.targetPackage.getVersionRolledBackFrom().getLongVersionCode()); + element.put("lowerVersionCode", + rollback.targetPackage.getVersionRolledBackTo().getLongVersionCode()); array.put(element); } @@ -231,9 +236,9 @@ class RollbackStore { String packageName = infoJson.getString("packageName"); long higherVersionCode = infoJson.getLong("higherVersionCode"); long lowerVersionCode = infoJson.getLong("lowerVersionCode"); - data.packages.add(new PackageRollbackInfo(packageName, - new PackageRollbackInfo.PackageVersion(higherVersionCode), - new PackageRollbackInfo.PackageVersion(lowerVersionCode))); + data.packages.add(new PackageRollbackInfo( + new VersionedPackage(packageName, higherVersionCode), + new VersionedPackage(packageName, lowerVersionCode))); } data.timestamp = Instant.parse(dataJson.getString("timestamp")); diff --git a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java index ec6f4b55d3ea7..b9271fe5d1353 100644 --- a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java +++ b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java @@ -22,6 +22,7 @@ import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.content.rollback.PackageRollbackInfo; import android.content.rollback.RollbackInfo; import android.content.rollback.RollbackManager; import android.net.Uri; @@ -98,7 +99,7 @@ public class RollbackTest { // so that's not the case! for (int i = 0; i < 5; ++i) { for (RollbackInfo info : rm.getRecentlyExecutedRollbacks()) { - if (TEST_APP_A.equals(info.targetPackage.packageName)) { + if (TEST_APP_A.equals(info.targetPackage.getPackageName())) { Log.i(TAG, "Sleeping 1 second to wait for uninstall to take effect."); Thread.sleep(1000); break; @@ -116,7 +117,7 @@ public class RollbackTest { // There should be no recently executed rollbacks for this package. for (RollbackInfo info : rm.getRecentlyExecutedRollbacks()) { - assertNotEquals(TEST_APP_A, info.targetPackage.packageName); + assertNotEquals(TEST_APP_A, info.targetPackage.getPackageName()); } // Install v1 of the app (without rollbacks enabled). @@ -135,9 +136,7 @@ public class RollbackTest { assertTrue(rm.getPackagesWithAvailableRollbacks().contains(TEST_APP_A)); RollbackInfo rollback = rm.getAvailableRollback(TEST_APP_A); assertNotNull(rollback); - assertEquals(TEST_APP_A, rollback.targetPackage.packageName); - assertEquals(2, rollback.targetPackage.higherVersion.versionCode); - assertEquals(1, rollback.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollback.targetPackage); // We should not have received any rollback requests yet. // TODO: Possibly flaky if, by chance, some other app on device @@ -159,15 +158,13 @@ public class RollbackTest { // Verify the recent rollback has been recorded. rollback = null; for (RollbackInfo r : rm.getRecentlyExecutedRollbacks()) { - if (TEST_APP_A.equals(r.targetPackage.packageName)) { + if (TEST_APP_A.equals(r.targetPackage.getPackageName())) { assertNull(rollback); rollback = r; } } assertNotNull(rollback); - assertEquals(TEST_APP_A, rollback.targetPackage.packageName); - assertEquals(2, rollback.targetPackage.higherVersion.versionCode); - assertEquals(1, rollback.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollback.targetPackage); broadcastReceiver.unregister(); context.unregisterReceiver(enableRollbackReceiver); @@ -208,16 +205,12 @@ public class RollbackTest { assertTrue(rm.getPackagesWithAvailableRollbacks().contains(TEST_APP_A)); RollbackInfo rollbackA = rm.getAvailableRollback(TEST_APP_A); assertNotNull(rollbackA); - assertEquals(TEST_APP_A, rollbackA.targetPackage.packageName); - assertEquals(2, rollbackA.targetPackage.higherVersion.versionCode); - assertEquals(1, rollbackA.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollbackA.targetPackage); assertTrue(rm.getPackagesWithAvailableRollbacks().contains(TEST_APP_B)); RollbackInfo rollbackB = rm.getAvailableRollback(TEST_APP_B); assertNotNull(rollbackB); - assertEquals(TEST_APP_B, rollbackB.targetPackage.packageName); - assertEquals(2, rollbackB.targetPackage.higherVersion.versionCode); - assertEquals(1, rollbackB.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_B, 2, 1, rollbackB.targetPackage); // Reload the persisted data. rm.reloadPersistedData(); @@ -225,16 +218,12 @@ public class RollbackTest { // The apps should still be available for rollback. rollbackA = rm.getAvailableRollback(TEST_APP_A); assertNotNull(rollbackA); - assertEquals(TEST_APP_A, rollbackA.targetPackage.packageName); - assertEquals(2, rollbackA.targetPackage.higherVersion.versionCode); - assertEquals(1, rollbackA.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollbackA.targetPackage); assertTrue(rm.getPackagesWithAvailableRollbacks().contains(TEST_APP_B)); rollbackB = rm.getAvailableRollback(TEST_APP_B); assertNotNull(rollbackB); - assertEquals(TEST_APP_B, rollbackB.targetPackage.packageName); - assertEquals(2, rollbackB.targetPackage.higherVersion.versionCode); - assertEquals(1, rollbackB.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_B, 2, 1, rollbackB.targetPackage); // Rollback of B should not rollback A RollbackTestUtils.rollback(rollbackB); @@ -278,16 +267,12 @@ public class RollbackTest { assertTrue(rm.getPackagesWithAvailableRollbacks().contains(TEST_APP_A)); RollbackInfo rollbackA = rm.getAvailableRollback(TEST_APP_A); assertNotNull(rollbackA); - assertEquals(TEST_APP_A, rollbackA.targetPackage.packageName); - assertEquals(2, rollbackA.targetPackage.higherVersion.versionCode); - assertEquals(1, rollbackA.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollbackA.targetPackage); assertTrue(rm.getPackagesWithAvailableRollbacks().contains(TEST_APP_B)); RollbackInfo rollbackB = rm.getAvailableRollback(TEST_APP_B); assertNotNull(rollbackB); - assertEquals(TEST_APP_B, rollbackB.targetPackage.packageName); - assertEquals(2, rollbackB.targetPackage.higherVersion.versionCode); - assertEquals(1, rollbackB.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_B, 2, 1, rollbackB.targetPackage); // Reload the persisted data. rm.reloadPersistedData(); @@ -295,16 +280,12 @@ public class RollbackTest { // The apps should still be available for rollback. rollbackA = rm.getAvailableRollback(TEST_APP_A); assertNotNull(rollbackA); - assertEquals(TEST_APP_A, rollbackA.targetPackage.packageName); - assertEquals(2, rollbackA.targetPackage.higherVersion.versionCode); - assertEquals(1, rollbackA.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollbackA.targetPackage); assertTrue(rm.getPackagesWithAvailableRollbacks().contains(TEST_APP_B)); rollbackB = rm.getAvailableRollback(TEST_APP_B); assertNotNull(rollbackB); - assertEquals(TEST_APP_B, rollbackB.targetPackage.packageName); - assertEquals(2, rollbackB.targetPackage.higherVersion.versionCode); - assertEquals(1, rollbackB.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_B, 2, 1, rollbackB.targetPackage); // Rollback of B should rollback A as well RollbackTestUtils.rollback(rollbackB); @@ -348,15 +329,13 @@ public class RollbackTest { // Verify the recent rollback has been recorded. rollback = null; for (RollbackInfo r : rm.getRecentlyExecutedRollbacks()) { - if (TEST_APP_A.equals(r.targetPackage.packageName)) { + if (TEST_APP_A.equals(r.targetPackage.getPackageName())) { assertNull(rollback); rollback = r; } } assertNotNull(rollback); - assertEquals(TEST_APP_A, rollback.targetPackage.packageName); - assertEquals(2, rollback.targetPackage.higherVersion.versionCode); - assertEquals(1, rollback.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollback.targetPackage); // Reload the persisted data. rm.reloadPersistedData(); @@ -364,15 +343,13 @@ public class RollbackTest { // Verify the recent rollback is still recorded. rollback = null; for (RollbackInfo r : rm.getRecentlyExecutedRollbacks()) { - if (TEST_APP_A.equals(r.targetPackage.packageName)) { + if (TEST_APP_A.equals(r.targetPackage.getPackageName())) { assertNull(rollback); rollback = r; } } assertNotNull(rollback); - assertEquals(TEST_APP_A, rollback.targetPackage.packageName); - assertEquals(2, rollback.targetPackage.higherVersion.versionCode); - assertEquals(1, rollback.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollback.targetPackage); } finally { RollbackTestUtils.dropShellPermissionIdentity(); } @@ -404,9 +381,7 @@ public class RollbackTest { assertTrue(rm.getPackagesWithAvailableRollbacks().contains(TEST_APP_A)); RollbackInfo rollback = rm.getAvailableRollback(TEST_APP_A); assertNotNull(rollback); - assertEquals(TEST_APP_A, rollback.targetPackage.packageName); - assertEquals(2, rollback.targetPackage.higherVersion.versionCode); - assertEquals(1, rollback.targetPackage.lowerVersion.versionCode); + assertPackageRollbackInfoEquals(TEST_APP_A, 2, 1, rollback.targetPackage); // Expire the rollback. rm.expireRollbackForPackage(TEST_APP_A); @@ -549,11 +524,11 @@ public class RollbackTest { Thread.sleep(1000); RollbackInfo rollbackA = rm.getAvailableRollback(TEST_APP_A); assertNotNull(rollbackA); - assertEquals(TEST_APP_A, rollbackA.targetPackage.packageName); + assertEquals(TEST_APP_A, rollbackA.targetPackage.getPackageName()); RollbackInfo rollbackB = rm.getAvailableRollback(TEST_APP_B); assertNotNull(rollbackB); - assertEquals(TEST_APP_B, rollbackB.targetPackage.packageName); + assertEquals(TEST_APP_B, rollbackB.targetPackage.getPackageName()); // Executing rollback should roll back the correct package. RollbackTestUtils.rollback(rollbackA); @@ -670,7 +645,7 @@ public class RollbackTest { // We should not see a recent rollback listed for TEST_APP_B for (RollbackInfo r : rm.getRecentlyExecutedRollbacks()) { - assertNotEquals(TEST_APP_B, r.targetPackage.packageName); + assertNotEquals(TEST_APP_B, r.targetPackage.getPackageName()); } // TODO: Test the listed dependent apps for the recently executed @@ -680,4 +655,15 @@ public class RollbackTest { RollbackTestUtils.dropShellPermissionIdentity(); } } + + // Helper function to test the value of a PackageRollbackInfo + private void assertPackageRollbackInfoEquals(String packageName, + long versionRolledBackFrom, long versionRolledBackTo, + PackageRollbackInfo info) { + assertEquals(packageName, info.getPackageName()); + assertEquals(packageName, info.getVersionRolledBackFrom().getPackageName()); + assertEquals(versionRolledBackFrom, info.getVersionRolledBackFrom().getLongVersionCode()); + assertEquals(packageName, info.getVersionRolledBackTo().getPackageName()); + assertEquals(versionRolledBackTo, info.getVersionRolledBackTo().getLongVersionCode()); + } } From f8f1b38fd1a97ded49c15a32d3189082b2db63e4 Mon Sep 17 00:00:00 2001 From: Richard Uhler Date: Wed, 23 Jan 2019 10:46:30 +0000 Subject: [PATCH 3/3] Remove package name from ROLLBACK_EXECUTED broadcast. The receiver of the broadcast would likely want to query the rolled back packages via RollbackManager regardless, and it's not clear which package to put when an atomic set of packages is rolled back. Bug: 112431924 Test: atest RollbackTest Change-Id: Ic8db00b62d8993e00a0dd2cb79ae68c430b45bb8 --- core/java/android/content/Intent.java | 1 - .../server/rollback/RollbackManagerServiceImpl.java | 8 +++----- .../android/tests/rollback/RollbackBroadcastReceiver.java | 1 - .../src/com/android/tests/rollback/RollbackTest.java | 5 +---- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index 3c487a13af4bc..13ce714decbd8 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -2363,7 +2363,6 @@ public class Intent implements Parcelable, Cloneable { /** * Broadcast Action: An existing version of an application package has been * rolled back to a previous version. - * The data contains the name of the package. * *

This is a protected intent that can only be sent * by the system. diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 2b76d00481820..7f515bf63bc1c 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -16,7 +16,6 @@ package com.android.server.rollback; -import android.Manifest; import android.app.AppOpsManager; import android.content.BroadcastReceiver; import android.content.Context; @@ -36,7 +35,6 @@ import android.content.pm.VersionedPackage; import android.content.rollback.IRollbackManager; import android.content.rollback.PackageRollbackInfo; import android.content.rollback.RollbackInfo; -import android.net.Uri; import android.os.Binder; import android.os.Bundle; import android.os.Environment; @@ -384,12 +382,12 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { addRecentlyExecutedRollback(rollback); sendSuccess(statusReceiver); - Intent broadcast = new Intent(Intent.ACTION_PACKAGE_ROLLBACK_EXECUTED, - Uri.fromParts("package", targetPackageName, - Manifest.permission.MANAGE_ROLLBACKS)); + Intent broadcast = new Intent(Intent.ACTION_PACKAGE_ROLLBACK_EXECUTED); // TODO: This call emits the warning "Calling a method in the // system process without a qualified user". Fix that. + // TODO: Limit this to receivers holding the + // MANAGE_ROLLBACKS permission? mContext.sendBroadcast(broadcast); } ); diff --git a/tests/RollbackTest/src/com/android/tests/rollback/RollbackBroadcastReceiver.java b/tests/RollbackTest/src/com/android/tests/rollback/RollbackBroadcastReceiver.java index d3c39f0b82488..030641bf08958 100644 --- a/tests/RollbackTest/src/com/android/tests/rollback/RollbackBroadcastReceiver.java +++ b/tests/RollbackTest/src/com/android/tests/rollback/RollbackBroadcastReceiver.java @@ -44,7 +44,6 @@ class RollbackBroadcastReceiver extends BroadcastReceiver { RollbackBroadcastReceiver() { IntentFilter filter = new IntentFilter(); filter.addAction(Intent.ACTION_PACKAGE_ROLLBACK_EXECUTED); - filter.addDataScheme("package"); InstrumentationRegistry.getContext().registerReceiver(this, filter); } diff --git a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java index b9271fe5d1353..9d67cea05fc8b 100644 --- a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java +++ b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java @@ -25,7 +25,6 @@ import android.content.IntentFilter; import android.content.rollback.PackageRollbackInfo; import android.content.rollback.RollbackInfo; import android.content.rollback.RollbackManager; -import android.net.Uri; import android.os.Handler; import android.os.HandlerThread; import android.support.test.InstrumentationRegistry; @@ -152,7 +151,6 @@ public class RollbackTest { // received could lead to test flakiness. Intent broadcast = broadcastReceiver.poll(5, TimeUnit.SECONDS); assertNotNull(broadcast); - assertEquals(TEST_APP_A, broadcast.getData().getSchemeSpecificPart()); assertNull(broadcastReceiver.poll(0, TimeUnit.SECONDS)); // Verify the recent rollback has been recorded. @@ -474,8 +472,7 @@ public class RollbackTest { @Test public void testRollbackBroadcastRestrictions() throws Exception { RollbackBroadcastReceiver broadcastReceiver = new RollbackBroadcastReceiver(); - Intent broadcast = new Intent(Intent.ACTION_PACKAGE_ROLLBACK_EXECUTED, - Uri.fromParts("package", "com.android.tests.rollback.bogus", null)); + Intent broadcast = new Intent(Intent.ACTION_PACKAGE_ROLLBACK_EXECUTED); try { InstrumentationRegistry.getContext().sendBroadcast(broadcast); fail("Succeeded in sending restricted broadcast from app context.");