1303025 - Incorrect TypeError: Response body is given with a null body status.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Xidorn Quan [:xidorn] UTC+11]

Steps to reproduce:
1. Open Web Console
2. Enter "new Response(null, {status: 204})"

Expected result:
return a Response object

Actual result:
TypeError: Response body is given with a null body status.


This is an issue for Fetch, but I have no idea what component Fetch belongs to, so just put in ServiceWorker.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

I guess the issue is at [1].

The spec says in [2]:
> If body is non-null, run these substeps:
but the code at [1] only checks whether the value is passed.


[1] https://dxr.mozilla.org/mozilla-central/rev/8a494adbc5cced90a4edf0c98cffde906bf7f3ae/dom/fetch/Response.cpp#194
[2] https://fetch.spec.whatwg.org/#dom-response

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Xidorn, I believe null is being converted to "null" because BodyInit is not a nullable type and DOMString is one of its union types.  If you do this, then it works:

  var r = new Response(undefined, { status: 204 })

Does this make sense?

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

And fetch just goes in Core/DOM

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Except the constructor itself has a nullable operator on the BodyInit type.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Which we are missing in our webidl:

https://dxr.mozilla.org/mozilla-central/source/dom/webidl/Response.webidl#10

Seems we need `optional BodyInit? body` there.

Comment 6

[chrsmrtn]

I too have run into this issue. Please refer to the following specs for fetch. The body getter must return null or stream, so 'undefined' will never occur [1]. Also the status reference for response says that it must be null for 101, 204, 205, or 304 [2]. And finally body is allowed to be null, and "body is said to be done if body is null or body’s stream is closed or errored" [3], So creating a Response with a body of 'undefined' is actually not "done" as the spec describes

[1] https://fetch.spec.whatwg.org/#dom-body-body
[2] https://fetch.spec.whatwg.org/#concept-status
[3] https://fetch.spec.whatwg.org/#concept-body

Comment 7

[chrsmrtn]

noted that Chrome and Edge both allow null as the body when building a Response object

Comment 8

[chrsmrtn]

Please ignore my previous comment. This is not about what other browsers allow, but rather how Firefox has implemented the standard. Cheers

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1303025 - Accept null for body param in constructor of Response.

Review commit: https://reviewboard.mozilla.org/r/212712/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212712/

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

If there is no test checking this, I'll add a wpt later.

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

Looks like there is no such test at the moment...

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8942406 [details]
Bug 1303025 - Accept null for body param in constructor of Response.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/212712/diff/1-2/

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8942406 [details]
Bug 1303025 - Accept null for body param in constructor of Response.

https://reviewboard.mozilla.org/r/212712/#review218884

Thanks!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62f5c4a36823
Accept null for body param in constructor of Response. r=bkelly

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/62f5c4a36823

Comment 16

[Chris Mills [:cmills]]

I've documented this by:

* Updating the description and support info on the constructor page: https://developer.mozilla.org/en-US/docs/Web/API/Response/Response
* Adding a note to the Fx59 rel notes: https://developer.mozilla.org/en-US/Firefox/Releases/59#DOM

Let me know if this is OK, thanks!

Comment 17

[Xidorn Quan [:xidorn] UTC+11]

Looks good, thanks.