Clamp platform version client hint.

mkarolin · mkarolin · commit f9466784d159 · 2023-04-26T10:01:03.000-04:00
diff --git a/browser/about_flags.cc b/browser/about_flags.cc
@@ -390,6 +390,14 @@
           kOsAll,                                                              \
           FEATURE_VALUE_TYPE(blink::features::kAllowCertainClientHints),       \
       },                                                                       \
+      {                                                                        \
+          "clamp-platform-version-client-hint",                                \
+          "Clamp platform version client hint",                                \
+          "Clamps the patch field of the platform version client hint",        \
+          kOsAll,                                                              \
+          FEATURE_VALUE_TYPE(                                                  \
+              blink::features::kClampPlatformVersionClientHint),               \
+      },                                                                       \
       {                                                                        \
           "brave-ntp-branded-wallpaper-demo",                                  \
           "New Tab Page Demo Branded Wallpaper",                               \
diff --git a/browser/test/disabled_features/disable_client_hints_browsertest.cc b/browser/test/disabled_features/disable_client_hints_browsertest.cc
@@ -12,6 +12,7 @@
 #include "base/functional/bind.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "brave/components/constants/brave_paths.h"
@@ -36,6 +37,8 @@ const char KClientHintsMetaHTTPEquivAcceptCH[] =
     "/ch-meta-http-equiv-accept-ch.html";
 const char KClientHintsMetaNameAcceptCH[] = "/ch-meta-name-accept-ch.html";
 
+const char kPlatformVersionClientHintPatchValue[] = "x";
+
 const std::reference_wrapper<const base::Feature> kTestFeatures[] = {
     // Individual hints features
     blink::features::kClientHintsDeviceMemory,
@@ -93,23 +96,30 @@ class ClientHintsBrowserTest
 
   void SetUp() override {
     // Test that even with CH features enabled, there is no header.
-    std::vector<base::test::FeatureRef> enabled_features;
+    std::vector<base::test::FeatureRefAndParams> enabled_features;
     std::vector<base::test::FeatureRef> disabled_features;
     for (const auto& feature : kTestFeatures) {
       if (IsClientHintHeaderEnabled()) {
-        enabled_features.push_back(feature.get());
+        enabled_features.emplace_back(feature.get(), base::FieldTrialParams());
       } else {
         disabled_features.push_back(feature.get());
       }
     }
 
     if (IsBraveClientHintFeatureEnabled()) {
-      enabled_features.push_back(blink::features::kAllowCertainClientHints);
+      enabled_features.emplace_back(blink::features::kAllowCertainClientHints,
+                                    base::FieldTrialParams());
+      base::FieldTrialParams parameters;
+      parameters[blink::features::kClampPlatformVersionClientHintPatchValue
+                     .name] = kPlatformVersionClientHintPatchValue;
+      enabled_features.emplace_back(
+          blink::features::kClampPlatformVersionClientHint, parameters);
     } else {
       disabled_features.push_back(blink::features::kAllowCertainClientHints);
     }
 
-    scoped_feature_list_.InitWithFeatures(enabled_features, disabled_features);
+    scoped_feature_list_.InitWithFeaturesAndParameters(enabled_features,
+                                                       disabled_features);
 
     if (IsBraveClientHintFeatureEnabled()) {
       PopulateDefaultClientHints();
@@ -154,28 +164,39 @@ class ClientHintsBrowserTest
     return unexpected_client_hints_headers_seen_.size();
   }
 
-  std::string default_client_hints_headers_seen() {
+  std::string default_client_hints_headers_seen() const {
     return base::JoinString(
         std::vector<std::string>(default_client_hints_headers_seen_.begin(),
                                  default_client_hints_headers_seen_.end()),
         ", ");
   }
 
-  std::string allowed_client_hints_headers_seen() {
+  std::string allowed_client_hints_headers_seen() const {
     return base::JoinString(
         std::vector<std::string>(allowed_client_hints_headers_seen_.begin(),
                                  allowed_client_hints_headers_seen_.end()),
         ", ");
   }
 
-  std::string unexpected_client_hints_headers_seen() {
+  std::string unexpected_client_hints_headers_seen() const {
     return base::JoinString(unexpected_client_hints_headers_seen_, ", ");
   }
 
+  std::string platform_version_client_hint_value() const {
+    return platform_version_client_hint_value_;
+  }
+
   void reset_client_hints_headers_seen() {
     default_client_hints_headers_seen_.clear();
     allowed_client_hints_headers_seen_.clear();
     unexpected_client_hints_headers_seen_.clear();
+    platform_version_client_hint_value_ = "";
+  }
+
+  bool VerifyPlatformVersionClientHintPatchValue() const {
+    return base::EndsWith(
+        platform_version_client_hint_value_,
+        base::StrCat({".", kPlatformVersionClientHintPatchValue, "\""}));
   }
 
  private:
@@ -197,6 +218,21 @@ class ClientHintsBrowserTest
         hints_map.at(network::mojom::WebClientHintsType::kUAPlatformVersion));
   }
 
+  bool IsPlatformVersionClientHintHeader(const std::string& header) const {
+    return header ==
+           network::GetClientHintToNameMap().at(
+               network::mojom::WebClientHintsType::kUAPlatformVersion);
+  }
+
+  void StorePlatformVersionClientHintHeaderValue(
+      const net::test_server::HttpRequest::HeaderMap& headers) {
+    const auto& hints_map = network::GetClientHintToNameMap();
+    const std::string& platform_version_header_name =
+        hints_map.at(network::mojom::WebClientHintsType::kUAPlatformVersion);
+    platform_version_client_hint_value_ =
+        headers.at(platform_version_header_name);
+  }
+
   void MonitorResourceRequest(const net::test_server::HttpRequest& request) {
     for (const auto& elem : network::GetClientHintToNameMap()) {
       const auto& header = elem.second;
@@ -207,6 +243,9 @@ class ClientHintsBrowserTest
             continue;
           } else if (base::Contains(allowed_hints_, header)) {
             allowed_client_hints_headers_seen_.insert(header);
+            if (IsPlatformVersionClientHintHeader(header)) {
+              StorePlatformVersionClientHintHeaderValue(request.headers);
+            }
             continue;
           }
         }
@@ -230,10 +269,12 @@ class ClientHintsBrowserTest
   std::vector<std::string> default_hints_;
   std::vector<std::string> allowed_hints_;
 
+  std::string platform_version_client_hint_value_;
+
   base::test::ScopedFeatureList scoped_feature_list_;
 };
 
-IN_PROC_BROWSER_TEST_P(ClientHintsBrowserTest, ClientHintsDisabled) {
+IN_PROC_BROWSER_TEST_P(ClientHintsBrowserTest, CheckClientHints) {
   for (const auto& feature : kTestFeatures) {
     EXPECT_EQ(IsClientHintHeaderEnabled(),
               base::FeatureList::IsEnabled(feature));
@@ -269,6 +310,13 @@ IN_PROC_BROWSER_TEST_P(ClientHintsBrowserTest, ClientHintsDisabled) {
       << "Allowed headers seen: " << allowed_client_hints_headers_seen();
   EXPECT_EQ(0u, client_hints_headers_seen_count())
       << "Unexpected headers: " << unexpected_client_hints_headers_seen();
+  if (IsClientHintHeaderEnabled() && IsBraveClientHintFeatureEnabled()) {
+    EXPECT_TRUE(VerifyPlatformVersionClientHintPatchValue())
+        << "Expected the patch field value to be: '"
+        << kPlatformVersionClientHintPatchValue << "'. "
+        << "Actual platform version value: "
+        << platform_version_client_hint_value();
+  }
 
   reset_client_hints_headers_seen();
   ASSERT_TRUE(ui_test_utils::NavigateToURL(
diff --git a/chromium_src/components/embedder_support/user_agent_utils.cc b/chromium_src/components/embedder_support/user_agent_utils.cc
@@ -3,6 +3,13 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "components/embedder_support/user_agent_utils.h"
+
+#include "base/check_op.h"
+#include "base/strings/stringprintf.h"
+#include "base/version.h"
+#include "third_party/blink/public/common/features.h"
+
 namespace {
 
 constexpr char kBraveBrandNameForCHUA[] = "Brave";
@@ -18,5 +25,34 @@ constexpr char kBraveBrandNameForCHUA[] = "Brave";
 // can't use it here in the //components.
 #define BRAVE_GET_USER_AGENT_BRAND_LIST brand = kBraveBrandNameForCHUA;
 
+#define GetUserAgentMetadata GetUserAgentMetadata_ChromiumImpl
 #include "src/components/embedder_support/user_agent_utils.cc"
+#undef GetUserAgentMetadata
 #undef BRAVE_GET_USER_AGENT_BRAND_LIST
+
+namespace embedder_support {
+
+blink::UserAgentMetadata GetUserAgentMetadata() {
+  return GetUserAgentMetadata(nullptr);
+}
+
+blink::UserAgentMetadata GetUserAgentMetadata(const PrefService* pref_service) {
+  blink::UserAgentMetadata metadata =
+      GetUserAgentMetadata_ChromiumImpl(pref_service);
+  if (base::FeatureList::IsEnabled(
+          blink::features::kClampPlatformVersionClientHint)) {
+    // Clamp platform version
+    base::Version platform_version(metadata.platform_version);
+    // Expect Chromium code to return `major.minor.patch` version. If that
+    // changes we need to re-evaluate what we want to send.
+    CHECK_EQ(3u, platform_version.components().size());
+    metadata.platform_version = base::StringPrintf(
+        "%d.%d.%s", platform_version.components()[0],
+        platform_version.components()[1],
+        blink::features::kClampPlatformVersionClientHintPatchValue.Get()
+            .c_str());
+  }
+  return metadata;
+}
+
+}  // namespace embedder_support
diff --git a/chromium_src/components/embedder_support/user_agent_utils.h b/chromium_src/components/embedder_support/user_agent_utils.h
@@ -0,0 +1,21 @@
+/* Copyright (c) 2023 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_EMBEDDER_SUPPORT_USER_AGENT_UTILS_H_
+#define BRAVE_CHROMIUM_SRC_COMPONENTS_EMBEDDER_SUPPORT_USER_AGENT_UTILS_H_
+
+#define GetUserAgentMetadata GetUserAgentMetadata_ChromiumImpl
+#include "src/components/embedder_support/user_agent_utils.h"  // IWYU pragma: export
+#undef GetUserAgentMetadata
+
+namespace embedder_support {
+
+blink::UserAgentMetadata GetUserAgentMetadata();
+
+blink::UserAgentMetadata GetUserAgentMetadata(const PrefService* pref_service);
+
+}  // namespace embedder_support
+
+#endif  // BRAVE_CHROMIUM_SRC_COMPONENTS_EMBEDDER_SUPPORT_USER_AGENT_UTILS_H_
diff --git a/chromium_src/third_party/blink/common/client_hints/enabled_client_hints.cc b/chromium_src/third_party/blink/common/client_hints/enabled_client_hints.cc
@@ -17,8 +17,7 @@
 // Additionally, if we receive CH requests for
 // kUAPlatformVersion and/or kUAModel, we will send these, but:
 // - kUAModel will be always set to an empty string;
-// - kUAPlatformVersion will be clamped to the same value we report in the
-//   User-Agent string.
+// - kUAPlatformVersion will have a clamped patch field value.
 
 namespace blink {
 
diff --git a/chromium_src/third_party/blink/common/features.cc b/chromium_src/third_party/blink/common/features.cc
@@ -51,6 +51,17 @@ BASE_FEATURE(kAllowCertainClientHints,
              "AllowCertainClientHints",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
+// Clamp platform version client hint's patch field.
+// Platform version client hint is only sent when requested and when the
+// AllowCertainClientHints feature is turned on.
+BASE_FEATURE(kClampPlatformVersionClientHint,
+             "ClampPlatformVersionClientHint",
+             base::FEATURE_ENABLED_BY_DEFAULT);
+
+constexpr base::FeatureParam<std::string>
+    kClampPlatformVersionClientHintPatchValue{&kClampPlatformVersionClientHint,
+                                              "patch_value", "0"};
+
 BASE_FEATURE(kFileSystemAccessAPI,
              "FileSystemAccessAPI",
              base::FEATURE_DISABLED_BY_DEFAULT);
diff --git a/chromium_src/third_party/blink/public/common/features.h b/chromium_src/third_party/blink/public/common/features.h
@@ -8,10 +8,13 @@
 
 #include "src/third_party/blink/public/common/features.h"  // IWYU pragma: export
 
+#include <string>
+
 namespace blink {
 namespace features {
 
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kAllowCertainClientHints);
+BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kClampPlatformVersionClientHint);
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kFileSystemAccessAPI);
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kNavigatorConnectionAttribute);
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kPartitionBlinkMemoryCache);
@@ -20,6 +23,11 @@ BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kBraveBlockScreenFingerprinting);
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kBraveRoundTimeStamps);
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kRestrictEventSourcePool);
 
+// Specifies the value of the third component of the dotted version of the
+// platform version client hint.
+BLINK_COMMON_EXPORT extern const base::FeatureParam<std::string>
+    kClampPlatformVersionClientHintPatchValue;
+
 // Chromium used this function to control Prerender2 feature, but then the
 // feature was permanently enabled and the function was removed. We still want
 // to keep the Prerender2 functionality disabled, so putting back the function
diff --git a/components/embedder_support/BUILD.gn b/components/embedder_support/BUILD.gn
@@ -11,6 +11,7 @@ source_set("unit_tests") {
   sources = [ "user_agent_utils_unittest.cc" ]
 
   deps = [
+    "//base/test:test_support",
     "//components/embedder_support:browser_util",
     "//components/version_info",
     "//testing/gtest",
diff --git a/components/embedder_support/user_agent_utils_unittest.cc b/components/embedder_support/user_agent_utils_unittest.cc
@@ -4,8 +4,12 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "components/embedder_support/user_agent_utils.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/version.h"
 #include "components/version_info/version_info.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/user_agent/user_agent_metadata.h"
 
 namespace embedder_support {
@@ -33,4 +37,45 @@ TEST(UserAgentUtilsTest, UserAgentMetadata) {
       ContainsBrandVersion(metadata.brand_version_list, product_brand_version));
 }
 
+TEST(UserAgentUtilsTest, DoNotClampPlatformVersion) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitWithFeatures(
+      {blink::features::kAllowCertainClientHints},
+      {blink::features::kClampPlatformVersionClientHint});
+  auto metadata = GetUserAgentMetadata_ChromiumImpl();
+  auto brave_metadata = GetUserAgentMetadata(nullptr);
+  EXPECT_EQ(metadata, brave_metadata);
+}
+
+TEST(UserAgentUtilsTest, ClampPlatformVersion) {
+  static const char kClampedValue[] = "7775777";
+  base::test::ScopedFeatureList feature_list;
+  std::vector<base::test::FeatureRefAndParams> enabled_features;
+  enabled_features.emplace_back(blink::features::kAllowCertainClientHints,
+                                base::FieldTrialParams());
+  base::FieldTrialParams parameters;
+  parameters[blink::features::kClampPlatformVersionClientHintPatchValue.name] =
+      kClampedValue;
+  enabled_features.emplace_back(
+      blink::features::kClampPlatformVersionClientHint, parameters);
+  feature_list.InitWithFeaturesAndParameters(enabled_features, {});
+
+  auto metadata = GetUserAgentMetadata_ChromiumImpl();
+  auto brave_metadata = GetUserAgentMetadata(nullptr);
+
+  base::Version platform_version(metadata.platform_version);
+  base::Version brave_platform_version(brave_metadata.platform_version);
+
+  EXPECT_EQ(3u, platform_version.components().size());
+  EXPECT_EQ(3u, brave_platform_version.components().size());
+  EXPECT_EQ(platform_version.components()[0],
+            brave_platform_version.components()[0]);
+  EXPECT_EQ(platform_version.components()[1],
+            brave_platform_version.components()[1]);
+  EXPECT_NE(platform_version.components()[2],
+            brave_platform_version.components()[2]);
+  EXPECT_EQ(kClampedValue,
+            base::NumberToString(brave_platform_version.components()[2]));
+}
+
 }  // namespace embedder_support
