1500879 - Fetch should not consume Request/Response with null-body

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Andrea Marchesini [:baku]]

http://w3c-test.org/fetch/api/request/request-consume-empty.html

Comment 1

[Andrea Marchesini [:baku]]

Attachment: fetch_empty.patch

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9018997 [details] [diff] [review]
fetch_empty.patch

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

Please add a commit message, something like the following.  The extra context of a commit message like this and a slightly more spec-explaining comment like the below would have made it much easier to address this review.  I'll try and be better in the future about providing an immediate "r-" like :smaug and others do instead of trying to figure the patch out myself.

The fetch spec treats null bodies specially.  Their Body cannot become disturbed or locked and a fresh empty ReadableStream is returned whenever an attempt is made to consume the body.  We currently fail a number of WPT tests in this area because we do mark the body consumed as exposed via bodyUsed.

::: dom/fetch/Fetch.cpp
@@ +1238,5 @@
>      aRv.ThrowTypeError<MSG_FETCH_BODY_CONSUMED_ERROR>();
>      return nullptr;
>    }
>  
> +  // By spec, if the body is null, we have to create an empty ReadableStream.

This comment could be a little more illuminating given the unusual behavior of the spec around null bodies.  I'd suggest something like the following.

null bodies are a special-case in the fetch spec.  The Body mix-in can only be "disturbed" or "locked" if its associated "body" is non-null.  Additionally, the Body min-in's "consume body" algorithm explicitly creates a fresh empty ReadableStream object in step 2.  This means that `bodyUsed` will never return true for a null body.

To this end, we create a fresh (empty) body every time a request is made and consume its body here, without marking this FetchBody consumed via SetBodyUsed.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/01232c002d2d
Fetch should not consume Request/Response with null-body, r=asuth

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/01232c002d2d