update_engine: Add Cohort, CohortName, CohortHint to Omaha protocol.
This patch persists and sends back to omaha the cohort, cohorthint and
cohortname arguments in the <app> tag. To avoid problems, these strings
are limited to 1024 chars.
BUG=chromium:448995
TEST=unittest added.
Change-Id: I05e7677ee43ab795b670b274d3984803cb6b9522
Reviewed-on: https://chromium-review.googlesource.com/262967
Trybot-Ready: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
diff --git a/constants.cc b/constants.cc
index 4129176..0e052c0 100644
--- a/constants.cc
+++ b/constants.cc
@@ -45,6 +45,9 @@
"metrics-check-last-reporting-time";
const char kPrefsNumReboots[] = "num-reboots";
const char kPrefsNumResponsesSeen[] = "num-responses-seen";
+const char kPrefsOmahaCohort[] = "omaha-cohort";
+const char kPrefsOmahaCohortHint[] = "omaha-cohort-hint";
+const char kPrefsOmahaCohortName[] = "omaha-cohort-name";
const char kPrefsP2PEnabled[] = "p2p-enabled";
const char kPrefsP2PFirstAttemptTimestamp[] = "p2p-first-attempt-timestamp";
const char kPrefsP2PNumAttempts[] = "p2p-num-attempts";
diff --git a/constants.h b/constants.h
index 0f200cb..b933f62 100644
--- a/constants.h
+++ b/constants.h
@@ -46,6 +46,9 @@
extern const char kPrefsMetricsCheckLastReportingTime[];
extern const char kPrefsNumReboots[];
extern const char kPrefsNumResponsesSeen[];
+extern const char kPrefsOmahaCohort[];
+extern const char kPrefsOmahaCohortHint[];
+extern const char kPrefsOmahaCohortName[];
extern const char kPrefsP2PEnabled[];
extern const char kPrefsP2PFirstAttemptTimestamp[];
extern const char kPrefsP2PNumAttempts[];
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 0955983..a1cc429 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -141,6 +141,33 @@
return app_body;
}
+// Returns the cohort* argument to include in the <app> tag for the passed
+// |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.
+string GetCohortArgXml(PrefsInterface* prefs,
+ const std::string arg_name,
+ const std::string prefs_key) {
+ // There's nothing wrong with not having a given cohort setting, so we check
+ // existance first to avoid the warning log message.
+ if (!prefs->Exists(prefs_key))
+ return "";
+ string cohort_value;
+ 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.
+ if (cohort_value.size() > 1024) {
+ LOG(WARNING) << "The omaha cohort setting " << arg_name
+ << " has a too big value, which must be an error or an "
+ "attacker trying to inhibit updates.";
+ return "";
+ }
+
+ return base::StringPrintf("%s=\"%s\" ",
+ arg_name.c_str(), XmlEncode(cohort_value).c_str());
+}
+
// Returns an XML that corresponds to the entire <app> node of the Omaha
// request based on the given parameters.
string GetAppXml(const OmahaEvent* event,
@@ -184,8 +211,17 @@
install_date_in_days);
}
+ string app_cohort_args;
+ app_cohort_args += GetCohortArgXml(system_state->prefs(),
+ "cohort", kPrefsOmahaCohort);
+ app_cohort_args += GetCohortArgXml(system_state->prefs(),
+ "cohorthint", kPrefsOmahaCohortHint);
+ app_cohort_args += GetCohortArgXml(system_state->prefs(),
+ "cohortname", kPrefsOmahaCohortName);
+
string app_xml =
" <app appid=\"" + XmlEncode(params->GetAppId()) + "\" " +
+ app_cohort_args +
app_versions +
app_channels +
"lang=\"" + XmlEncode(params->app_lang()) + "\" " +
@@ -260,6 +296,12 @@
string current_path;
// These are the values extracted from the XML.
+ string app_cohort;
+ string app_cohorthint;
+ string app_cohortname;
+ bool app_cohort_set = false;
+ bool app_cohorthint_set = false;
+ bool app_cohortname_set = false;
string updatecheck_status;
string updatecheck_poll_interval;
string daystart_elapsed_days;
@@ -292,7 +334,20 @@
}
}
- if (data->current_path == "/response/app/updatecheck") {
+ if (data->current_path == "/response/app") {
+ if (attrs.find("cohort") != attrs.end()) {
+ data->app_cohort_set = true;
+ data->app_cohort = attrs["cohort"];
+ }
+ if (attrs.find("cohorthint") != attrs.end()) {
+ data->app_cohorthint_set = true;
+ data->app_cohorthint = attrs["cohorthint"];
+ }
+ if (attrs.find("cohortname") != attrs.end()) {
+ data->app_cohortname_set = true;
+ data->app_cohortname = attrs["cohortname"];
+ }
+ } else if (data->current_path == "/response/app/updatecheck") {
// There is only supposed to be a single <updatecheck> element.
data->updatecheck_status = attrs["status"];
data->updatecheck_poll_interval = attrs["PollInterval"];
@@ -629,6 +684,13 @@
if (!ParseParams(parser_data, output_object, completer))
return false;
+ if (parser_data->app_cohort_set)
+ PersistCohortData(kPrefsOmahaCohort, parser_data->app_cohort);
+ if (parser_data->app_cohorthint_set)
+ PersistCohortData(kPrefsOmahaCohortHint, parser_data->app_cohorthint);
+ if (parser_data->app_cohortname_set)
+ PersistCohortData(kPrefsOmahaCohortName, parser_data->app_cohortname);
+
return true;
}
@@ -1232,6 +1294,19 @@
return true;
}
+bool OmahaRequestAction::PersistCohortData(
+ const string& prefs_key,
+ const string& new_value) {
+ if (new_value.empty() && system_state_->prefs()->Exists(prefs_key)) {
+ LOG(INFO) << "Removing stored " << prefs_key << " value.";
+ return system_state_->prefs()->Delete(prefs_key);
+ } else if (!new_value.empty()) {
+ LOG(INFO) << "Storing new setting " << prefs_key << " as " << new_value;
+ return system_state_->prefs()->SetString(prefs_key, new_value);
+ }
+ return true;
+}
+
void OmahaRequestAction::ActionCompleted(ErrorCode code) {
// We only want to report this on "update check".
if (ping_only_ || event_ != nullptr)
diff --git a/omaha_request_action.h b/omaha_request_action.h
index b9f70f0..0cb2244 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -189,6 +189,13 @@
int install_date_days,
InstallDateProvisioningSource source);
+ // Persist the new cohort* value received in the XML file in the |prefs_key|
+ // preference file. If the |new_value| is empty, the currently stored value
+ // will be deleted. Don't call this function with an empty |new_value| if the
+ // value was not set in the XML, since that would delete the stored value.
+ bool PersistCohortData(const std::string& prefs_key,
+ const std::string& new_value);
+
// If this is an update check request, initializes
// |ping_active_days_| and |ping_roll_call_days_| to values that may
// be sent as pings to Omaha.
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index c40fa3b..8a2090c 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -10,6 +10,7 @@
#include <string>
#include <vector>
+#include <base/strings/string_number_conversions.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
#include <base/time/time.h>
@@ -46,6 +47,89 @@
using testing::SetArgumentPointee;
using testing::_;
+namespace {
+
+// This is a helper struct to allow unit tests build an update response with the
+// values they care about.
+struct FakeUpdateResponse {
+ string GetNoUpdateResponse() const {
+ string entity_str;
+ if (include_entity)
+ entity_str = "<!DOCTYPE response [<!ENTITY CrOS \"ChromeOS\">]>";
+ return base::StringPrintf(
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+ "%s<response protocol=\"3.0\">"
+ "<daystart elapsed_seconds=\"100\"/>"
+ "<app appid=\"%s\" status=\"ok\"><ping status=\"ok\"/>"
+ "<updatecheck status=\"noupdate\"/></app></response>",
+ entity_str.c_str(), app_id.c_str());
+ }
+
+ string GetUpdateResponse() const {
+ return
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
+ "protocol=\"3.0\">"
+ "<daystart elapsed_seconds=\"100\"" +
+ (elapsed_days.empty() ? "" : (" elapsed_days=\"" + elapsed_days + "\""))
+ + "/>"
+ "<app appid=\"" + app_id + "\" " +
+ (include_cohorts ? "cohort=\"" + cohort + "\" cohorthint=\"" +
+ cohorthint + "\" cohortname=\"" + cohortname + "\" " : "") +
+ " status=\"ok\">"
+ "<ping status=\"ok\"/><updatecheck status=\"ok\">"
+ "<urls><url codebase=\"" + codebase + "\"/></urls>"
+ "<manifest version=\"" + version + "\">"
+ "<packages><package hash=\"not-used\" name=\"" + filename + "\" "
+ "size=\"" + base::Int64ToString(size) + "\"/></packages>"
+ "<actions><action event=\"postinstall\" "
+ "ChromeOSVersion=\"" + version + "\" "
+ "MoreInfo=\"" + more_info_url + "\" Prompt=\"" + prompt + "\" "
+ "IsDelta=\"true\" "
+ "IsDeltaPayload=\"true\" "
+ "MaxDaysToScatter=\"" + max_days_to_scatter + "\" "
+ "sha256=\"" + hash + "\" "
+ "needsadmin=\"" + needsadmin + "\" " +
+ (deadline.empty() ? "" : ("deadline=\"" + deadline + "\" ")) +
+ (disable_p2p_for_downloading ?
+ "DisableP2PForDownloading=\"true\" " : "") +
+ (disable_p2p_for_sharing ? "DisableP2PForSharing=\"true\" " : "") +
+ "/></actions></manifest></updatecheck></app></response>";
+ }
+
+ // Return the payload URL, which is split in two fields in the XML response.
+ string GetPayloadUrl() {
+ return codebase + filename;
+ }
+
+ string app_id = chromeos_update_engine::OmahaRequestParams::kAppId;
+ string version = "1.2.3.4";
+ string more_info_url = "http://more/info";
+ string prompt = "true";
+ string codebase = "http://code/base/";
+ string filename = "file.signed";
+ string hash = "HASH1234=";
+ string needsadmin = "false";
+ int64_t size = 123;
+ string deadline = "";
+ string max_days_to_scatter = "7";
+ string elapsed_days = "42";
+
+ // P2P setting defaults to allowed.
+ bool disable_p2p_for_downloading = false;
+ bool disable_p2p_for_sharing = false;
+
+ // Omaha cohorts settings.
+ bool include_cohorts = false;
+ string cohort = "";
+ string cohorthint = "";
+ string cohortname = "";
+
+ // Whether to include the CrOS <!ENTITY> in the XML response.
+ bool include_entity = false;
+};
+
+} // namespace
+
namespace chromeos_update_engine {
class OmahaRequestActionTest : public ::testing::Test {
@@ -103,6 +187,7 @@
const string& expected_p2p_url);
FakeSystemState fake_system_state_;
+ FakeUpdateResponse fake_update_response_;
// By default, all tests use these objects unless they replace them in the
// fake_system_state_.
@@ -128,96 +213,6 @@
};
namespace {
-
-string GetNoUpdateResponse(const string& app_id) {
- return string(
- "<?xml version=\"1.0\" encoding=\"UTF-8\"?><response protocol=\"3.0\">"
- "<daystart elapsed_seconds=\"100\"/>"
- "<app appid=\"") + app_id + "\" status=\"ok\"><ping "
- "status=\"ok\"/><updatecheck status=\"noupdate\"/></app></response>";
-}
-
-string GetNoUpdateResponseWithEntity(const string& app_id) {
- return string(
- "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
- "<!DOCTYPE response ["
- "<!ENTITY CrOS \"ChromeOS\">"
- "]>"
- "<response protocol=\"3.0\">"
- "<daystart elapsed_seconds=\"100\"/>"
- "<app appid=\"") + app_id + "\" status=\"ok\"><ping "
- "status=\"ok\"/><updatecheck status=\"noupdate\"/></app></response>";
-}
-
-string GetUpdateResponse2(const string& app_id,
- const string& version,
- const string& more_info_url,
- const string& prompt,
- const string& codebase,
- const string& filename,
- const string& hash,
- const string& needsadmin,
- const string& size,
- const string& deadline,
- const string& max_days_to_scatter,
- const string& elapsed_days,
- bool disable_p2p_for_downloading,
- bool disable_p2p_for_sharing) {
- string response =
- "<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
- "protocol=\"3.0\">"
- "<daystart elapsed_seconds=\"100\"" +
- (elapsed_days.empty() ? "" : (" elapsed_days=\"" + elapsed_days + "\"")) +
- "/>"
- "<app appid=\"" + app_id + "\" status=\"ok\">"
- "<ping status=\"ok\"/><updatecheck status=\"ok\">"
- "<urls><url codebase=\"" + codebase + "\"/></urls>"
- "<manifest version=\"" + version + "\">"
- "<packages><package hash=\"not-used\" name=\"" + filename + "\" "
- "size=\"" + size + "\"/></packages>"
- "<actions><action event=\"postinstall\" "
- "ChromeOSVersion=\"" + version + "\" "
- "MoreInfo=\"" + more_info_url + "\" Prompt=\"" + prompt + "\" "
- "IsDelta=\"true\" "
- "IsDeltaPayload=\"true\" "
- "MaxDaysToScatter=\"" + max_days_to_scatter + "\" "
- "sha256=\"" + hash + "\" "
- "needsadmin=\"" + needsadmin + "\" " +
- (deadline.empty() ? "" : ("deadline=\"" + deadline + "\" ")) +
- (disable_p2p_for_downloading ?
- "DisableP2PForDownloading=\"true\" " : "") +
- (disable_p2p_for_sharing ? "DisableP2PForSharing=\"true\" " : "") +
- "/></actions></manifest></updatecheck></app></response>";
- LOG(INFO) << "Response = " << response;
- return response;
-}
-
-string GetUpdateResponse(const string& app_id,
- const string& version,
- const string& more_info_url,
- const string& prompt,
- const string& codebase,
- const string& filename,
- const string& hash,
- const string& needsadmin,
- const string& size,
- const string& deadline) {
- return GetUpdateResponse2(app_id,
- version,
- more_info_url,
- prompt,
- codebase,
- filename,
- hash,
- needsadmin,
- size,
- deadline,
- "7",
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false); // disable_p2p_for sharing
-}
-
class OmahaRequestActionTestProcessorDelegate : public ActionProcessorDelegate {
public:
OmahaRequestActionTestProcessorDelegate()
@@ -378,9 +373,10 @@
TEST_F(OmahaRequestActionTest, RejectEntities) {
OmahaResponse response;
+ fake_update_response_.include_entity = true;
ASSERT_FALSE(
TestUpdateCheck(nullptr, // request_params
- GetNoUpdateResponseWithEntity(OmahaRequestParams::kAppId),
+ fake_update_response_.GetNoUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kOmahaRequestXMLHasEntityDecl,
@@ -396,7 +392,7 @@
OmahaResponse response;
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ fake_update_response_.GetNoUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -408,20 +404,14 @@
EXPECT_FALSE(response.update_exists);
}
+// Test that all the values in the response are parsed in a normal update
+// response.
TEST_F(OmahaRequestActionTest, ValidUpdateTest) {
OmahaResponse response;
+ fake_update_response_.deadline = "20101020";
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetUpdateResponse(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "20101020"), // deadline
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -432,13 +422,18 @@
nullptr));
EXPECT_TRUE(response.update_exists);
EXPECT_TRUE(response.update_exists);
- EXPECT_EQ("1.2.3.4", response.version);
- EXPECT_EQ("http://code/base/file.signed", response.payload_urls[0]);
- EXPECT_EQ("http://more/info", response.more_info_url);
- EXPECT_EQ("HASH1234=", response.hash);
- EXPECT_EQ(123, response.size);
- EXPECT_TRUE(response.prompt);
- EXPECT_EQ("20101020", response.deadline);
+ EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ(fake_update_response_.GetPayloadUrl(), response.payload_urls[0]);
+ EXPECT_EQ(fake_update_response_.more_info_url, response.more_info_url);
+ EXPECT_EQ(fake_update_response_.hash, response.hash);
+ EXPECT_EQ(fake_update_response_.size, response.size);
+ EXPECT_EQ(fake_update_response_.prompt == "true", response.prompt);
+ EXPECT_EQ(fake_update_response_.deadline, response.deadline);
+ // Omaha cohort attribets are not set in the response, so they should not be
+ // persisted.
+ EXPECT_FALSE(fake_prefs_.Exists(kPrefsOmahaCohort));
+ EXPECT_FALSE(fake_prefs_.Exists(kPrefsOmahaCohortHint));
+ EXPECT_FALSE(fake_prefs_.Exists(kPrefsOmahaCohortName));
}
TEST_F(OmahaRequestActionTest, ValidUpdateBlockedByConnection) {
@@ -459,16 +454,7 @@
ASSERT_FALSE(
TestUpdateCheck(nullptr, // request_params
- GetUpdateResponse(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- ""), // deadline
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kOmahaUpdateIgnoredPerPolicy,
@@ -490,18 +476,10 @@
EXPECT_CALL(mock_payload_state, GetRollbackVersion())
.WillRepeatedly(Return(rollback_version));
+ fake_update_response_.version = rollback_version;
ASSERT_FALSE(
TestUpdateCheck(nullptr, // request_params
- GetUpdateResponse(OmahaRequestParams::kAppId,
- rollback_version, // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- ""), // deadline
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kOmahaUpdateIgnoredPerPolicy,
@@ -522,20 +500,7 @@
ASSERT_FALSE(
TestUpdateCheck(¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kOmahaUpdateDeferredPerPolicy,
@@ -550,20 +515,7 @@
params.set_interactive(true);
ASSERT_TRUE(
TestUpdateCheck(¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -587,20 +539,7 @@
ASSERT_TRUE(
TestUpdateCheck(¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -622,22 +561,10 @@
params.set_min_update_checks_needed(1);
params.set_max_update_checks_allowed(8);
+ fake_update_response_.max_days_to_scatter = "0";
ASSERT_TRUE(
TestUpdateCheck(¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "0", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -662,20 +589,7 @@
ASSERT_TRUE(TestUpdateCheck(
¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -703,20 +617,7 @@
ASSERT_FALSE(TestUpdateCheck(
¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kOmahaUpdateDeferredPerPolicy,
@@ -735,20 +636,7 @@
params.set_interactive(true);
ASSERT_TRUE(
TestUpdateCheck(¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -774,20 +662,7 @@
ASSERT_FALSE(TestUpdateCheck(
¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kOmahaUpdateDeferredPerPolicy,
@@ -808,20 +683,7 @@
params.set_interactive(true);
ASSERT_TRUE(
TestUpdateCheck(¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -833,8 +695,94 @@
EXPECT_TRUE(response.update_exists);
}
+TEST_F(OmahaRequestActionTest, CohortsArePersisted) {
+ OmahaResponse response;
+ OmahaRequestParams params = request_params_;
+ fake_update_response_.include_cohorts = true;
+ fake_update_response_.cohort = "s/154454/8479665";
+ fake_update_response_.cohorthint = "please-put-me-on-beta";
+ fake_update_response_.cohortname = "stable";
+
+ ASSERT_TRUE(TestUpdateCheck(¶ms,
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+
+ string value;
+ EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohort, &value));
+ EXPECT_EQ(fake_update_response_.cohort, value);
+
+ EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohortHint, &value));
+ EXPECT_EQ(fake_update_response_.cohorthint, value);
+
+ EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohortName, &value));
+ EXPECT_EQ(fake_update_response_.cohortname, value);
+}
+
+TEST_F(OmahaRequestActionTest, CohortsAreUpdated) {
+ OmahaResponse response;
+ OmahaRequestParams params = request_params_;
+ EXPECT_TRUE(fake_prefs_.SetString(kPrefsOmahaCohort, "old_value"));
+ EXPECT_TRUE(fake_prefs_.SetString(kPrefsOmahaCohortHint, "old_hint"));
+ EXPECT_TRUE(fake_prefs_.SetString(kPrefsOmahaCohortName, "old_name"));
+ fake_update_response_.include_cohorts = true;
+ fake_update_response_.cohort = "s/154454/8479665";
+ fake_update_response_.cohorthint = "please-put-me-on-beta";
+ fake_update_response_.cohortname = "";
+
+ ASSERT_TRUE(TestUpdateCheck(¶ms,
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+
+ string value;
+ EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohort, &value));
+ EXPECT_EQ(fake_update_response_.cohort, value);
+
+ EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohortHint, &value));
+ EXPECT_EQ(fake_update_response_.cohorthint, value);
+
+ EXPECT_FALSE(fake_prefs_.GetString(kPrefsOmahaCohortName, &value));
+}
+
+TEST_F(OmahaRequestActionTest, CohortsAreNotModifiedWhenMissing) {
+ OmahaResponse response;
+ OmahaRequestParams params = request_params_;
+ EXPECT_TRUE(fake_prefs_.SetString(kPrefsOmahaCohort, "old_value"));
+
+ ASSERT_TRUE(TestUpdateCheck(¶ms,
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+
+ string value;
+ EXPECT_TRUE(fake_prefs_.GetString(kPrefsOmahaCohort, &value));
+ EXPECT_EQ("old_value", value);
+
+ EXPECT_FALSE(fake_prefs_.GetString(kPrefsOmahaCohortHint, &value));
+ EXPECT_FALSE(fake_prefs_.GetString(kPrefsOmahaCohortName, &value));
+}
+
TEST_F(OmahaRequestActionTest, NoOutputPipeTest) {
- const string http_response(GetNoUpdateResponse(OmahaRequestParams::kAppId));
+ const string http_response(fake_update_response_.GetNoUpdateResponse());
GMainLoop *loop = g_main_loop_new(g_main_context_default(), FALSE);
@@ -1054,6 +1002,11 @@
false, // interactive
"http://url",
""); // target_version_prefix
+ fake_prefs_.SetString(kPrefsOmahaCohort, "evil\nstring");
+ fake_prefs_.SetString(kPrefsOmahaCohortHint, "evil&string\\");
+ fake_prefs_.SetString(kPrefsOmahaCohortName,
+ JoinString(vector<string>(100, "My spoon is too big."),
+ ' '));
OmahaResponse response;
ASSERT_FALSE(
TestUpdateCheck(¶ms,
@@ -1068,30 +1021,30 @@
&post_data));
// convert post_data to string
string post_str(post_data.begin(), post_data.end());
- EXPECT_NE(post_str.find("testtheservice_pack>"), string::npos);
- EXPECT_EQ(post_str.find("testtheservice_pack>"), string::npos);
- EXPECT_NE(post_str.find("x86 generic<id"), string::npos);
- EXPECT_EQ(post_str.find("x86 generic<id"), string::npos);
- EXPECT_NE(post_str.find("unittest_track&lt;"), string::npos);
- EXPECT_EQ(post_str.find("unittest_track<"), string::npos);
- EXPECT_NE(post_str.find("<OEM MODEL>"), string::npos);
- EXPECT_EQ(post_str.find("<OEM MODEL>"), string::npos);
+ EXPECT_NE(string::npos, post_str.find("testtheservice_pack>"));
+ EXPECT_EQ(string::npos, post_str.find("testtheservice_pack>"));
+ EXPECT_NE(string::npos, post_str.find("x86 generic<id"));
+ EXPECT_EQ(string::npos, post_str.find("x86 generic<id"));
+ EXPECT_NE(string::npos, post_str.find("unittest_track&lt;"));
+ EXPECT_EQ(string::npos, post_str.find("unittest_track<"));
+ EXPECT_NE(string::npos, post_str.find("<OEM MODEL>"));
+ EXPECT_EQ(string::npos, post_str.find("<OEM MODEL>"));
+ EXPECT_NE(string::npos, post_str.find("cohort=\"evil\nstring\""));
+ EXPECT_EQ(string::npos, post_str.find("cohorthint=\"evil&string\\\""));
+ EXPECT_NE(string::npos, post_str.find("cohorthint=\"evil&string\\\""));
+ // Values from Prefs that are too big are removed from the XML instead of
+ // encoded.
+ EXPECT_EQ(string::npos, post_str.find("cohortname="));
}
TEST_F(OmahaRequestActionTest, XmlDecodeTest) {
OmahaResponse response;
+ fake_update_response_.deadline = "<20110101";
+ fake_update_response_.more_info_url = "testthe<url";
+ fake_update_response_.codebase = "testthe&codebase/";
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetUpdateResponse(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "testthe<url", // more info
- "true", // prompt
- "testthe&codebase/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "<20110101"), // deadline
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -1108,19 +1061,11 @@
TEST_F(OmahaRequestActionTest, ParseIntTest) {
OmahaResponse response;
+ // overflows int32_t:
+ fake_update_response_.size = 123123123123123ll;
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetUpdateResponse(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "theurl", // more info
- "true", // prompt
- "thecodebase/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- // overflows int32_t:
- "123123123123123", // size
- "deadline"),
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -1349,7 +1294,7 @@
chromeos::Blob post_data;
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ fake_update_response_.GetNoUpdateResponse(),
-1,
ping_only,
ErrorCode::kSuccess,
@@ -1396,7 +1341,7 @@
chromeos::Blob post_data;
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ fake_update_response_.GetNoUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -1428,7 +1373,7 @@
chromeos::Blob post_data;
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ fake_update_response_.GetNoUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -1465,7 +1410,7 @@
chromeos::Blob post_data;
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ fake_update_response_.GetNoUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -1492,7 +1437,7 @@
chromeos::Blob post_data;
EXPECT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ fake_update_response_.GetNoUpdateResponse(),
-1,
true, // ping_only
ErrorCode::kSuccess,
@@ -1685,20 +1630,7 @@
ASSERT_FALSE(TestUpdateCheck(
¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kOmahaUpdateDeferredPerPolicy,
@@ -1717,20 +1649,7 @@
params.set_interactive(true);
ASSERT_TRUE(
TestUpdateCheck(¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -1757,20 +1676,7 @@
kPrefsUpdateFirstSeenAt, t1.ToInternalValue()));
ASSERT_TRUE(TestUpdateCheck(
¶ms,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -1895,7 +1801,7 @@
chromeos::Blob post_data;
ASSERT_TRUE(
TestUpdateCheck(nullptr, // request_params
- GetNoUpdateResponse(OmahaRequestParams::kAppId),
+ fake_update_response_.GetNoUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -1949,22 +1855,12 @@
EXPECT_CALL(mock_p2p_manager, LookupUrlForFile(_, _, timeout, _))
.Times(expect_p2p_client_lookup ? 1 : 0);
+ fake_update_response_.disable_p2p_for_downloading =
+ omaha_disable_p2p_for_downloading;
+ fake_update_response_.disable_p2p_for_sharing = omaha_disable_p2p_for_sharing;
ASSERT_TRUE(
TestUpdateCheck(&request_params,
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- "42", // elapsed_days
- omaha_disable_p2p_for_downloading,
- omaha_disable_p2p_for_sharing),
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
@@ -2066,22 +1962,10 @@
bool OmahaRequestActionTest::InstallDateParseHelper(const string &elapsed_days,
OmahaResponse *response) {
+ fake_update_response_.elapsed_days = elapsed_days;
return
TestUpdateCheck(nullptr, // request_params
- GetUpdateResponse2(OmahaRequestParams::kAppId,
- "1.2.3.4", // version
- "http://more/info",
- "true", // prompt
- "http://code/base/", // dl url
- "file.signed", // file name
- "HASH1234=", // checksum
- "false", // needs admin
- "123", // size
- "", // deadline
- "7", // max days to scatter
- elapsed_days,
- false, // disable_p2p_for_downloading
- false), // disable_p2p_for sharing
+ fake_update_response_.GetUpdateResponse(),
-1,
false, // ping_only
ErrorCode::kSuccess,
diff --git a/omaha_response.h b/omaha_response.h
index c1e8e4d..d58bf46 100644
--- a/omaha_response.h
+++ b/omaha_response.h
@@ -17,25 +17,11 @@
// This struct encapsulates the data Omaha's response for the request.
// The strings in this struct are not XML escaped.
struct OmahaResponse {
- OmahaResponse()
- : update_exists(false),
- poll_interval(0),
- size(0),
- metadata_size(0),
- max_days_to_scatter(0),
- max_failure_count_per_url(0),
- prompt(false),
- is_delta_payload(false),
- disable_payload_backoff(false),
- disable_p2p_for_downloading(false),
- disable_p2p_for_sharing(false),
- install_date_days(-1) {}
-
// True iff there is an update to be downloaded.
- bool update_exists;
+ bool update_exists = false;
// If non-zero, server-dictated poll interval in seconds.
- int poll_interval;
+ int poll_interval = 0;
// These are only valid if update_exists is true:
std::string version;
@@ -48,28 +34,28 @@
std::string hash;
std::string metadata_signature;
std::string deadline;
- off_t size;
- off_t metadata_size;
- int max_days_to_scatter;
+ off_t size = 0;
+ off_t metadata_size = 0;
+ int max_days_to_scatter = 0;
// The number of URL-related failures to tolerate before moving on to the
// next URL in the current pass. This is a configurable value from the
// Omaha Response attribute, if ever we need to fine tune the behavior.
- uint32_t max_failure_count_per_url;
- bool prompt;
+ uint32_t max_failure_count_per_url = 0;
+ bool prompt = false;
// True if the payload described in this response is a delta payload.
// False if it's a full payload.
- bool is_delta_payload;
+ bool is_delta_payload = false;
// True if the Omaha rule instructs us to disable the back-off logic
// on the client altogether. False otherwise.
- bool disable_payload_backoff;
+ bool disable_payload_backoff = false;
// True if the Omaha rule instructs us to disable p2p for downloading.
- bool disable_p2p_for_downloading;
+ bool disable_p2p_for_downloading = false;
// True if the Omaha rule instructs us to disable p2p for sharing.
- bool disable_p2p_for_sharing;
+ bool disable_p2p_for_sharing = false;
// If not blank, a base-64 encoded representation of the PEM-encoded
// public key in the response.
@@ -78,7 +64,7 @@
// If not -1, the number of days since the epoch Jan 1, 2007 0:00
// PST, according to the Omaha Server's clock and timezone (PST8PDT,
// aka "Pacific Time".)
- int install_date_days;
+ int install_date_days = -1;
};
COMPILE_ASSERT(sizeof(off_t) == 8, off_t_not_64bit);