Thomas Wisniewski Assignee
	Description • 8 years ago

The Fetch spec section 5.5.3 states that HEAD requests with a null body should be given a "0" content-length, just like is already being done for null-bodied POST and PUT requests. This change will let the following web platform tests pass:

http://w3c-test.org/fetch/api/basic/request-headers.html
http://w3c-test.org/fetch/api/basic/request-headers-worker.html
http://w3c-test.org/XMLHttpRequest/send-entity-body-get-head.htm
http://w3c-test.org/XMLHttpRequest/send-entity-body-get-head-async.htm

Note that Chrome currently fails these tests as well though, in case that makes this controversial.

	Thomas Wisniewski Assignee
	Comment 1 • 8 years ago

Here's a patch to make Firefox match the spec.

Successful try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=24fba7e55ebb

	Dragana Damjanovic [:dragana]
	Comment 2 • 8 years ago

> 
> Note that Chrome currently fails these tests as well though, in case that
> makes this controversial.

I do not think that this is going to break something. Just to check, you have not seen any chrome bug that revert this behavior?

	Thomas Wisniewski Assignee
	Comment 3 • 8 years ago

Good question. I do actually see that they changed their behavior in Aug 2015 to match Firefox, despite IE and Safari still sending the 0: https://bugs.chromium.org/p/chromium/issues/detail?id=455939

Now that I've read a bit more, I disagree with their decision to do so. The HTTP 1.1 spec is very clear here:

>The Content-Length entity-header field indicates the size of the entity-body, in decimal number of OCTETs, sent to the recipient or, in the case of the HEAD method, the size of the entity-body that would have been sent had the request been a GET.
>   Content-Length    = "Content-Length" ":" 1*DIGIT

It sounds to me like they intentionally went against the spec and other browsers all for one buggy site, rather than doing the right thing and reaching out to have the site in question fix a long-standing bug with multiple browsers. Maybe I'm wrong in that assessment, though?

	Patrick McManus [:mcmanus]
	Comment 4 • 8 years ago

in general head and get don't have a content-length header - I'm not surprised that sites are surprised by that.

Other than following what somebody wrote down in the fetch spec, what's the upside of making the change? It doesn't even sound like we would match chrome. webcompat has to be the primary driver here.

	Thomas Wisniewski Assignee
	Comment 5 • 8 years ago

Hmm. I'm certainly not against skipping this, but it seems like something that should be sorted out at the spec-level, so I'll needinfo :annevk to give us some insight (when he's back next week). It seems to me that if only one site noticed this recently in 2015 after many years of breakage, then this is less a webcompat issue and more something that needs to be clearly agreed-upon.

	Anne (:annevk)
	Comment 6 • 8 years ago

See https://www.w3.org/Bugs/Public/show_bug.cgi?id=25684 for why this is the case. The rationale there though says that this was in part to match Chrome. Perhaps they fail the tests for a different reason.

	Thomas Wisniewski Assignee
	Comment 7 • 8 years ago

Based on your link and what I found above, It seems to me like Chrome was always sending Content-Length:0 like WebKit, and the spec aligned to match them on in May '05. However, Chrome later changed it that August to match Firefox's behavior of not sending Content-Length:0 (in https://bugs.chromium.org/p/chromium/issues/detail?id=455939) regardless of the spec's choice.

If that's how it played out, then I suspect it would be best to just change the spec. Chrome and Firefox are now aligned to not send Content-Length:0, and I don't see a documented reason on why WebKit/IE ever chose to send the header in the first place.

	Anne (:annevk)
	Comment 8 • 8 years ago

Fair, done: https://github.com/whatwg/fetch/commit/674b4d3f9606b22993bc61e135f2739b668c85a1.

	Patrick McManus [:mcmanus]
	Comment 9 • 8 years ago

I think that means wontfix.. reopen if I got that wrong

	Anne (:annevk)
	Comment 10 • 8 years ago

We probably want to update the tests, but we could also file a bug against WPT I suppose.

	Thomas Wisniewski Assignee
	Comment 11 • 8 years ago

Well, here's a patch to fix the tests and mark Firefox as passing them (Chrome still has another issue with the fetch ones, but also passes the XHR ones with this change).

	Anne (:annevk)
	Comment 12 • 8 years ago

These changes look fine, but it seems we could also change the tests to test "POST" and "PUT" and make sure it's "0" and not "NO" for them. Although some of the other lines suggest they're tested by their own tests?

	Thomas Wisniewski Assignee
	Comment 13 • 8 years ago

Sure, here's a version that expands the existing XHR tests "send-entity-body-empty" and "send-entity-body-none" to test POST, PUT and HEAD rather than just POST. The fetch tests were already checking all three in the file I was modifying.

	Anne (:annevk)
	Comment 14 • 8 years ago

Comment on attachment 8772060 [details] [diff] [review]
1286743-correct_wpts_and_mark_fx_as_passing_them.diff

Review of attachment 8772060 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

	Thomas Wisniewski Assignee
	Comment 15 • 8 years ago

No problem. Might as well request checkin now, too.

	Pulsebot
	Comment 16 • 8 years ago

Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1b89c4a8e6a
Correct WPTs and mark Firefox as passing them where appropriate. r=annevk

	Carsten Book [:Tomcat]
	Comment 17 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/b1b89c4a8e6a