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

Should Content-Type header be stripped when redirecting from POST to GET? #609

Closed
ricea opened this issue Sep 28, 2017 · 12 comments
Closed
Labels
good first issue Ideal for someone new to a WHATWG standard or software project topic: redirects

Comments

@ricea
Copy link
Collaborator

ricea commented Sep 28, 2017

The reporter of Chrome issue https://bugs.chromium.org/p/chromium/issues/detail?id=765898 ("XHR preserves Content-Type on cross-origin 303 redirect") argues that when a 303 redirect causes a request to be redirected from POST to GET and the request body stripped, the Content-Type header should also be removed.

Chrome's current behaviour differs depending on the type of fetch that is being performed.

Before settling on a single behaviour in Chrome it would be nice to be definitive about what the standard behaviour is.

My understanding from reading the above Chrome issue is that the standard currently implies that the header is not removed, but removing it has better compatibility with servers.

@annevk
Copy link
Member

annevk commented Sep 28, 2017

I guess we should do this for 301/302 as well when we change the method to GET there? If someone is willing to write tests I'm happy to file all the browser bugs (if any) and change the specification.

It seems a little weird, but since we modify the request anyway we might as well do this too.

Should we remove other Content-* headers?

@annevk
Copy link
Member

annevk commented Sep 28, 2017

cc @mnot

@annevk annevk added needs tests Moving the issue forward requires someone to write tests needsinfo labels Sep 28, 2017
@mnot
Copy link
Member

mnot commented Sep 29, 2017

Yeah, for 303 it's truely a different request, so it doesn't make sense to talk about the old request body.

For 301 and 302 where you change the method, I think the same argument applies; it doesn't make sense to talk about a request body that isn't there.

@annevk
Copy link
Member

annevk commented Nov 7, 2017

@mnot thanks, but we already drop the body. The question is what should happen with Content-Type and potentially other Content-* request headers.

@mnot
Copy link
Member

mnot commented Nov 8, 2017

Yep; Content-* headers talk about the body.

There are cases when there are headers that reference a missing body, but I don't think this is one of them; they're mostly around responses that refer to things in caches or the state of the resource after a change.

@annevk
Copy link
Member

annevk commented Nov 8, 2017

So should we drop all headers that start with Content- or only a set of known header names? E.g., if a developer set Content-Pony, would it be okay to drop that for these redirects?

@annevk
Copy link
Member

annevk commented Feb 20, 2018

@mnot ping.

@wisniewskit
Copy link

I just closed XHR issue 192 (whatwg/xhr#192) of this, but was wondering if the inconsistency I found there is related to this. Browsers are currently passing the "301 POST with string and explicit Content-Type safelisted" XHR WPT in /xhr/send-redirect-to-cors.htm, but I don't believe they should if we're meant to strip these content- headers (since it's a 301 redirect that expects the body to be nulled, so it seems it should not be expecting the content-type to remain).

@mnot
Copy link
Member

mnot commented Feb 20, 2018

Sorry, missed the notification.

Content-* is a convention; there isn't a hard-and-fast rule that all content-specific headers need to start with it, or that all headers starting with it are about the content (although it would be weird if they weren't).

Safest thing to do for now would probably be to enumerate the applicable headers to remove; since we're talking about a request, I think that's Content-Disposition, Content-Encoding, Content-Language, Content-Location, Content-Range, Content-Type, ETag.

I'm assuming that things "below" fetch will be managed appropriately (e.g., Connection, CORS) for the new request.

That's not terribly satisfying. When the method is rewritten, I'd argue that the request is fundamentally new, and the application needs to decide whether the various headers it might have added are still applicable -- i.e., the redirect should NOT be automatic. But I recognise that allowing that causes other headaches.

You'll probably want to enumerate the headers as a list in Fetch, so you can add to it; e.g., the proposed Integrity header might be a candidate.

@annevk
Copy link
Member

annevk commented Feb 21, 2018

The web platform follows redirects (and includes any headers on them), by default. And exposing redirects is a security concern at this point (unless they provide some opt-in I'm okay to be read header we haven't defined yet). So yeah, redirects are just different on "web platform HTTP" I guess.

@annevk annevk added good first issue Ideal for someone new to a WHATWG standard or software project and removed needsinfo labels Feb 21, 2018
@annevk
Copy link
Member

annevk commented Feb 21, 2018

Okay, so what I think we should here is:

  1. Define "request-body-header name" in https://fetch.spec.whatwg.org/#terminology-headers and make it the list of headers given by @mnot above.
  2. Then in https://fetch.spec.whatwg.org/#concept-http-redirect-fetch change the "If either actualResponse’s status is 301 or 302 and request’s method is POST, or actualResponse’s status is 303" step to make the actions ("set request’s method to GET and request’s body to null") take place as two steps in a nested list and add a step that removes headers from the header list that match the earlier introduced definition.

I'm happy to help out with the tests.

@annevk
Copy link
Member

annevk commented Dec 3, 2019

@JuniorHsu wrote a PR for this in #977!

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: redirects
Development

No branches or pull requests

5 participants
@mnot @wisniewskit @annevk @ricea and others