1508699 - Fetch() should not store lowercase header names

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Andrea Marchesini [:baku]]

We currently "lowercase" header names. This is not written in the spec and it makes several WPTs to fail. We also don't merge headers. I'll fix this in the same patch.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: name.patch

"A header list list contains a name name if list contains a header whose name is a byte-case-insensitive match for name. "

Comment 2

[Andrea Marchesini [:baku]]

Attachment: name.patch

Comment 3

[Andrea Marchesini [:baku]]

asuth, can you take a look at this patch?

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9026441 [details] [diff] [review]
name.patch

Review of attachment 9026441 [details] [diff] [review]:
-----------------------------------------------------------------

restating:
- Some nsACString types are converted to nsCString because .get() is used and nsCString guarantees zero-termination but nsACString is dumb and does not.
- Capitalization is ignored throughout the logic except...
- The first capitalization seen for a (case-insensitive) header name is latched so that if we see "foo" then "fOO" then "Foo", "foo" will win.

::: dom/fetch/FetchDriver.cpp
@@ +1450,5 @@
>  FetchDriver::SetRequestHeaders(nsIHttpChannel* aChannel) const
>  {
>    MOZ_ASSERT(aChannel);
>  
> +  // nsIHttpChannel has a set of pre-configured headers (Accept,

Thanks for this very helpful comment!

::: dom/fetch/InternalHeaders.cpp
@@ +524,5 @@
>      }
>  
>      if (!found) {
> +      Entry newEntry = entry;
> +      ToLowerCase(newEntry.mName);

Restating: The "sort and combine" algorithm at https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine calls for the headers to be byte-lowercased.  Previously, this was handled by everything being lowercased, but since the names are no longer normalized to be lowercased elsewhere in the class, the normalization needs to happen here.

::: dom/fetch/InternalHeaders.h
@@ +160,5 @@
>             IsForbiddenRequestNoCorsHeader(aName, aValue) ||
>             IsForbiddenResponseHeader(aName);
>    }
>  
> +  void ReuseExistingNameIfExists(nsCString& aName) const;

Please add a comment explaining the intent of this method.  Something like:

This method updates the passed name to match the capitalization of a header with the same name (ignoring case, per the spec).

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0edf62695f1d
Fetch() should not send lowercase header names, r=asuth

Comment 6

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/0edf62695f1d