Commit aa416553 authored by Sorin Jianu's avatar Sorin Jianu Committed by Commit Bot

Handle app status errors such as error-unknownApplication in update_client.

This change introduces new error codes for these responses.

Bug: 850720
Change-Id: I85462e964a573bf310b7b0ae5e9180e2e87eafdc
Reviewed-on: https://chromium-review.googlesource.com/1091876Reviewed-by: default avatarJoshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565719}
parent eba23a11
<?xml version='1.0' encoding='UTF-8'?>
<response protocol='3.1'>
<app appid='jebgalgnebhfojomionfpkfelancnnkf' status="error-unknownApplication"/>
</response>
...@@ -133,6 +133,7 @@ bundle_data("unit_tests_bundle_data") { ...@@ -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_4.xml",
"//components/test/data/update_client/updatecheck_reply_noupdate.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_parse_error.xml",
"//components/test/data/update_client/updatecheck_reply_unknownapp.xml",
] ]
outputs = [ outputs = [
"{{bundle_resources_dir}}/" + "{{bundle_resources_dir}}/" +
......
...@@ -321,7 +321,25 @@ bool ParseAppTag(xmlNode* app, ...@@ -321,7 +321,25 @@ bool ParseAppTag(xmlNode* app,
return false; 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"); std::vector<xmlNode*> updates = GetChildren(app, "updatecheck");
if (updates.empty()) { if (updates.empty()) {
*error = "Missing updatecheck on app."; *error = "Missing updatecheck on app.";
......
...@@ -990,4 +990,36 @@ TEST_F(UpdateCheckerTest, ParseErrorProtocolVersionMismatch) { ...@@ -990,4 +990,36 @@ TEST_F(UpdateCheckerTest, ParseErrorProtocolVersionMismatch) {
EXPECT_FALSE(results_); 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 } // namespace update_client
...@@ -100,12 +100,16 @@ enum class ServiceError { ...@@ -100,12 +100,16 @@ enum class ServiceError {
// error in any meaningful way, but this value may be reported in UMA stats, // error in any meaningful way, but this value may be reported in UMA stats,
// therefore avoiding collisions with known network errors is desirable. // therefore avoiding collisions with known network errors is desirable.
enum class ProtocolError : int { enum class ProtocolError : int {
NONE = 0,
RESPONSE_NOT_TRUSTED = -10000, RESPONSE_NOT_TRUSTED = -10000,
MISSING_PUBLIC_KEY = -10001, MISSING_PUBLIC_KEY = -10001,
MISSING_URLS = -10002, MISSING_URLS = -10002,
PARSE_FAILED = -10003, PARSE_FAILED = -10003,
UPDATE_RESPONSE_NOT_FOUND = -10004, UPDATE_RESPONSE_NOT_FOUND = -10004,
URL_FETCHER_FAILED = -10005, URL_FETCHER_FAILED = -10005,
UNKNOWN_APPLICATION = -10006,
RESTRICTED_APPLICATION = -10007,
INVALID_APPID = -10008,
}; };
} // namespace update_client } // namespace update_client
......
...@@ -228,12 +228,31 @@ void UpdateEngine::UpdateCheckResultsAvailable( ...@@ -228,12 +228,31 @@ void UpdateEngine::UpdateCheckResultsAvailable(
DCHECK_EQ(1u, update_context->components.count(id)); DCHECK_EQ(1u, update_context->components.count(id));
auto& component = update_context->components.at(id); auto& component = update_context->components.at(id);
const auto& it = id_to_result.find(id); const auto& it = id_to_result.find(id);
if (it != id_to_result.end()) if (it != id_to_result.end()) {
component->SetUpdateCheckResult(it->second, ErrorCategory::kNone, 0); const auto result = it->second;
else const auto error = [](const std::string& status) {
// First, handle app status literals which can be folded down as an
// updatecheck status
if (status == "error-unknownApplication")
return std::make_pair(ErrorCategory::kUpdateCheck,
ProtocolError::UNKNOWN_APPLICATION);
if (status == "restricted")
return std::make_pair(ErrorCategory::kUpdateCheck,
ProtocolError::RESTRICTED_APPLICATION);
if (status == "error-invalidAppId")
return std::make_pair(ErrorCategory::kUpdateCheck,
ProtocolError::INVALID_APPID);
// If the parser has return a valid result and the status is not one of
// the literals above, then this must be a success an not a parse error.
return std::make_pair(ErrorCategory::kNone, ProtocolError::NONE);
}(result.status);
component->SetUpdateCheckResult(result, error.first,
static_cast<int>(error.second));
} else {
component->SetUpdateCheckResult( component->SetUpdateCheckResult(
base::nullopt, ErrorCategory::kUpdateCheck, base::nullopt, ErrorCategory::kUpdateCheck,
static_cast<int>(ProtocolError::UPDATE_RESPONSE_NOT_FOUND)); static_cast<int>(ProtocolError::UPDATE_RESPONSE_NOT_FOUND));
}
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment