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

303 redirects should preserve HEAD #753

Closed
davidben opened this issue Jun 1, 2018 · 9 comments
Closed

303 redirects should preserve HEAD #753

davidben opened this issue Jun 1, 2018 · 9 comments
Labels
good first issue Ideal for someone new to a WHATWG standard or software project topic: http topic: redirects

Comments

@davidben
Copy link
Collaborator

davidben commented Jun 1, 2018

(@yutakahirano passed this along to me to dig into.)

The Fetch spec, in step 11 here, says: "If either actualResponse’s status is 301 or 302 and request’s method is POST, or actualResponse’s status is 303, set request’s method to GET and request’s body to null."
https://fetch.spec.whatwg.org/#http-redirect-fetch

This implies that a 303 in response to a HEAD gets converted to GET. However wpt says it stays as a HEAD. That is Chrome's behavior as well and, loading the tests in Firefox, seem to be Firefox's behavior.
http://w3c-test.org/fetch/api/redirect/redirect-method.any.html

Our behavior appears to date to here:
https://bugs.chromium.org/p/chromium/issues/detail?id=102130
https://trac.ietf.org/trac/httpbis/ticket/310
which cites what ultimately became the new HTTP RFCs.

The "clarification" in the IETF specification is absurdly unclear, but the intent and reality do seem to align on 303 not transforming HEAD. Additionally, it seems odd for a HEAD request (which expects no body) to ultimately receive a GET response (which has a body). So I believe this is just a mistake in the Fetch spec.

@annevk
Copy link
Member

annevk commented Jun 2, 2018

Thanks for raising this. So we'd add a conditional for method not being HEAD when status is 303.

(Note that there's also another issue here, that we want to remove various Content-* headers as per #609.)

@annevk annevk added good first issue Ideal for someone new to a WHATWG standard or software project topic: redirects labels Jun 2, 2018
@yutakahirano
Copy link
Member

cc: @jungkees

@lorddaedra
Copy link

I send DELETE requests via fetch() to my server and get back 303 status redirects. I expect HEAD requests here... Actually I see GET.

I expect to HEAD because of next thing I will do is window.location.replace(response.url);, so I do not need twice GET requests here...

@lorddaedra
Copy link

even better add option to fetch() for users to give them ability select, which method to use after redirects

follow_redirect_method: {
301: 'HEAD'
302: 'HEAD',
303: 'GET'
}

@annevk
Copy link
Member

annevk commented Jun 15, 2018

@lorddaedra that's not something that the HTTP specification allows for.

@ericlaw1979
Copy link

We encountered this issue on Edge today and plan to align with Chrome to keep HEAD when following a 303.

@annevk
Copy link
Member

annevk commented Aug 2, 2018

@ericlaw1979 great, are you all planning on updating web-platform-tests? Apart from a PR against Fetch that's pretty much all it takes to fix this.

@davidben
Copy link
Collaborator Author

davidben commented Aug 2, 2018

I believe the tests were fine. It's just the spec that was wrong.

@annevk
Copy link
Member

annevk commented Aug 17, 2018

My bad, I should have read OP better. I guess I'll fix Fetch then.

annevk added a commit that referenced this issue Aug 17, 2018
Tests: already tested by fetch/api/redirect/redirect-method.any.js.

Fixes #753.
annevk added a commit that referenced this issue Aug 17, 2018
Tests: already tested by fetch/api/redirect/redirect-method.any.js.

Fixes #753.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Ideal for someone new to a WHATWG standard or software project topic: http topic: redirects
Development

No branches or pull requests

5 participants