Commit e555be86 authored by Mike West's avatar Mike West Committed by Commit Bot

Sec-CH-UA: Send only the major version by default.

Rather than sending a `Sec-CH-UA` header containing the full version
number by default (e.g. "Chromium 99.0.1232.12"), send only the major
version (e.g. "Chromium 99").

This does not effect the value of `UserAgent.version` obtained from
`navigator.getUserAgent()`, but only the HTTP request header.

Bug: 928669
Change-Id: I074e244a3918b0bdab4453c2f56dc737c506f732
Reviewed-on: https://chromium-review.googlesource.com/c/1475438Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633726}
parent bdb0e8df
......@@ -1063,7 +1063,8 @@ blink::UserAgentMetadata GetUserAgentMetadata() {
blink::UserAgentMetadata metadata;
metadata.brand = version_info::GetProductName();
metadata.version = version_info::GetVersionNumber();
metadata.full_version = version_info::GetVersionNumber();
metadata.major_version = version_info::GetMajorVersionNumber();
metadata.platform = version_info::GetOSType();
// TODO(mkwst): Poke at BuildUserAgentFromProduct to split out these pieces.
......
......@@ -464,7 +464,8 @@ TEST(ChromeContentBrowserClient, UserAgentMetadata) {
auto metadata = content_browser_client.GetUserAgentMetadata();
EXPECT_EQ(metadata.brand, version_info::GetProductName());
EXPECT_EQ(metadata.version, version_info::GetVersionNumber());
EXPECT_EQ(metadata.full_version, version_info::GetVersionNumber());
EXPECT_EQ(metadata.major_version, version_info::GetMajorVersionNumber());
EXPECT_EQ(metadata.platform, version_info::GetOSType());
EXPECT_EQ(metadata.architecture, "");
EXPECT_EQ(metadata.model, "");
......
......@@ -395,14 +395,16 @@ void ClientHints::GetAdditionalNavigationRequestClientHintsHeaders(
// (intentionally) different than other client hints.
//
// https://tools.ietf.org/html/draft-west-ua-client-hints-00#section-2.4
std::string version =
web_client_hints.IsEnabled(blink::mojom::WebClientHintsType::kUA)
? ua.full_version
: ua.major_version;
additional_headers->SetHeader(
blink::kClientHintsHeaderMapping[static_cast<int>(
blink::mojom::WebClientHintsType::kUA)],
// TODO(mkwst): This should include only the major version if the
// recipient hasn't opted into the hint.
ua.version.empty() ? ua.brand.c_str()
: base::StringPrintf("%s %s", ua.brand.c_str(),
ua.version.c_str()));
version.empty()
? ua.brand.c_str()
: base::StringPrintf("%s %s", ua.brand.c_str(), version.c_str()));
if (web_client_hints.IsEnabled(blink::mojom::WebClientHintsType::kUAArch)) {
additional_headers->SetHeader(
......
......@@ -10,8 +10,10 @@
#include "base/metrics/field_trial_param_associator.h"
#include "base/run_loop.h"
#include "base/stl_util.h"
#include "base/strings/string_util.h"
#include "base/test/metrics/histogram_tester.h"
#include "build/build_config.h"
#include "chrome/browser/chrome_content_browser_client.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/content_settings/tab_specific_content_settings.h"
......@@ -398,6 +400,10 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
return request_interceptor_->client_hints_count_seen();
}
const std::string& main_frame_ua_observed() const {
return main_frame_ua_observed_;
}
base::test::ScopedFeatureList scoped_feature_list_;
std::string intercept_iframe_resource_;
......@@ -477,6 +483,9 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
}
if (is_main_frame_navigation) {
if (request.headers.find("sec-ch-ua") != request.headers.end())
main_frame_ua_observed_ = request.headers.find("sec-ch-ua")->second;
VerifyClientHintsReceived(expect_client_hints_on_main_frame_, request);
if (expect_client_hints_on_main_frame_) {
double value = 0.0;
......@@ -503,6 +512,7 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
EXPECT_EQ(980, value);
#endif
main_frame_viewport_width_observed_ = value;
VerifyNetworkQualityClientHints(request);
}
}
......@@ -669,6 +679,8 @@ class ClientHintsBrowserTest : public InProcessBrowserTest,
GURL http_equiv_accept_ch_with_lifetime_;
GURL redirect_url_;
std::string main_frame_ua_observed_;
double main_frame_dpr_observed_ = -1;
double main_frame_viewport_width_observed_ = -1;
double main_frame_device_memory_observed_ = -1;
......@@ -823,6 +835,30 @@ IN_PROC_BROWSER_TEST_P(ClientHintsBrowserTest,
EXPECT_EQ(1u, third_party_client_hints_count_seen());
}
// Verify that we send only major version information in the `Sec-CH-UA` header
// by default, and full version information after an opt-in.
IN_PROC_BROWSER_TEST_P(ClientHintsBrowserTest, UserAgentVersion) {
const GURL gurl = GetParam() ? http_equiv_accept_ch_with_lifetime()
: accept_ch_with_lifetime_url();
blink::UserAgentMetadata ua = ::GetUserAgentMetadata();
// Navigate to a page that opts-into the header: the value should end with
// the major version, and not contain the full version.
SetClientHintExpectationsOnMainFrame(false);
ui_test_utils::NavigateToURL(browser(), gurl);
EXPECT_TRUE(base::EndsWith(main_frame_ua_observed(), ua.major_version,
base::CompareCase::SENSITIVE));
EXPECT_EQ(std::string::npos, main_frame_ua_observed().find(ua.full_version));
// Navigate again, after the opt-in: the value should end with the full
// version.
SetClientHintExpectationsOnMainFrame(true);
ui_test_utils::NavigateToURL(browser(), gurl);
EXPECT_TRUE(base::EndsWith(main_frame_ua_observed(), ua.full_version,
base::CompareCase::SENSITIVE));
}
// Test that client hints are attached to third party subresources if
// AllowClientHintsToThirdParty feature is enabled.
IN_PROC_BROWSER_TEST_P(ClientHintsAllowThirdPartyBrowserTest,
......
......@@ -6,6 +6,7 @@
#include "base/logging.h"
#include "base/no_destructor.h"
#include "base/strings/string_number_conversions.h"
#include "base/version.h"
#include "build/build_config.h"
#include "components/version_info/version_info_values.h"
......@@ -24,6 +25,11 @@ std::string GetVersionNumber() {
return PRODUCT_VERSION;
}
std::string GetMajorVersionNumber() {
DCHECK(version_info::GetVersion().IsValid());
return base::UintToString(version_info::GetVersion().components()[0]);
}
const base::Version& GetVersion() {
static const base::NoDestructor<base::Version> version(GetVersionNumber());
return *version;
......
......@@ -25,6 +25,9 @@ std::string GetProductName();
// Returns the version number, e.g. "6.0.490.1".
std::string GetVersionNumber();
// Returns the major component of the version, e.g. "6".
std::string GetMajorVersionNumber();
// Returns the result of GetVersionNumber() as a base::Version.
const base::Version& GetVersion();
......
......@@ -26,6 +26,7 @@ if (is_android) {
declare_args() {
content_shell_product_name = "Content Shell"
content_shell_version = "99.77.34.5"
content_shell_major_version = "99"
}
config("content_shell_lib_warnings") {
......@@ -238,7 +239,10 @@ jumbo_static_library("content_shell_lib") {
"//build/config/compiler:no_size_t_to_int_warning",
]
defines = [ "CONTENT_SHELL_VERSION=\"$content_shell_version\"" ]
defines = [
"CONTENT_SHELL_VERSION=\"$content_shell_version\"",
"CONTENT_SHELL_MAJOR_VERSION=\"$content_shell_major_version\"",
]
public_deps = [
":android_shell_descriptors",
......
......@@ -240,7 +240,8 @@ blink::UserAgentMetadata GetShellUserAgentMetadata() {
blink::UserAgentMetadata metadata;
metadata.brand = "content_shell";
metadata.version = CONTENT_SHELL_VERSION;
metadata.full_version = CONTENT_SHELL_VERSION;
metadata.major_version = CONTENT_SHELL_MAJOR_VERSION;
metadata.platform = BuildOSCpuInfo(false);
// TODO(mkwst): Split these out from BuildOSCpuInfo().
......
......@@ -17,9 +17,13 @@ bool StructTraits<blink::mojom::UserAgentMetadataDataView,
return false;
out->brand = string;
if (!data.ReadVersion(&string))
if (!data.ReadFullVersion(&string))
return false;
out->version = string;
out->full_version = string;
if (!data.ReadMajorVersion(&string))
return false;
out->major_version = string;
if (!data.ReadPlatform(&string))
return false;
......
......@@ -13,7 +13,8 @@ namespace blink {
struct BLINK_COMMON_EXPORT UserAgentMetadata {
std::string brand;
std::string version;
std::string full_version;
std::string major_version;
std::string platform;
std::string architecture;
std::string model;
......
......@@ -21,8 +21,13 @@ struct BLINK_COMMON_EXPORT StructTraits<blink::mojom::UserAgentMetadataDataView,
static const std::string& brand(const ::blink::UserAgentMetadata& data) {
return data.brand;
}
static const std::string& version(const ::blink::UserAgentMetadata& data) {
return data.version;
static const std::string& full_version(
const ::blink::UserAgentMetadata& data) {
return data.full_version;
}
static const std::string& major_version(
const ::blink::UserAgentMetadata& data) {
return data.major_version;
}
static const std::string& platform(const ::blink::UserAgentMetadata& data) {
return data.platform;
......
......@@ -6,7 +6,8 @@ module blink.mojom;
struct UserAgentMetadata {
string brand;
string version;
string full_version;
string major_version;
string platform;
string architecture;
string model;
......
......@@ -18,7 +18,7 @@ ScriptPromise NavigatorUserAgent::getUserAgent(ScriptState* script_state) {
blink::UserAgent* idl_metadata = blink::UserAgent::Create();
idl_metadata->setBrand(String::FromUTF8(metadata.brand.data()));
idl_metadata->setVersion(String::FromUTF8(metadata.version.data()));
idl_metadata->setVersion(String::FromUTF8(metadata.full_version.data()));
idl_metadata->setPlatform(String::FromUTF8(metadata.platform.data()));
idl_metadata->setArchitecture(String::FromUTF8(metadata.architecture.data()));
idl_metadata->setModel(String::FromUTF8(metadata.model.data()));
......
......@@ -737,16 +737,14 @@ void FrameFetchContext::AddClientHintsIfNecessary(
if (RuntimeEnabledFeatures::UserAgentClientHintEnabled()) {
StringBuilder result;
result.Append(ua.brand.data());
if (!ua.version.empty()) {
const std::string& version =
ShouldSendClientHint(mojom::WebClientHintsType::kUA, hints_preferences,
enabled_hints)
? ua.full_version
: ua.major_version;
if (!version.empty()) {
result.Append(' ');
if (ShouldSendClientHint(mojom::WebClientHintsType::kUA,
hints_preferences, enabled_hints)) {
result.Append(ua.version.data());
} else {
// TODO(mkwst): This should only send the major version, but we haven't
// piped that through yet.
result.Append(ua.version.data());
}
result.Append(version.data());
}
request.AddHTTPHeaderField(
blink::kClientHintsHeaderMapping[static_cast<size_t>(
......
def main(request, response):
ua = request.headers.get('sec-ch-ua', '')
response.headers.set("Content-Type", "text/html")
response.headers.set("Accept-CH", "UA")
response.headers.set("Accept-CH-Lifetime", "10")
response.content = '''
<script>
window.opener.postMessage({ header: "%s" }, "*");
</script>
Sec-CH-UA: %s
''' % (ua, ua)
<!DOCTYPE html>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
promise_test(t => {
return new Promise((resolve, reject) => {
var w;
window.onmessage = e => {
assert_equals(e.data.header, "", "The `Sec-CH-UA` header is not delivered.");
w.close();
resolve();
};
w = window.open("./resources/sec-ch-ua.py");
});
}, "Open HTTP window: no `Sec-CH-UA` header.")
</script>
This is a testharness.js-based test.
FAIL Open HTTPS window prior to opt-in: `Sec-CH-UA` header with minor version. assert_not_equals: The `Sec-CH-UA` header is delivered. got disallowed value ""
FAIL Open HTTPS window post-opt-in: `Sec-CH-UA` header with minor version. assert_not_equals: The `Sec-CH-UA` header is delivered. got disallowed value ""
Harness: the test ran to completion.
<!DOCTYPE html>
<head>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
var minor = "";
promise_test(t => {
return new Promise((resolve, reject) => {
var w;
window.onmessage = e => {
try {
assert_not_equals(e.data.header, "", "The `Sec-CH-UA` header is delivered.");
minor = e.data.header;
} catch (ex) {
reject(ex);
}
w.close();
resolve();
};
w = window.open("./resources/sec-ch-ua.py");
});
}, "Open HTTPS window prior to opt-in: `Sec-CH-UA` header with minor version.")
promise_test(t => {
return new Promise((resolve, reject) => {
var w;
window.onmessage = e => {
try {
assert_not_equals(e.data.header, "", "The `Sec-CH-UA` header is delivered.");
assert_not_equals(e.data.header, minor, "The `Sec-CH-UA` header is different after the opt-in than before.");
} catch (ex) {
reject(ex);
}
w.close();
resolve();
};
w = window.open("./resources/sec-ch-ua.py");
});
}, "Open HTTPS window post-opt-in: `Sec-CH-UA` header with minor version.")
</script>
</head>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment