Snap for 6507793 from 3e7a22b4b08df528038567498fefdc2c4e41bd89 to rvc-d1-release

Change-Id: I388dac8f2b4477d708463eb701006ac9a1dcec9e

13 files changed, 179 insertions, 120 deletions

diff --git a/apexd/Android.bp b/apexd/Android.bp
index e5c7fd95..e8d221d5 100644
--- a/apexd/Android.bp
+++ b/apexd/Android.bp
@@ -178,7 +178,6 @@ cc_defaults {
     "libbase",
     "libcrypto",
     "libcutils",
-    "libjsoncpp",
     "libprotobuf-cpp-full",
     "libziparchive",
   ],
diff --git a/apexd/apex_file.cpp b/apexd/apex_file.cpp
index 850226b7..f91b5d18 100644
--- a/apexd/apex_file.cpp
+++ b/apexd/apex_file.cpp
@@ -80,18 +80,9 @@ Result<ApexFile> ApexFile::Open(const std::string& path) {
   image_size = entry.uncompressed_length;
 
   ret = FindEntry(handle, kManifestFilenamePb, &entry);
-  bool isJsonManifest = false;
   if (ret < 0) {
-    LOG(ERROR) << "Could not find entry \"" << kManifestFilenamePb
-               << "\" in package " << path << ": " << ErrorCodeString(ret);
-    LOG(ERROR) << "Falling back to JSON if present.";
-    isJsonManifest = true;
-    ret = FindEntry(handle, kManifestFilenameJson, &entry);
-    if (ret < 0) {
-      return Error() << "Could not find entry \"" << kManifestFilenameJson
-                     << "\" in package " << path << ": "
-                     << ErrorCodeString(ret);
-    }
+    return Error() << "Could not find entry \"" << kManifestFilenamePb
+                   << "\" in package " << path << ": " << ErrorCodeString(ret);
   }
 
   uint32_t length = entry.uncompressed_length;
@@ -116,18 +107,13 @@ Result<ApexFile> ApexFile::Open(const std::string& path) {
     }
   }
 
-  Result<ApexManifest> manifest;
-  if (isJsonManifest) {
-    manifest = ParseManifestJson(manifest_content);
-  } else {
-    manifest = ParseManifest(manifest_content);
-  }
+  Result<ApexManifest> manifest = ParseManifest(manifest_content);
   if (!manifest.ok()) {
     return manifest.error();
   }
 
-  return ApexFile(path, image_offset, image_size, std::move(*manifest),
-                  isJsonManifest, pubkey, isPathForBuiltinApexes(path));
+  return ApexFile(path, image_offset, image_size, std::move(*manifest), pubkey,
+                  isPathForBuiltinApexes(path));
 }
 
 // AVB-related code.
@@ -353,14 +339,7 @@ Result<void> ApexFile::VerifyManifestMatches(
   Result<ApexManifest> verifiedManifest =
       ReadManifest(mount_path + "/" + kManifestFilenamePb);
   if (!verifiedManifest.ok()) {
-    LOG(ERROR) << "Could not read manifest from  " << mount_path << "/"
-               << kManifestFilenamePb << " : " << verifiedManifest.error();
-    // Fallback to Json manifest if present.
-    LOG(ERROR) << "Trying to find a JSON manifest";
-    verifiedManifest = ReadManifest(mount_path + "/" + kManifestFilenameJson);
-    if (!verifiedManifest.ok()) {
-      return verifiedManifest.error();
-    }
+    return verifiedManifest.error();
   }
 
   if (!MessageDifferencer::Equals(manifest_, *verifiedManifest)) {
diff --git a/apexd/apex_file.h b/apexd/apex_file.h
index a50048aa..0476911c 100644
--- a/apexd/apex_file.h
+++ b/apexd/apex_file.h
@@ -51,7 +51,6 @@ class ApexFile {
   int32_t GetImageOffset() const { return image_offset_; }
   size_t GetImageSize() const { return image_size_; }
   const ApexManifest& GetManifest() const { return manifest_; }
-  bool HasOnlyJsonManifest() const { return has_only_json_manifest_; }
   const std::string& GetBundledPublicKey() const { return apex_pubkey_; }
   bool IsBuiltin() const { return is_builtin_; }
   android::base::Result<ApexVerityData> VerifyApexVerity() const;
@@ -61,13 +60,11 @@ class ApexFile {
  private:
   ApexFile(const std::string& apex_path, int32_t image_offset,
            size_t image_size, ApexManifest manifest,
-           bool has_only_json_manifest, const std::string& apex_pubkey,
-           bool is_builtin)
+           const std::string& apex_pubkey, bool is_builtin)
       : apex_path_(apex_path),
         image_offset_(image_offset),
         image_size_(image_size),
         manifest_(std::move(manifest)),
-        has_only_json_manifest_(has_only_json_manifest),
         apex_pubkey_(apex_pubkey),
         is_builtin_(is_builtin) {}
 
@@ -75,7 +72,6 @@ class ApexFile {
   int32_t image_offset_;
   size_t image_size_;
   ApexManifest manifest_;
-  bool has_only_json_manifest_;
   std::string apex_pubkey_;
   bool is_builtin_;
 };
diff --git a/apexd/apex_manifest.cpp b/apexd/apex_manifest.cpp
index af3bfb8f..1994633c 100644
--- a/apexd/apex_manifest.cpp
+++ b/apexd/apex_manifest.cpp
@@ -16,62 +16,18 @@
 
 #include "apex_manifest.h"
 #include <android-base/file.h>
-#include <android-base/logging.h>
-#include <android-base/strings.h>
-#include "apex_constants.h"
-#include "string_log.h"
 
-#include <google/protobuf/util/json_util.h>
 #include <memory>
 #include <string>
 
-using android::base::EndsWith;
 using android::base::Error;
 using android::base::Result;
-using google::protobuf::util::JsonParseOptions;
 
 namespace android {
 namespace apex {
-namespace {
-
-Result<void> JsonToApexManifestMessage(const std::string& content,
-                                       ApexManifest* apex_manifest) {
-  JsonParseOptions options;
-  options.ignore_unknown_fields = true;
-  auto parse_status = JsonStringToMessage(content, apex_manifest, options);
-  if (!parse_status.ok()) {
-    return Error() << "Failed to parse APEX Manifest JSON config: "
-                   << parse_status.error_message().as_string();
-  }
-  return {};
-}
-
-}  // namespace
-
-Result<ApexManifest> ParseManifestJson(const std::string& content) {
-  ApexManifest apex_manifest;
-  Result<void> parse_manifest_status =
-      JsonToApexManifestMessage(content, &apex_manifest);
-  if (!parse_manifest_status.ok()) {
-    return parse_manifest_status.error();
-  }
-
-  // Verifying required fields.
-  // name
-  if (apex_manifest.name().empty()) {
-    return Error() << "Missing required field \"name\" from APEX manifest.";
-  }
-
-  // version
-  if (apex_manifest.version() == 0) {
-    return Error() << "Missing required field \"version\" from APEX manifest.";
-  }
-  return apex_manifest;
-}
 
 Result<ApexManifest> ParseManifest(const std::string& content) {
   ApexManifest apex_manifest;
-  std::string err;
 
   if (!apex_manifest.ParseFromString(content)) {
     return Error() << "Can't parse APEX manifest.";
@@ -99,9 +55,6 @@ Result<ApexManifest> ReadManifest(const std::string& path) {
   if (!android::base::ReadFileToString(path, &content)) {
     return Error() << "Failed to read manifest file: " << path;
   }
-  if (EndsWith(path, kManifestFilenameJson)) {
-    return ParseManifestJson(content);
-  }
   return ParseManifest(content);
 }
 
diff --git a/apexd/apex_manifest.h b/apexd/apex_manifest.h
index 58f24bc9..f2996483 100644
--- a/apexd/apex_manifest.h
+++ b/apexd/apex_manifest.h
@@ -29,9 +29,6 @@ namespace android {
 namespace apex {
 // Parses and validates APEX manifest.
 android::base::Result<ApexManifest> ParseManifest(const std::string& content);
-// Parses and validates APEX manifest (in JSON format);
-android::base::Result<ApexManifest> ParseManifestJson(
-    const std::string& content);
 // Returns package id of an ApexManifest
 std::string GetPackageId(const ApexManifest& apex_manifest);
 // Reads and parses APEX manifest from the file on disk.
diff --git a/apexd/apex_manifest_test.cpp b/apexd/apex_manifest_test.cpp
index 64f02172..5a4e3da5 100644
--- a/apexd/apex_manifest_test.cpp
+++ b/apexd/apex_manifest_test.cpp
@@ -25,9 +25,21 @@
 namespace android {
 namespace apex {
 
+namespace {
+
+std::string ToString(const ApexManifest& manifest) {
+  std::string out;
+  manifest.SerializeToString(&out);
+  return out;
+}
+
+}  // namespace
+
 TEST(ApexManifestTest, SimpleTest) {
-  auto apex_manifest = ParseManifestJson(
-      "{\"name\": \"com.android.example.apex\", \"version\": 1}\n");
+  ApexManifest manifest;
+  manifest.set_name("com.android.example.apex");
+  manifest.set_version(1);
+  auto apex_manifest = ParseManifest(ToString(manifest));
   ASSERT_RESULT_OK(apex_manifest);
   EXPECT_EQ("com.android.example.apex", std::string(apex_manifest->name()));
   EXPECT_EQ(1u, apex_manifest->version());
@@ -35,7 +47,9 @@ TEST(ApexManifestTest, SimpleTest) {
 }
 
 TEST(ApexManifestTest, NameMissing) {
-  auto apex_manifest = ParseManifestJson("{\"version\": 1}\n");
+  ApexManifest manifest;
+  manifest.set_version(1);
+  auto apex_manifest = ParseManifest(ToString(manifest));
   ASSERT_FALSE(apex_manifest.ok());
   EXPECT_EQ(apex_manifest.error().message(),
             std::string("Missing required field \"name\" from APEX manifest."))
@@ -43,8 +57,9 @@ TEST(ApexManifestTest, NameMissing) {
 }
 
 TEST(ApexManifestTest, VersionMissing) {
-  auto apex_manifest =
-      ParseManifestJson("{\"name\": \"com.android.example.apex\"}\n");
+  ApexManifest manifest;
+  manifest.set_name("com.android.example.apex");
+  auto apex_manifest = ParseManifest(ToString(manifest));
   ASSERT_FALSE(apex_manifest.ok());
   EXPECT_EQ(
       apex_manifest.error().message(),
@@ -52,61 +67,59 @@ TEST(ApexManifestTest, VersionMissing) {
       << apex_manifest.error();
 }
 
-TEST(ApexManifestTest, VersionNotNumber) {
-  auto apex_manifest = ParseManifestJson(
-      "{\"name\": \"com.android.example.apex\", \"version\": \"a\"}\n");
-
-  ASSERT_FALSE(apex_manifest.ok());
-  EXPECT_EQ(apex_manifest.error().message(),
-            std::string("Failed to parse APEX Manifest JSON config: "
-                        "(version): invalid value \"a\" for type TYPE_INT64"))
-      << apex_manifest.error();
-}
-
 TEST(ApexManifestTest, NoPreInstallHook) {
-  auto apex_manifest = ParseManifestJson(
-      "{\"name\": \"com.android.example.apex\", \"version\": 1}\n");
+  ApexManifest manifest;
+  manifest.set_name("com.android.example.apex");
+  manifest.set_version(1);
+  auto apex_manifest = ParseManifest(ToString(manifest));
   ASSERT_RESULT_OK(apex_manifest);
   EXPECT_EQ("", std::string(apex_manifest->preinstallhook()));
 }
 
 TEST(ApexManifestTest, PreInstallHook) {
-  auto apex_manifest = ParseManifestJson(
-      "{\"name\": \"com.android.example.apex\", \"version\": 1, "
-      "\"preInstallHook\": \"bin/preInstallHook\"}\n");
+  ApexManifest manifest;
+  manifest.set_name("com.android.example.apex");
+  manifest.set_version(1);
+  manifest.set_preinstallhook("bin/preInstallHook");
+  auto apex_manifest = ParseManifest(ToString(manifest));
   ASSERT_RESULT_OK(apex_manifest);
   EXPECT_EQ("bin/preInstallHook", std::string(apex_manifest->preinstallhook()));
 }
 
 TEST(ApexManifestTest, NoPostInstallHook) {
-  auto apex_manifest = ParseManifestJson(
-      "{\"name\": \"com.android.example.apex\", \"version\": 1}\n");
+  ApexManifest manifest;
+  manifest.set_name("com.android.example.apex");
+  manifest.set_version(1);
+  auto apex_manifest = ParseManifest(ToString(manifest));
   ASSERT_RESULT_OK(apex_manifest);
   EXPECT_EQ("", std::string(apex_manifest->postinstallhook()));
 }
 
 TEST(ApexManifestTest, PostInstallHook) {
-  auto apex_manifest = ParseManifestJson(
-      "{\"name\": \"com.android.example.apex\", \"version\": 1, "
-      "\"postInstallHook\": \"bin/postInstallHook\"}\n");
+  ApexManifest manifest;
+  manifest.set_name("com.android.example.apex");
+  manifest.set_version(1);
+  manifest.set_postinstallhook("bin/postInstallHook");
+  auto apex_manifest = ParseManifest(ToString(manifest));
   ASSERT_RESULT_OK(apex_manifest);
   EXPECT_EQ("bin/postInstallHook",
             std::string(apex_manifest->postinstallhook()));
 }
 
 TEST(ApexManifestTest, UnparsableManifest) {
-  auto apex_manifest = ParseManifestJson("This is an invalid pony");
+  auto apex_manifest = ParseManifest("This is an invalid pony");
   ASSERT_FALSE(apex_manifest.ok());
   EXPECT_EQ(apex_manifest.error().message(),
-            std::string("Failed to parse APEX Manifest JSON config: Unexpected "
-                        "token.\nThis is an invalid p\n^"))
+            std::string("Can't parse APEX manifest."))
       << apex_manifest.error();
 }
 
 TEST(ApexManifestTest, NoCode) {
-  auto apex_manifest = ParseManifestJson(
-      "{\"name\": \"com.android.example.apex\", \"version\": 1, "
-      "\"noCode\": true}\n");
+  ApexManifest manifest;
+  manifest.set_name("com.android.example.apex");
+  manifest.set_version(1);
+  manifest.set_nocode(true);
+  auto apex_manifest = ParseManifest(ToString(manifest));
   ASSERT_RESULT_OK(apex_manifest);
   EXPECT_TRUE(apex_manifest->nocode());
 }
diff --git a/apexd/apexd.cpp b/apexd/apexd.cpp
index f52781af..5c0a4711 100644
--- a/apexd/apexd.cpp
+++ b/apexd/apexd.cpp
@@ -1176,8 +1176,7 @@ Result<void> ActivateApexPackages(const std::vector<ApexFile>& apexes) {
 }
 
 bool ShouldActivateApexOnData(const ApexFile& apex) {
-  return HasPreInstalledVersion(apex.GetManifest().name()) &&
-         !apex.HasOnlyJsonManifest();
+  return HasPreInstalledVersion(apex.GetManifest().name());
 }
 
 }  // namespace
@@ -2105,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()) {
@@ -2116,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/corrupted_b146895998.apex b/apexd/apexd_testdata/corrupted_b146895998.apex
index fc6b4747..1d02697e 100644
--- a/apexd/apexd_testdata/corrupted_b146895998.apex
+++ b/apexd/apexd_testdata/corrupted_b146895998.apex
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/apexd/apexservice_test.cpp b/apexd/apexservice_test.cpp
index 2fc3e8b1..adcc5d62 100644
--- a/apexd/apexservice_test.cpp
+++ b/apexd/apexservice_test.cpp
@@ -86,6 +86,7 @@ using ::testing::Contains;
 using ::testing::EndsWith;
 using ::testing::HasSubstr;
 using ::testing::Not;
+using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 using ::testing::UnorderedElementsAreArray;
 
@@ -1401,7 +1402,7 @@ TEST_F(ApexServiceTest, NoPackagesAreBothActiveAndInactive) {
       activePackagesStrings.begin(), activePackagesStrings.end(),
       inactivePackagesStrings.begin(), inactivePackagesStrings.end(),
       std::back_inserter(intersection));
-  ASSERT_EQ(intersection.size(), 0UL);
+  ASSERT_THAT(intersection, SizeIs(0));
 }
 
 TEST_F(ApexServiceTest, GetAllPackages) {
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);
+            }
+        }
+    }
 }