Skip to content

Commit

Permalink
Updater: Enhanced architecture support
Browse files Browse the repository at this point in the history 
The `arch` attribute in the offline update response is now used to
determine compatibility of the app being installed with the host
processor architecture.

`arch` can be a single entry, or multiple entries separated with `,`.
Entries prefixed with a `-` (negative entries) indicate non-compatible
hosts.

Examples:
* `arch` == "x86": the app will install if run on all systems, because
  the Updater is x86, and is running the logic to determine
  compatibility)
* `arch` == "x64": the app will install if run on x64 or many arm64
  systems.
* `arch` == "x86,x64,-arm64": the app will fail installation if the
  underlying host is arm64.

Bug: 1375366
Change-Id: I11175116901fa3b406e960d5cfb5a3ab65053be5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4014666
Auto-Submit: S Ganesh <ganesh@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Commit-Queue: S Ganesh <ganesh@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1068937}
  • Loading branch information
@GitHubGanesh
GitHubGanesh authored and Chromium LUCI CQ committed Nov 9, 2022
1 parent 18ce8ce commit 38ffbd2
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 29 deletions.
44 changes: 33 additions & 11 deletions chrome/updater/win/app_install_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ class AppInstallControllerImpl : public AppInstallController,
void DoInstallAppOffline(const base::FilePath& installer_path,
const std::string& install_args,
const std::string& install_data);
void HandleArchNotCompatible();
void InstallComplete(UpdateService::Result result);

[[nodiscard]] static ObserverCompletionInfo HandleInstallResult(
Expand Down Expand Up @@ -692,6 +693,7 @@ void AppInstallControllerImpl::InstallAppOffline(
GetCommandLineLegacyCompatible();
// Parse the offline manifest to get the install
// command and install data.
update_client::ProtocolParser::Results results;
base::FilePath installer_path;
std::string install_args;
std::string install_data;
Expand All @@ -700,25 +702,33 @@ void AppInstallControllerImpl::InstallAppOffline(
app_id,
GetInstallDataIndexFromAppArgsForCommandLine(cmd_line,
app_id),
installer_path, install_args, install_data);
results, installer_path, install_args, install_data);

const std::string client_install_data =
GetDecodedInstallDataFromAppArgsForCommandLine(
cmd_line, app_id);
return std::make_tuple(installer_path, install_args,
client_install_data.empty()
? install_data
: client_install_data);
return std::make_tuple(
results, installer_path, install_args,
client_install_data.empty() ? install_data
: client_install_data);
},
self->app_id_),
base::BindOnce(
[](scoped_refptr<AppInstallControllerImpl> self,
const std::tuple<base::FilePath /*installer_path*/,
std::string /*arguments*/,
std::string /*install_data*/>& result) {
self->DoInstallAppOffline(std::get<0>(result),
std::get<1>(result),
std::get<2>(result));
const std::tuple<
update_client::ProtocolParser::Results /*results*/,
base::FilePath /*installer_path*/,
std::string /*arguments*/,
std::string /*install_data*/>& result) {
if (!IsArchCompatible(
std::get<0>(result).system_requirements.arch)) {
self->HandleArchNotCompatible();
return;
}

self->DoInstallAppOffline(std::get<1>(result),
std::get<2>(result),
std::get<3>(result));
},
self));
},
Expand Down Expand Up @@ -800,6 +810,18 @@ void AppInstallControllerImpl::DoInstallAppOffline(
install_data, install_settings)));
}

void AppInstallControllerImpl::HandleArchNotCompatible() {
UpdateService::UpdateState update_state;
update_state.app_id = app_id_;
update_state.state = UpdateService::UpdateState::State::kUpdateError;
update_state.error_category = UpdateService::ErrorCategory::kInstall;
update_state.error_code = UNSUPPORTED_WINDOWS_VERSION;
observer_completion_info_ = HandleInstallResult(update_state);
observer_completion_info_->completion_text =
GetLocalizedString(IDS_INSTALL_OS_NOT_SUPPORTED_BASE);
InstallComplete(UpdateService::Result::kInstallFailed);
}

// TODO(crbug.com/1218219) - propagate error code in case of errors.
void AppInstallControllerImpl::InstallComplete(UpdateService::Result result) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
Expand Down
50 changes: 41 additions & 9 deletions chrome/updater/win/manifest_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/ranges/algorithm.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "chrome/updater/win/protocol_parser_xml.h"
#include "chrome/updater/win/win_util.h"
#include "components/update_client/protocol_parser.h"
#include "components/update_client/utils.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace updater {
Expand Down Expand Up @@ -74,12 +78,14 @@ std::unique_ptr<ProtocolParserXML> ParseOfflineManifest(

} // namespace

void ReadInstallCommandFromManifest(const base::FilePath& offline_dir,
const std::string& app_id,
const std::string& install_data_index,
base::FilePath& installer_path,
std::string& install_args,
std::string& install_data) {
void ReadInstallCommandFromManifest(
const base::FilePath& offline_dir,
const std::string& app_id,
const std::string& install_data_index,
update_client::ProtocolParser::Results& results,
base::FilePath& installer_path,
std::string& install_args,
std::string& install_data) {
if (offline_dir.empty()) {
VLOG(1) << "Unexpected: offline install without an offline directory.";
return;
Expand All @@ -91,13 +97,14 @@ void ReadInstallCommandFromManifest(const base::FilePath& offline_dir,
return;
}

const std::vector<update_client::ProtocolParser::Result>& results =
results = manifest_parser->results();
const std::vector<update_client::ProtocolParser::Result>& app_list =
manifest_parser->results().list;
auto it = base::ranges::find_if(
results, [&app_id](const update_client::ProtocolParser::Result& result) {
app_list, [&app_id](const update_client::ProtocolParser::Result& result) {
return base::EqualsCaseInsensitiveASCII(result.extension_id, app_id);
});
if (it == std::end(results)) {
if (it == std::end(app_list)) {
VLOG(2) << "No manifest data for app: " << app_id;
return;
}
Expand All @@ -116,4 +123,29 @@ void ReadInstallCommandFromManifest(const base::FilePath& offline_dir,
}
}

bool IsArchCompatible(const std::string& arch_list) {
std::vector<std::string> architectures = base::SplitString(
arch_list, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);

if (architectures.empty()) {
return true;
}

const std::string arch = update_client::GetArchitecture();

// This code accounts for Omaha 3 Offline manifests having `arch` as "x64",
// but `GetArchitecture` returning "x86_64" for amd64.
const std::string current_architecture =
arch == update_client::kArchAmd64 ? kArchAmd64Omaha3 : arch;

base::ranges::sort(architectures);
if (base::ranges::find(architectures, '-' + current_architecture) !=
architectures.end()) {
return false;
}

return base::ranges::find_if(architectures, IsArchitectureSupported) !=
architectures.end();
}

} // namespace updater
40 changes: 34 additions & 6 deletions chrome/updater/win/manifest_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include <string>

#include "components/update_client/protocol_parser.h"

namespace base {
class FilePath;
}
Expand All @@ -25,18 +27,44 @@ namespace updater {
//
// The function extracts the values from the manifest using a best-effort
// approach. If matching values are found, then:
// `results`: contains the protocol parser results.
// `installer_path`: contains the full path to the app installer.
// `install_args`: the command line arguments for the app installer.
// `install_data`: the text value for the key `install_data_index` if such
// key/value pair exists in <data> element. During
// installation, the text will be serialized to a file and
// passed to the app installer.
void ReadInstallCommandFromManifest(const base::FilePath& offline_dir,
const std::string& app_id,
const std::string& install_data_index,
base::FilePath& installer_path,
std::string& install_args,
std::string& install_data);
void ReadInstallCommandFromManifest(
const base::FilePath& offline_dir,
const std::string& app_id,
const std::string& install_data_index,
update_client::ProtocolParser::Results& results,
base::FilePath& installer_path,
std::string& install_args,
std::string& install_data);

// Checks if the current architecture is compatible with the entries in
// `arch_list`. `arch_list` can be a single entry, or multiple entries separated
// with `,`. Entries prefixed with `-` (negative entries) indicate
// non-compatible hosts.
//
// Returns `true` if:
//* `arch_list` is empty, or
// * (none of the negative entries within `arch_list` match the current
// architecture, and
// * one of the non-negative entries within `arch_list` matches the current
// architecture, or is compatible with the current architecture as determined
// by `::IsWow64GuestMachineSupported()`.
// * If `::IsWow64GuestMachineSupported()` is not available, returns `true`
// if `arch` is x86.
//
// Examples:
// * `arch_list` == "x86": returns `true` if run on all systems, because the
// Updater is x86, and is running the logic to determine compatibility).
// * `arch_list` == "x64": returns `true` if run on x64 or many arm64 systems.
// * `arch_list` == "x86,x64,-arm64": returns `false` if the underlying host is
// arm64.
bool IsArchCompatible(const std::string& arch_list);

} // namespace updater

Expand Down
37 changes: 36 additions & 1 deletion chrome/updater/win/manifest_util_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
#include "base/files/file_path.h"
#include "base/path_service.h"
#include "chrome/common/chrome_paths.h"
#include "components/update_client/protocol_parser.h"
#include "components/update_client/utils.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace updater {

TEST(ManifestUtil, ReadInstallCommandFromManifest) {
update_client::ProtocolParser::Results results;
base::FilePath installer_path;
std::string install_args;
std::string install_data;
Expand All @@ -24,7 +27,7 @@ TEST(ManifestUtil, ReadInstallCommandFromManifest) {

ReadInstallCommandFromManifest(
offline_dir, "{CDABE316-39CD-43BA-8440-6D1E0547AEE6}", "verboselogging",
installer_path, install_args, install_data);
results, installer_path, install_args, install_data);
EXPECT_EQ(installer_path, offline_dir.AppendASCII("my_installer.exe"));
EXPECT_EQ(install_args, "-baz");
EXPECT_EQ(install_data,
Expand All @@ -35,4 +38,36 @@ TEST(ManifestUtil, ReadInstallCommandFromManifest) {
" }");
}

TEST(ManifestUtil, IsArchCompatible_ArchEmpty) {
EXPECT_TRUE(IsArchCompatible({}));
}

TEST(ManifestUtil, IsArchCompatible_ArchUnknown) {
EXPECT_FALSE(IsArchCompatible("unknown"));
}

TEST(ManifestUtil, IsArchCompatible_Archx86) {
EXPECT_TRUE(IsArchCompatible("x86"));
}

TEST(ManifestUtil, IsArchCompatible_Archx64) {
EXPECT_EQ(update_client::GetArchitecture() == update_client::kArchAmd64,
IsArchCompatible("x64"));
}

TEST(ManifestUtil, IsArchCompatible_NotArchx64) {
EXPECT_EQ(update_client::GetArchitecture() != update_client::kArchAmd64,
IsArchCompatible("-x64"));
}

TEST(ManifestUtil, IsArchCompatible_Archx86Notx64) {
EXPECT_EQ(update_client::GetArchitecture() != update_client::kArchAmd64,
IsArchCompatible("x86,-x64"));
}

TEST(ManifestUtil, IsArchCompatible_Archx86x64NotArm64) {
EXPECT_EQ(update_client::GetArchitecture() != update_client::kArchArm64,
IsArchCompatible("x86,x64,-arm64"));
}

} // namespace updater
5 changes: 3 additions & 2 deletions chrome/updater/win/win_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace updater {

const char kArchAmd64Omaha3[] = "x64";

namespace {

HResultOr<bool> IsUserRunningSplitToken() {
Expand Down Expand Up @@ -991,8 +994,6 @@ bool StopGoogleUpdateProcesses(UpdaterScope scope) {
}

bool IsArchitectureSupported(const std::string& arch) {
constexpr char kArchAmd64Omaha3[] = "x64";

if (arch.empty())
return true;

Expand Down
2 changes: 2 additions & 0 deletions chrome/updater/win/win_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ struct std::hash<IID> {

namespace updater {

extern const char kArchAmd64Omaha3[];

// Helper for methods which perform system operations which may fail. The
// failure reason is returned as an HRESULT.
// TODO(crbug.com/1369769): Remove the following warning once resolved in
Expand Down
21 changes: 21 additions & 0 deletions docs/updater/functional_spec.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,27 @@ event that Omaha listens to, so that Omaha processes can shut down gracefully.
The updater then proceeds to overinstall the Omaha binaries with the updater
binaries.

### Offline installs

The updater supports offline installations, for which no update check or file
download is performed against the server during installation. All data is read
from the files in the directory instead.

Offline installs include:
* an offline manifest file, which contains the update check response in XML
format.
* app installer.

The `arch` attribute in the offline update response is used to determine
compatibility of the app being installed with the host processor architecture.

Omaha 3 offline manifests have `arch` as "x64", but the Chromium functions
return "x86_64" as the architecture for amd64. The updater accounts for this by
treating "x64" the same as "x86_64".

For more information, see the
[protocol document](protocol_3_1.md#update-checks-body-update-check-response-objects-update-check-response-3).

### Enterprise Enrollment
The updater may be enrolled with a particular enterprise. Enrollment is
coordinated with a device management server by means of an enrollment token and
Expand Down

0 comments on commit 38ffbd2

Please sign in to comment.