From f6370c35ee4f448247485c6e875d3c60679d0454 Mon Sep 17 00:00:00 2001 From: Daniel Hosseinian Date: Thu, 29 Jul 2021 22:46:57 +0000 Subject: [PATCH] Revert "Add Sec-CH-UA-Reduced client hint header when Origin Trial token present" This reverts commit d9c057ea37355847f0c9530eada84aca335c96b4. Reason for revert: Browser tests is failing on linux-chromeos-chrome. See [1]. [1] https://ci.chromium.org/ui/p/chrome/builders/ci/linux-chromeos-chrome/16242/overview Original change's description: > Add Sec-CH-UA-Reduced client hint header when Origin Trial token present > > In this CL, we add logic to parse and accept the Sec-CH-UA-Reduced > client hint in the Accept-CH header and and only send the > Sec-CH-UA-Reduced client hint only in the presence of a valid > UserAgentReduction Origin Trial token. > > If the Origin Trial token is present and valid, and the origin > contains Sec-CH-UA-Reduced in the Accept-CH cache, then on all > subsequent requests to the origin, the Sec-CH-UA-Reduced request > header will be set with a value of "?1" (sh-boolean). If the > Accept-CH cache does not contain Sec-CH-UA-Reduced, or the Origin > Trial token is not valid, then Sec-CH-UA-Reduced will not be sent > in the request headers. > > NB: A subsequent CL will change the User-Agent request header to > the reduced UA string if the Sec-CH-UA-Reduced header is sent. > > Chrome Platform Status: https://chromestatus.com/feature/5704553745874944 > Design Doc: https://docs.google.com/document/d/1feIxK9S7oNgT2oGGebbxE9X0O-4wTKcsP_gRaY99tq4 > > Bug: 1222742 > Change-Id: If855d4bb393540d49de3be80aa9bc7c80f861c50 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3042810 > Commit-Queue: Ali Beyad > Reviewed-by: Aaron Tagliaboschi > Reviewed-by: Robert Kroeger > Reviewed-by: Mike West > Cr-Commit-Position: refs/heads/master@{#906810} Bug: 1222742 Change-Id: I0d8aab1d56d78f8323d4a3e3ed88b6eef370e1b9 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3061119 Auto-Submit: Daniel Hosseinian Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Owners-Override: Daniel Hosseinian Cr-Commit-Position: refs/heads/master@{#906884} --- .../client_hints/client_hints_browsertest.cc | 200 +----------------- ...ua_reduced_missing_valid_origin_trial.html | 5 - ..._valid_origin_trial.html.mock-http-headers | 3 - ..._ua_reduced_with_invalid_origin_trial.html | 7 - ...nvalid_origin_trial.html.mock-http-headers | 3 - ...pt_ch_ua_reduced_with_no_origin_trial.html | 8 - ...ith_no_origin_trial.html.mock-http-headers | 2 - ...ch_ua_reduced_with_valid_origin_trial.html | 8 - ..._valid_origin_trial.html.mock-http-headers | 3 - ..._ua_reduced_with_invalid_origin_trial.html | 7 - ...nvalid_origin_trial.html.mock-http-headers | 4 - ...ch_ua_reduced_with_valid_origin_trial.html | 8 - ..._valid_origin_trial.html.mock-http-headers | 4 - content/browser/client_hints/client_hints.cc | 187 ++++++++-------- content/browser/client_hints/client_hints.h | 13 +- .../critical_client_hints_throttle.cc | 6 +- .../renderer_host/navigation_request.cc | 6 +- content/test/BUILD.gn | 1 - .../client_hints/enabled_client_hints.cc | 34 +-- .../enabled_client_hints_unittest.cc | 96 +-------- .../client_hints/enabled_client_hints.h | 26 +-- .../core/loader/base_fetch_context.cc | 48 ++--- .../loader/fetch/client_hints_preferences.cc | 1 - 23 files changed, 124 insertions(+), 556 deletions(-) delete mode 100644 chrome/test/data/client_hints/accept_ch_ua_reduced_missing_valid_origin_trial.html delete mode 100644 chrome/test/data/client_hints/accept_ch_ua_reduced_missing_valid_origin_trial.html.mock-http-headers delete mode 100644 chrome/test/data/client_hints/accept_ch_ua_reduced_with_invalid_origin_trial.html delete mode 100644 chrome/test/data/client_hints/accept_ch_ua_reduced_with_invalid_origin_trial.html.mock-http-headers delete mode 100644 chrome/test/data/client_hints/accept_ch_ua_reduced_with_no_origin_trial.html delete mode 100644 chrome/test/data/client_hints/accept_ch_ua_reduced_with_no_origin_trial.html.mock-http-headers delete mode 100644 chrome/test/data/client_hints/accept_ch_ua_reduced_with_valid_origin_trial.html delete mode 100644 chrome/test/data/client_hints/accept_ch_ua_reduced_with_valid_origin_trial.html.mock-http-headers delete mode 100644 chrome/test/data/client_hints/critical_ch_ua_reduced_with_invalid_origin_trial.html delete mode 100644 chrome/test/data/client_hints/critical_ch_ua_reduced_with_invalid_origin_trial.html.mock-http-headers delete mode 100644 chrome/test/data/client_hints/critical_ch_ua_reduced_with_valid_origin_trial.html delete mode 100644 chrome/test/data/client_hints/critical_ch_ua_reduced_with_valid_origin_trial.html.mock-http-headers diff --git a/chrome/browser/client_hints/client_hints_browsertest.cc b/chrome/browser/client_hints/client_hints_browsertest.cc index 6b6b5192dd373..c44f7f87667f0 100644 --- a/chrome/browser/client_hints/client_hints_browsertest.cc +++ b/chrome/browser/client_hints/client_hints_browsertest.cc @@ -11,10 +11,8 @@ #include "base/containers/contains.h" #include "base/metrics/field_trial_param_associator.h" #include "base/run_loop.h" -#include "base/strings/strcat.h" #include "base/strings/string_util.h" #include "base/synchronization/lock.h" -#include "base/test/bind.h" #include "base/test/metrics/histogram_tester.h" #include "build/build_config.h" #include "chrome/browser/content_settings/cookie_settings_factory.h" @@ -30,7 +28,6 @@ #include "components/content_settings/core/browser/cookie_settings.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/content_settings/core/common/pref_names.h" -#include "components/embedder_support/switches.h" #include "components/embedder_support/user_agent_utils.h" #include "components/metrics/content/subprocess_metrics_provider.h" #include "components/page_load_metrics/browser/page_load_metrics_test_waiter.h" @@ -42,8 +39,6 @@ #include "content/public/common/content_switches.h" #include "content/public/test/browser_test.h" #include "content/public/test/browser_test_utils.h" -#include "content/public/test/content_browser_test.h" -#include "content/public/test/content_browser_test_utils.h" #include "content/public/test/test_utils.h" #include "content/public/test/url_loader_interceptor.h" #include "net/dns/mock_host_resolver.h" @@ -57,9 +52,7 @@ #include "services/network/public/cpp/features.h" #include "services/network/public/cpp/network_switches.h" #include "services/network/public/mojom/web_client_hints_types.mojom-shared.h" -#include "testing/gmock/include/gmock/gmock-matchers.h" #include "testing/gmock/include/gmock/gmock.h" -#include "third_party/abseil-cpp/absl/types/optional.h" #include "third_party/blink/public/common/client_hints/client_hints.h" #include "third_party/blink/public/common/web_preferences/web_preferences.h" @@ -699,10 +692,8 @@ class ClientHintsBrowserTest : public InProcessBrowserTest, continue; } - // Skip over the `Sec-CH-UA-Reduced` client hint because it is only added - // in the presence of a valid "UserAgentReduction" Origin Trial token. - // `Sec-CH-UA-Reduced` is tested via UaReducedOriginTrialBrowserTest - // below. + // TODO(crbug.com/1226193): Remove this block when support for + // `Sec-CH-UA-Reduced` has been added. if (std::string(blink::kClientHintsHeaderMapping[i]) == "sec-ch-ua-reduced") { continue; @@ -2484,190 +2475,3 @@ IN_PROC_BROWSER_TEST_F(ClientHintsBrowserTestWithEmulatedMedia, ui_test_utils::NavigateToURL(browser(), test_url()); EXPECT_EQ(prefers_color_scheme_observed(), "dark"); } - -// Tests that the Sec-CH-UA-Reduced client hint is sent if and only if the -// UserAgentReduction Origin Trial token is present and valid in the response -// headers. -// -// The test Origin Trial token found in the test files was generated by running -// (in tools/origin_trials): -// generate_token.py https://127.0.0.1:44444 UserAgentReduction -// --expire-timestamp=2000000000 -// -// The Origin Trial token expires in 2033. Generate a new token by then, or -// find a better way to re-generate a test trial token. -class UaReducedOriginTrialBrowserTest : public InProcessBrowserTest { - public: - UaReducedOriginTrialBrowserTest() - : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) { - https_server_.ServeFilesFromSourceDirectory( - "chrome/test/data/client_hints"); - https_server_.RegisterRequestMonitor(base::BindRepeating( - &UaReducedOriginTrialBrowserTest::MonitorResourceRequest, - base::Unretained(this))); - // Always start the server on the same port, because Origin Trial tokens are - // bound to an origin (schema/host/port). - EXPECT_TRUE(https_server_.Start(/*port=*/44444)); - } - - void SetUpCommandLine(base::CommandLine* command_line) override { - // The public key for the default privatey key used by the - // tools/origin_trials/generate_token.py tool. - static constexpr char kOriginTrialTestPublicKey[] = - "dRCs+TocuKkocNKa0AtZ4awrt9XKH2SQCI6o4FY6BNA="; - command_line->AppendSwitchASCII(embedder_support::kOriginTrialPublicKey, - kOriginTrialTestPublicKey); - } - - void SetUpOnMainThread() override { - host_resolver()->AddRule("*", "127.0.0.1"); - } - - static absl::optional GetHeaderValue( - const net::test_server::HttpRequest& request, - const std::string& header) { - if (request.headers.find(header) != request.headers.end()) { - return request.headers.find(header)->second; - } else { - return absl::nullopt; - } - } - - // Called by `https_server_`, as a callback when navigating to a URL and a - // request is about to be sent. - void MonitorResourceRequest(const net::test_server::HttpRequest& request) { - absl::optional ua_reduced_value = - GetHeaderValue(request, "sec-ch-ua-reduced"); - if (ua_reduced_value.has_value()) { - SetUaReducedClientHintValue(*ua_reduced_value); - } else { - ClearUaReducedClientHintValue(); - } - } - - absl::optional UaReducedClientHintValue() { - base::AutoLock lock(ua_reduced_client_hint_value_mutex_); - return ua_reduced_client_hint_value_; - } - - void SetUaReducedClientHintValue(const std::string& value) { - base::AutoLock lock(ua_reduced_client_hint_value_mutex_); - ua_reduced_client_hint_value_ = value; - } - - void ClearUaReducedClientHintValue() { - base::AutoLock lock(ua_reduced_client_hint_value_mutex_); - ua_reduced_client_hint_value_.reset(); - } - - GURL ua_reduced_with_valid_origin_trial_token_url() const { - return https_server_.GetURL( - "/accept_ch_ua_reduced_with_valid_origin_trial.html"); - } - - GURL ua_reduced_with_invalid_origin_trial_token_url() const { - return https_server_.GetURL( - "/accept_ch_ua_reduced_with_invalid_origin_trial.html"); - } - - GURL ua_reduced_with_no_origin_trial_token_url() const { - return https_server_.GetURL( - "/accept_ch_ua_reduced_with_no_origin_trial.html"); - } - - GURL ua_reduced_missing_with_valid_origin_trial_token_url() const { - return https_server_.GetURL( - "/accept_ch_ua_reduced_missing_valid_origin_trial.html"); - } - - GURL critical_ch_ua_reduced_with_valid_origin_trial_token_url() const { - return https_server_.GetURL( - "/critical_ch_ua_reduced_with_valid_origin_trial.html"); - } - - GURL critical_ch_ua_reduced_with_invalid_origin_trial_token_url() const { - return https_server_.GetURL( - "/critical_ch_ua_reduced_with_invalid_origin_trial.html"); - } - - void NavigateTwiceWithoutCriticalCH(const GURL& url) { - // The first navigation is in order to set the Accept-CH cache with the - // client hints in the Accept-CH response header. - ui_test_utils::NavigateToURL(browser(), url); - // In the first navigation, the client hints should not be present. - EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt)); - // The next navigation is to inspect the request headers sent in subsequent - // requests. - ui_test_utils::NavigateToURL(browser(), url); - } - - private: - net::EmbeddedTestServer https_server_; - base::Lock ua_reduced_client_hint_value_mutex_; - // Guarded by a mutex because it's written to by the IO sequence and read from - // the main test execution thread. - absl::optional ua_reduced_client_hint_value_ - GUARDED_BY(ua_reduced_client_hint_value_mutex_); -}; - -IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest, - AcceptChUaReducedWithValidOriginTrialToken) { - NavigateTwiceWithoutCriticalCH( - ua_reduced_with_valid_origin_trial_token_url()); - - EXPECT_THAT(UaReducedClientHintValue(), Optional(Eq("?1"))); -} - -IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest, - AcceptChUaReducedWithInvalidOriginTrialToken) { - NavigateTwiceWithoutCriticalCH( - ua_reduced_with_invalid_origin_trial_token_url()); - - // The response contained Sec-CH-UA-Reduced in the Accept-CH header, but the - // origin trial token is invalid. - EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt)); -} - -IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest, - AcceptChUaReducedWithNoOriginTrialToken) { - NavigateTwiceWithoutCriticalCH(ua_reduced_with_no_origin_trial_token_url()); - - // The response contained Sec-CH-UA-Reduced in the Accept-CH header, but the - // origin trial token is not present. - EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt)); -} - -IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest, - NoAcceptChUaReducedWithValidOriginTrialToken) { - NavigateTwiceWithoutCriticalCH( - ua_reduced_missing_with_valid_origin_trial_token_url()); - - // The response contained a valid Origin Trial token, but no Sec-CH-UA-Reduced - // in the Accept-CH header. - EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt)); -} - -IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest, - CriticalChUaReducedWithValidOriginTrialToken) { - ui_test_utils::NavigateToURL( - browser(), critical_ch_ua_reduced_with_valid_origin_trial_token_url()); - - // The initial navigation also contains the Critical-CH header, so the - // Sec-CH-UA-Reduced header should be set after the first navigation. - EXPECT_THAT(UaReducedClientHintValue(), Optional(Eq("?1"))); -} - -IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest, - CriticalChUaReducedWithInvalidOriginTrialToken) { - ui_test_utils::NavigateToURL( - browser(), critical_ch_ua_reduced_with_invalid_origin_trial_token_url()); - - // The Origin Trial token is invalid, so the Critical-CH should not have - // resulted in the Sec-CH-UA-Reduced header being sent. - EXPECT_THAT(UaReducedClientHintValue(), Eq(absl::nullopt)); -} - -IN_PROC_BROWSER_TEST_F(UaReducedOriginTrialBrowserTest, - ThirdPartyUaReducedWithValidOriginTrialToken) { - // TODO(crbug.com/1222742): Implement this test. -} diff --git a/chrome/test/data/client_hints/accept_ch_ua_reduced_missing_valid_origin_trial.html b/chrome/test/data/client_hints/accept_ch_ua_reduced_missing_valid_origin_trial.html deleted file mode 100644 index c340ceba5d9bb..0000000000000 --- a/chrome/test/data/client_hints/accept_ch_ua_reduced_missing_valid_origin_trial.html +++ /dev/null @@ -1,5 +0,0 @@ - - - -The Origin Trial token is valid, but the Sec-CH-UA-Reduced header is not present. - diff --git a/chrome/test/data/client_hints/accept_ch_ua_reduced_missing_valid_origin_trial.html.mock-http-headers b/chrome/test/data/client_hints/accept_ch_ua_reduced_missing_valid_origin_trial.html.mock-http-headers deleted file mode 100644 index f59cf0c2dfef4..0000000000000 --- a/chrome/test/data/client_hints/accept_ch_ua_reduced_missing_valid_origin_trial.html.mock-http-headers +++ /dev/null @@ -1,3 +0,0 @@ -HTTP/1.1 200 OK -Accept-CH: sec-ch-ua-full-version -Origin-Trial: A93QtcQ0CRKf5ioPasUwNbweXQWgbI4ZEshiz+YS7dkQEWVfW9Ua2pTnA866sZwRzuElkPwsUdGdIaW0fRUP8AwAAABceyJvcmlnaW4iOiAiaHR0cHM6Ly8xMjcuMC4wLjE6NDQ0NDQiLCAiZmVhdHVyZSI6ICJVc2VyQWdlbnRSZWR1Y3Rpb24iLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0= diff --git a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_invalid_origin_trial.html b/chrome/test/data/client_hints/accept_ch_ua_reduced_with_invalid_origin_trial.html deleted file mode 100644 index d291bbe251fa6..0000000000000 --- a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_invalid_origin_trial.html +++ /dev/null @@ -1,7 +0,0 @@ - - - -Empty file which uses link-rel to disable favicon fetches. The corresponding -.mock-http-headers sets client hints. The Origin Trial token in the headers -file is a corruption of the valid Origin Trial token. - diff --git a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_invalid_origin_trial.html.mock-http-headers b/chrome/test/data/client_hints/accept_ch_ua_reduced_with_invalid_origin_trial.html.mock-http-headers deleted file mode 100644 index b2e0d453079d6..0000000000000 --- a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_invalid_origin_trial.html.mock-http-headers +++ /dev/null @@ -1,3 +0,0 @@ -HTTP/1.1 200 OK -Accept-CH: sec-ch-ua-full-version,sec-ch-ua-reduced -Origin-Trial: A23QtcQ0CRKf5ioPasUwNbweXQWgbI4ZEshiz+YS7dkQEWVfW9Ua2pTnA866sZwRzuElkPwsUdGdIaW0fRUP8AwAAABceyJvcmlnbW4iOiAiaHR0cHM6Ly8xMjcuMC4wLjE6NDQ0NDQiLCAiZmVhdHVyZSI6ICJVc2VyQWdlbnRSZWR1Y3Rpb24iLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0= diff --git a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_no_origin_trial.html b/chrome/test/data/client_hints/accept_ch_ua_reduced_with_no_origin_trial.html deleted file mode 100644 index 8988d58fe5b34..0000000000000 --- a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_no_origin_trial.html +++ /dev/null @@ -1,8 +0,0 @@ - - - -Empty file which uses link-rel to disable favicon fetches. The corresponding -.mock-http-headers sets client hints. The Origin Trial token in the headers -file is missing, even though the Sec-CH-UA-Reduced client hint is present in -the Accept-CH header. - diff --git a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_no_origin_trial.html.mock-http-headers b/chrome/test/data/client_hints/accept_ch_ua_reduced_with_no_origin_trial.html.mock-http-headers deleted file mode 100644 index 2d964e57241db..0000000000000 --- a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_no_origin_trial.html.mock-http-headers +++ /dev/null @@ -1,2 +0,0 @@ -HTTP/1.1 200 OK -Accept-CH: sec-ch-ua-full-version,sec-ch-ua-reduced diff --git a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_valid_origin_trial.html b/chrome/test/data/client_hints/accept_ch_ua_reduced_with_valid_origin_trial.html deleted file mode 100644 index db1b98472c0d4..0000000000000 --- a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_valid_origin_trial.html +++ /dev/null @@ -1,8 +0,0 @@ - - - -Empty file which uses link-rel to disable favicon fetches. The corresponding -.mock-http-headers sets client hints. The Origin Trial token was generated by -running (in tools/origin_trials): -generate_token.py https://127.0.0.1:44444 UserAgentReduction --expire-timestamp=2000000000 - diff --git a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_valid_origin_trial.html.mock-http-headers b/chrome/test/data/client_hints/accept_ch_ua_reduced_with_valid_origin_trial.html.mock-http-headers deleted file mode 100644 index eb492a2227a0d..0000000000000 --- a/chrome/test/data/client_hints/accept_ch_ua_reduced_with_valid_origin_trial.html.mock-http-headers +++ /dev/null @@ -1,3 +0,0 @@ -HTTP/1.1 200 OK -Accept-CH: sec-ch-ua-full-version,sec-ch-ua-reduced -Origin-Trial: A93QtcQ0CRKf5ioPasUwNbweXQWgbI4ZEshiz+YS7dkQEWVfW9Ua2pTnA866sZwRzuElkPwsUdGdIaW0fRUP8AwAAABceyJvcmlnaW4iOiAiaHR0cHM6Ly8xMjcuMC4wLjE6NDQ0NDQiLCAiZmVhdHVyZSI6ICJVc2VyQWdlbnRSZWR1Y3Rpb24iLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0= diff --git a/chrome/test/data/client_hints/critical_ch_ua_reduced_with_invalid_origin_trial.html b/chrome/test/data/client_hints/critical_ch_ua_reduced_with_invalid_origin_trial.html deleted file mode 100644 index 9666ba7bc4fe0..0000000000000 --- a/chrome/test/data/client_hints/critical_ch_ua_reduced_with_invalid_origin_trial.html +++ /dev/null @@ -1,7 +0,0 @@ - - - -Empty file which uses link-rel to disable favicon fetches. The corresponding -.mock-http-headers sets client hints. The Origin Trial token is a corrupted -version of the original. - diff --git a/chrome/test/data/client_hints/critical_ch_ua_reduced_with_invalid_origin_trial.html.mock-http-headers b/chrome/test/data/client_hints/critical_ch_ua_reduced_with_invalid_origin_trial.html.mock-http-headers deleted file mode 100644 index 4798643e6d20e..0000000000000 --- a/chrome/test/data/client_hints/critical_ch_ua_reduced_with_invalid_origin_trial.html.mock-http-headers +++ /dev/null @@ -1,4 +0,0 @@ -HTTP/1.1 200 OK -Accept-CH: sec-ch-ua-full-version,sec-ch-ua-reduced -Critical-CH: sec-ch-ua-reduced -Origin-Trial: A23QtcQ0CRKf5ioPasUwNbweXQWgbI4ZEshiz+YS7dkQEWVfW9Ua2pTnA866sZwRzsElkPwsUdGdIaW0fRUP8AwAAABceyJvcmlnaW4iOiAiaHR0cHM6Ly8xMjcuMC4wLjE6NDQ0NDQiLCAiZmVhdHVyZSI6ICJVc2VyQWdlbnRSZWR1Y3Rpb24iLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0= diff --git a/chrome/test/data/client_hints/critical_ch_ua_reduced_with_valid_origin_trial.html b/chrome/test/data/client_hints/critical_ch_ua_reduced_with_valid_origin_trial.html deleted file mode 100644 index db1b98472c0d4..0000000000000 --- a/chrome/test/data/client_hints/critical_ch_ua_reduced_with_valid_origin_trial.html +++ /dev/null @@ -1,8 +0,0 @@ - - - -Empty file which uses link-rel to disable favicon fetches. The corresponding -.mock-http-headers sets client hints. The Origin Trial token was generated by -running (in tools/origin_trials): -generate_token.py https://127.0.0.1:44444 UserAgentReduction --expire-timestamp=2000000000 - diff --git a/chrome/test/data/client_hints/critical_ch_ua_reduced_with_valid_origin_trial.html.mock-http-headers b/chrome/test/data/client_hints/critical_ch_ua_reduced_with_valid_origin_trial.html.mock-http-headers deleted file mode 100644 index f034407022313..0000000000000 --- a/chrome/test/data/client_hints/critical_ch_ua_reduced_with_valid_origin_trial.html.mock-http-headers +++ /dev/null @@ -1,4 +0,0 @@ -HTTP/1.1 200 OK -Accept-CH: sec-ch-ua-full-version,sec-ch-ua-reduced -Critical-CH: sec-ch-ua-reduced -Origin-Trial: A93QtcQ0CRKf5ioPasUwNbweXQWgbI4ZEshiz+YS7dkQEWVfW9Ua2pTnA866sZwRzuElkPwsUdGdIaW0fRUP8AwAAABceyJvcmlnaW4iOiAiaHR0cHM6Ly8xMjcuMC4wLjE6NDQ0NDQiLCAiZmVhdHVyZSI6ICJVc2VyQWdlbnRSZWR1Y3Rpb24iLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0= diff --git a/content/browser/client_hints/client_hints.cc b/content/browser/client_hints/client_hints.cc index 876c0263bfaf3..4ed9891e00d67 100644 --- a/content/browser/client_hints/client_hints.cc +++ b/content/browser/client_hints/client_hints.cc @@ -18,7 +18,6 @@ #include "content/browser/devtools/devtools_instrumentation.h" #include "content/browser/renderer_host/frame_tree.h" #include "content/browser/renderer_host/frame_tree_node.h" -#include "content/browser/renderer_host/navigation_request.h" #include "content/browser/renderer_host/navigator.h" #include "content/browser/renderer_host/navigator_delegate.h" #include "content/browser/renderer_host/render_view_host_impl.h" @@ -29,8 +28,6 @@ #include "content/public/common/content_features.h" #include "content/public/common/content_switches.h" #include "net/base/url_util.h" -#include "net/http/http_request_headers.h" -#include "net/http/http_response_headers.h" #include "net/http/structured_headers.h" #include "net/nqe/effective_connection_type.h" #include "net/nqe/network_quality_estimator_params.h" @@ -50,8 +47,6 @@ namespace content { namespace { -using ::network::mojom::WebClientHintsType; - uint8_t randomization_salt = 0; constexpr size_t kMaxRandomNumbers = 21; @@ -202,7 +197,7 @@ GetWebHoldbackEffectiveConnectionType() { } void SetHeaderToDouble(net::HttpRequestHeaders* headers, - WebClientHintsType client_hint_type, + network::mojom::WebClientHintsType client_hint_type, double value) { headers->SetHeader( blink::kClientHintsHeaderMapping[static_cast(client_hint_type)], @@ -210,7 +205,7 @@ void SetHeaderToDouble(net::HttpRequestHeaders* headers, } void SetHeaderToInt(net::HttpRequestHeaders* headers, - WebClientHintsType client_hint_type, + network::mojom::WebClientHintsType client_hint_type, double value) { headers->SetHeader( blink::kClientHintsHeaderMapping[static_cast(client_hint_type)], @@ -218,14 +213,14 @@ void SetHeaderToInt(net::HttpRequestHeaders* headers, } void SetHeaderToString(net::HttpRequestHeaders* headers, - WebClientHintsType client_hint_type, + network::mojom::WebClientHintsType client_hint_type, const std::string& value) { headers->SetHeader( blink::kClientHintsHeaderMapping[static_cast(client_hint_type)], value); } -void RemoveClientHintHeader(WebClientHintsType client_hint_type, +void RemoveClientHintHeader(network::mojom::WebClientHintsType client_hint_type, net::HttpRequestHeaders* headers) { headers->RemoveHeader( blink::kClientHintsHeaderMapping[static_cast(client_hint_type)]); @@ -237,7 +232,8 @@ void AddDeviceMemoryHeader(net::HttpRequestHeaders* headers) { const float device_memory = blink::ApproximatedDeviceMemory::GetApproximatedDeviceMemory(); DCHECK_LT(0.0, device_memory); - SetHeaderToDouble(headers, WebClientHintsType::kDeviceMemory, device_memory); + SetHeaderToDouble(headers, network::mojom::WebClientHintsType::kDeviceMemory, + device_memory); } void AddDPRHeader(net::HttpRequestHeaders* headers, @@ -247,7 +243,7 @@ void AddDPRHeader(net::HttpRequestHeaders* headers, DCHECK(context); double device_scale_factor = GetDeviceScaleFactor(); double zoom_factor = GetZoomFactor(context, url); - SetHeaderToDouble(headers, WebClientHintsType::kDpr, + SetHeaderToDouble(headers, network::mojom::WebClientHintsType::kDpr, device_scale_factor * zoom_factor); } @@ -271,7 +267,8 @@ void AddViewportWidthHeader(net::HttpRequestHeaders* headers, DCHECK_LT(0, viewport_width); // TODO(yoav): Find out why this 0 check is needed... if (viewport_width > 0) { - SetHeaderToInt(headers, WebClientHintsType::kViewportWidth, viewport_width); + SetHeaderToInt(headers, network::mojom::WebClientHintsType::kViewportWidth, + viewport_width); } } @@ -293,7 +290,7 @@ void AddRttHeader(net::HttpRequestHeaders* headers, http_rtt = net::NetworkQualityEstimatorParams::GetDefaultTypicalHttpRtt( net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN); } - SetHeaderToInt(headers, WebClientHintsType::kRtt, + SetHeaderToInt(headers, network::mojom::WebClientHintsType::kRtt, RoundRtt(url.host(), http_rtt)); } @@ -319,7 +316,7 @@ void AddDownlinkHeader(net::HttpRequestHeaders* headers, net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN); } - SetHeaderToDouble(headers, WebClientHintsType::kDownlink, + SetHeaderToDouble(headers, network::mojom::WebClientHintsType::kDownlink, RoundKbpsToMbps(url.host(), downlink_throughput_kbps)); } @@ -347,13 +344,13 @@ void AddEctHeader(net::HttpRequestHeaders* headers, } SetHeaderToString( - headers, WebClientHintsType::kEct, + headers, network::mojom::WebClientHintsType::kEct, blink::kWebEffectiveConnectionTypeMapping[effective_connection_type]); } void AddLangHeader(net::HttpRequestHeaders* headers, BrowserContext* context) { SetHeaderToString( - headers, WebClientHintsType::kLang, + headers, network::mojom::WebClientHintsType::kLang, blink::SerializeLangClientHint( GetContentClient()->browser()->GetAcceptLangs(context))); } @@ -366,7 +363,8 @@ void AddPrefersColorSchemeHeader(net::HttpRequestHeaders* headers, frame_tree_node->current_frame_host()->GetPreferredColorScheme(); bool is_dark_mode = preferred_color_scheme == blink::mojom::PreferredColorScheme::kDark; - SetHeaderToString(headers, WebClientHintsType::kPrefersColorScheme, + SetHeaderToString(headers, + network::mojom::WebClientHintsType::kPrefersColorScheme, is_dark_mode ? "dark" : "light"); } @@ -385,18 +383,15 @@ bool UserAgentClientHintEnabled() { } void AddUAHeader(net::HttpRequestHeaders* headers, - WebClientHintsType type, + network::mojom::WebClientHintsType type, const std::string& value) { SetHeaderToString(headers, type, value); } -// Creates a serialized string header value out of the input type, using -// structured headers as described in -// https://www.rfc-editor.org/rfc/rfc8941.html. -template -const std::string SerializeHeaderString(const T& value) { +// Use structured headers to escape and quote headers +std::string SerializeHeaderString(std::string str) { return net::structured_headers::SerializeItem( - net::structured_headers::Item(value)) + net::structured_headers::Item(str)) .value_or(std::string()); } @@ -441,7 +436,7 @@ struct ClientHintsExtendedData { }; bool IsClientHintAllowed(const ClientHintsExtendedData& data, - WebClientHintsType type) { + network::mojom::WebClientHintsType type) { if (!IsPermissionsPolicyForClientHintsEnabled() || data.is_main_frame) return data.is_1p_origin; return data.permissions_policy && @@ -452,7 +447,7 @@ bool IsClientHintAllowed(const ClientHintsExtendedData& data, } bool ShouldAddClientHint(const ClientHintsExtendedData& data, - WebClientHintsType type) { + network::mojom::WebClientHintsType type) { if (!blink::IsClientHintSentByDefault(type) && !data.hints.IsEnabled(type)) return false; return IsClientHintAllowed(data, type); @@ -514,60 +509,70 @@ void UpdateNavigationRequestClientUaHeadersImpl( // Permissions Policy. // // https://wicg.github.io/client-hints-infrastructure/#abstract-opdef-append-client-hints-to-request - if (ShouldAddClientHint(data, WebClientHintsType::kUA)) { - AddUAHeader(headers, WebClientHintsType::kUA, + if (ShouldAddClientHint(data, network::mojom::WebClientHintsType::kUA)) { + AddUAHeader(headers, network::mojom::WebClientHintsType::kUA, ua_metadata->SerializeBrandVersionList()); } // The `Sec-CH-UA-Mobile client hint was also deemed "low entropy" and can // safely be sent with every request. Similarly to UA, ShouldAddClientHints // makes sure it's controlled by Permissions Policy. - if (ShouldAddClientHint(data, WebClientHintsType::kUAMobile)) { - AddUAHeader(headers, WebClientHintsType::kUAMobile, - SerializeHeaderString(ua_metadata->mobile)); + if (ShouldAddClientHint(data, + network::mojom::WebClientHintsType::kUAMobile)) { + AddUAHeader(headers, network::mojom::WebClientHintsType::kUAMobile, + ua_metadata->mobile ? "?1" : "?0"); } - if (ShouldAddClientHint(data, WebClientHintsType::kUAFullVersion)) { - AddUAHeader(headers, WebClientHintsType::kUAFullVersion, + if (ShouldAddClientHint( + data, network::mojom::WebClientHintsType::kUAFullVersion)) { + AddUAHeader(headers, network::mojom::WebClientHintsType::kUAFullVersion, SerializeHeaderString(ua_metadata->full_version)); } - if (ShouldAddClientHint(data, WebClientHintsType::kUAArch)) { - AddUAHeader(headers, WebClientHintsType::kUAArch, + if (ShouldAddClientHint(data, + network::mojom::WebClientHintsType::kUAArch)) { + AddUAHeader(headers, network::mojom::WebClientHintsType::kUAArch, SerializeHeaderString(ua_metadata->architecture)); } - if (ShouldAddClientHint(data, WebClientHintsType::kUAPlatform)) { - AddUAHeader(headers, WebClientHintsType::kUAPlatform, + if (ShouldAddClientHint(data, + network::mojom::WebClientHintsType::kUAPlatform)) { + AddUAHeader(headers, network::mojom::WebClientHintsType::kUAPlatform, SerializeHeaderString(ua_metadata->platform)); } - if (ShouldAddClientHint(data, WebClientHintsType::kUAPlatformVersion)) { - AddUAHeader(headers, WebClientHintsType::kUAPlatformVersion, + if (ShouldAddClientHint( + data, network::mojom::WebClientHintsType::kUAPlatformVersion)) { + AddUAHeader(headers, + network::mojom::WebClientHintsType::kUAPlatformVersion, SerializeHeaderString(ua_metadata->platform_version)); } - if (ShouldAddClientHint(data, WebClientHintsType::kUAModel)) { - AddUAHeader(headers, WebClientHintsType::kUAModel, + if (ShouldAddClientHint(data, + network::mojom::WebClientHintsType::kUAModel)) { + AddUAHeader(headers, network::mojom::WebClientHintsType::kUAModel, SerializeHeaderString(ua_metadata->model)); } - if (ShouldAddClientHint(data, WebClientHintsType::kUABitness)) { - AddUAHeader(headers, WebClientHintsType::kUABitness, + if (ShouldAddClientHint(data, + network::mojom::WebClientHintsType::kUABitness)) { + AddUAHeader(headers, network::mojom::WebClientHintsType::kUABitness, SerializeHeaderString(ua_metadata->bitness)); } - if (ShouldAddClientHint(data, WebClientHintsType::kUAReduced)) { - AddUAHeader(headers, WebClientHintsType::kUAReduced, - SerializeHeaderString(true)); - } } else if (call_type == ClientUaHeaderCallType::kAfterCreated) { - RemoveClientHintHeader(WebClientHintsType::kUA, headers); - RemoveClientHintHeader(WebClientHintsType::kUAMobile, headers); - RemoveClientHintHeader(WebClientHintsType::kUAFullVersion, headers); - RemoveClientHintHeader(WebClientHintsType::kUAArch, headers); - RemoveClientHintHeader(WebClientHintsType::kUAPlatform, headers); - RemoveClientHintHeader(WebClientHintsType::kUAPlatformVersion, headers); - RemoveClientHintHeader(WebClientHintsType::kUAModel, headers); - RemoveClientHintHeader(WebClientHintsType::kUABitness, headers); - RemoveClientHintHeader(WebClientHintsType::kUAReduced, headers); + RemoveClientHintHeader(network::mojom::WebClientHintsType::kUA, headers); + RemoveClientHintHeader(network::mojom::WebClientHintsType::kUAMobile, + headers); + RemoveClientHintHeader(network::mojom::WebClientHintsType::kUAFullVersion, + headers); + RemoveClientHintHeader(network::mojom::WebClientHintsType::kUAArch, + headers); + RemoveClientHintHeader(network::mojom::WebClientHintsType::kUAPlatform, + headers); + RemoveClientHintHeader( + network::mojom::WebClientHintsType::kUAPlatformVersion, headers); + RemoveClientHintHeader(network::mojom::WebClientHintsType::kUAModel, + headers); + RemoveClientHintHeader(network::mojom::WebClientHintsType::kUABitness, + headers); } } @@ -633,27 +638,30 @@ void AddRequestClientHintsHeaders( } // Add Headers - if (ShouldAddClientHint(data, WebClientHintsType::kDeviceMemory)) { + if (ShouldAddClientHint(data, + network::mojom::WebClientHintsType::kDeviceMemory)) { AddDeviceMemoryHeader(headers); } - if (ShouldAddClientHint(data, WebClientHintsType::kDpr)) { + if (ShouldAddClientHint(data, network::mojom::WebClientHintsType::kDpr)) { AddDPRHeader(headers, context, url); } - if (ShouldAddClientHint(data, WebClientHintsType::kViewportWidth)) { + if (ShouldAddClientHint(data, + network::mojom::WebClientHintsType::kViewportWidth)) { AddViewportWidthHeader(headers, context, url); } network::NetworkQualityTracker* network_quality_tracker = delegate->GetNetworkQualityTracker(); - if (ShouldAddClientHint(data, WebClientHintsType::kRtt)) { + if (ShouldAddClientHint(data, network::mojom::WebClientHintsType::kRtt)) { AddRttHeader(headers, network_quality_tracker, url); } - if (ShouldAddClientHint(data, WebClientHintsType::kDownlink)) { + if (ShouldAddClientHint(data, + network::mojom::WebClientHintsType::kDownlink)) { AddDownlinkHeader(headers, network_quality_tracker, url); } - if (ShouldAddClientHint(data, WebClientHintsType::kEct)) { + if (ShouldAddClientHint(data, network::mojom::WebClientHintsType::kEct)) { AddEctHeader(headers, network_quality_tracker, url); } - if (ShouldAddClientHint(data, WebClientHintsType::kLang)) { + if (ShouldAddClientHint(data, network::mojom::WebClientHintsType::kLang)) { AddLangHeader(headers, context); } @@ -663,7 +671,8 @@ void AddRequestClientHintsHeaders( ClientUaHeaderCallType::kDuringCreation, headers); } - if (ShouldAddClientHint(data, WebClientHintsType::kPrefersColorScheme)) { + if (ShouldAddClientHint( + data, network::mojom::WebClientHintsType::kPrefersColorScheme)) { AddPrefersColorSchemeHeader(headers, frame_tree_node); } @@ -672,7 +681,8 @@ void AddRequestClientHintsHeaders( // If possible, logic should be added above so that the request headers for // the newly added client hint can be added to the request. static_assert( - WebClientHintsType::kUAReduced == WebClientHintsType::kMaxValue, + network::mojom::WebClientHintsType::kUAReduced == + network::mojom::WebClientHintsType::kMaxValue, "Consider adding client hint request headers from the browser process"); // TODO(crbug.com/735518): If the request is redirected, the client hint @@ -733,19 +743,18 @@ void AddNavigationRequestClientHintsHeaders( container_policy); } -absl::optional> +absl::optional> ParseAndPersistAcceptCHForNavigation( const GURL& url, - const network::mojom::ParsedHeadersPtr& parsed_headers, - const net::HttpResponseHeaders* response_headers, + const ::network::mojom::ParsedHeadersPtr& headers, BrowserContext* context, ClientHintsControllerDelegate* delegate, FrameTreeNode* frame_tree_node) { DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK(context); - DCHECK(parsed_headers); + DCHECK(headers); - if (!parsed_headers->accept_ch) + if (!headers->accept_ch) return absl::nullopt; if (!IsValidURLForClientHints(url)) @@ -765,13 +774,6 @@ ParseAndPersistAcceptCHForNavigation( if (!frame_tree_node->IsMainFrame()) return absl::nullopt; - blink::EnabledClientHints enabled_hints; - for (const WebClientHintsType type : parsed_headers->accept_ch.value()) { - enabled_hints.SetIsEnabled(url, response_headers, type, true); - } - const std::vector persisted_hints = - enabled_hints.GetEnabledHints(); - base::TimeDelta persist_duration; if (IsPermissionsPolicyForClientHintsEnabled()) { // JSON cannot store "non-finite" values (i.e. NaN or infinite) so @@ -780,35 +782,42 @@ ParseAndPersistAcceptCHForNavigation( // large was chosen instead persist_duration = base::TimeDelta::FromDays(1000000); } else { - persist_duration = parsed_headers->accept_ch_lifetime; + persist_duration = headers->accept_ch_lifetime; if (persist_duration.is_zero()) - return persisted_hints; + return headers->accept_ch; } - delegate->PersistClientHints(url::Origin::Create(url), persisted_hints, - persist_duration); + delegate->PersistClientHints(url::Origin::Create(url), + headers->accept_ch.value(), persist_duration); - return persisted_hints; + return headers->accept_ch; } -CONTENT_EXPORT std::vector LookupAcceptCHForCommit( - const GURL& url, - ClientHintsControllerDelegate* delegate, - FrameTreeNode* frame_tree_node) { - std::vector result; +CONTENT_EXPORT std::vector<::network::mojom::WebClientHintsType> +LookupAcceptCHForCommit(const GURL& url, + ClientHintsControllerDelegate* delegate, + FrameTreeNode* frame_tree_node) { + std::vector<::network::mojom::WebClientHintsType> result; if (!ShouldAddClientHints(url, frame_tree_node, delegate)) { return result; } const ClientHintsExtendedData data(url, frame_tree_node, delegate); - return data.hints.GetEnabledHints(); + for (int v = 0; + v <= static_cast(network::mojom::WebClientHintsType::kMaxValue); + ++v) { + auto hint = static_cast(v); + if (data.hints.IsEnabled(hint)) + result.push_back(hint); + } + return result; } bool AreCriticalHintsMissing( const GURL& url, FrameTreeNode* frame_tree_node, ClientHintsControllerDelegate* delegate, - const std::vector& critical_hints) { + const std::vector& critical_hints) { ClientHintsExtendedData data(url, frame_tree_node, delegate); // Note: these only check for per-hint origin/permissions policy settings, not diff --git a/content/browser/client_hints/client_hints.h b/content/browser/client_hints/client_hints.h index 3c6d738958a10..9c0bccd11aa96 100644 --- a/content/browser/client_hints/client_hints.h +++ b/content/browser/client_hints/client_hints.h @@ -13,10 +13,6 @@ #include "services/network/public/mojom/parsed_headers.mojom-forward.h" #include "third_party/blink/public/common/permissions_policy/permissions_policy.h" -namespace net { -class HttpResponseHeaders; -} // namespace net - namespace content { class BrowserContext; @@ -86,17 +82,10 @@ CONTENT_EXPORT void AddPrefetchNavigationRequestClientHintsHeaders( // persisted. The distinction is relevant in legacy case where permissions // policy is off and there is no valid Accept-CH-Lifetime, where the header // still applies locally within frame. -// -// The ParsedHeaders are used to retrieve the already parsed Accept-CH header -// values. The HttpResponseHeaders are not meant to be used by non-sandboxed -// processes, but here, we just pass the HttpRequestHeaders to the -// TrialTokenValidator library. There is precedent for calling the -// TrialTokenValidator from the browser process, see crrev.com/c/2142580. CONTENT_EXPORT absl::optional> ParseAndPersistAcceptCHForNavigation( const GURL& url, - const network::mojom::ParsedHeadersPtr& parsed_headers, - const net::HttpResponseHeaders* response_headers, + const ::network::mojom::ParsedHeadersPtr& headers, BrowserContext* context, ClientHintsControllerDelegate* delegate, FrameTreeNode*); diff --git a/content/browser/client_hints/critical_client_hints_throttle.cc b/content/browser/client_hints/critical_client_hints_throttle.cc index a173549158716..cbf620c1e7cdb 100644 --- a/content/browser/client_hints/critical_client_hints_throttle.cc +++ b/content/browser/client_hints/critical_client_hints_throttle.cc @@ -66,7 +66,7 @@ void CriticalClientHintsThrottle::BeforeWillProcessResponse( blink::EnabledClientHints hints; for (const WebClientHintsType hint : response_head.parsed_headers->accept_ch.value()) - hints.SetIsEnabled(response_url, response_head.headers.get(), hint, true); + hints.SetIsEnabled(hint, true); std::vector critical_hints; for (const WebClientHintsType hint : @@ -85,8 +85,8 @@ void CriticalClientHintsThrottle::BeforeWillProcessResponse( if (ShouldRestartWithHints(response_url, critical_hints, modified_headers)) { LogCriticalCHStatus(CriticalCHRestart::kNavigationRestarted); ParseAndPersistAcceptCHForNavigation( - response_url, response_head.parsed_headers, response_head.headers.get(), - context_, client_hint_delegate_, + response_url, response_head.parsed_headers, context_, + client_hint_delegate_, FrameTreeNode::GloballyFindByID(frame_tree_node_id_)); delegate_->RestartWithURLResetAndFlags(/*additional_load_flags=*/0); } diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc index 7d77b8a997172..1905447cc1959 100644 --- a/content/browser/renderer_host/navigation_request.cc +++ b/content/browser/renderer_host/navigation_request.cc @@ -3782,7 +3782,6 @@ void NavigationRequest::OnRedirectChecksComplete( ParseAndPersistAcceptCHForNavigation( commit_params_->redirects.back(), commit_params_->redirect_response.back()->parsed_headers, - commit_params_->redirect_response.back()->headers.get(), browser_context, client_hints_delegate, frame_tree_node_); AddNavigationRequestClientHintsHeaders( common_params_->url, &client_hints_extra_headers, browser_context, @@ -4174,9 +4173,8 @@ void NavigationRequest::CommitNavigation() { opt_in_hints_from_response; if (response()) { opt_in_hints_from_response = ParseAndPersistAcceptCHForNavigation( - common_params_->url, response()->parsed_headers, - response()->headers.get(), browser_context, client_hints_delegate, - frame_tree_node_); + common_params_->url, response()->parsed_headers, browser_context, + client_hints_delegate, frame_tree_node_); } commit_params_->enabled_client_hints = LookupAcceptCHForCommit( common_params_->url, client_hints_delegate, frame_tree_node_); diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn index 811d8db3f8dd5..338da387f5e4a 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn @@ -2471,7 +2471,6 @@ test("content_unittests") { "//third_party/blink/public:blink", "//third_party/blink/public:test_support", "//third_party/blink/public/common:font_enumeration_table_proto", - "//third_party/blink/public/common:headers", "//third_party/icu", "//third_party/inspector_protocol:crdtp_test", "//third_party/leveldatabase", diff --git a/third_party/blink/common/client_hints/enabled_client_hints.cc b/third_party/blink/common/client_hints/enabled_client_hints.cc index 390470ccd0f8e..59226fd49c308 100644 --- a/third_party/blink/common/client_hints/enabled_client_hints.cc +++ b/third_party/blink/common/client_hints/enabled_client_hints.cc @@ -5,11 +5,7 @@ #include "third_party/blink/public/common/client_hints/enabled_client_hints.h" #include "base/feature_list.h" -#include "base/time/time.h" -#include "net/http/http_response_headers.h" #include "third_party/blink/public/common/features.h" -#include "third_party/blink/public/common/origin_trials/trial_token_validator.h" -#include "url/gurl.h" namespace blink { @@ -48,37 +44,15 @@ bool IsDisabledByFeature(const WebClientHintsType type) { } // namespace bool EnabledClientHints::IsEnabled(const WebClientHintsType type) const { + if (IsDisabledByFeature(type)) { + return false; + } return enabled_types_[static_cast(type)]; } void EnabledClientHints::SetIsEnabled(const WebClientHintsType type, const bool should_send) { - enabled_types_[static_cast(type)] = - IsDisabledByFeature(type) ? false : should_send; -} - -void EnabledClientHints::SetIsEnabled( - const GURL& url, - const net::HttpResponseHeaders* response_headers, - const network::mojom::WebClientHintsType type, - const bool should_send) { - bool enabled = should_send; - if (type == WebClientHintsType::kUAReduced) { - enabled &= blink::TrialTokenValidator().RequestEnablesFeature( - url, response_headers, "UserAgentReduction", base::Time::Now()); - } - SetIsEnabled(type, enabled); -} - -std::vector EnabledClientHints::GetEnabledHints() const { - std::vector hints; - for (int v = 0; v <= static_cast(WebClientHintsType::kMaxValue); ++v) { - const auto hint = static_cast(v); - if (IsEnabled(hint)) { - hints.push_back(hint); - } - } - return hints; + enabled_types_[static_cast(type)] = should_send; } } // namespace blink diff --git a/third_party/blink/common/client_hints/enabled_client_hints_unittest.cc b/third_party/blink/common/client_hints/enabled_client_hints_unittest.cc index b28a503655b45..77be96a67ac59 100644 --- a/third_party/blink/common/client_hints/enabled_client_hints_unittest.cc +++ b/third_party/blink/common/client_hints/enabled_client_hints_unittest.cc @@ -4,93 +4,26 @@ #include "third_party/blink/public/common/client_hints/enabled_client_hints.h" -#include "base/memory/scoped_refptr.h" #include "base/test/scoped_feature_list.h" -#include "net/http/http_response_headers.h" -#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/blink/public/common/features.h" -#include "third_party/blink/public/common/origin_trials/origin_trial_policy.h" -#include "third_party/blink/public/common/origin_trials/origin_trial_public_key.h" -#include "third_party/blink/public/common/origin_trials/trial_token_validator.h" namespace blink { using ::network::mojom::WebClientHintsType; -using ::testing::ElementsAre; - -static constexpr char kOriginUrl[] = "https://127.0.0.1:44444"; -static const OriginTrialPublicKey kTestPublicKey = { - 0x75, 0x10, 0xac, 0xf9, 0x3a, 0x1c, 0xb8, 0xa9, 0x28, 0x70, 0xd2, - 0x9a, 0xd0, 0x0b, 0x59, 0xe1, 0xac, 0x2b, 0xb7, 0xd5, 0xca, 0x1f, - 0x64, 0x90, 0x08, 0x8e, 0xa8, 0xe0, 0x56, 0x3a, 0x04, 0xd0, -}; -// Generated by running (in tools/origin_trials): -// generate_token.py https://127.0.0.1:44444 UserAgentReduction -// --expire-timestamp=2000000000 -// -// The Origin Trial token expires in 2033. Generate a new token by then, or -// find a better way to re-generate a test trial token. -static constexpr char kValidOriginTrialToken[] = - "A93QtcQ0CRKf5ioPasUwNbweXQWgbI4ZEshiz+" - "YS7dkQEWVfW9Ua2pTnA866sZwRzuElkPwsUdGdIaW0fRUP8AwAAABceyJvcmlnaW4iOiAiaHR0" - "cHM6Ly8xMjcuMC4wLjE6NDQ0NDQiLCAiZmVhdHVyZSI6ICJVc2VyQWdlbnRSZWR1Y3Rpb24iLC" - "AiZXhwaXJ5IjogMjAwMDAwMDAwMH0="; -// A slight corruption (changing a character) of kValidOriginTrialToken. -static constexpr char kInvalidOriginTrialToken[] = - "A93QtcQ0CRKf5ioPasUwNbweXQWgbI4ZEshiz+" - "YS7dkQEWVfW9Ua2pTnA866sZwRzuElkPwsUdGdIaW0fRUP8AwAAABceyJvcmlnaW4iOiAiaHR0" - "cHM6Ly8xMjcuMC4wLjE6NDQ0NDQiLCAiZmVhdHVyzSI6ICJVc2VyQWdlbnRSZWR1Y3Rpb24iLC" - "AiZXhwaXJ5IjogMjAwMDAwMDAwMH0="; class EnabledClientHintsTest : public testing::Test { public: - EnabledClientHintsTest() - : response_headers_(base::MakeRefCounted("")) { + EnabledClientHintsTest() { // The UserAgentClientHint feature is enabled, and the LangClientHintHeader // feature is disabled. scoped_feature_list_.InitWithFeatures( /*enabled_features=*/{blink::features::kUserAgentClientHint}, /*disabled_features=*/{blink::features::kLangClientHintHeader}); - TrialTokenValidator::SetOriginTrialPolicyGetter( - base::BindRepeating([](OriginTrialPolicy* policy) { return policy; }, - base::Unretained(&policy_))); - policy_.SetPublicKeys({kTestPublicKey}); - } - - ~EnabledClientHintsTest() override { - TrialTokenValidator::ResetOriginTrialPolicyGetter(); - } - - const net::HttpResponseHeaders* response_headers() const { - return response_headers_.get(); - } - - void AddHeader(const std::string& header, const std::string& value) { - response_headers_->AddHeader(header, value); } private: - class TestOriginTrialPolicy : public OriginTrialPolicy { - public: - bool IsOriginTrialsSupported() const override { return true; } - bool IsOriginSecure(const GURL& url) const override { - return url.SchemeIs("https"); - } - const std::vector& GetPublicKeys() const override { - return keys_; - } - void SetPublicKeys(const std::vector& keys) { - keys_ = keys; - } - - private: - std::vector keys_; - }; - base::test::ScopedFeatureList scoped_feature_list_; - TestOriginTrialPolicy policy_; - scoped_refptr response_headers_; }; TEST_F(EnabledClientHintsTest, EnabledClientHint) { @@ -117,31 +50,4 @@ TEST_F(EnabledClientHintsTest, EnabledClientHintOnDisabledFeature) { EXPECT_FALSE(hints.IsEnabled(WebClientHintsType::kLang)); } -TEST_F(EnabledClientHintsTest, - EnabledUaReducedClientHintWithValidOriginTrialToken) { - AddHeader("Origin-Trial", kValidOriginTrialToken); - EnabledClientHints hints; - hints.SetIsEnabled(GURL(kOriginUrl), response_headers(), - WebClientHintsType::kUAReduced, true); - EXPECT_TRUE(hints.IsEnabled(WebClientHintsType::kUAReduced)); -} - -TEST_F(EnabledClientHintsTest, - EnabledUaReducedClientHintWithInvalidOriginTrialToken) { - AddHeader("Origin-Trial", kInvalidOriginTrialToken); - EnabledClientHints hints; - hints.SetIsEnabled(GURL(kOriginUrl), response_headers(), - WebClientHintsType::kUAReduced, true); - EXPECT_FALSE(hints.IsEnabled(WebClientHintsType::kUAReduced)); -} - -TEST_F(EnabledClientHintsTest, GetEnabledHints) { - EnabledClientHints hints; - hints.SetIsEnabled(WebClientHintsType::kUAFullVersion, true); - hints.SetIsEnabled(WebClientHintsType::kRtt, true); - EXPECT_THAT(hints.GetEnabledHints(), - ElementsAre(WebClientHintsType::kRtt, - WebClientHintsType::kUAFullVersion)); -} - } // namespace blink diff --git a/third_party/blink/public/common/client_hints/enabled_client_hints.h b/third_party/blink/public/common/client_hints/enabled_client_hints.h index f3b515f3785b4..70fac17a2800a 100644 --- a/third_party/blink/public/common/client_hints/enabled_client_hints.h +++ b/third_party/blink/public/common/client_hints/enabled_client_hints.h @@ -8,13 +8,6 @@ #include "services/network/public/mojom/web_client_hints_types.mojom-shared.h" #include "third_party/blink/public/common/common_export.h" -// Forward declarations. -class GURL; - -namespace net { -class HttpResponseHeaders; -} // namespace net - namespace blink { // EnabledClientHints stores all the client hints along with whether the hint @@ -34,30 +27,13 @@ class BLINK_COMMON_EXPORT EnabledClientHints { // Sets the client hint as enabled for sending in an HTTP request header. Even // if the client hint header is set to enabled, it is still possible that - // other factors (such as feature toggles) should cause the client hint to not + // other factors (such as feature toggles) could cause the client hint to not // be sent, and in that case, IsEnabled() would return false. // // If `type` is not a valid WebClientHintsType value, nothing is changed (no // client hints get enabled). void SetIsEnabled(network::mojom::WebClientHintsType type, bool should_send); - // Sets the client hint as enabled for sending in an HTTP request header. - // - // In addition to the client hint checks outlined in the SetIsEnabled() - // function above, this function also checks if the origin and/or the - // response headers indicate that the client hint should not be enabled (e.g. - // if the client hint should only be enabled in an Origin Trial). - // - // If `type` is not a valid WebClientHintsType value, nothing is changed (no - // client hints get enabled). - void SetIsEnabled(const GURL& url, - const net::HttpResponseHeaders* response_headers, - network::mojom::WebClientHintsType type, - bool should_send); - - // Returns a list of the enabled client hints. - std::vector GetEnabledHints() const; - private: std::array(network::mojom::WebClientHintsType::kMaxValue) + diff --git a/third_party/blink/renderer/core/loader/base_fetch_context.cc b/third_party/blink/renderer/core/loader/base_fetch_context.cc index 0cda0e6cf233f..07ce40cf2bb5c 100644 --- a/third_party/blink/renderer/core/loader/base_fetch_context.cc +++ b/third_party/blink/renderer/core/loader/base_fetch_context.cc @@ -35,10 +35,8 @@ namespace { -// Creates a serialized AtomicString header value out of the input string, using -// structured headers as described in -// https://www.rfc-editor.org/rfc/rfc8941.html. -const AtomicString SerializeStringHeader(std::string str) { +// Simple function to add quotes to make headers strings. +const AtomicString SerializeHeaderString(std::string str) { std::string output; if (!str.empty()) { output = net::structured_headers::SerializeItem( @@ -49,17 +47,6 @@ const AtomicString SerializeStringHeader(std::string str) { return AtomicString(output.c_str()); } -// Creates a serialized AtomicString header value out of the input boolean, -// using structured headers as described in -// https://www.rfc-editor.org/rfc/rfc8941.html. -const AtomicString SerializeBoolHeader(const bool value) { - const std::string output = net::structured_headers::SerializeItem( - net::structured_headers::Item(value)) - .value_or(std::string()); - - return AtomicString(output.c_str()); -} - } // namespace namespace blink { @@ -164,7 +151,9 @@ void BaseFetchContext::AddClientHintsIfNecessary( } // We also send Sec-CH-UA-Mobile to all hints. It is a one-bit header - // identifying if the browser has opted for a "mobile" experience. + // identifying if the browser has opted for a "mobile" experience + // Formatted using the "sh-boolean" format from: + // https://httpwg.org/http-extensions/draft-ietf-httpbis-header-structure.html#boolean // ShouldSendClientHint is called to make sure it's controlled by // PermissionsPolicy. if (ShouldSendClientHint( @@ -174,7 +163,7 @@ void BaseFetchContext::AddClientHintsIfNecessary( request.SetHttpHeaderField( blink::kClientHintsHeaderMapping[static_cast( network::mojom::blink::WebClientHintsType::kUAMobile)], - SerializeBoolHeader(ua->mobile)); + ua->mobile ? "?1" : "?0"); } } @@ -304,7 +293,7 @@ void BaseFetchContext::AddClientHintsIfNecessary( request.SetHttpHeaderField( blink::kClientHintsHeaderMapping[static_cast( network::mojom::blink::WebClientHintsType::kUAArch)], - SerializeStringHeader(ua->architecture)); + SerializeHeaderString(ua->architecture)); } if (ShouldSendClientHint( @@ -314,7 +303,7 @@ void BaseFetchContext::AddClientHintsIfNecessary( request.SetHttpHeaderField( blink::kClientHintsHeaderMapping[static_cast( network::mojom::blink::WebClientHintsType::kUAPlatform)], - SerializeStringHeader(ua->platform)); + SerializeHeaderString(ua->platform)); } if (ShouldSendClientHint( @@ -324,7 +313,7 @@ void BaseFetchContext::AddClientHintsIfNecessary( request.SetHttpHeaderField( blink::kClientHintsHeaderMapping[static_cast( network::mojom::blink::WebClientHintsType::kUAPlatformVersion)], - SerializeStringHeader(ua->platform_version)); + SerializeHeaderString(ua->platform_version)); } if (ShouldSendClientHint( @@ -334,7 +323,7 @@ void BaseFetchContext::AddClientHintsIfNecessary( request.SetHttpHeaderField( blink::kClientHintsHeaderMapping[static_cast( network::mojom::blink::WebClientHintsType::kUAModel)], - SerializeStringHeader(ua->model)); + SerializeHeaderString(ua->model)); } if (ShouldSendClientHint( @@ -344,7 +333,7 @@ void BaseFetchContext::AddClientHintsIfNecessary( request.SetHttpHeaderField( blink::kClientHintsHeaderMapping[static_cast( network::mojom::blink::WebClientHintsType::kUAFullVersion)], - SerializeStringHeader(ua->full_version)); + SerializeHeaderString(ua->full_version)); } if (ShouldSendClientHint( @@ -354,20 +343,7 @@ void BaseFetchContext::AddClientHintsIfNecessary( request.SetHttpHeaderField( blink::kClientHintsHeaderMapping[static_cast( network::mojom::blink::WebClientHintsType::kUABitness)], - SerializeStringHeader(ua->bitness)); - } - - if (ShouldSendClientHint( - ClientHintsMode::kStandard, policy, resource_origin, is_1p_origin, - network::mojom::blink::WebClientHintsType::kUAReduced, - hints_preferences)) { - // If the UA-Reduced client hint should be sent according to the hints - // preferences, it means the Origin Trial token for User-Agent Reduction - // has already been validated. - request.SetHttpHeaderField( - blink::kClientHintsHeaderMapping[static_cast( - network::mojom::blink::WebClientHintsType::kUAReduced)], - SerializeBoolHeader(true)); + SerializeHeaderString(ua->bitness)); } } diff --git a/third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.cc b/third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.cc index f1f127a0d87e2..8ffd2aa64a087 100644 --- a/third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.cc +++ b/third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.cc @@ -64,7 +64,6 @@ void ClientHintsPreferences::UpdateFromHttpEquivAcceptCH( // Note: .Ascii() would convert tab to ?, which is undesirable. absl::optional> parsed_ch = network::ParseClientHintsHeader(header_value.Latin1()); - if (!parsed_ch.has_value()) return;