strip request-body-header for redireting from POST to GET

[JuniorHsu]

---

Preview | Diff

[JuniorHsu]

This addresses #609. I'll work on test and gecko implementation.
@annevk does the patch make sense?

[annevk]

This looks pretty good to me, modulo nits. Thanks for working on this!

@ricea do you have any further feedback? Should someone else from Chrome be consulted?

@youennf I'm not sure if WebKit does this at the moment, but your review would be appreciated. And we'll file a bug if there are any tests you might fail.

[annevk]

There's a markup error here.

[annevk]

No need for the space before "Set".

[annevk]

#944 is a bit related to this, if you wanted to pick up more issues in this space.

[ricea]

lgtm. It would be nice if there were tests, but I guess whichever browser implements this first will write them. CC: @yutakahirano who was also active on https://bugs.chromium.org/p/chromium/issues/detail?id=765898.

[annevk]

@davidben are you on board with stripping more headers than Content-Type and Content-Length?

[davidben]

I like how in 2019, we still don't know what HTTP redirects are. :-)

This is a list of request headers to remove, right? This list is strange. Content-Disposition, Content-Range, and ETag are defined as response headers.

Adding @MattMenke2 and @mnot who may have thoughts about this. One hopes the risk of changes here is tiny, but we should incur that risk at most once.

[MattMenke2]

There's also Transfer-Encoding (No idea if Transfer-Encoding is generally used in place of Content-Encoding for uploads, but if we're encouraging using these sorts of entity-body headers symmetrically for request and response bodies, seems like it should be here).

Does seem like we should create some more formal grouping of entity-body headers that can apply both to uploads and downloads, if we want to have some sort of support for consumers setting them in web-platform uploads.

[davidben]

Can Transfer-Encoding be set by web content? I would hope not since it's a property of how the browser arranges bytes on the wire, rather than the request itself. In that case, hopefully it gets applied below redirect handling anyway.

[MattMenke2]

The same is true of Content-Encoding, no? I mean, they're nominally different, but in practice, not so much. Regardless, there's discussion of a streaming upload API, which would use chunked uploads. No idea if Chrome's pre-existing chunked upload support uses.

[annevk]

Transfer-Encoding is blocked, Content-Encoding is not, see https://fetch.spec.whatwg.org/#forbidden-header-name.

[MattMenke2]

But we still have rules for how forbidden headers are mutated as we follow redirects. Also, I don't think any browser should be letting web content set "Content-Encoding: chunked" directly.

[MattMenke2]

Though I suppose chunked is generally advertised as a Transfer-Encoding, it just seems not a great idea.

[davidben]

I don't think Content-Encoding: chunked means anything, so it shouldn't be any different from Content-Encoding: garbage. While Transfer-Encoding nominally can also do compression, I don't think any browser supports it.

The two are effectively orthogonal. Content-Encoding is whether there is compression applied to the body and should be supplied by whoever supplied the body. Transfer-Encoding is largely a symptom of HTTP/1.1's poor layering. It should be supplied by the low-level HTTP/1.1 (and not HTTP/2) encoding logic, well below Fetch and the web platform. Alas, HTTP headers are a leaky abstraction, so we're stuck needling to forbid Transfer-Encoding at the Fetch level, but we should layer things so we otherwise don't need to think about it.

[MattMenke2]

I do think that consumers have just as much control over Transfer-Encoding: chunked (Assuming we expose a streaming upload API) as they have over Content-Length. Neither one can they set directly, but they can set them indirectly (By their selection of upload body or their choice of upload API).

[JuniorHsu]

We're wondering if we need to remove request-body-header (Content-Type, Content-Language... ) from request headers for GET->GET redirection if the client adds them, e.g, fetch(url, {headers: {"Content-Encoding": "Identity"}} for a 301/302/303 response.

We suspect that leaving GET->GET alone is probably the quickest way to consensus, and I'd like to include GET->GET in the web-platform tests.

[annevk]

@JuniorHsu if we want to leave GET -> GET alone we do have to add a check for 303 in the specification: that method is not GET or HEAD. As the current wording does affect GET -> GET for 303s.

[JuniorHsu]

@JuniorHsu if we want to leave GET -> GET alone we do have to add a check for 303 in the specification: that method is not GET or HEAD. As the current wording does affect GET -> GET for 303s.

Thanks for finding this. It's good to be consistent

[annevk]

Thanks, this looks great. Also thanks all for the comments. If there are no further issues here I plan to merge this before the end of the week and @JuniorHsu or I will ensure browser bugs as a result of failing tests are filed as well.

[JuniorHsu]

Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1530230

[annevk]

More bugs:

• https://bugs.webkit.org/show_bug.cgi?id=205119
• https://bugs.chromium.org/p/chromium/issues/detail?id=1033056