Closed Bug 1286743 Opened 4 years ago Closed 4 years ago

[XHR2][Fetch] HEAD requests with a null body should be assigned a Content-Length of 0

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: wisniewskit, Assigned: wisniewskit)

References

Details

(Whiteboard: [necko-active])

Attachments

(1 file, 2 obsolete files)

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.
Here's a patch to make Firefox match the spec.

Successful try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=24fba7e55ebb
Attachment #8770800 - Flags: review?(dd.mozilla)
Whiteboard: [necko-active]
Attachment #8770800 - Flags: review?(dd.mozilla) → review+
> 
> 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?
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?
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.
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.
Flags: needinfo?(annevk)
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.
Flags: needinfo?(annevk)
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.
Flags: needinfo?(annevk)
I think that means wontfix.. reopen if I got that wrong
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
We probably want to update the tests, but we could also file a bug against WPT I suppose.
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).
Attachment #8770800 - Attachment is obsolete: true
Attachment #8772033 - Flags: review?(annevk)
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?
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.
Attachment #8772033 - Attachment is obsolete: true
Attachment #8772033 - Flags: review?(annevk)
Attachment #8772060 - Flags: review?(annevk)
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!
Attachment #8772060 - Flags: review?(annevk) → review+
No problem. Might as well request checkin now, too.
Keywords: checkin-needed
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Status: REOPENED → ASSIGNED
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b1b89c4a8e6a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.