update_engine: move autoupdate_token to cohorthint attribute
If policy DeviceQuickFixBuildToken is set and is not empty then set "cohorthint" attribute
to the value of DeviceQuickFixBuildToken in update request. Takes precedence over kPrefsOmahaCohortHint pref.
BUG=chromium:932465
TEST=./build_packages --board=amd64-generic && \
cros_run_unit_tests --board=amd64-generic --packages update_engine
Change-Id: Ia4143b0854742ec22a535ce75b3b54a937a47b5a
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 41b2520..8008e00 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -1471,7 +1471,6 @@
request_params_.set_current_channel("unittest_track<");
request_params_.set_target_channel("unittest_track<");
request_params_.set_hwid("<OEM MODEL>");
- request_params_.set_autoupdate_token("autoupdate_token>");
fake_prefs_.SetString(kPrefsOmahaCohort, "evil\nstring");
fake_prefs_.SetString(kPrefsOmahaCohortHint, "evil&string\\");
fake_prefs_.SetString(
@@ -1498,8 +1497,6 @@
// Values from Prefs that are too big are removed from the XML instead of
// encoded.
EXPECT_EQ(string::npos, post_str.find("cohortname="));
- EXPECT_NE(string::npos, post_str.find("autoupdate_token>"));
- EXPECT_EQ(string::npos, post_str.find("autoupdate_token>"));
}
TEST_F(OmahaRequestActionTest, XmlDecodeTest) {
@@ -1706,18 +1703,44 @@
}
TEST_F(OmahaRequestActionTest, DeviceQuickFixBuildTokenIsSetTest) {
- constexpr char autoupdate_token[] = "autoupdate_token";
+ // If DeviceQuickFixBuildToken value is set it takes precedence over pref
+ // value.
+ constexpr char autoupdate_token[] = "autoupdate_token>";
+ constexpr char xml_encoded_autoupdate_token[] = "autoupdate_token>";
+ constexpr char omaha_cohort_hint[] = "cohort_hint";
tuc_params_.http_response = fake_update_response_.GetNoUpdateResponse();
tuc_params_.expected_check_result = metrics::CheckResult::kNoUpdateAvailable;
tuc_params_.expected_check_reaction = metrics::CheckReaction::kUnset;
request_params_.set_autoupdate_token(autoupdate_token);
+ fake_prefs_.SetString(kPrefsOmahaCohortHint, omaha_cohort_hint);
ASSERT_TRUE(TestUpdateCheck());
- EXPECT_NE(post_str.find(" <updatecheck token=\"" +
- string(autoupdate_token) + "\"></updatecheck>\n"),
- string::npos);
+ EXPECT_NE(string::npos,
+ post_str.find("cohorthint=\"" +
+ string(xml_encoded_autoupdate_token) + "\""));
+ EXPECT_EQ(string::npos, post_str.find(autoupdate_token));
+ EXPECT_EQ(string::npos, post_str.find(omaha_cohort_hint));
+}
+
+TEST_F(OmahaRequestActionTest, DeviceQuickFixBuildTokenIsNotSetTest) {
+ // If DeviceQuickFixBuildToken is not set, pref value will be provided in
+ // cohorthint attribute.
+ constexpr char omaha_cohort_hint[] = "evil_string>";
+ constexpr char xml_encoded_cohort_hint[] = "evil_string>";
+
+ tuc_params_.http_response = fake_update_response_.GetNoUpdateResponse();
+ tuc_params_.expected_check_result = metrics::CheckResult::kNoUpdateAvailable;
+ tuc_params_.expected_check_reaction = metrics::CheckReaction::kUnset;
+ fake_prefs_.SetString(kPrefsOmahaCohortHint, omaha_cohort_hint);
+
+ ASSERT_TRUE(TestUpdateCheck());
+
+ EXPECT_NE(
+ string::npos,
+ post_str.find("cohorthint=\"" + string(xml_encoded_cohort_hint) + "\""));
+ EXPECT_EQ(string::npos, post_str.find(omaha_cohort_hint));
}
void OmahaRequestActionTest::PingTest(bool ping_only) {
diff --git a/omaha_request_builder_xml.cc b/omaha_request_builder_xml.cc
index 3e4a335..95fb183 100644
--- a/omaha_request_builder_xml.cc
+++ b/omaha_request_builder_xml.cc
@@ -118,12 +118,6 @@
app_body += " rollback_allowed=\"true\"";
}
}
- string autoupdate_token = params_->autoupdate_token();
- if (!autoupdate_token.empty()) {
- app_body += base::StringPrintf(
- " token=\"%s\"", XmlEncodeWithDefault(autoupdate_token).c_str());
- }
-
app_body += "></updatecheck>\n";
}
@@ -172,14 +166,20 @@
}
string OmahaRequestBuilderXml::GetCohortArg(const string arg_name,
- const string prefs_key) const {
- // There's nothing wrong with not having a given cohort setting, so we check
- // existence first to avoid the warning log message.
- if (!prefs_->Exists(prefs_key))
- return "";
+ const string prefs_key,
+ const string override_value) const {
string cohort_value;
- if (!prefs_->GetString(prefs_key, &cohort_value) || cohort_value.empty())
- return "";
+ if (!override_value.empty()) {
+ // |override_value| take precedence over pref value.
+ cohort_value = override_value;
+ } else {
+ // There's nothing wrong with not having a given cohort setting, so we check
+ // existence first to avoid the warning log message.
+ if (!prefs_->Exists(prefs_key))
+ return "";
+ if (!prefs_->GetString(prefs_key, &cohort_value) || cohort_value.empty())
+ return "";
+ }
// This is a sanity check to avoid sending a huge XML file back to Ohama due
// to a compromised stateful partition making the update check fail in low
// network environments envent after a reboot.
@@ -246,9 +246,14 @@
string app_cohort_args;
app_cohort_args += GetCohortArg("cohort", kPrefsOmahaCohort);
- app_cohort_args += GetCohortArg("cohorthint", kPrefsOmahaCohortHint);
app_cohort_args += GetCohortArg("cohortname", kPrefsOmahaCohortName);
+ // Policy provided value overrides pref.
+ string autoupdate_token = params_->autoupdate_token();
+ app_cohort_args += GetCohortArg("cohorthint",
+ kPrefsOmahaCohortHint,
+ autoupdate_token /* override_value */);
+
string fingerprint_arg;
if (!params_->os_build_fingerprint().empty()) {
fingerprint_arg = "fingerprint=\"" +
diff --git a/omaha_request_builder_xml.h b/omaha_request_builder_xml.h
index 0ba44b8..495ddd7 100644
--- a/omaha_request_builder_xml.h
+++ b/omaha_request_builder_xml.h
@@ -161,7 +161,8 @@
// |arg_name| and |prefs_key|, if any. The return value is suitable to
// concatenate to the list of arguments and includes a space at the end.
std::string GetCohortArg(const std::string arg_name,
- const std::string prefs_key) const;
+ const std::string prefs_key,
+ const std::string override_value = "") const;
// Returns an XML ping element if any of the elapsed days need to be
// sent, or an empty string otherwise.