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(&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
-                                         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(&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
-                                         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(&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
-                                         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(&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
-                                         "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(
                       &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
-                                         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(
                       &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
-                                         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(&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
-                                         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(
                       &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
-                                         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(&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
-                                         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(&params,
+                              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(&params,
+                              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(&params,
+                              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(&params,
@@ -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&gt;"), string::npos);
-  EXPECT_EQ(post_str.find("testtheservice_pack>"), string::npos);
-  EXPECT_NE(post_str.find("x86 generic&lt;id"), string::npos);
-  EXPECT_EQ(post_str.find("x86 generic<id"), string::npos);
-  EXPECT_NE(post_str.find("unittest_track&amp;lt;"), string::npos);
-  EXPECT_EQ(post_str.find("unittest_track&lt;"), string::npos);
-  EXPECT_NE(post_str.find("&lt;OEM MODEL&gt;"), string::npos);
-  EXPECT_EQ(post_str.find("<OEM MODEL>"), string::npos);
+  EXPECT_NE(string::npos, post_str.find("testtheservice_pack&gt;"));
+  EXPECT_EQ(string::npos, post_str.find("testtheservice_pack>"));
+  EXPECT_NE(string::npos, post_str.find("x86 generic&lt;id"));
+  EXPECT_EQ(string::npos, post_str.find("x86 generic<id"));
+  EXPECT_NE(string::npos, post_str.find("unittest_track&amp;lt;"));
+  EXPECT_EQ(string::npos, post_str.find("unittest_track&lt;"));
+  EXPECT_NE(string::npos, post_str.find("&lt;OEM MODEL&gt;"));
+  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&amp;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 = "&lt;20110101";
+  fake_update_response_.more_info_url = "testthe&lt;url";
+  fake_update_response_.codebase = "testthe&amp;codebase/";
   ASSERT_TRUE(
       TestUpdateCheck(nullptr,  // request_params
-                      GetUpdateResponse(OmahaRequestParams::kAppId,
-                                        "1.2.3.4",  // version
-                                        "testthe&lt;url",  // more info
-                                        "true",  // prompt
-                                        "testthe&amp;codebase/",  // dl url
-                                        "file.signed",  // file name
-                                        "HASH1234=",  // checksum
-                                        "false",  // needs admin
-                                        "123",  // size
-                                        "&lt;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(
                       &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
-                                         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(&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
-                                         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(
                       &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
-                                         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);