update_engine: Query Omaha with correct version/delta_okay during DLC Installations
When DLC(s) are being installed, the Omaha request to check should pass
a version of "0.0.0.0" instead of the same version as the platform and
the delta_okay should always be false.
During a DLC installation, the "<updatecheck></updatecheck>" is also
skipped for the platform and tests are included for those checks.
This means that dlcservice should be extra cautious in not passing in
DLC(s) that are already installed as update_engine can potentially
overwrite the file that's already in use/installed+mounted.
Example Omaha request for DLC installations:
<?xml version="1.0" encoding="UTF-8"?>
<request requestid="79a08366-3a0a-4d18-a1a7-a01a6ad7c34c" sessionid="" protocol="3.0" updater="ChromeOSUpdateEngine" updaterversion="0.1.0.0" installsource="scheduler" ismachine="1">
<os version="Indy" platform="Chrome OS" sp=""></os>
<app appid="" version="" track="" board="" hardware_class="" delta_okay="false" lang="" fw_version="" ec_version="" installdate="0" >
<event eventtype="54" eventresult="1" previousversion="0.0.0.0"></event>
</app>
<app appid="_dlc_1" version="0.0.0.0" track="" board="" hardware_class="" delta_okay="false" >
<updatecheck></updatecheck>
<event eventtype="54" eventresult="1" previousversion="0.0.0.0"></event>
</app>
<app appid="_dlc_2" version="0.0.0.0" track="" board="" hardware_class="" delta_okay="false" >
<updatecheck></updatecheck>
<event eventtype="54" eventresult="1" previousversion="0.0.0.0"></event>
</app>
</request>
BUG=chromium:1039898
TEST=FEATURES=test emerge-$B update_engine
Change-Id: Ibc1a29449e9244f38deb661d400d3fc569e7478f
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/1992194
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/omaha_request_builder_xml.cc b/omaha_request_builder_xml.cc
index 823894e..8439b42 100644
--- a/omaha_request_builder_xml.cc
+++ b/omaha_request_builder_xml.cc
@@ -37,6 +37,7 @@
namespace chromeos_update_engine {
const int kNeverPinged = -1;
+const char kNoVersion[] = "0.0.0.0";
bool XmlEncode(const string& input, string* output) {
if (std::find_if(input.begin(), input.end(), [](const char c) {
@@ -131,7 +132,7 @@
// sent for this new version with a previous updatecheck.
string prev_version;
if (!prefs_->GetString(kPrefsPreviousVersion, &prev_version)) {
- prev_version = "0.0.0.0";
+ prev_version = kNoVersion;
}
// We only store a non-empty previous version value after a successful
// update in the previous boot. After reporting it back to the server,
@@ -142,7 +143,7 @@
"previousversion=\"%s\"></event>\n",
OmahaEvent::kTypeRebootedAfterUpdate,
OmahaEvent::kResultSuccess,
- XmlEncodeWithDefault(prev_version, "0.0.0.0").c_str());
+ XmlEncodeWithDefault(prev_version, kNoVersion).c_str());
LOG_IF(WARNING, !prefs_->SetString(kPrefsPreviousVersion, ""))
<< "Unable to reset the previous version.";
}
@@ -219,11 +220,11 @@
if (params_->ShouldPowerwash()) {
LOG(INFO) << "Passing OS version as 0.0.0.0 as we are set to powerwash "
<< "on downgrading to the version in the more stable channel";
- app_versions = "version=\"0.0.0.0\" from_version=\"" +
- XmlEncodeWithDefault(app_data.version, "0.0.0.0") + "\" ";
+ app_versions = "version=\"" + string(kNoVersion) + "\" from_version=\"" +
+ XmlEncodeWithDefault(app_data.version, kNoVersion) + "\" ";
} else {
app_versions = "version=\"" +
- XmlEncodeWithDefault(app_data.version, "0.0.0.0") + "\" ";
+ XmlEncodeWithDefault(app_data.version, kNoVersion) + "\" ";
}
string download_channel = params_->download_channel();
@@ -234,7 +235,8 @@
XmlEncodeWithDefault(params_->current_channel()) + "\" ";
}
- string delta_okay_str = params_->delta_okay() ? "true" : "false";
+ string delta_okay_str =
+ params_->delta_okay() && !params_->is_install() ? "true" : "false";
// If install_date_days is not set (e.g. its value is -1 ), don't
// include the attribute.
@@ -382,7 +384,7 @@
for (const auto& dlc_module_id : params_->dlc_module_ids()) {
OmahaAppData dlc_module_app = {
.id = params_->GetAppId() + "_" + dlc_module_id,
- .version = params_->app_version(),
+ .version = params_->is_install() ? kNoVersion : params_->app_version(),
.skip_update = false,
.is_dlc = true};
app_xml += GetApp(dlc_module_app);
diff --git a/omaha_request_builder_xml.h b/omaha_request_builder_xml.h
index 488be8a..d7a81d3 100644
--- a/omaha_request_builder_xml.h
+++ b/omaha_request_builder_xml.h
@@ -39,6 +39,7 @@
namespace chromeos_update_engine {
extern const int kNeverPinged;
+extern const char kNoVersion[];
// This struct encapsulates the Omaha event information. For a
// complete list of defined event types and results, see
diff --git a/omaha_request_builder_xml_unittest.cc b/omaha_request_builder_xml_unittest.cc
index ecab0e0..8cf7473 100644
--- a/omaha_request_builder_xml_unittest.cc
+++ b/omaha_request_builder_xml_unittest.cc
@@ -44,6 +44,13 @@
return "";
return xml.substr(val_start_pos + key_with_quotes.size(), val_size);
}
+// Helper to find the count of substring in a string.
+static size_t CountSubstringInString(const string& str, const string& substr) {
+ size_t count = 0, pos = 0;
+ while ((pos = str.find(substr, pos ? pos + 1 : 0)) != string::npos)
+ ++count;
+ return count;
+}
} // namespace
class OmahaRequestBuilderXmlTest : public ::testing::Test {
@@ -131,9 +138,8 @@
}
TEST_F(OmahaRequestBuilderXmlTest, GetRequestXmlRequestIdTest) {
- OmahaEvent omaha_event;
OmahaRequestParams omaha_request_params{&fake_system_state_};
- OmahaRequestBuilderXml omaha_request{&omaha_event,
+ OmahaRequestBuilderXml omaha_request{nullptr,
&omaha_request_params,
false,
false,
@@ -153,9 +159,8 @@
TEST_F(OmahaRequestBuilderXmlTest, GetRequestXmlSessionIdTest) {
const string gen_session_id = base::GenerateGUID();
- OmahaEvent omaha_event;
OmahaRequestParams omaha_request_params{&fake_system_state_};
- OmahaRequestBuilderXml omaha_request{&omaha_event,
+ OmahaRequestBuilderXml omaha_request{nullptr,
&omaha_request_params,
false,
false,
@@ -175,4 +180,74 @@
EXPECT_EQ(gen_session_id, session_id);
}
+TEST_F(OmahaRequestBuilderXmlTest, GetRequestXmlPlatformUpdateTest) {
+ OmahaRequestParams omaha_request_params{&fake_system_state_};
+ OmahaRequestBuilderXml omaha_request{nullptr,
+ &omaha_request_params,
+ false,
+ false,
+ 0,
+ 0,
+ 0,
+ fake_system_state_.prefs(),
+ ""};
+ const string request_xml = omaha_request.GetRequest();
+ EXPECT_EQ(1, CountSubstringInString(request_xml, "<updatecheck"))
+ << request_xml;
+}
+
+TEST_F(OmahaRequestBuilderXmlTest, GetRequestXmlPlatformUpdateWithDlcsTest) {
+ OmahaRequestParams omaha_request_params{&fake_system_state_};
+ omaha_request_params.set_dlc_module_ids({"dlc_1", "dlc_2"});
+ OmahaRequestBuilderXml omaha_request{nullptr,
+ &omaha_request_params,
+ false,
+ false,
+ 0,
+ 0,
+ 0,
+ fake_system_state_.prefs(),
+ ""};
+ const string request_xml = omaha_request.GetRequest();
+ EXPECT_EQ(3, CountSubstringInString(request_xml, "<updatecheck"))
+ << request_xml;
+}
+
+TEST_F(OmahaRequestBuilderXmlTest, GetRequestXmlDlcInstallationTest) {
+ OmahaRequestParams omaha_request_params{&fake_system_state_};
+ const vector<string> dlcs = {"dlc_1", "dlc_2"};
+ omaha_request_params.set_dlc_module_ids(dlcs);
+ omaha_request_params.set_is_install(true);
+ OmahaRequestBuilderXml omaha_request{nullptr,
+ &omaha_request_params,
+ false,
+ false,
+ 0,
+ 0,
+ 0,
+ fake_system_state_.prefs(),
+ ""};
+ const string request_xml = omaha_request.GetRequest();
+ EXPECT_EQ(2, CountSubstringInString(request_xml, "<updatecheck"))
+ << request_xml;
+
+ auto FindAppId = [request_xml](size_t pos) -> size_t {
+ return request_xml.find("<app appid", pos);
+ };
+ // Skip over the Platform AppID, which is always first.
+ size_t pos = FindAppId(0);
+ for (auto&& _ : dlcs) {
+ (void)_;
+ EXPECT_NE(string::npos, (pos = FindAppId(pos + 1))) << request_xml;
+ const string dlc_app_id_version = FindAttributeKeyValueInXml(
+ request_xml.substr(pos), "version", string(kNoVersion).size());
+ EXPECT_EQ(kNoVersion, dlc_app_id_version);
+
+ const string false_str = "false";
+ const string dlc_app_id_delta_okay = FindAttributeKeyValueInXml(
+ request_xml.substr(pos), "delta_okay", false_str.length());
+ EXPECT_EQ(false_str, dlc_app_id_delta_okay);
+ }
+}
+
} // namespace chromeos_update_engine