Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent redirect loop in fetch/api/redirect/redirect-location.html #9970

Merged

Conversation

Projects
None yet
6 participants
@ricea
Copy link
Contributor

ricea commented Mar 12, 2018

The value of invalidLocationUrl, "#invalidurl:" is actually treated as a valid
URL by redirect.py, leading to a redirect loop. This makes the test take over 6
seconds in Chrome, causing it to time out. Change invalidLocationUrl to
"invalidurl:" to avoid the redirect loop and make the test faster.

See Chrome issue http://crbug.com/815286 about the test timing out.

Prevent redirect loop in fetch/api/redirect/redirect-location.html
The value of invalidLocationUrl, "#invalidurl:" is actually treated as a valid
URL by redirect.py, leading to a redirect loop. This makes the test take over 6
seconds in Chrome, causing it to time out. Change invalidLocationUrl to
"invalidurl:" to avoid the redirect loop and make the test faster.

See Chrome issue http://crbug.com/815286 about the test timing out.

@wpt-pr-bot wpt-pr-bot added the fetch label Mar 12, 2018

@wpt-pr-bot wpt-pr-bot requested review from annevk, jdm, mnot, youennf and yutakahirano Mar 12, 2018

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 12, 2018

Build PASSED

Started: 2018-03-13 08:32:21
Finished: 2018-03-13 08:43:54

Failing Jobs

  • chrome:dev

View more information about this build on:

@jdm

jdm approved these changes Mar 12, 2018

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 12, 2018

@ricea In what sense do you mean "invalid" for URLs? We have tests that ensure specifically that the URL parser accepts input with only a scheme followed by a colon: https://github.com/w3c/web-platform-tests/blob/3c5d8637bb/url/urltestdata.json#L4735-L4772

Runnable test case: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5824

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Mar 12, 2018

Yeah, something: would be rejected because it's not an HTTP(S) URL, not because it doesn't parse. If you want something that doesn't parse, use https://test:test/ or some such.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 12, 2018

Ok, sounds good.

@ricea

This comment has been minimized.

Copy link
Contributor Author

ricea commented Mar 13, 2018

@SimonSapin The intent of the test is not completely clear to me. In this context, I think "invalid URL" probably means "something which you can't successfully redirect to". It doesn't appear that the redirect loop was intentional.

"invalidurl:" prevents redirect loop by causing redirect.py to parse the scheme as "invalidurl". When redirect.py parses the scheme as "", "http" or "https", then it adds its query arguments to the URL, creating the redirect loop. Something like "https://invalid.invalid./" would be another way to prevent a redirect loop, by causing redirect.py to construct a new URL that wouldn't point back to itself.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 13, 2018

Yeah, feel free to ignore my earlier comment. I was thinking of validity in terms of URL parsing exclusively, but that’s not what’s relevant here.

@ricea ricea merged commit 8f7d065 into web-platform-tests:master Mar 13, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ricea ricea deleted the ricea:fetch-api-redirect-location-invalid.url branch Mar 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.