Handle app status errors such as error-unknownApplication in update_c…

…lient.

This change introduces new error codes for these responses.

Bug: 850720
Change-Id: I85462e964a573bf310b7b0ae5e9180e2e87eafdc
Reviewed-on: https://chromium-review.googlesource.com/1091876
Reviewed-by: Joshua Pawlicki <[email protected]>
Commit-Queue: Sorin Jianu <[email protected]>
Cr-Commit-Position: refs/heads/master@{#565719}

components/test/data/update_client/updatecheck_reply_unknownapp.xml

@@ -0,0 +1,4 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<response protocol='3.1'>
+  <app appid='jebgalgnebhfojomionfpkfelancnnkf' status="error-unknownApplication"/>
+</response>

components/update_client/BUILD.gn

@@ -133,6 +133,7 @@ bundle_data("unit_tests_bundle_data") {
     "//components/test/data/update_client/updatecheck_reply_4.xml",
     "//components/test/data/update_client/updatecheck_reply_noupdate.xml",
     "//components/test/data/update_client/updatecheck_reply_parse_error.xml",
+    "//components/test/data/update_client/updatecheck_reply_unknownapp.xml",
   ]
   outputs = [
     "{{bundle_resources_dir}}/" +

components/update_client/protocol_parser.cc

@@ -321,7 +321,25 @@ bool ParseAppTag(xmlNode* app,
     return false;
   }
 
-  // Get the <updatecheck> tag.
+  // Read the |status| attribute for the app.
+  // If the status is one of the defined app status error literals, then return
+  // it in the result as if it were an updatecheck status, then stop parsing,
+  // and return success.
+  result->status = GetAttribute(app, "status");
+  if (result->status == "restricted" ||
+      result->status == "error-unknownApplication" ||
+      result->status == "error-invalidAppId")
+    return true;
+
+  // If the status was not handled above and the status is not "ok", then
+  // this must be a status literal that that the parser does not know about.
+  if (!result->status.empty() && result->status != "ok") {
+    *error = "Unknown app status";
+    return false;
+  }
+
+  // Get the <updatecheck> tag if the status is missing or the status is "ok".
+  DCHECK(result->status.empty() || result->status == "ok");
   std::vector<xmlNode*> updates = GetChildren(app, "updatecheck");
   if (updates.empty()) {
     *error = "Missing updatecheck on app.";

components/update_client/protocol_parser_unittest.cc

@@ -289,6 +289,24 @@ const char* kUpdateCheckStatusErrorWithRunAction =
     " </app>"
     "</response>";
 
+// Includes four <app> tags with status different than "ok".
+const char* kAppsStatusError =
+    "<?xml version='1.0' encoding='UTF-8'?>"
+    "<response protocol='3.1'>"
+    " <app appid='aaaaaaaa' status='error-unknownApplication'>"
+    "  <updatecheck status='error-internal'/>"
+    " </app>"
+    " <app appid='bbbbbbbb' status='restricted'>"
+    "  <updatecheck status='error-internal'/>"
+    " </app>"
+    " <app appid='cccccccc' status='error-invalidAppId'>"
+    "  <updatecheck status='error-internal'/>"
+    " </app>"
+    " <app appid='dddddddd' status='foobar'>"
+    "  <updatecheck status='error-internal'/>"
+    " </app>"
+    "</response>";
+
 TEST(ComponentUpdaterProtocolParserTest, Parse) {
   ProtocolParser parser;
 
@@ -324,15 +342,15 @@ TEST(ComponentUpdaterProtocolParserTest, Parse) {
   EXPECT_TRUE(parser.Parse(kValidXml));
   EXPECT_TRUE(parser.errors().empty());
   EXPECT_EQ(1u, parser.results().list.size());
-  const ProtocolParser::Result* firstResult = &parser.results().list[0];
-  EXPECT_STREQ("ok", firstResult->status.c_str());
-  EXPECT_EQ(1u, firstResult->crx_urls.size());
-  EXPECT_EQ(GURL("http://example.com/"), firstResult->crx_urls[0]);
-  EXPECT_EQ(GURL("http://diff.example.com/"), firstResult->crx_diffurls[0]);
-  EXPECT_EQ("1.2.3.4", firstResult->manifest.version);
-  EXPECT_EQ("2.0.143.0", firstResult->manifest.browser_min_version);
-  EXPECT_EQ(1u, firstResult->manifest.packages.size());
-  EXPECT_EQ("extension_1_2_3_4.crx", firstResult->manifest.packages[0].name);
+  const ProtocolParser::Result* first_result = &parser.results().list[0];
+  EXPECT_STREQ("ok", first_result->status.c_str());
+  EXPECT_EQ(1u, first_result->crx_urls.size());
+  EXPECT_EQ(GURL("http://example.com/"), first_result->crx_urls[0]);
+  EXPECT_EQ(GURL("http://diff.example.com/"), first_result->crx_diffurls[0]);
+  EXPECT_EQ("1.2.3.4", first_result->manifest.version);
+  EXPECT_EQ("2.0.143.0", first_result->manifest.browser_min_version);
+  EXPECT_EQ(1u, first_result->manifest.packages.size());
+  EXPECT_EQ("extension_1_2_3_4.crx", first_result->manifest.packages[0].name);
 
   // Parse some xml that uses namespace prefixes.
   EXPECT_TRUE(parser.Parse(kUsesNamespacePrefix));
@@ -344,23 +362,23 @@ TEST(ComponentUpdaterProtocolParserTest, Parse) {
   EXPECT_TRUE(parser.Parse(valid_xml_with_hash));
   EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
-  firstResult = &parser.results().list[0];
-  EXPECT_FALSE(firstResult->manifest.packages.empty());
-  EXPECT_EQ("1234", firstResult->manifest.packages[0].hash_sha256);
-  EXPECT_EQ("5678", firstResult->manifest.packages[0].hashdiff_sha256);
+  first_result = &parser.results().list[0];
+  EXPECT_FALSE(first_result->manifest.packages.empty());
+  EXPECT_EQ("1234", first_result->manifest.packages[0].hash_sha256);
+  EXPECT_EQ("5678", first_result->manifest.packages[0].hashdiff_sha256);
 
   // Parse xml with package size value
   EXPECT_TRUE(parser.Parse(valid_xml_with_invalid_sizes));
   EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
-  firstResult = &parser.results().list[0];
-  EXPECT_FALSE(firstResult->manifest.packages.empty());
-  EXPECT_EQ(1234, firstResult->manifest.packages[0].size);
-  EXPECT_EQ(-1234, firstResult->manifest.packages[1].size);
-  EXPECT_EQ(0, firstResult->manifest.packages[2].size);
-  EXPECT_EQ(0, firstResult->manifest.packages[3].size);
-  EXPECT_EQ(0, firstResult->manifest.packages[4].size);
-  EXPECT_EQ(0, firstResult->manifest.packages[5].size);
+  first_result = &parser.results().list[0];
+  EXPECT_FALSE(first_result->manifest.packages.empty());
+  EXPECT_EQ(1234, first_result->manifest.packages[0].size);
+  EXPECT_EQ(-1234, first_result->manifest.packages[1].size);
+  EXPECT_EQ(0, first_result->manifest.packages[2].size);
+  EXPECT_EQ(0, first_result->manifest.packages[3].size);
+  EXPECT_EQ(0, first_result->manifest.packages[4].size);
+  EXPECT_EQ(0, first_result->manifest.packages[5].size);
 
   // Parse xml with a <daystart> element.
   EXPECT_TRUE(parser.Parse(kWithDaystart));
@@ -372,63 +390,82 @@ TEST(ComponentUpdaterProtocolParserTest, Parse) {
   EXPECT_TRUE(parser.Parse(kNoUpdate));
   EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
-  firstResult = &parser.results().list[0];
-  EXPECT_STREQ("noupdate", firstResult->status.c_str());
-  EXPECT_EQ(firstResult->extension_id, "12345");
-  EXPECT_EQ(firstResult->manifest.version, "");
+  first_result = &parser.results().list[0];
+  EXPECT_STREQ("noupdate", first_result->status.c_str());
+  EXPECT_EQ(first_result->extension_id, "12345");
+  EXPECT_EQ(first_result->manifest.version, "");
 
   // Parse xml with one error and one success <app> tag.
   EXPECT_TRUE(parser.Parse(kTwoAppsOneError));
-  EXPECT_FALSE(parser.errors().empty());
-  EXPECT_EQ(1u, parser.results().list.size());
-  firstResult = &parser.results().list[0];
-  EXPECT_EQ(firstResult->extension_id, "bbbbbbbb");
-  EXPECT_STREQ("ok", firstResult->status.c_str());
-  EXPECT_EQ("1.2.3.4", firstResult->manifest.version);
+  EXPECT_TRUE(parser.errors().empty());
+  EXPECT_EQ(2u, parser.results().list.size());
+  first_result = &parser.results().list[0];
+  EXPECT_EQ(first_result->extension_id, "aaaaaaaa");
+  EXPECT_STREQ("error-unknownApplication", first_result->status.c_str());
+  EXPECT_TRUE(first_result->manifest.version.empty());
+  const ProtocolParser::Result* second_result = &parser.results().list[1];
+  EXPECT_EQ(second_result->extension_id, "bbbbbbbb");
+  EXPECT_STREQ("ok", second_result->status.c_str());
+  EXPECT_EQ("1.2.3.4", second_result->manifest.version);
 
   // Parse xml with two apps setting the cohort info.
   EXPECT_TRUE(parser.Parse(kTwoAppsSetCohort));
   EXPECT_TRUE(parser.errors().empty());
   EXPECT_EQ(2u, parser.results().list.size());
-  firstResult = &parser.results().list[0];
-  EXPECT_EQ(firstResult->extension_id, "aaaaaaaa");
-  EXPECT_NE(firstResult->cohort_attrs.find("cohort"),
-            firstResult->cohort_attrs.end());
-  EXPECT_EQ(firstResult->cohort_attrs.find("cohort")->second, "1:2q3/");
-  EXPECT_EQ(firstResult->cohort_attrs.find("cohortname"),
-            firstResult->cohort_attrs.end());
-  EXPECT_EQ(firstResult->cohort_attrs.find("cohorthint"),
-            firstResult->cohort_attrs.end());
-  const ProtocolParser::Result* secondResult = &parser.results().list[1];
-  EXPECT_EQ(secondResult->extension_id, "bbbbbbbb");
-  EXPECT_NE(secondResult->cohort_attrs.find("cohort"),
-            secondResult->cohort_attrs.end());
-  EXPECT_EQ(secondResult->cohort_attrs.find("cohort")->second, "1:[email protected]");
-  EXPECT_NE(secondResult->cohort_attrs.find("cohortname"),
-            secondResult->cohort_attrs.end());
-  EXPECT_EQ(secondResult->cohort_attrs.find("cohortname")->second, "cname");
-  EXPECT_EQ(secondResult->cohort_attrs.find("cohorthint"),
-            secondResult->cohort_attrs.end());
+  first_result = &parser.results().list[0];
+  EXPECT_EQ(first_result->extension_id, "aaaaaaaa");
+  EXPECT_NE(first_result->cohort_attrs.find("cohort"),
+            first_result->cohort_attrs.end());
+  EXPECT_EQ(first_result->cohort_attrs.find("cohort")->second, "1:2q3/");
+  EXPECT_EQ(first_result->cohort_attrs.find("cohortname"),
+            first_result->cohort_attrs.end());
+  EXPECT_EQ(first_result->cohort_attrs.find("cohorthint"),
+            first_result->cohort_attrs.end());
+  EXPECT_EQ(second_result->extension_id, "bbbbbbbb");
+  EXPECT_NE(second_result->cohort_attrs.find("cohort"),
+            second_result->cohort_attrs.end());
+  EXPECT_EQ(second_result->cohort_attrs.find("cohort")->second, "1:[email protected]");
+  EXPECT_NE(second_result->cohort_attrs.find("cohortname"),
+            second_result->cohort_attrs.end());
+  EXPECT_EQ(second_result->cohort_attrs.find("cohortname")->second, "cname");
+  EXPECT_EQ(second_result->cohort_attrs.find("cohorthint"),
+            second_result->cohort_attrs.end());
 
   EXPECT_TRUE(parser.Parse(kUpdateCheckStatusOkWithRunAction));
   EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
-  firstResult = &parser.results().list[0];
-  EXPECT_STREQ("ok", firstResult->status.c_str());
-  EXPECT_EQ(firstResult->extension_id, "12345");
-  EXPECT_STREQ("this", firstResult->action_run.c_str());
+  first_result = &parser.results().list[0];
+  EXPECT_STREQ("ok", first_result->status.c_str());
+  EXPECT_EQ(first_result->extension_id, "12345");
+  EXPECT_STREQ("this", first_result->action_run.c_str());
 
   EXPECT_TRUE(parser.Parse(kUpdateCheckStatusNoUpdateWithRunAction));
   EXPECT_TRUE(parser.errors().empty());
   EXPECT_FALSE(parser.results().list.empty());
-  firstResult = &parser.results().list[0];
-  EXPECT_STREQ("noupdate", firstResult->status.c_str());
-  EXPECT_EQ(firstResult->extension_id, "12345");
-  EXPECT_STREQ("this", firstResult->action_run.c_str());
+  first_result = &parser.results().list[0];
+  EXPECT_STREQ("noupdate", first_result->status.c_str());
+  EXPECT_EQ(first_result->extension_id, "12345");
+  EXPECT_STREQ("this", first_result->action_run.c_str());
 
   EXPECT_TRUE(parser.Parse(kUpdateCheckStatusErrorWithRunAction));
   EXPECT_FALSE(parser.errors().empty());
   EXPECT_TRUE(parser.results().list.empty());
+
+  EXPECT_TRUE(parser.Parse(kAppsStatusError));
+  EXPECT_STREQ("Unknown app status", parser.errors().c_str());
+  EXPECT_EQ(3u, parser.results().list.size());
+  first_result = &parser.results().list[0];
+  EXPECT_EQ(first_result->extension_id, "aaaaaaaa");
+  EXPECT_STREQ("error-unknownApplication", first_result->status.c_str());
+  EXPECT_TRUE(first_result->manifest.version.empty());
+  second_result = &parser.results().list[1];
+  EXPECT_EQ(second_result->extension_id, "bbbbbbbb");
+  EXPECT_STREQ("restricted", second_result->status.c_str());
+  EXPECT_TRUE(second_result->manifest.version.empty());
+  const ProtocolParser::Result* third_result = &parser.results().list[2];
+  EXPECT_EQ(third_result->extension_id, "cccccccc");
+  EXPECT_STREQ("error-invalidAppId", third_result->status.c_str());
+  EXPECT_TRUE(third_result->manifest.version.empty());
 }
 
 }  // namespace update_client

components/update_client/update_checker_unittest.cc

@@ -990,4 +990,36 @@ TEST_F(UpdateCheckerTest, ParseErrorProtocolVersionMismatch) {
   EXPECT_FALSE(results_);
 }
 
+// The update response contains a status |error-unknownApplication| for the
+// app. The response is succesfully parsed and a result is extracted to
+// indicate this status.
+TEST_F(UpdateCheckerTest, ParseErrorAppStatusErrorUnknownApplication) {
+  EXPECT_TRUE(post_interceptor_->ExpectRequest(
+      std::make_unique<PartialMatch>("updatecheck"),
+      test_file("updatecheck_reply_unknownapp.xml")));
+
+  update_checker_ = UpdateChecker::Create(config_, metadata_.get());
+
+  IdToComponentPtrMap components;
+  components[kUpdateItemId] = MakeComponent();
+
+  update_checker_->CheckForUpdates(
+      update_context_->session_id, {kUpdateItemId}, components, "", true,
+      base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete,
+                     base::Unretained(this)));
+  RunThreads();
+
+  EXPECT_EQ(1, post_interceptor_->GetHitCount())
+      << post_interceptor_->GetRequestsAsString();
+  ASSERT_EQ(1, post_interceptor_->GetCount())
+      << post_interceptor_->GetRequestsAsString();
+
+  EXPECT_EQ(ErrorCategory::kNone, error_category_);
+  EXPECT_EQ(0, error_);
+  EXPECT_TRUE(results_);
+  EXPECT_EQ(1u, results_->list.size());
+  const auto& result = results_->list.front();
+  EXPECT_STREQ("error-unknownApplication", result.status.c_str());
+}
+
 }  // namespace update_client

components/update_client/update_client_errors.h

@@ -100,12 +100,16 @@ enum class ServiceError {
 // error in any meaningful way, but this value may be reported in UMA stats,
 // therefore avoiding collisions with known network errors is desirable.
 enum class ProtocolError : int {
+  NONE = 0,
   RESPONSE_NOT_TRUSTED = -10000,
   MISSING_PUBLIC_KEY = -10001,
   MISSING_URLS = -10002,
   PARSE_FAILED = -10003,
   UPDATE_RESPONSE_NOT_FOUND = -10004,
   URL_FETCHER_FAILED = -10005,
+  UNKNOWN_APPLICATION = -10006,
+  RESTRICTED_APPLICATION = -10007,
+  INVALID_APPID = -10008,
 };
 
 }  // namespace update_client