Sort header names in Access-Control-Request-Headers

Sort header names in Access-Control-Request-Headers

This is required by the Fetch API Spec:
https://fetch.spec.whatwg.org/#cors-preflight-fetch-0

BUG=452391
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189183

[hiroshige, 2015-01-27 09:41:39]

hiroshige@chromium.org changed reviewers:
+ mkwst@chromium.org, tyoshino@chromium.org

[Mike West, 2015-01-27 10:56:49]

Test?

[tyoshino (SeeGerritForStatus), 2015-01-27 12:22:01]

On 2015/01/27 10:56:49, Mike West wrote:
> Test?

HTTPHeaderMap is using case folding hash but the key itself is stored as-is?

[tyoshino (SeeGerritForStatus), 2015-01-27 12:22:21]

On 2015/01/27 12:22:01, tyoshino wrote:
> On 2015/01/27 10:56:49, Mike West wrote:
> > Test?
> 
> HTTPHeaderMap is using case folding hash but the key itself is stored as-is?

Sorry. This isn't reply to Mike.

[hiroshige, 2015-01-27 13:07:31]

Thanks for comments!

> Test?
I'll add tests and update this CL later.
(Actually I have some tests but am waiting for another CL to be landed to avoid
git conflicts)

> HTTPHeaderMap is using case folding hash but the key itself is stored as-is?
Hmm, I'll take a look.
(and try to make test cases for this; the current tests I have for this don't
expose this.)

[hiroshige, 2015-01-28 10:57:44]

> > Test?
Add Fetch and XHR tests.

> > HTTPHeaderMap is using case folding hash but the key itself is stored as-is?
Yes, stored as-is.
In Fetch API case, the header names are lowered in
Source/modules/fetch/FetchHeaderList.cpp before constructing HTTPHeaderMap.
In XHR case, this is not the case so I added lower() before sorting.

[Mike West, 2015-01-28 11:22:44]

LGTM if bots are happy, thanks for adding tests.

[tyoshino (SeeGerritForStatus), 2015-01-29 10:44:53]

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
File LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js
(right):

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js:134: //
Test Access-Control-ARequest-Headers is sorted https://crbug.com/452391
ARequest -> Request

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js:238: //
Test Access-Control-ARequest-Headers is sorted https://crbug.com/452391
ARequest -> Request

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js:349: //
Test Access-Control-ARequest-Headers is sorted https://crbug.com/452391
ARequest -> Request

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js:461: //
Test Access-Control-ARequest-Headers is sorted https://crbug.com/452391
ditto

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/x...
File
LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-sorted.php
(right):

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-sorted.php:2:
require_once '../../resources/portabilityLayer.php';
is this needed?

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-sorted.php:10:
// Spec: http://www.w3.org/TR/cors/#cross-origin-request-with-preflight-0
please cite fetch.spec.whatwg.org

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-sorted.php:15:
header("HTTP/1.1 400");
does this work even after calling header() emitting? even so, i slightly prefer
invoking L4-5 after this call.

https://codereview.chromium.org/875363002/diff/60001/Source/core/fetch/CrossO...
File Source/core/fetch/CrossOriginAccessControl.cpp (right):

https://codereview.chromium.org/875363002/diff/60001/Source/core/fetch/CrossO...
Source/core/fetch/CrossOriginAccessControl.cpp:92: //  
http://www.w3.org/TR/cors/#cross-origin-request-with-preflight-0
it's ok to omit the W3C version.

[hiroshige, 2015-01-29 11:08:11]

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
File LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js
(right):

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js:134: //
Test Access-Control-ARequest-Headers is sorted https://crbug.com/452391
On 2015/01/29 10:44:53, tyoshino wrote:
> ARequest -> Request

Done.

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js:238: //
Test Access-Control-ARequest-Headers is sorted https://crbug.com/452391
On 2015/01/29 10:44:53, tyoshino wrote:
> ARequest -> Request

Done.

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js:349: //
Test Access-Control-ARequest-Headers is sorted https://crbug.com/452391
On 2015/01/29 10:44:53, tyoshino wrote:
> ARequest -> Request

Done.

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/f...
LayoutTests/http/tests/fetch/script-tests/fetch-access-control-cors.js:461: //
Test Access-Control-ARequest-Headers is sorted https://crbug.com/452391
On 2015/01/29 10:44:53, tyoshino wrote:
> ditto

Done.

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/x...
File
LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-sorted.php
(right):

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-sorted.php:2:
require_once '../../resources/portabilityLayer.php';
Removed. Also removed from *-lowercase.php, based on which I created
*-sorted.php.

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-sorted.php:10:
// Spec: http://www.w3.org/TR/cors/#cross-origin-request-with-preflight-0
On 2015/01/29 10:44:53, tyoshino wrote:
> please cite http://fetch.spec.whatwg.org

Done.

https://codereview.chromium.org/875363002/diff/60001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-sorted.php:15:
header("HTTP/1.1 400");
On 2015/01/29 10:44:53, tyoshino wrote:
> does this work even after calling header() emitting? even so, i slightly
prefer
> invoking L4-5 after this call.

Done.

https://codereview.chromium.org/875363002/diff/60001/Source/core/fetch/CrossO...
File Source/core/fetch/CrossOriginAccessControl.cpp (right):

https://codereview.chromium.org/875363002/diff/60001/Source/core/fetch/CrossO...
Source/core/fetch/CrossOriginAccessControl.cpp:92: //  
http://www.w3.org/TR/cors/#cross-origin-request-with-preflight-0
On 2015/01/29 10:44:53, tyoshino wrote:
> it's ok to omit the W3C version.

Done.

[tyoshino (SeeGerritForStatus), 2015-01-29 11:13:24]

lgtm

[hiroshige, 2015-01-29 11:18:21]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2015-01-29 11:19:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/875363002/70001

[commit-bot: I haz the power, 2015-01-29 12:34:34]

Committed patchset #5 (id:70001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189183