Fetch API: Stop lowercasing header names.

Adapt to whatwg/fetch#476, which basically says a
header list should not lowercase the header names it is passed, but rather
only do that in the "sort and combine" algorithm. This means several test
expectations had to be updated because casing _will_ be preserved when a
header is set.

To accommodate the change, turn FetchHeaderList::header_list_ into an
std::multimap with a custom comparison function in order to implement the
concept of multiple headers with possibly different casing that should all
be treated as the same header and kept sorted.

Special care has been taken not to change FetchHeaderList's API unless
absolutely necessary, but simply to avoid making the diff needlessly large.

BUG=687155
R=haraken@chromium.org,tyoshino@chromium.org,yhirano@chromium.org

Review-Url: https://codereview.chromium.org/2787003002
Cr-Commit-Position: refs/heads/master@{#465214}

content/browser/service_worker/service_worker_browsertest.cc

@@ -1229,8 +1229,8 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, FetchEvent_Response) {
   EXPECT_EQ(301, response.status_code);
   EXPECT_EQ("Moved Permanently", response.status_text);
   ServiceWorkerHeaderMap expected_headers;
-  expected_headers["content-language"] = "fi";
-  expected_headers["content-type"] = "text/html; charset=UTF-8";
+  expected_headers["Content-Language"] = "fi";
+  expected_headers["Content-Type"] = "text/html; charset=UTF-8";
   EXPECT_EQ(expected_headers, response.headers);
 
   std::string body;

third_party/WebKit/LayoutTests/http/tests/fetch/resources/thorough-util.js

@@ -163,16 +163,16 @@ var checkJsonpError = checkJsonpResult.bind(this, 'error');
 var checkJsonpSuccess = checkJsonpResult.bind(this, 'success');
 var checkJsonpNoRedirect = checkJsonpResult.bind(this, 'noredirect');
 var hasCustomHeader =
-  checkJsonpHeader.bind(this, 'x-serviceworker-test', 'test');
+  checkJsonpHeader.bind(this, 'X-ServiceWorker-Test', 'test');
 var hasCustomHeader2 = function(url, data) {
-  checkJsonpHeader('x-serviceworker-s', 'test1', url, data);
-  checkJsonpHeader('x-serviceworker-test', 'test2,test3', url, data);
-  checkJsonpHeader('x-serviceworker-ua', 'test4', url, data);
-  checkJsonpHeader('x-serviceworker-u', 'test5', url, data);
-  checkJsonpHeader('x-serviceworker-v', 'test6', url, data);
+  checkJsonpHeader('X-ServiceWorker-s', 'test1', url, data);
+  checkJsonpHeader('X-ServiceWorker-Test', 'test2,test3', url, data);
+  checkJsonpHeader('X-ServiceWorker-ua', 'test4', url, data);
+  checkJsonpHeader('X-ServiceWorker-U', 'test5', url, data);
+  checkJsonpHeader('X-ServiceWorker-V', 'test6', url, data);
 };
 var noCustomHeader =
-  checkJsonpHeader.bind(this, 'x-serviceworker-test', undefined);
+  checkJsonpHeader.bind(this, 'X-ServiceWorker-Test', undefined);
 var methodIsGET = checkJsonpMethod.bind(this, 'GET');
 var methodIsPOST = checkJsonpMethod.bind(this, 'POST');
 var methodIsPUT = checkJsonpMethod.bind(this, 'PUT');

third_party/WebKit/Source/modules/BUILD.gn

@@ -261,6 +261,7 @@ source_set("unit_tests") {
     "fetch/DataConsumerHandleTestUtil.cpp",
     "fetch/DataConsumerHandleTestUtil.h",
     "fetch/FetchDataLoaderTest.cpp",
+    "fetch/FetchHeaderListTest.cpp",
     "fetch/FetchResponseDataTest.cpp",
     "fetch/FormDataBytesConsumerTest.cpp",
     "fetch/ReadableStreamBytesConsumerTest.cpp",

third_party/WebKit/Source/modules/cachestorage/Cache.cpp

@@ -205,16 +205,13 @@ void RecordResponseTypeForAdd(const Member<Response>& response) {
 
 bool VaryHeaderContainsAsterisk(const Response* response) {
   const FetchHeaderList* headers = response->headers()->HeaderList();
-  for (size_t i = 0; i < headers->size(); ++i) {
-    const FetchHeaderList::Header& header = headers->Entry(i);
-    if (header.first == "vary") {
-      Vector<String> fields;
-      header.second.Split(',', fields);
-      for (size_t j = 0; j < fields.size(); ++j) {
-        if (fields[j].StripWhiteSpace() == "*")
-          return true;
-      }
-    }
+  String varyHeader;
+  if (headers->Get("vary", varyHeader)) {
+    Vector<String> fields;
+    varyHeader.Split(',', fields);
+    return std::any_of(fields.begin(), fields.end(), [](const String& field) {
+      return field.StripWhiteSpace() == "*";
+    });
   }
   return false;
 }

third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp

@@ -5,9 +5,10 @@
 #include "modules/fetch/FetchHeaderList.h"
 
 #include <algorithm>
+#include <utility>
 #include "platform/loader/fetch/FetchUtils.h"
 #include "platform/network/HTTPParsers.h"
-#include "platform/wtf/PtrUtil.h"
+#include "platform/wtf/text/StringBuilder.h"
 
 namespace blink {
 
@@ -17,8 +18,8 @@ FetchHeaderList* FetchHeaderList::Create() {
 
 FetchHeaderList* FetchHeaderList::Clone() const {
   FetchHeaderList* list = Create();
-  for (size_t i = 0; i < header_list_.size(); ++i)
-    list->Append(header_list_[i]->first, header_list_[i]->second);
+  for (const auto& header : header_list_)
+    list->Append(header.first, header.second);
   return list;
 }
 
@@ -27,35 +28,36 @@ FetchHeaderList::FetchHeaderList() {}
 FetchHeaderList::~FetchHeaderList() {}
 
 void FetchHeaderList::Append(const String& name, const String& value) {
+  // https://fetch.spec.whatwg.org/#concept-header-list-append
   // "To append a name/value (|name|/|value|) pair to a header list (|list|),
-  // append a new header whose name is |name|, byte lowercased, and value is
-  // |value|, to |list|."
-  header_list_.push_back(
-      WTF::WrapUnique(new Header(name.DeprecatedLower(), value)));
+  // run these steps:
+  // 1. If |list| contains |name|, then set |name| to the first such header’s
+  //    name. This reuses the casing of the name of the header already in the
+  //    header list, if any. If there are multiple matched headers their names
+  //    will all be identical.
+  // 2. Append a new header whose name is |name| and |value| is |value| to
+  //    |list|."
+  auto header = header_list_.find(name);
+  if (header != header_list_.end())
+    header_list_.insert(std::make_pair(header->first, value));
+  else
+    header_list_.insert(std::make_pair(name, value));
 }
 
 void FetchHeaderList::Set(const String& name, const String& value) {
+  // https://fetch.spec.whatwg.org/#concept-header-list-set
   // "To set a name/value (|name|/|value|) pair in a header list (|list|), run
   // these steps:
-  // 1. Byte lowercase |name|.
-  // 2. If there are any headers in |list| whose name is |name|, set the value
-  //    of the first such header to |value| and remove the others.
-  // 3. Otherwise, append a new header whose name is |name| and value is
-  //    |value|, to |list|."
-  const String lowercased_name = name.DeprecatedLower();
-  for (size_t i = 0; i < header_list_.size(); ++i) {
-    if (header_list_[i]->first == lowercased_name) {
-      header_list_[i]->second = value;
-      for (size_t j = i + 1; j < header_list_.size();) {
-        if (header_list_[j]->first == lowercased_name)
-          header_list_.erase(j);
-        else
-          ++j;
-      }
-      return;
-    }
-  }
-  header_list_.push_back(WTF::MakeUnique<Header>(lowercased_name, value));
+  // 1. If |list| contains |name|, then set the value of the first such header
+  //    to |value| and remove the others.
+  // 2. Otherwise, append a new header whose name is |name| and value is
+  //    |value| to |list|."
+  auto existingHeader = header_list_.find(name);
+  const FetchHeaderList::Header newHeader = std::make_pair(
+      existingHeader != header_list_.end() ? existingHeader->first : name,
+      value);
+  header_list_.erase(name);
+  header_list_.insert(newHeader);
 }
 
 String FetchHeaderList::ExtractMIMEType() const {
@@ -75,85 +77,87 @@ size_t FetchHeaderList::size() const {
 }
 
 void FetchHeaderList::Remove(const String& name) {
-  // "To delete a name (|name|) from a header list (|list|), remove all headers
-  // whose name is |name|, byte lowercased, from |list|."
-  const String lowercased_name = name.DeprecatedLower();
-  for (size_t i = 0; i < header_list_.size();) {
-    if (header_list_[i]->first == lowercased_name)
-      header_list_.erase(i);
-    else
-      ++i;
-  }
+  // https://fetch.spec.whatwg.org/#concept-header-list-delete
+  // "To delete a name (name) from a header list (list), remove all headers
+  // whose name is a byte-case-insensitive match for name from list."
+  header_list_.erase(name);
 }
 
 bool FetchHeaderList::Get(const String& name, String& result) const {
-  const String lowercased_name = name.DeprecatedLower();
+  // https://fetch.spec.whatwg.org/#concept-header-list-combine
+  // "To combine a name/value (|name|/|value|) pair in a header list (|list|),
+  // run these steps:
+  // 1. If |list| contains |name|, then set the value of the first such header
+  //    to its value, followed by 0x2C 0x20, followed by |value|.
+  // 2. Otherwise, append a new header whose name is |name| and value is
+  //    |value| to |list|."
+  StringBuilder resultBuilder;
   bool found = false;
-  for (const auto& header : header_list_) {
-    if (header->first == lowercased_name) {
-      if (!found) {
-        result = "";
-        result.Append(header->second);
-        found = true;
-      } else {
-        result.Append(",");
-        result.Append(header->second);
-      }
+  auto range = header_list_.equal_range(name);
+  for (auto header = range.first; header != range.second; ++header) {
+    if (!found) {
+      resultBuilder.Append(header->second);
+      found = true;
+    } else {
+      // TODO(rakuco): This must be ", " instead. crbug.com/700434.
+      resultBuilder.Append(',');
+      resultBuilder.Append(header->second);
     }
   }
+  if (found)
+    result = resultBuilder.ToString();
   return found;
 }
 
+// This is going to be removed: see crbug.com/645492.
 void FetchHeaderList::GetAll(const String& name, Vector<String>& result) const {
-  const String lowercased_name = name.DeprecatedLower();
   result.Clear();
-  for (size_t i = 0; i < header_list_.size(); ++i) {
-    if (header_list_[i]->first == lowercased_name)
-      result.push_back(header_list_[i]->second);
+  auto range = header_list_.equal_range(name);
+  for (auto header = range.first; header != range.second; ++header) {
+    result.push_back(header->second);
   }
 }
 
 bool FetchHeaderList::Has(const String& name) const {
-  const String lowercased_name = name.DeprecatedLower();
-  for (size_t i = 0; i < header_list_.size(); ++i) {
-    if (header_list_[i]->first == lowercased_name)
-      return true;
-  }
-  return false;
+  // https://fetch.spec.whatwg.org/#header-list-contains
+  // "A header list (|list|) contains a name (|name|) if |list| contains a
+  // header whose name is a byte-case-insensitive match for |name|."
+  return header_list_.find(name) != header_list_.end();
 }
 
 void FetchHeaderList::ClearList() {
-  header_list_.Clear();
+  header_list_.clear();
 }
 
 bool FetchHeaderList::ContainsNonSimpleHeader() const {
-  for (size_t i = 0; i < header_list_.size(); ++i) {
-    if (!FetchUtils::IsSimpleHeader(AtomicString(header_list_[i]->first),
-                                    AtomicString(header_list_[i]->second)))
-      return true;
-  }
-  return false;
+  return std::any_of(
+      header_list_.cbegin(), header_list_.cend(), [](const Header& header) {
+        return !FetchUtils::IsSimpleHeader(AtomicString(header.first),
+                                           AtomicString(header.second));
+      });
 }
 
-void FetchHeaderList::SortAndCombine() {
+Vector<FetchHeaderList::Header> FetchHeaderList::SortAndCombine() const {
   // https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine
-  // "To sort and combine a header list..."
-  if (header_list_.IsEmpty())
-    return;
-
-  std::sort(
-      header_list_.begin(), header_list_.end(),
-      [](const std::unique_ptr<Header>& a, const std::unique_ptr<Header>& b) {
-        return WTF::CodePointCompareLessThan(a->first, b->first);
-      });
-
-  for (size_t index = header_list_.size() - 1; index > 0; --index) {
-    if (header_list_[index - 1]->first == header_list_[index]->first) {
-      header_list_[index - 1]->second.Append(",");
-      header_list_[index - 1]->second.Append(header_list_[index]->second);
-      header_list_.erase(index, 1);
-    }
+  // "To sort and combine a header list (|list|), run these steps:
+  // 1. Let |headers| be an empty list of name-value pairs with the key being
+  //    the name and value the value.
+  // 2. Let |names| be all the names of the headers in |list|, byte-lowercased,
+  //    with duplicates removed, and finally sorted lexicographically.
+  // 3. For each |name| in |names|, run these substeps:
+  //    1. Let |value| be the combined value given |name| and |list|.
+  //    2. Append |name-value| to |headers|.
+  // 4. Return |headers|."
+  Vector<FetchHeaderList::Header> ret;
+  for (auto it = header_list_.cbegin(); it != header_list_.cend();) {
+    const String& headerName = it->first.LowerASCII();
+    String combinedValue;
+    Get(headerName, combinedValue);
+    ret.emplace_back(std::make_pair(headerName, combinedValue));
+    // Skip to the next distinct key.
+    it = header_list_.upper_bound(headerName);
   }
+  return ret;
 }
 
 bool FetchHeaderList::IsValidHeaderName(const String& name) {

third_party/WebKit/Source/modules/fetch/FetchHeaderList.h

@@ -5,11 +5,10 @@
 #ifndef FetchHeaderList_h
 #define FetchHeaderList_h
 
-#include <memory>
+#include <map>
 #include <utility>
 #include "modules/ModulesExport.h"
 #include "platform/heap/Handle.h"
-#include "platform/wtf/PassRefPtr.h"
 #include "platform/wtf/Vector.h"
 #include "platform/wtf/text/WTFString.h"
 
@@ -21,6 +20,12 @@ class Header;
 class MODULES_EXPORT FetchHeaderList final
     : public GarbageCollectedFinalized<FetchHeaderList> {
  public:
+  struct ByteCaseInsensitiveCompare {
+    bool operator()(const String& lhs, const String& rhs) const {
+      return CodePointCompareLessThan(lhs.LowerASCII(), rhs.LowerASCII());
+    }
+  };
+
   typedef std::pair<String, String> Header;
   static FetchHeaderList* Create();
   FetchHeaderList* Clone() const;
@@ -39,11 +44,11 @@ class MODULES_EXPORT FetchHeaderList final
   void ClearList();
 
   bool ContainsNonSimpleHeader() const;
-  void SortAndCombine();
+  Vector<Header> SortAndCombine() const;
 
-  const Vector<std::unique_ptr<Header>>& List() const { return header_list_; }
-  const Header& Entry(size_t index) const {
-    return *(header_list_[index].get());
+  const std::multimap<String, String, ByteCaseInsensitiveCompare>& List()
+      const {
+    return header_list_;
   }
 
   static bool IsValidHeaderName(const String&);
@@ -53,7 +58,20 @@ class MODULES_EXPORT FetchHeaderList final
 
  private:
   FetchHeaderList();
-  Vector<std::unique_ptr<Header>> header_list_;
+
+  // While using STL data structures in Blink is not very common or
+  // encouraged, we do need a multimap here. The closest WTF structure
+  // comparable to what we need would be a
+  //   HashMap<String, Vector<String>>
+  // but it is not a "flat" data structure like std::multimap is. The
+  // size() of the HashMap is the number of distinct header names, not
+  // the total number of headers and values on the list.
+  // This would cause FetchHeaderList::size() to have to manually
+  // iterate through all keys and vectors in the HashMap. Similarly,
+  // list() would require callers to manually iterate through the
+  // HashMap's keys and value vector, and so would
+  // containsNonSimpleHeader().
+  std::multimap<String, String, ByteCaseInsensitiveCompare> header_list_;
 };
 
 }  // namespace blink