[client hints] Factor out Accept-CH parsing into common/

Needed since it sometimes need to be parsed on browser as well,
not just renderer.

This also makes it uses structured headers parsing, which is what
it is spec'd as. This changes parsing behavior somewhat; for
example accepting (and ignoring) parameters. It is also supposed
to not accept non-space whitespace (which tests are updated for),
but the parser doesn't match the spec yet.

Bug: 1050726
Change-Id: Ib3953ba74efd2ae74d9003cb59edcba9bb49010f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2079295
Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746540}

third_party/blink/common/client_hints/client_hints.cc

@@ -4,8 +4,16 @@
 
 #include "third_party/blink/public/common/client_hints/client_hints.h"
 
+#include <utility>
+#include <vector>
+
+#include "base/containers/flat_map.h"
+#include "base/no_destructor.h"
+#include "base/optional.h"
 #include "base/stl_util.h"
 #include "base/strings/string_tokenizer.h"
+#include "base/strings/string_util.h"
+#include "net/http/structured_headers.h"
 
 namespace blink {
 
@@ -37,6 +45,11 @@ static_assert(base::size(kClientHintsNameMapping) ==
               "The Client Hint name and header mappings must contain the same "
               "number of entries.");
 
+static_assert(
+    base::size(kClientHintsNameMapping) ==
+        (static_cast<int>(mojom::WebClientHintsType::kMaxValue) + 1),
+    "Client Hint name table size must match mojom::WebClientHintsType range");
+
 const char* const kWebEffectiveConnectionTypeMapping[] = {
     "4g" /* Unknown */, "4g" /* Offline */, "slow-2g" /* Slow 2G */,
     "2g" /* 2G */,      "3g" /* 3G */,      "4g" /* 4G */
@@ -45,6 +58,34 @@ const char* const kWebEffectiveConnectionTypeMapping[] = {
 const size_t kWebEffectiveConnectionTypeMappingCount =
     base::size(kWebEffectiveConnectionTypeMapping);
 
+namespace {
+
+struct ClientHintNameCompator {
+  bool operator()(const std::string& lhs, const std::string& rhs) const {
+    return base::CompareCaseInsensitiveASCII(lhs, rhs) < 0;
+  }
+};
+
+using DecodeMap = base::
+    flat_map<std::string, mojom::WebClientHintsType, ClientHintNameCompator>;
+
+DecodeMap MakeDecodeMap() {
+  DecodeMap result;
+  for (size_t i = 0;
+       i < static_cast<int>(mojom::WebClientHintsType::kMaxValue) + 1; ++i) {
+    result.insert(std::make_pair(kClientHintsNameMapping[i],
+                                 static_cast<mojom::WebClientHintsType>(i)));
+  }
+  return result;
+}
+
+const DecodeMap& GetDecodeMap() {
+  static const base::NoDestructor<DecodeMap> decode_map(MakeDecodeMap());
+  return *decode_map;
+}
+
+}  // namespace
+
 std::string SerializeLangClientHint(const std::string& raw_language_list) {
   base::StringTokenizer t(raw_language_list, ",");
   std::string result;
@@ -59,4 +100,58 @@ std::string SerializeLangClientHint(const std::string& raw_language_list) {
   return result;
 }
 
+base::Optional<std::vector<blink::mojom::WebClientHintsType>> ParseAcceptCH(
+    const std::string& header,
+    bool permit_lang_hints,
+    bool permit_ua_hints) {
+  // Accept-CH is an sh-list of tokens; see:
+  // https://httpwg.org/http-extensions/client-hints.html#rfc.section.3.1
+  base::Optional<net::structured_headers::List> maybe_list =
+      net::structured_headers::ParseList(header);
+  if (!maybe_list.has_value())
+    return base::nullopt;
+
+  // Standard validation rules: we want a list of tokens, so this better
+  // only have tokens (but params are OK!)
+  for (const auto& list_item : maybe_list.value()) {
+    // Make sure not a nested list.
+    if (list_item.member.size() != 1u)
+      return base::nullopt;
+    if (!list_item.member[0].item.is_token())
+      return base::nullopt;
+  }
+
+  std::vector<blink::mojom::WebClientHintsType> result;
+
+  // Now convert those to actual hint enums.
+  const DecodeMap& decode_map = GetDecodeMap();
+  for (const auto& list_item : maybe_list.value()) {
+    const std::string& token_value = list_item.member[0].item.GetString();
+
+    auto iter = decode_map.find(token_value);
+    if (iter != decode_map.end()) {
+      mojom::WebClientHintsType hint = iter->second;
+
+      // Some hints are supported only conditionally.
+      switch (hint) {
+        case mojom::WebClientHintsType::kLang:
+          if (permit_lang_hints)
+            result.push_back(hint);
+          break;
+        case mojom::WebClientHintsType::kUA:
+        case mojom::WebClientHintsType::kUAArch:
+        case mojom::WebClientHintsType::kUAPlatform:
+        case mojom::WebClientHintsType::kUAModel:
+        case mojom::WebClientHintsType::kUAMobile:
+          if (permit_ua_hints)
+            result.push_back(hint);
+          break;
+        default:
+          result.push_back(hint);
+      }  // switch (hint)
+    }    // if iter != end
+  }      // for list_item
+  return base::make_optional(std::move(result));
+}
+
 }  // namespace blink

third_party/blink/common/client_hints/client_hints_unittest.cc

@@ -4,10 +4,24 @@
 
 #include "third_party/blink/public/common/client_hints/client_hints.h"
 
+#include <iostream>
+
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/mojom/web_client_hints/web_client_hints_types.mojom-shared.h"
+
+using testing::UnorderedElementsAre;
 
 namespace blink {
 
+namespace mojom {
+
+void PrintTo(const blink::mojom::WebClientHintsType& value, std::ostream* os) {
+  *os << ::testing::PrintToString(static_cast<int>(value));
+}
+
+}  // namespace mojom
+
 TEST(ClientHintsTest, SerializeLangClientHint) {
   std::string header = SerializeLangClientHint("");
   EXPECT_TRUE(header.empty());
@@ -23,4 +37,108 @@ TEST(ClientHintsTest, SerializeLangClientHint) {
             header);
 }
 
+TEST(ClientHintsTest, ParseAcceptCH) {
+  base::Optional<std::vector<blink::mojom::WebClientHintsType>> result;
+
+  // Empty is OK.
+  result = ParseAcceptCH(" ",
+                         /* permit_lang_hints = */ true,
+                         /* permit_ua_hints = */ true);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_TRUE(result.value().empty());
+
+  // Normal case.
+  result = ParseAcceptCH("device-memory,  rtt, lang ",
+                         /* permit_lang_hints = */ true,
+                         /* permit_ua_hints = */ true);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_THAT(result.value(),
+              UnorderedElementsAre(mojom::WebClientHintsType::kDeviceMemory,
+                                   mojom::WebClientHintsType::kRtt,
+                                   mojom::WebClientHintsType::kLang));
+
+  // Must be a list of tokens, not other things.
+  result = ParseAcceptCH("\"device-memory\", \"rtt\", \"lang\"",
+                         /* permit_lang_hints = */ true,
+                         /* permit_ua_hints = */ true);
+  EXPECT_FALSE(result.has_value());
+
+  // Parameters to the tokens are ignored, as encourageed by structured headers
+  // spec.
+  result = ParseAcceptCH("device-memory;resolution=GIB, rtt, lang",
+                         /* permit_lang_hints = */ true,
+                         /* permit_ua_hints = */ true);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_THAT(result.value(),
+              UnorderedElementsAre(mojom::WebClientHintsType::kDeviceMemory,
+                                   mojom::WebClientHintsType::kRtt,
+                                   mojom::WebClientHintsType::kLang));
+
+  // Unknown tokens are fine, since this meant to be extensible.
+  result = ParseAcceptCH("device-memory,  rtt, lang , nosuchtokenwhywhywhy",
+                         /* permit_lang_hints = */ true,
+                         /* permit_ua_hints = */ true);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_THAT(result.value(),
+              UnorderedElementsAre(mojom::WebClientHintsType::kDeviceMemory,
+                                   mojom::WebClientHintsType::kRtt,
+                                   mojom::WebClientHintsType::kLang));
+}
+
+TEST(ClientHintsTest, ParseAcceptCHCaseInsensitive) {
+  base::Optional<std::vector<blink::mojom::WebClientHintsType>> result;
+
+  // Matching is case-insensitive.
+  result = ParseAcceptCH("Device-meMory,  Rtt, lanG ",
+                         /* permit_lang_hints = */ true,
+                         /* permit_ua_hints = */ true);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_THAT(result.value(),
+              UnorderedElementsAre(mojom::WebClientHintsType::kDeviceMemory,
+                                   mojom::WebClientHintsType::kRtt,
+                                   mojom::WebClientHintsType::kLang));
+}
+
+// Checks to make sure that language-controlled things are filtered.
+TEST(ClientHintsTest, ParseAcceptCHFlag) {
+  base::Optional<std::vector<blink::mojom::WebClientHintsType>> result;
+
+  result = ParseAcceptCH("device-memory,  rtt, lang, ua",
+                         /* permit_lang_hints = */ false,
+                         /* permit_ua_hints = */ true);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_THAT(result.value(),
+              UnorderedElementsAre(mojom::WebClientHintsType::kDeviceMemory,
+                                   mojom::WebClientHintsType::kRtt,
+                                   mojom::WebClientHintsType::kUA));
+
+  result = ParseAcceptCH("rtt, lang, ua, arch, platform, model, mobile",
+                         /* permit_lang_hints = */ true,
+                         /* permit_ua_hints = */ false);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_THAT(result.value(),
+              UnorderedElementsAre(mojom::WebClientHintsType::kRtt,
+                                   mojom::WebClientHintsType::kLang));
+
+  result = ParseAcceptCH("rtt, lang, ua, arch, platform, model, mobile",
+                         /* permit_lang_hints = */ true,
+                         /* permit_ua_hints = */ true);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_THAT(result.value(),
+              UnorderedElementsAre(mojom::WebClientHintsType::kRtt,
+                                   mojom::WebClientHintsType::kLang,
+                                   mojom::WebClientHintsType::kUA,
+                                   mojom::WebClientHintsType::kUAArch,
+                                   mojom::WebClientHintsType::kUAPlatform,
+                                   mojom::WebClientHintsType::kUAModel,
+                                   mojom::WebClientHintsType::kUAMobile));
+
+  result = ParseAcceptCH("rtt, lang, ua, arch, platform, model, mobile",
+                         /* permit_lang_hints = */ false,
+                         /* permit_ua_hints = */ false);
+  ASSERT_TRUE(result.has_value());
+  EXPECT_THAT(result.value(),
+              UnorderedElementsAre(mojom::WebClientHintsType::kRtt));
+}
+
 }  // namespace blink

third_party/blink/public/common/client_hints/client_hints.h

@@ -8,7 +8,9 @@
 #include <stddef.h>
 #include <string>
 
+#include "base/optional.h"
 #include "third_party/blink/public/common/common_export.h"
+#include "third_party/blink/public/mojom/web_client_hints/web_client_hints_types.mojom-shared.h"
 
 namespace blink {
 
@@ -37,6 +39,17 @@ BLINK_COMMON_EXPORT extern const size_t kWebEffectiveConnectionTypeMappingCount;
 std::string BLINK_COMMON_EXPORT
 SerializeLangClientHint(const std::string& raw_language_list);
 
+// Tries to parse an Accept-CH header. Returns base::nullopt if parsing
+// failed and the header should be ignored; otherwise returns a (possibly
+// empty) list of hints to accept.
+//
+// Language hints will only be in the result if |permit_lang_hints| is true;
+// UA-related ones if |permit_ua_hints| is.
+base::Optional<std::vector<blink::mojom::WebClientHintsType>>
+    BLINK_COMMON_EXPORT ParseAcceptCH(const std::string& header,
+                                      bool permit_lang_hints,
+                                      bool permit_ua_hints);
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_PUBLIC_COMMON_CLIENT_HINTS_CLIENT_HINTS_H_

third_party/blink/renderer/core/html/parser/html_preload_scanner_test.cc

@@ -634,11 +634,11 @@ TEST_F(HTMLPreloadScannerTest, testMetaAcceptCH) {
        "640w'>",
        "blabla.gif", "http://example.test/", ResourceType::kImage, 0},
       {"http://example.test",
-       "<meta http-equiv='accept-ch' content='dpr \t'><img srcset='bla.gif "
+       "<meta http-equiv='accept-ch' content='dpr  '><img srcset='bla.gif "
        "320w, blabla.gif 640w'>",
        "blabla.gif", "http://example.test/", ResourceType::kImage, 0, dpr},
       {"http://example.test",
-       "<meta http-equiv='accept-ch' content='bla,dpr \t'><img srcset='bla.gif "
+       "<meta http-equiv='accept-ch' content='bla,dpr  '><img srcset='bla.gif "
        "320w, blabla.gif 640w'>",
        "blabla.gif", "http://example.test/", ResourceType::kImage, 0, dpr},
       {"http://example.test",
@@ -663,7 +663,7 @@ TEST_F(HTMLPreloadScannerTest, testMetaAcceptCH) {
        viewport_width},
       {"http://example.test",
        "<meta http-equiv='accept-ch' content='  viewport-width  ,width, "
-       "wutever, dpr \t'><img sizes='90vw' srcset='bla.gif 320w, blabla.gif "
+       "wutever, dpr  '><img sizes='90vw' srcset='bla.gif 320w, blabla.gif "
        "640w'>",
        "blabla.gif", "http://example.test/", ResourceType::kImage, 450, all},
   };
@@ -685,7 +685,7 @@ TEST_F(HTMLPreloadScannerTest, testMetaAcceptCHInsecureDocument) {
   const PreloadScannerTestCase expect_no_client_hint = {
       "http://example.test",
       "<meta http-equiv='accept-ch' content='  viewport-width  ,width, "
-      "wutever, dpr \t'><img sizes='90vw' srcset='bla.gif 320w, blabla.gif "
+      "wutever, dpr  '><img sizes='90vw' srcset='bla.gif 320w, blabla.gif "
       "640w'>",
       "blabla.gif",
       "http://example.test/",
@@ -695,7 +695,7 @@ TEST_F(HTMLPreloadScannerTest, testMetaAcceptCHInsecureDocument) {
   const PreloadScannerTestCase expect_client_hint = {
       "http://example.test",
       "<meta http-equiv='accept-ch' content='  viewport-width  ,width, "
-      "wutever, dpr \t'><img sizes='90vw' srcset='bla.gif 320w, blabla.gif "
+      "wutever, dpr  '><img sizes='90vw' srcset='bla.gif 320w, blabla.gif "
       "640w'>",
       "blabla.gif",
       "http://example.test/",

third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.cc

@@ -14,69 +14,6 @@
 
 namespace blink {
 
-namespace {
-
-void ParseAcceptChHeader(const String& header_value,
-                         WebEnabledClientHints& enabled_hints) {
-  CommaDelimitedHeaderSet accept_client_hints_header;
-  ParseCommaDelimitedHeader(header_value, accept_client_hints_header);
-
-  for (size_t i = 0;
-       i < static_cast<int>(mojom::WebClientHintsType::kMaxValue) + 1; ++i) {
-    enabled_hints.SetIsEnabled(
-        static_cast<mojom::WebClientHintsType>(i),
-        accept_client_hints_header.Contains(kClientHintsNameMapping[i]));
-  }
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kDeviceMemory,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kDeviceMemory));
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kRtt,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kRtt));
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kDownlink,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kDownlink));
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kEct,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kEct));
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kLang,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kLang) &&
-          RuntimeEnabledFeatures::LangClientHintHeaderEnabled());
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kUA,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kUA) &&
-          RuntimeEnabledFeatures::UserAgentClientHintEnabled());
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kUAArch,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kUAArch) &&
-          RuntimeEnabledFeatures::UserAgentClientHintEnabled());
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kUAPlatform,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kUAPlatform) &&
-          RuntimeEnabledFeatures::UserAgentClientHintEnabled());
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kUAModel,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kUAModel) &&
-          RuntimeEnabledFeatures::UserAgentClientHintEnabled());
-
-  enabled_hints.SetIsEnabled(
-      mojom::WebClientHintsType::kUAMobile,
-      enabled_hints.IsEnabled(mojom::WebClientHintsType::kUAMobile) &&
-          RuntimeEnabledFeatures::UserAgentClientHintEnabled());
-}
-
-}  // namespace
-
 ClientHintsPreferences::ClientHintsPreferences() {
   DCHECK_EQ(static_cast<size_t>(mojom::WebClientHintsType::kMaxValue) + 1,
             kClientHintsMappingsCount);
@@ -102,16 +39,24 @@ void ClientHintsPreferences::UpdateFromAcceptClientHintsHeader(
   if (!IsClientHintsAllowed(url))
     return;
 
-  WebEnabledClientHints new_enabled_types;
+  // 8-bit conversions from String can turn non-ASCII characters into ?,
+  // turning syntax errors into "correct" syntax, so reject those first.
+  // (.Utf8() doesn't have this problem, but it does a lot of expensive
+  //  work that would be wasted feeding to an ASCII-only syntax).
+  if (!header_value.ContainsOnlyASCIIOrEmpty())
+    return;
 
-  ParseAcceptChHeader(header_value, new_enabled_types);
+  // Note: .Ascii() would convert tab to ?, which is undesirable.
+  base::Optional<std::vector<blink::mojom::WebClientHintsType>> parsed_ch =
+      ParseAcceptCH(header_value.Latin1(),
+                    RuntimeEnabledFeatures::LangClientHintHeaderEnabled(),
+                    RuntimeEnabledFeatures::UserAgentClientHintEnabled());
+  if (!parsed_ch.has_value())
+    return;
 
-  for (size_t i = 0;
-       i < static_cast<int>(mojom::WebClientHintsType::kMaxValue) + 1; ++i) {
-    mojom::WebClientHintsType type = static_cast<mojom::WebClientHintsType>(i);
-    enabled_hints_.SetIsEnabled(type, enabled_hints_.IsEnabled(type) ||
-                                          new_enabled_types.IsEnabled(type));
-  }
+  // Note: this keeps previously enabled hints.
+  for (blink::mojom::WebClientHintsType newly_enabled : parsed_ch.value())
+    enabled_hints_.SetIsEnabled(newly_enabled, true);
 
   if (context) {
     for (size_t i = 0;