Fix the xml parsing of multi-package sessions.

The existing code didn't restore the child sessions, since the
XmlPullParser object was advanced within the
PackageInstallerSession#readFromXml code (in order to read grant
permissions).

Slightly refactor the code to make it more self-contained and testable,
and adding a test to make sure that the most important bits of a
session, or of a group of sessions, are dumped and restored properly.

Bug: 118865310
Bug: 109941548
Test: atest FrameworksServicesTests:PackageInstallerSessionTest;
verified that install_sessions.xml is correctly written and restored in
presence of multi-group sessions.
Change-Id: I5aef61f64e7223844f11661ee068dd3c1e112611

3 files changed, 347 insertions, 42 deletions

diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java
index bf4e272d4004..1281b7056f21 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerService.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerService.java
@@ -310,7 +310,6 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
             in.setInput(fis, StandardCharsets.UTF_8.name());
 
             int type;
-            PackageInstallerSession currentSession = null;
             while ((type = in.next()) != END_DOCUMENT) {
                 if (type == START_TAG) {
                     final String tag = in.getName();
@@ -320,9 +319,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
                             session = PackageInstallerSession.readFromXml(in, mInternalCallback,
                                     mContext, mPm, mInstallThread.getLooper(), mStagingManager,
                                     mSessionsDir, this);
-                            currentSession = session;
                         } catch (Exception e) {
-                            currentSession = null;
                             Slog.e(TAG, "Could not read session", e);
                             continue;
                         }
@@ -347,10 +344,6 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
                             addHistoricalSessionLocked(session);
                         }
                         mAllocatedSessions.put(session.sessionId, true);
-                    } else if (currentSession != null
-                            && PackageInstallerSession.TAG_CHILD_SESSION.equals(tag)) {
-                        currentSession.addChildSessionIdInternal(
-                                PackageInstallerSession.readChildSessionIdFromXml(in));
                     }
                 }
             }
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 12d335dd87ab..33d23e75cc11 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -2057,6 +2057,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         return mStagedSessionFailed;
     }
 
+    /** {@hide} */
+    @StagedSessionErrorCode int getStagedSessionErrorCode() {
+        return mStagedSessionErrorCode;
+    }
+
     private void destroyInternal() {
         synchronized (mLock) {
             mSealed = true;
@@ -2221,35 +2226,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         out.endTag(null, TAG_SESSION);
     }
 
-    private static String[] readGrantedRuntimePermissions(XmlPullParser in)
-            throws IOException, XmlPullParserException {
-        List<String> permissions = null;
-
-        final int outerDepth = in.getDepth();
-        int type;
-        while ((type = in.next()) != XmlPullParser.END_DOCUMENT
-                && (type != XmlPullParser.END_TAG || in.getDepth() > outerDepth)) {
-            if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
-                continue;
-            }
-            if (TAG_GRANTED_RUNTIME_PERMISSION.equals(in.getName())) {
-                String permission = readStringAttribute(in, ATTR_NAME);
-                if (permissions == null) {
-                    permissions = new ArrayList<>();
-                }
-                permissions.add(permission);
-            }
-        }
-
-        if (permissions == null) {
-            return null;
-        }
-
-        String[] permissionsArray = new String[permissions.size()];
-        permissions.toArray(permissionsArray);
-        return permissionsArray;
-    }
-
     // Sanity check to be performed when the session is restored from an external file. Only one
     // of the session states should be true, or none of them.
     private static boolean isStagedSessionStateValid(boolean isReady, boolean isApplied,
@@ -2273,8 +2249,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
      * @param sessionProvider
      * @return The newly created session
      */
-    // TODO(patb,109941548): modify readFromXml to consume to the next tag session tag so we
-    //                       can have a complete session for the constructor
     public static PackageInstallerSession readFromXml(@NonNull XmlPullParser in,
             @NonNull PackageInstallerService.InternalCallback callback, @NonNull Context context,
             @NonNull PackageManagerService pm, Looper installerThread,
@@ -2314,8 +2288,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         params.volumeUuid = readStringAttribute(in, ATTR_VOLUME_UUID);
         params.installReason = readIntAttribute(in, ATTR_INSTALL_REASON);
 
-        params.grantedRuntimePermissions = readGrantedRuntimePermissions(in);
-
         final File appIconFile = buildAppIconFile(sessionId, sessionsDir);
         if (appIconFile.exists()) {
             params.appIcon = BitmapFactory.decodeFile(appIconFile.getAbsolutePath());
@@ -2324,16 +2296,51 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         final boolean isReady = readBooleanAttribute(in, ATTR_IS_READY);
         final boolean isFailed = readBooleanAttribute(in, ATTR_IS_FAILED);
         final boolean isApplied = readBooleanAttribute(in, ATTR_IS_APPLIED);
-        final int stagedSessionErrorCode = readIntAttribute(in, ATTR_STAGED_SESSION_ERROR_CODE);
+        final int stagedSessionErrorCode = readIntAttribute(in, ATTR_STAGED_SESSION_ERROR_CODE,
+                SessionInfo.NO_ERROR);
 
         if (!isStagedSessionStateValid(isReady, isApplied, isFailed)) {
             throw new IllegalArgumentException("Can't restore staged session with invalid state.");
         }
 
+        // Parse sub tags of this session, typically used for repeated values / arrays.
+        // Sub tags can come in any order, therefore we need to keep track of what we find while
+        // parsing and only set the right values at the end.
+
+        // Store the current depth. We should stop parsing when we reach an end tag at the same
+        // depth.
+        List<String> permissions = new ArrayList<>();
+        List<Integer> childSessionIds = new ArrayList<>();
+        int outerDepth = in.getDepth();
+        int type;
+        while ((type = in.next()) != XmlPullParser.END_DOCUMENT
+                && (type != XmlPullParser.END_TAG || in.getDepth() > outerDepth)) {
+            if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) {
+                continue;
+            }
+            if (TAG_GRANTED_RUNTIME_PERMISSION.equals(in.getName())) {
+                permissions.add(readStringAttribute(in, ATTR_NAME));
+            }
+            if (TAG_CHILD_SESSION.equals(in.getName())) {
+                childSessionIds.add(readIntAttribute(in, ATTR_SESSION_ID, SessionInfo.INVALID_ID));
+            }
+        }
+
+        if (permissions.size() > 0) {
+            params.grantedRuntimePermissions = permissions.stream().toArray(String[]::new);
+        }
+
+        int[] childSessionIdsArray;
+        if (childSessionIds.size() > 0) {
+            childSessionIdsArray = childSessionIds.stream().mapToInt(i -> i).toArray();
+        } else {
+            childSessionIdsArray = EMPTY_CHILD_SESSION_ARRAY;
+        }
+
         return new PackageInstallerSession(callback, context, pm, sessionProvider,
                 installerThread, stagingManager, sessionId, userId, installerPackageName,
                 installerUid, params, createdMillis, stageDir, stageCid, prepared, sealed,
-                EMPTY_CHILD_SESSION_ARRAY, parentSessionId, isReady, isFailed, isApplied,
+                childSessionIdsArray, parentSessionId, isReady, isFailed, isApplied,
                 stagedSessionErrorCode);
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java b/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java
new file mode 100644
index 000000000000..73e96134167a
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/pm/PackageInstallerSessionTest.java
@@ -0,0 +1,305 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.pm;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.xmlpull.v1.XmlPullParser.END_DOCUMENT;
+import static org.xmlpull.v1.XmlPullParser.START_TAG;
+
+import android.content.pm.PackageInstaller;
+import android.util.AtomicFile;
+import android.util.Slog;
+import android.util.Xml;
+
+import androidx.test.runner.AndroidJUnit4;
+
+import com.android.internal.os.BackgroundThread;
+import com.android.internal.util.FastXmlSerializer;
+
+import libcore.io.IoUtils;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.xmlpull.v1.XmlPullParser;
+import org.xmlpull.v1.XmlPullParserException;
+import org.xmlpull.v1.XmlSerializer;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(AndroidJUnit4.class)
+public class PackageInstallerSessionTest {
+    private File mTmpDir;
+    private AtomicFile mSessionsFile;
+    private static final String TAG_SESSIONS = "sessions";
+
+    @Mock
+    PackageManagerService mMockPackageManagerInternal;
+
+    @Before
+    public void setUp() throws Exception {
+        mTmpDir = IoUtils.createTemporaryDirectory("PackageInstallerSessionTest");
+        mSessionsFile = new AtomicFile(
+                new File(mTmpDir.getAbsolutePath() + "/sessions.xml"), "package-session");
+        MockitoAnnotations.initMocks(this);
+    }
+
+    @Test
+    public void testWriteAndRestoreSessionXmlSimpleSession() {
+        PackageInstallerSession session = createSimpleSession();
+        dumpSession(session);
+        List<PackageInstallerSession> restored = restoreSessions();
+        assertEquals(1, restored.size());
+        assertSessionsEquivalent(session, restored.get(0));
+    }
+
+    @Test
+    public void testWriteAndRestoreSessionXmlStagedSession() {
+        PackageInstallerSession session = createStagedSession();
+        dumpSession(session);
+        List<PackageInstallerSession> restored = restoreSessions();
+        assertEquals(1, restored.size());
+        assertSessionsEquivalent(session, restored.get(0));
+    }
+
+    @Test
+    public void testWriteAndRestoreSessionXmlGrantedPermission() {
+        PackageInstallerSession session = createSessionWithGrantedPermissions();
+        dumpSession(session);
+        List<PackageInstallerSession> restored = restoreSessions();
+        assertEquals(1, restored.size());
+        assertSessionsEquivalent(session, restored.get(0));
+    }
+
+    @Test
+    public void testWriteAndRestoreSessionXmlMultiPackageSessions() {
+        PackageInstallerSession session = createMultiPackageParentSession(123, new int[]{234, 345});
+        PackageInstallerSession childSession1 = createMultiPackageChildSession(234, 123);
+        PackageInstallerSession childSession2 = createMultiPackageChildSession(345, 123);
+        List<PackageInstallerSession> sessionGroup =
+                Arrays.asList(session, childSession1, childSession2);
+        dumpSessions(sessionGroup);
+        List<PackageInstallerSession> restored = restoreSessions();
+        assertEquals(3, restored.size());
+        assertSessionsEquivalent(sessionGroup, restored);
+    }
+
+    private PackageInstallerSession createSimpleSession() {
+        return createSession(false, false, 123, false, PackageInstaller.SessionInfo.INVALID_ID,
+                null);
+    }
+
+    private PackageInstallerSession createStagedSession() {
+        return createSession(true, false, 123, false, PackageInstaller.SessionInfo.INVALID_ID,
+                null);
+    }
+
+    private PackageInstallerSession createSessionWithGrantedPermissions() {
+        return createSession(false, true, 123, false, PackageInstaller.SessionInfo.INVALID_ID,
+                null);
+    }
+
+    private PackageInstallerSession createMultiPackageParentSession(int sessionId,
+                                                                    int[] childSessionIds) {
+        return createSession(false, false, sessionId, true,
+                PackageInstaller.SessionInfo.INVALID_ID, childSessionIds);
+    }
+
+    private PackageInstallerSession createMultiPackageChildSession(int sessionId,
+                                                                   int parentSessionId) {
+        return createSession(false, false, sessionId, false, parentSessionId, null);
+    }
+
+    private PackageInstallerSession createSession(boolean staged, boolean withGrantedPermissions,
+                                                  int sessionId, boolean isMultiPackage,
+                                                  int parentSessionId, int[] childSessionIds) {
+        PackageInstaller.SessionParams params = new PackageInstaller.SessionParams(
+                PackageInstaller.SessionParams.MODE_FULL_INSTALL);
+        if (staged) {
+            params.isStaged = true;
+        }
+        if (withGrantedPermissions) {
+            params.grantedRuntimePermissions = new String[]{"permission1", "permission2"};
+        }
+        if (isMultiPackage) {
+            params.isMultiPackage = true;
+        }
+        return new PackageInstallerSession(
+                /* callback */ null,
+                /* context */null,
+                /* pm */ mMockPackageManagerInternal,
+                /* sessionProvider */ null,
+                /* looper */ BackgroundThread.getHandler().getLooper(),
+                /* stagingManager */ null,
+                /* sessionId */ sessionId,
+                /* userId */  456,
+                /* installerPackageName */ "testInstaller",
+                /* installerUid */ -1,
+                /* sessionParams */ params,
+                /* createdMillis */ 0L,
+                /* stageDir */ mTmpDir,
+                /* stageCid */ null,
+                /* prepared */ true,
+                /* sealed */ false,  // Setting to true would trigger some PM logic.
+                /* childSessionIds */ childSessionIds != null ? childSessionIds : new int[0],
+                /* parentSessionId */ parentSessionId,
+                /* isReady */ staged ? true : false,
+                /* isFailed */ false,
+                /* isApplied */false,
+                /* stagedSessionErrorCode */ PackageInstaller.SessionInfo.VERIFICATION_FAILED);
+    }
+
+    private void dumpSession(PackageInstallerSession session) {
+        dumpSessions(Arrays.asList(session));
+    }
+
+    private void dumpSessions(List<PackageInstallerSession> sessions) {
+        FileOutputStream fos = null;
+        try {
+            fos = mSessionsFile.startWrite();
+
+            XmlSerializer out = new FastXmlSerializer();
+            out.setOutput(fos, StandardCharsets.UTF_8.name());
+            out.startDocument(null, true);
+            out.startTag(null, TAG_SESSIONS);
+            for (PackageInstallerSession session : sessions) {
+                session.write(out, mTmpDir);
+            }
+            out.endTag(null, TAG_SESSIONS);
+            out.endDocument();
+
+            mSessionsFile.finishWrite(fos);
+            Slog.d("PackageInstallerSessionTest", new String(mSessionsFile.readFully()));
+        } catch (IOException e) {
+            if (fos != null) {
+                mSessionsFile.failWrite(fos);
+            }
+        }
+    }
+
+    // This is roughly the logic used in PackageInstallerService to read the session. Note that
+    // this test stresses readFromXml method from PackageInstallerSession, and doesn't cover the
+    // PackageInstallerService portion of the parsing.
+    private List<PackageInstallerSession> restoreSessions() {
+        List<PackageInstallerSession> ret = new ArrayList<>();
+        FileInputStream fis = null;
+        try {
+            fis = mSessionsFile.openRead();
+            final XmlPullParser in = Xml.newPullParser();
+            in.setInput(fis, StandardCharsets.UTF_8.name());
+
+            int type;
+            while ((type = in.next()) != END_DOCUMENT) {
+                if (type == START_TAG) {
+                    final String tag = in.getName();
+                    if (PackageInstallerSession.TAG_SESSION.equals(tag)) {
+                        final PackageInstallerSession session;
+                        try {
+                            session = PackageInstallerSession.readFromXml(in, null,
+                                    null, mMockPackageManagerInternal,
+                                    BackgroundThread.getHandler().getLooper(), null,
+                                    mTmpDir, null);
+                            ret.add(session);
+                        } catch (Exception e) {
+                            Slog.e("PackageInstallerSessionTest", "Exception ", e);
+                            continue;
+                        }
+                    }
+                }
+            }
+        } catch (FileNotFoundException e) {
+            // Missing sessions are okay, probably first boot
+        } catch (IOException | XmlPullParserException e) {
+
+        } finally {
+            IoUtils.closeQuietly(fis);
+        }
+        return ret;
+    }
+
+    private void assertSessionParamsEquivalent(PackageInstaller.SessionParams expected,
+                                               PackageInstaller.SessionParams actual) {
+        assertEquals(expected.mode, actual.mode);
+        assertEquals(expected.installFlags, actual.installFlags);
+        assertEquals(expected.installLocation, actual.installLocation);
+        assertEquals(expected.installReason, actual.installReason);
+        assertEquals(expected.sizeBytes, actual.sizeBytes);
+        assertEquals(expected.appPackageName, actual.appPackageName);
+        assertEquals(expected.appIcon, actual.appIcon);
+        assertEquals(expected.originatingUri, actual.originatingUri);
+        assertEquals(expected.originatingUid, actual.originatingUid);
+        assertEquals(expected.referrerUri, actual.referrerUri);
+        assertEquals(expected.abiOverride, actual.abiOverride);
+        assertEquals(expected.volumeUuid, actual.volumeUuid);
+        assertArrayEquals(expected.grantedRuntimePermissions, actual.grantedRuntimePermissions);
+        assertEquals(expected.installerPackageName, actual.installerPackageName);
+        assertEquals(expected.isMultiPackage, actual.isMultiPackage);
+        assertEquals(expected.isStaged, actual.isStaged);
+    }
+
+    private void assertSessionsEquivalent(List<PackageInstallerSession> expected,
+                                          List<PackageInstallerSession> actual) {
+        assertEquals(expected.size(), actual.size());
+        for (PackageInstallerSession expectedSession : expected) {
+            boolean foundSession = false;
+            for (PackageInstallerSession actualSession : actual) {
+                if (expectedSession.sessionId == actualSession.sessionId) {
+                    // We should only encounter each expected session once.
+                    assertFalse(foundSession);
+                    foundSession = true;
+                    assertSessionsEquivalent(expectedSession, actualSession);
+                }
+            }
+            assertTrue(foundSession);
+        }
+    }
+
+    private void assertSessionsEquivalent(PackageInstallerSession expected,
+                                          PackageInstallerSession actual) {
+        assertEquals(expected.sessionId, actual.sessionId);
+        assertEquals(expected.userId, actual.userId);
+        assertSessionParamsEquivalent(expected.params, actual.params);
+        assertEquals(expected.getInstallerUid(), actual.getInstallerUid());
+        assertEquals(expected.stageDir.getAbsolutePath(), actual.stageDir.getAbsolutePath());
+        assertEquals(expected.stageCid, actual.stageCid);
+        assertEquals(expected.isPrepared(), actual.isPrepared());
+        assertEquals(expected.isStaged(), actual.isStaged());
+        assertEquals(expected.isStagedSessionApplied(), actual.isStagedSessionApplied());
+        assertEquals(expected.isStagedSessionFailed(), actual.isStagedSessionFailed());
+        assertEquals(expected.isStagedSessionReady(), actual.isStagedSessionReady());
+        assertEquals(expected.getStagedSessionErrorCode(), actual.getStagedSessionErrorCode());
+        assertEquals(expected.isPrepared(), actual.isPrepared());
+        assertEquals(expected.isSealed(), actual.isSealed());
+        assertEquals(expected.isMultiPackage(), actual.isMultiPackage());
+        assertEquals(expected.hasParentSessionId(), actual.hasParentSessionId());
+        assertEquals(expected.getParentSessionId(), actual.getParentSessionId());
+        assertArrayEquals(expected.getChildSessionIds(), actual.getChildSessionIds());
+    }
+}