Extract new system version from Omaha responses
When processing an Omaha response, line up the app elements for the main
product and system apps, by their appid fields. This allows the (product)
version and system version to be differentiated and recorded in the
InstallPlan.
Bug: 65422956
Test: Unit-tests, and using a live OTA
Change-Id: I5217b7ac6fbf6f7ae595c1cec6fbc329051925eb
(cherry picked from commit 19ef6db4885387a5e3fbd2affcbd57a9a40d8b9b)
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index c3bbf9d..cce4287 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -392,6 +392,7 @@
string daystart_elapsed_seconds;
struct App {
+ string id;
vector<string> url_codebase;
string manifest_version;
map<string, string> action_postinstall_attrs;
@@ -430,6 +431,9 @@
if (data->current_path == "/response/app") {
data->apps.emplace_back();
+ if (attrs.find("appid") != attrs.end()) {
+ data->apps.back().id = attrs["appid"];
+ }
if (attrs.find("cohort") != attrs.end()) {
data->app_cohort_set = true;
data->app_cohort = attrs["cohort"];
@@ -983,10 +987,18 @@
ScopedActionCompleter* completer) {
map<string, string> attrs;
for (auto& app : parser_data->apps) {
- if (!app.manifest_version.empty() && output_object->version.empty())
+ if (app.id == params_->GetAppId()) {
+ // this is the app (potentially the only app)
output_object->version = app.manifest_version;
- if (!app.action_postinstall_attrs.empty() && attrs.empty())
+ } else if (!params_->system_app_id().empty() &&
+ app.id == params_->system_app_id()) {
+ // this is the system app (this check is intentionally skipped if there is
+ // no system_app_id set)
+ output_object->system_version = app.manifest_version;
+ }
+ if (!app.action_postinstall_attrs.empty() && attrs.empty()) {
attrs = app.action_postinstall_attrs;
+ }
}
if (output_object->version.empty()) {
LOG(ERROR) << "Omaha Response does not have version in manifest!";
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 9091031..7e38956 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -68,6 +68,7 @@
namespace {
const char kTestAppId[] = "test-app-id";
+const char kTestAppId2[] = "test-app2-id";
// This is a helper struct to allow unit tests build an update response with the
// values they care about.
@@ -89,7 +90,8 @@
"<ping status=\"ok\"/>"
"<updatecheck status=\"noupdate\"/></app>" +
(multi_app_no_update
- ? "<app><updatecheck status=\"noupdate\"/></app>"
+ ? "<app appid=\"" + app_id2 +
+ "\"><updatecheck status=\"noupdate\"/></app>"
: "") +
"</response>";
}
@@ -142,9 +144,10 @@
(disable_p2p_for_sharing ? "DisableP2PForSharing=\"true\" " : "") +
"/></actions></manifest></updatecheck></app>" +
(multi_app
- ? "<app><updatecheck status=\"ok\"><urls><url codebase=\"" +
- codebase2 +
- "\"/></urls><manifest><packages>"
+ ? "<app appid=\"" + app_id2 +
+ "\"><updatecheck status=\"ok\"><urls><url codebase=\"" +
+ codebase2 + "\"/></urls><manifest version=\"" + version2 +
+ "\"><packages>"
"<package name=\"package3\" size=\"333\" "
"hash_sha256=\"hash3\"/></packages>"
"<actions><action event=\"postinstall\" " +
@@ -166,7 +169,9 @@
}
string app_id = kTestAppId;
+ string app_id2 = kTestAppId2;
string version = "1.2.3.4";
+ string version2 = "2.3.4.5";
string more_info_url = "http://more/info";
string prompt = "true";
string codebase = "http://code/base/";
@@ -549,6 +554,7 @@
nullptr));
EXPECT_TRUE(response.update_exists);
EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ("", response.system_version);
EXPECT_EQ(fake_update_response_.GetPayloadUrl(),
response.packages[0].payload_urls[0]);
EXPECT_EQ(fake_update_response_.more_info_url, response.more_info_url);
@@ -557,7 +563,7 @@
EXPECT_EQ(true, response.packages[0].is_delta);
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
+ // Omaha cohort attributes are not set in the response, so they should not be
// persisted.
EXPECT_FALSE(fake_prefs_.Exists(kPrefsOmahaCohort));
EXPECT_FALSE(fake_prefs_.Exists(kPrefsOmahaCohortHint));
@@ -624,6 +630,40 @@
EXPECT_EQ(false, response.packages[1].is_delta);
}
+TEST_F(OmahaRequestActionTest, MultiAppAndSystemUpdateTest) {
+ OmahaResponse response;
+ fake_update_response_.multi_app = true;
+ // trigger the lining up of the app and system versions
+ request_params_.set_system_app_id(fake_update_response_.app_id2);
+
+ ASSERT_TRUE(TestUpdateCheck(nullptr, // request_params
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_TRUE(response.update_exists);
+ EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ(fake_update_response_.version2, response.system_version);
+ EXPECT_EQ(fake_update_response_.GetPayloadUrl(),
+ response.packages[0].payload_urls[0]);
+ EXPECT_EQ(fake_update_response_.codebase2 + "package3",
+ response.packages[1].payload_urls[0]);
+ EXPECT_EQ(fake_update_response_.hash, response.packages[0].hash);
+ EXPECT_EQ(fake_update_response_.size, response.packages[0].size);
+ EXPECT_EQ(11u, response.packages[0].metadata_size);
+ EXPECT_EQ(true, response.packages[0].is_delta);
+ ASSERT_EQ(2u, response.packages.size());
+ EXPECT_EQ(string("hash3"), response.packages[1].hash);
+ EXPECT_EQ(333u, response.packages[1].size);
+ EXPECT_EQ(33u, response.packages[1].metadata_size);
+ EXPECT_EQ(false, response.packages[1].is_delta);
+}
+
TEST_F(OmahaRequestActionTest, MultiAppPartialUpdateTest) {
OmahaResponse response;
fake_update_response_.multi_app = true;
@@ -640,6 +680,7 @@
nullptr));
EXPECT_TRUE(response.update_exists);
EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ("", response.system_version);
EXPECT_EQ(fake_update_response_.GetPayloadUrl(),
response.packages[0].payload_urls[0]);
EXPECT_EQ(fake_update_response_.hash, response.packages[0].hash);
@@ -668,6 +709,7 @@
nullptr));
EXPECT_TRUE(response.update_exists);
EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ("", response.system_version);
EXPECT_EQ(fake_update_response_.GetPayloadUrl(),
response.packages[0].payload_urls[0]);
EXPECT_EQ(fake_update_response_.codebase + "package2",
@@ -1263,7 +1305,10 @@
string input_response =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response protocol=\"3.0\">"
"<daystart elapsed_seconds=\"100\"/>"
- "<app appid=\"xyz\" status=\"ok\">"
+ // the appid needs to match that in the request params
+ "<app appid=\"" +
+ fake_update_response_.app_id +
+ "\" status=\"ok\">"
"<updatecheck status=\"ok\">"
"<urls><url codebase=\"http://missing/field/test/\"/></urls>"
"<manifest version=\"10.2.3.4\">"
diff --git a/omaha_request_params.h b/omaha_request_params.h
index f8e9438..ba7f2c3 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -111,6 +111,9 @@
return image_props_.canary_product_id;
}
inline std::string system_app_id() const { return image_props_.system_id; }
+ inline void set_system_app_id(const std::string& system_app_id) {
+ image_props_.system_id = system_app_id;
+ }
inline void set_app_id(const std::string& app_id) {
image_props_.product_id = app_id;
image_props_.canary_product_id = app_id;
diff --git a/omaha_response.h b/omaha_response.h
index c702068..b973eb5 100644
--- a/omaha_response.h
+++ b/omaha_response.h
@@ -37,6 +37,7 @@
// These are only valid if update_exists is true:
std::string version;
+ std::string system_version;
struct Package {
// The ordered list of URLs in the Omaha response. Each item is a complete
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 189fe6b..75050e7 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -71,6 +71,7 @@
install_plan_.download_url = current_url;
install_plan_.version = response.version;
+ install_plan_.system_version = response.system_version;
OmahaRequestParams* const params = system_state_->request_params();
PayloadStateInterface* const payload_state = system_state_->payload_state();
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 75cd819..935ae5d 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -456,4 +456,28 @@
EXPECT_TRUE(install_plan.hash_checks_mandatory);
}
+TEST_F(OmahaResponseHandlerActionTest, SystemVersionTest) {
+ OmahaResponse in;
+ in.update_exists = true;
+ in.version = "a.b.c.d";
+ in.system_version = "b.c.d.e";
+ in.packages.push_back({.payload_urls = {"http://package/1"},
+ .size = 1,
+ .hash = kPayloadHashHex});
+ in.packages.push_back({.payload_urls = {"http://package/2"},
+ .size = 2,
+ .hash = kPayloadHashHex});
+ in.more_info_url = "http://more/info";
+ InstallPlan install_plan;
+ EXPECT_TRUE(DoTest(in, "", &install_plan));
+ EXPECT_EQ(in.packages[0].payload_urls[0], install_plan.download_url);
+ EXPECT_EQ(2u, install_plan.payloads.size());
+ EXPECT_EQ(in.packages[0].size, install_plan.payloads[0].size);
+ EXPECT_EQ(in.packages[1].size, install_plan.payloads[1].size);
+ EXPECT_EQ(expected_hash_, install_plan.payloads[0].hash);
+ EXPECT_EQ(expected_hash_, install_plan.payloads[1].hash);
+ EXPECT_EQ(in.version, install_plan.version);
+ EXPECT_EQ(in.system_version, install_plan.system_version);
+}
+
} // namespace chromeos_update_engine
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index d5d745b..b0dff31 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -74,7 +74,14 @@
InstallPayloadTypeToString(payload.type).c_str());
}
+ string version_str = base::StringPrintf(", version: %s", version.c_str());
+ if (!system_version.empty()) {
+ version_str +=
+ base::StringPrintf(", system_version: %s", system_version.c_str());
+ }
+
LOG(INFO) << "InstallPlan: " << (is_resume ? "resume" : "new_update")
+ << version_str
<< ", source_slot: " << BootControlInterface::SlotName(source_slot)
<< ", target_slot: " << BootControlInterface::SlotName(target_slot)
<< ", url: " << download_url << payloads_str << partitions_str
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index 6dd5a73..551f8c2 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -54,6 +54,8 @@
bool is_resume{false};
std::string download_url; // url to download from
std::string version; // version we are installing.
+ // system version, if present and separate from version
+ std::string system_version;
struct Payload {
uint64_t size = 0; // size of the payload