From 0f07c5e801f1ee35281c02c6600e4edd058a4f75 Mon Sep 17 00:00:00 2001
From: Nandana Dutt <nandana@google.com>
Date: Tue, 24 Jul 2018 14:18:09 +0100
Subject: [PATCH] Fix retrieval of instant app cookies from cache

The in-memory cache stores cookies by user-id and package so that
pending persistence can be easily canceled. CTS tests that do "set
cooke -> uninstall pkg -> install pkg -> get cookie" fail on devices
that are fast enough to complete the install within 1 second. This is
because the cache lookup used the package object, which would be
regenerated on install and no longer equal to the old package object.

Changing the look up to package name instead fixes this issue since package
names are stable across installs.

BUG: 110379319
Test: cts-tradefed run cts -m CtsAppSecurityHostTestCases -t
android.appsecurity.cts.InstantCookieHostTest (Reproduced the failed tests before the change, and ensured they now pass).

Change-Id: Ie0138399c715e7a69bea2421fad013559ba75ae3
---
 .../android/server/pm/InstantAppRegistry.java | 28 +++++++++----------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/services/core/java/com/android/server/pm/InstantAppRegistry.java b/services/core/java/com/android/server/pm/InstantAppRegistry.java
index fde13acb8f384..66f110296b349 100644
--- a/services/core/java/com/android/server/pm/InstantAppRegistry.java
+++ b/services/core/java/com/android/server/pm/InstantAppRegistry.java
@@ -1158,15 +1158,13 @@ class InstantAppRegistry {
     private final class CookiePersistence extends Handler {
         private static final long PERSIST_COOKIE_DELAY_MILLIS = 1000L; /* one second */
 
-        // In case you wonder why we stash the cookies aside, we use
-        // the user id for the message id and the package for the payload.
-        // Handler allows removing messages by id and tag where the
-        // tag is compared using ==. So to allow cancelling the
-        // pending persistence for an app under a given user we use
-        // the fact that package are cached by the system so the ==
-        // comparison would match and we end up with a way to cancel
-        // persisting the cookie for a user and package.
-        private final SparseArray<ArrayMap<PackageParser.Package, SomeArgs>> mPendingPersistCookies
+        // The cookies are cached per package name per user-id in this sparse
+        // array. The caching is so that pending persistence can be canceled within
+        // a short interval. To ensure we still return pending persist cookies
+        // for a package that uninstalled and reinstalled while the persistence
+        // was still pending, we use the package name as a key for
+        // mPendingPersistCookies, since that stays stable across reinstalls.
+        private final SparseArray<ArrayMap<String, SomeArgs>> mPendingPersistCookies
                 = new SparseArray<>();
 
         public CookiePersistence(Looper looper) {
@@ -1196,10 +1194,10 @@ class InstantAppRegistry {
 
         public @Nullable byte[] getPendingPersistCookieLPr(@NonNull PackageParser.Package pkg,
                 @UserIdInt int userId) {
-            ArrayMap<PackageParser.Package, SomeArgs> pendingWorkForUser =
+            ArrayMap<String, SomeArgs> pendingWorkForUser =
                     mPendingPersistCookies.get(userId);
             if (pendingWorkForUser != null) {
-                SomeArgs state = pendingWorkForUser.get(pkg);
+                SomeArgs state = pendingWorkForUser.get(pkg.packageName);
                 if (state != null) {
                     return (byte[]) state.arg1;
                 }
@@ -1219,7 +1217,7 @@ class InstantAppRegistry {
         private void addPendingPersistCookieLPw(@UserIdInt int userId,
                 @NonNull PackageParser.Package pkg, @NonNull byte[] cookie,
                 @NonNull File cookieFile) {
-            ArrayMap<PackageParser.Package, SomeArgs> pendingWorkForUser =
+            ArrayMap<String, SomeArgs> pendingWorkForUser =
                     mPendingPersistCookies.get(userId);
             if (pendingWorkForUser == null) {
                 pendingWorkForUser = new ArrayMap<>();
@@ -1228,16 +1226,16 @@ class InstantAppRegistry {
             SomeArgs args = SomeArgs.obtain();
             args.arg1 = cookie;
             args.arg2 = cookieFile;
-            pendingWorkForUser.put(pkg, args);
+            pendingWorkForUser.put(pkg.packageName, args);
         }
 
         private SomeArgs removePendingPersistCookieLPr(@NonNull PackageParser.Package pkg,
                 @UserIdInt int userId) {
-            ArrayMap<PackageParser.Package, SomeArgs> pendingWorkForUser =
+            ArrayMap<String, SomeArgs> pendingWorkForUser =
                     mPendingPersistCookies.get(userId);
             SomeArgs state = null;
             if (pendingWorkForUser != null) {
-                state = pendingWorkForUser.remove(pkg);
+                state = pendingWorkForUser.remove(pkg.packageName);
                 if (pendingWorkForUser.isEmpty()) {
                     mPendingPersistCookies.remove(userId);
                 }