Remove corrupt apexes in /data on boot complete

Previously, .json only apexes were treated as valid but those from /data
were cleaned up on boot completion. However corrupt apexes
(which fail to ApexFile::Open) are not cleaned up.

As we drop JSON support, .json only apexes are treated as corrupt. And
these need to be cleaned up as well.

Bug: 143973464
Test: apexd_host_tests
Change-Id: I252bfaf68cf93b02bb3b8fca02bc92b6b2e77c4a

5 files changed, 124 insertions, 2 deletions

diff --git a/apexd/apexd.cpp b/apexd/apexd.cpp
index 1269a359..5c0a4711 100644
--- a/apexd/apexd.cpp
+++ b/apexd/apexd.cpp
@@ -2104,7 +2104,8 @@ void UnmountDanglingMounts() {
   RemoveObsoleteHashTrees();
 }
 
-// Removes APEXes on /data that don't have corresponding pre-installed version.
+// Removes APEXes on /data that don't have corresponding pre-installed version
+// or that are corrupt
 void RemoveOrphanedApexes() {
   auto data_apexes = FindApexFilesByName(kActiveApexPackagesDataDir);
   if (!data_apexes.ok()) {
@@ -2115,7 +2116,10 @@ void RemoveOrphanedApexes() {
   for (const auto& path : *data_apexes) {
     auto apex = ApexFile::Open(path);
     if (!apex.ok()) {
-      LOG(ERROR) << "Failed to open " << path << " : " << apex.error();
+      LOG(DEBUG) << "Removing corrupt APEX " << path << " : " << apex.error();
+      if (unlink(path.c_str()) != 0) {
+        PLOG(ERROR) << "Failed to unlink " << path;
+      }
       continue;
     }
     if (!ShouldActivateApexOnData(*apex)) {
diff --git a/apexd/apexd_testdata/Android.bp b/apexd/apexd_testdata/Android.bp
index 974c22d9..1f74a532 100644
--- a/apexd/apexd_testdata/Android.bp
+++ b/apexd/apexd_testdata/Android.bp
@@ -67,6 +67,33 @@ apex {
     installable: false,
 }
 
+apex {
+    name: "apex.apexd_test_v2_legacy",
+    manifest: "manifest_v2.json",
+    file_contexts: ":apex.test-file_contexts",
+    prebuilts: ["sample_prebuilt_file"],
+    key: "com.android.apex.test_package.key",
+    installable: false,
+    min_sdk_version: "29", // add apex_manifest.json as well
+}
+
+genrule {
+  name: "apex.apexd_test_v2_no_pb",
+  srcs: [":apex.apexd_test_v2_legacy"],
+  out: ["apex.apexd_test_v2_no_pb.apex"],
+  tools: ["zip2zip"],
+  cmd: "$(location zip2zip) -i $(in) -x apex_manifest.pb -o $(out)", // remove apex_manifest.pb
+}
+
+apex {
+    name: "apex.apexd_test_v3",
+    manifest: "manifest_v3.json",
+    file_contexts: ":apex.test-file_contexts",
+    prebuilts: ["sample_prebuilt_file"],
+    key: "com.android.apex.test_package.key",
+    installable: false,
+}
+
 apex_key {
     name: "com.android.apex.test_package.preinstall.key",
     public_key: "com.android.apex.test_package.preinstall.avbpubkey",
diff --git a/apexd/apexd_testdata/manifest_v3.json b/apexd/apexd_testdata/manifest_v3.json
new file mode 100644
index 00000000..fbc02fb3
--- /dev/null
+++ b/apexd/apexd_testdata/manifest_v3.json
@@ -0,0 +1,4 @@
+{
+  "name": "com.android.apex.test_package",
+  "version": 3
+}
diff --git a/tests/Android.bp b/tests/Android.bp
index ed8ae9c8..dfa9b22d 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -298,7 +298,10 @@ java_test_host {
     test_config: "apexd-host-tests.xml",
     test_suites: ["general-tests"],
     data: [
+        ":apex.apexd_test",
         ":apex.apexd_test_v2",
+        ":apex.apexd_test_v2_no_pb",
+        ":apex.apexd_test_v3",
         ":com.android.apex.cts.shim.v2_prebuilt",
         ":com.android.apex.cts.shim.v2_no_pb",
     ],
diff --git a/tests/src/com/android/tests/apex/ApexdHostTest.java b/tests/src/com/android/tests/apex/ApexdHostTest.java
index 2e3a8328..2998ac81 100644
--- a/tests/src/com/android/tests/apex/ApexdHostTest.java
+++ b/tests/src/com/android/tests/apex/ApexdHostTest.java
@@ -26,6 +26,7 @@ import com.android.tradefed.device.ITestDevice;
 import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
 import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
 
+import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -114,4 +115,87 @@ public class ApexdHostTest extends BaseHostJUnit4Test  {
             getDevice().reboot();
         }
     }
+
+    @Test
+    public void testApexWithoutPbIsNotActivated_ProductPartitionHasOlderVersion()
+            throws Exception {
+        assumeTrue("Device does not support updating APEX", mTestUtils.isApexUpdateSupported());
+        assumeTrue("Device requires root", getDevice().isAdbRoot());
+
+        withTestFiles(new String[]{
+                "apex.apexd_test.apex", "/product/apex",
+                "apex.apexd_test_v2_no_pb.apex", "/data/apex/active"
+        }, () -> {
+                final Set<ITestDevice.ApexInfo> activeApexes = getDevice().getActiveApexes();
+                assertThat(activeApexes).contains(new ITestDevice.ApexInfo(
+                        "com.android.apex.test_package", 1L));
+                assertThat(activeApexes).doesNotContain(new ITestDevice.ApexInfo(
+                        "com.android.apex.test_package", 2L));
+
+                // v2_no_pb should be deleted
+                mTestUtils.waitForFileDeleted("/data/apex/active/apex.apexd_test_v2_no_pb.apex",
+                        Duration.ofMinutes(3));
+            });
+    }
+
+    @Test
+    public void testApexWithoutPbIsNotActivated_ProductPartitionHasNewerVersion()
+            throws Exception {
+        assumeTrue("Device does not support updating APEX", mTestUtils.isApexUpdateSupported());
+        assumeTrue("Device requires root", getDevice().isAdbRoot());
+
+        withTestFiles(new String[]{
+                "apex.apexd_test_v3.apex", "/product/apex",
+                "apex.apexd_test_v2_no_pb.apex", "/data/apex/active"
+        }, () -> {
+                final Set<ITestDevice.ApexInfo> activeApexes = getDevice().getActiveApexes();
+                assertThat(activeApexes).contains(new ITestDevice.ApexInfo(
+                        "com.android.apex.test_package", 3L));
+                assertThat(activeApexes).doesNotContain(new ITestDevice.ApexInfo(
+                        "com.android.apex.test_package", 2L));
+
+                // v2_no_pb should be deleted
+                mTestUtils.waitForFileDeleted("/data/apex/active/apex.apexd_test_v2_no_pb.apex",
+                        Duration.ofMinutes(3));
+            });
+    }
+
+    private interface ThrowingRunnable {
+        void run() throws Exception;
+    }
+
+    private void withTestFiles(String[] files, ThrowingRunnable body) throws Exception {
+        Assert.assertTrue("files should have even elements", files.length % 2 == 0);
+        try {
+            getDevice().remountSystemWritable();
+            // In case remount requires a reboot, wait for boot to complete.
+            assertWithMessage("Timed out waiting for device to boot").that(
+                    getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
+
+            // copy test files
+            for (int i = 0; i < files.length; i += 2) {
+                final String filename = files[i];
+                final String path = files[i + 1];
+                final File file = mTestUtils.getTestFile(filename);
+                getDevice().pushFile(file,  path + "/" + filename);
+            }
+
+            getDevice().reboot();
+            assertWithMessage("Timed out waiting for device to boot").that(
+                    getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
+
+            body.run();
+        } finally {
+            getDevice().remountSystemWritable();
+            assertWithMessage("Timed out waiting for device to boot").that(
+                    getDevice().waitForBootComplete(Duration.ofMinutes(2).toMillis())).isTrue();
+
+            // remove test files
+            for (int i = 0; i < files.length; i += 2) {
+                final String filename = files[i];
+                final String path = files[i + 1];
+                getDevice().executeShellV2Command("rm " + path + "/" + filename);
+            }
+        }
+    }
 }