Porting access-control-preflight-request-header-lowercase to WPT [chromium/src : master]
Austin James Ahlstrom (Gerrit)
Keep me posted with suggestions! Thank you!
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Blink WPT Bot (Gerrit)
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/w3c/web-platform-tests/pull/6931.
If this CL lands and Travis CI upstream is green, we will auto-merge the PR.
Note: Please check the Travis CI status (at the bottom of the PR) before landing this CL and only land this CL if the status is green. Otherwise a human needs to step in and resolve it manually. (This may be automated in the future, see https://crbug.com/711447)
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md#Automatic-export-process
Yutaka Hirano (Gerrit)
2 comments:
Patch Set #2, Line 7:
headersUnlike the below one, the expectation here is "x-test" is not appear in the header. This should be like:
if request.headers.get("x-test") is None and request.headers.get("X-Test") is None:
response.headers.set("Access-Control-Allow-Headers", "X-Test")
else:
response.status = 400Patch Set #2, Line 10:
if request.headers.get("x-test") != "":request.headers.get(name) returns None when |name| is not found. So I think this should be
if request.headers.get("x-test"):
or
if request.headers.get("x-test") == "foobar":
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Austin James Ahlstrom (Gerrit)
2 comments:
Patch Set #2, Line 7:
headers
Unlike the below one, the expectation here is "x-test" is not appear in the header. […]
That makes sense! Is it better to put 'if request.headers.get("x-test") is None:' rather than 'if request.headers.get("x-test"):'? For now, I've put it the way you recommended.
Patch Set #2, Line 10:
if request.headers.get("x-test") != "":
request.headers.get(name) returns None when |name| is not found. So I think this should be […]
Got it! Thank you for the tip!
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Blink WPT Bot (Gerrit)
Successfully updated WPT GitHub pull request with new revision "fix": https://github.com/w3c/web-platform-tests/pull/6931
Takeshi Yoshino (Gerrit)
2 comments:
File build/linux/bin/eu-strip:
Added unexpectedly?
Patch Set #3, Line 7:
if request.headers.get("x-test") is None and request.headers.get("X-Test") is None:use raw_headers on request. request.headers is a case in-sensitive dictionary wrapper.
see https://github.com/w3c/web-platform-tests/blob/master/tools/wptserve/wptserve/request.py
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Austin James Ahlstrom (Gerrit)
2 comments:
Added unexpectedly?
Yup, that was an accident! Glad that you noticed that
Patch Set #3, Line 7:
if request.raw_headers.get("x-test") is None and request.raw_headers.get("X-Test") is None:use raw_headers on request. request.headers is a case in-sensitive dictionary wrapper. […]
I didn't know that! Thank you so much for pointing that out, and for the explanatory note! Does it look all right now?
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Blink WPT Bot (Gerrit)
Successfully updated WPT GitHub pull request with new revision "fix": https://github.com/w3c/web-platform-tests/pull/6931
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Takeshi Yoshino (Gerrit)
2 comments:
Patch Set #3, Line 7:
if request.raw_headers.get("x-test") is None and request.raw_headers.get("X-Test") is None:
I didn't know that! Thank you so much for pointing that out, and for the explanatory note! Does it l […]
Right! But see my new comment.
Patch Set #4, Line 7:
if request.raw_headers.get("x-test") is None and request.raw_headers.get("X-Test") is None:Oops, sorry I should have noticed this earlier than I commented about use of raw_headers.
The original test is checking that entries in the Access-Control-Request-Headers header in a preflight are lower cased. So, here you need to extract the header and split the value at comma characters and verify that one of them is "x-test" (lower cased). You don't need raw_headers from the beginning as the value you need to check is in the value part, not the key.
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Blink WPT Bot (Gerrit)
Successfully updated WPT GitHub pull request with new revision "rebase and fix": https://github.com/w3c/web-platform-tests/pull/6931
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Yutaka Hirano (Gerrit)
Patch set 5:Code-Review +1
Austin James Ahlstrom (Gerrit)
1 comment:
Patch Set #4, Line 7:
if "x-test" in [header.strip(" ") for header inOops, sorry I should have noticed this earlier than I commented about use of raw_headers. […]
Thank you for your help on this issue!! I think I've got it now, but please let me know if I missed anything or if there's anything else I can fix!
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Blink WPT Bot (Gerrit)
Successfully updated WPT GitHub pull request with new revision "fix": https://github.com/w3c/web-platform-tests/pull/6931
To view, visit change 620448. To unsubscribe, or for help writing mail filters, visit settings.
Takeshi Yoshino (Gerrit)
Patch set 6:Code-Review +1
Austin James Ahlstrom (Gerrit)
Patch set 6:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot merged this change.
Porting access-control-preflight-request-header-lowercase to WPT
Bug: 745385
Change-Id: Ibbe62d83f0ea45940be1f89f7bfe71ed7c62bf77
Reviewed-on: https://chromium-review.googlesource.com/620448
Reviewed-by: Takeshi Yoshino <tyos...@chromium.org>
Reviewed-by: Yutaka Hirano <yhi...@chromium.org>
Commit-Queue: Austin James Ahlstrom <aahl...@google.com>
Cr-Commit-Position: refs/heads/master@{#498392}
---
M third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
A third_party/WebKit/LayoutTests/external/wpt/XMLHttpRequest/access-control-preflight-request-header-lowercase.htm
A third_party/WebKit/LayoutTests/external/wpt/XMLHttpRequest/resources/access-control-preflight-request-header-lowercase.py
D third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/access-control-preflight-request-header-lowercase-expected.txt
D third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/access-control-preflight-request-header-lowercase.html
D third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-lowercase.php
6 files changed, 45 insertions(+), 55 deletions(-)
Blink WPT Bot (Gerrit)
The WPT PR for this CL has been merged upstream! https://github.com/w3c/web-platform-tests/pull/6931