Remove JSON support from APEXd

ApexFile::Open() doesn't fall back to apex_manifest.json for APEX
manifest.

Bug: 143973464
Test: atest CtsStagedInstallHostTestCases
Test: atest ApexTestCases
Change-Id: Ia0fad621806f2dc91caaa3123fdb55fb35f3ab5c

9 files changed, 55 insertions, 118 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..1269a359 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
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/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) {