Bug 177829 – XMLHttpRequest.setRequestHeader() should allow Content-Transfer-Encoding header; remove duplicate logic to check for a forbidden XHR header field

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

Remove duplicate logic to check for a forbidden XHR header field.

[Daniel Bates]

Created attachment 322561 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 322561 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=322561&action=review

> LayoutTests/fast/xmlhttprequest/set-dangerous-headers-in-dashboard.html:26
> +    // CONTENT-TRANSFER-ENCODING is no longer forbidden since <https://www.w3.org/TR/2012/WD-XMLHttpRequest-20121206/>.
>      req.setRequestHeader("CONTENT-TRANSFER-ENCODING", "foobar");

It is strange to keep this header filed name tested here - we don't test other safe names in these tests. It would be cleaner to remove it from these tests, and to add a new one for this fix.

This is a suggested change, not a condition for r+.

[Daniel Bates]

(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 322561 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=322561&action=review
> 
> > LayoutTests/fast/xmlhttprequest/set-dangerous-headers-in-dashboard.html:26
> > +    // CONTENT-TRANSFER-ENCODING is no longer forbidden since <https://www.w3.org/TR/2012/WD-XMLHttpRequest-20121206/>.
> >      req.setRequestHeader("CONTENT-TRANSFER-ENCODING", "foobar");
> 
> It is strange to keep this header filed name tested here - we don't test
> other safe names in these tests. 

Notice that all these layout tests test the now-considered safe header AUTHORIZATION, e.g. <https://trac.webkit.org/browser/trunk/LayoutTests/http/tests/xmlhttprequest/set-dangerous-headers.html?rev=163915#L16>.

I thought to keep the test for Content-Type-Encoding for similar historical preservation.

[Daniel Bates]

(In reply to Daniel Bates from comment #3)
> (In reply to Alexey Proskuryakov from comment #2)
> > Comment on attachment 322561 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=322561&action=review
> > 
> > > LayoutTests/fast/xmlhttprequest/set-dangerous-headers-in-dashboard.html:26
> > > +    // CONTENT-TRANSFER-ENCODING is no longer forbidden since <https://www.w3.org/TR/2012/WD-XMLHttpRequest-20121206/>.
> > >      req.setRequestHeader("CONTENT-TRANSFER-ENCODING", "foobar");
> > 
> > It is strange to keep this header filed name tested here - we don't test
> > other safe names in these tests. 
> 
> Notice that all these layout tests test the now-considered safe header
> AUTHORIZATION, e.g.
> <https://trac.webkit.org/browser/trunk/LayoutTests/http/tests/xmlhttprequest/
> set-dangerous-headers.html?rev=163915#L16>.
> 
> I thought to keep the test for Content-Type-Encoding for similar historical
> preservation.

Will remove test for Content-Transfer-Encoding before landing.

[Daniel Bates]

Committed r222807: <http://trac.webkit.org/changeset/222807>

[Radar WebKit Bug Importer]

<rdar://problem/34798441>

[Daniel Bates]

Forgot to mention this change also removed User-Agent from the list of forbidden headers. This header is no longer forbidden in the XHR standard, <https://xhr.spec.whatwg.org>, last updated 09/08/2017.

[Daniel Bates]

Updated tests and expected results now that we allow setting the Content-Transfer-Encoding and User-Agent headers in <http://trac.webkit.org/changeset/222817>.

[Ryan Haddad]

One more test re-baseline in https://trac.webkit.org/changeset/222852/webkit to account for a change made to the tests themselves in http://trac.webkit.org/changeset/222817.