Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Request's total bytes isn't set #604

Closed
jakearchibald opened this issue Sep 18, 2017 · 5 comments
Closed

Request's total bytes isn't set #604

jakearchibald opened this issue Sep 18, 2017 · 5 comments
Labels
clarification Standard could be clearer

Comments

@jakearchibald
Copy link
Collaborator

jakearchibald commented Sep 18, 2017 •
edited
Loading

https://fetch.spec.whatwg.org/#concept-body-total-bytes isn't set for requests, unless the input is a Request object.

Feels like it should be set in https://fetch.spec.whatwg.org/#concept-bodyinit-extract for non-stream sources.
@annevk
Copy link
Member

annevk commented Sep 18, 2017

cc @yutakahirano

@wanderview
Copy link
Member

It would be nice, though, to acknowledge the size can be unknown. Zero really isn't a good representation of this.

@gterzian gterzian mentioned this issue May 6, 2020
Merged
5 tasks
@gterzian gterzian mentioned this issue Jun 16, 2020
Closed
@annevk annevk added the clarification Standard could be clearer label Feb 15, 2021
@annevk
Copy link
Member

annevk commented Feb 19, 2021

I really want to remove this field and for responses we can look at the Content-Length header (at least for HTTP/1, guess we might have to write/look at some XMLHttpRequest tests to see how the progress events behave).

For non-stream requests the main difficulty will be getting a length out of https://fetch.spec.whatwg.org/#concept-bodyinit-extract when object isn't a stream. That might also involve acknowledging that some serialization happens on the main thread. FormData is the trickiest as it can hold blobs so you would partially serialize it and then use the size of the blobs to calculate the remaining space that is required. Perhaps this can be implementation-defined until we have FormData serialization itself defined?

annevk added a commit that referenced this issue Feb 20, 2021
@annevk
Formalize reading bodies
64760c5 
This refactoring allows specifications building on top of Fetch to
avoid having to directly integrate with Streams. It makes these
changes:

* body's transmitted bytes is removed. body's total bytes will likely
  be removed in #604.
* body's done and wait concepts are removed.
* It introduces three primitives to read a body: incrementally read
  (process each chunk), fully read (get all), and
  fully reading body as promise (get all as promise).
* Removed transmit request body in favor of HTTP-network fetch using
  incrementally read directly to transmit the request body.
* Fetch's processRequestBody and processRequestEndOfBody are no longer
  passed a request. The former is passed the chunk length for progress
  reporting. (Needed by XMLHttpRequest.)
* Fetch's processResponseEndOfBody when passed will now cause the body
  to be fully read and will pass the result in the second argument.
  (This means that callers can no longer read the body themselves.)
* Subresource Integrity changed slightly with Fetch handing it the
  bytes and integrity metadata and no longer a response. Essentially
  fetch fully reads the body, does the check, and then creates a new
  body from the same bytes to hand to the caller.

Subresoruce Integrity PR: w3c/webappsec-subresource-integrity#97.

XMLHttpRequest PR: whatwg/xhr#313.

Closes #661.
annevk added a commit that referenced this issue Feb 24, 2021
@annevk
Formalize reading bodies
c32c1ea 
This refactoring allows specifications building on top of Fetch to
avoid having to directly integrate with Streams. It makes these
changes:

* body's transmitted bytes is removed. body's total bytes will likely
  be removed in #604.
* body's done and wait concepts are removed.
* It introduces three primitives to read a body: incrementally read
  (process each chunk), fully read (get all), and
  fully reading body as promise (get all as promise).
* Removed transmit request body in favor of HTTP-network fetch using
  incrementally read directly to transmit the request body.
* Fetch's processRequestBody and processRequestEndOfBody are no longer
  passed a request. The former is passed the chunk length for progress
  reporting. (Needed by XMLHttpRequest.)
* Fetch's processResponseEndOfBody when passed will now cause the body
  to be fully read and will pass the result in the second argument.
  (This means that callers can no longer read the body themselves.)
* Subresource Integrity changed slightly with Fetch handing it the
  bytes and integrity metadata and no longer a response. Essentially
  fetch fully reads the body, does the check, and then creates a new
  body from the same bytes to hand to the caller.

Subresoruce Integrity PR: w3c/webappsec-subresource-integrity#97.

XMLHttpRequest PR: whatwg/xhr#313.

Closes #661.
annevk added a commit that referenced this issue Feb 24, 2021
@annevk
Formalize reading bodies
6ad9998 
This refactoring allows specifications building on top of Fetch to
avoid having to directly integrate with Streams. It makes these
changes:

* body's transmitted bytes is removed. body's total bytes will likely
  be removed in #604.
* body's done and wait concepts are removed.
* It introduces three primitives to read a body: incrementally read
  (process each chunk), fully read (get all), and
  fully reading body as promise (get all as promise).
* Removed transmit request body in favor of HTTP-network fetch using
  incrementally read directly to transmit the request body.
* Fetch's processRequestBody and processRequestEndOfBody are no longer
  passed a request. The former is passed the chunk length for progress
  reporting. (Needed by XMLHttpRequest.)
* Fetch's processResponseEndOfBody when passed will now cause the body
  to be fully read and will pass the result in the second argument.
  (This means that callers can no longer read the body themselves.)
* Subresource Integrity changed slightly with Fetch handing it the
  bytes and integrity metadata, and no longer a response. Essentially
  fetch fully reads the body, does the check, and then creates a new
  body from the same bytes to hand to the caller.

Subresoruce Integrity PR: w3c/webappsec-subresource-integrity#97.

XMLHttpRequest PR: whatwg/xhr#313.

Closes #661.
@annevk
Copy link
Member

annevk commented Feb 26, 2021

Here's my plan:

  • Define convert BodyInit object to bytes routine that returns a tuple of a byte sequence and null or a multipart/form-data boundary. This will synchronous but implementations can optimize.
  • Define a size of BodyInit object routine that invokes convert and returns the length. We'll use this to set the Content-Type header in requests. (I kinda wanna use <a for="BodyInit,byte sequence">length</a> there, but if that's not a thing we'll just branch.)
  • Make extract use convert as well instead of all the "actions".

Add a bunch of notes that encourage implementations to optimize this. All ears to better ideas though.

@annevk
Copy link
Member

annevk commented Feb 27, 2021

My plan does not work for Blob and FormData: whatwg/html#6424. The other types should probably still be eagerly converted to a byte sequence...

So maybe we should have convert and that returns a tuple of

  • a byte sequence or a read operation of sorts
  • a length

but it's still rather inelegant to do extract and length (which would both use convert) determination separately. So maybe body should store a length after all. (Though we should also be clear that it's different between requests and responses as for responses it can lie, especially with service workers and maybe also with H/2 and H3/, which don't really have that concept although if you specify Content-Length maybe it's still required to match?)

@annevk annevk mentioned this issue Mar 1, 2021
Merged
annevk added a commit that referenced this issue Mar 1, 2021
@annevk
Define Content-Length parser
4525c22 
As the body concept is refactored for #604, XMLHttpRequest will need to use this algorithm for its events. We also want to require browsers to use this algorithm instead of the one defined by HTTP as part of #1156.

Tests: web-platform-tests/wpt#10548 & web-platform-tests/wpt#27837.
@annevk annevk mentioned this issue Mar 1, 2021
Merged
3 tasks
annevk added a commit that referenced this issue Mar 1, 2021
@annevk
Revamp body's total bytes as length
9870813 
Together with #1183 this solves the Fetch side of #604. Changes:

* No longer set a length for network responses. Callers will have to parse Content-Length themselves.
* Make extract set length so requests will set Content-Length correctly.

XMLHttpRequest PR: TODO.

Tests: web-platform-tests/wpt#27837.
@annevk annevk mentioned this issue Mar 1, 2021
Merged
3 tasks
annevk added a commit that referenced this issue Mar 3, 2021
@annevk
Define Content-Length parser
f2a7922 
As the body concept is refactored for #604, XMLHttpRequest will need to use this algorithm for its events. We also want to require browsers to use this algorithm instead of the one defined by HTTP as part of #1156.

Tests: web-platform-tests/wpt#10548.
annevk added a commit to web-platform-tests/wpt that referenced this issue Mar 10, 2021
@annevk
XMLHttpRequest: Content-Length tests
0adf967 
Some tests with progress events, Content-Length, and service workers.

For whatwg/fetch#604, whatwg/fetch#1184, and whatwg/xhr#317.
annevk added a commit that referenced this issue Mar 10, 2021
@annevk
Revamp body's total bytes as length
f0ae5a6 
Together with #1183 this solves the Fetch side of #604. Changes:

* No longer set a length for network responses. Callers will have to parse Content-Length themselves.
* Make extract set length so requests can set Content-Length correctly.

XMLHttpRequest PR: whatwg/xhr#317.

Tests: web-platform-tests/wpt#27837.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 19, 2021
@annevk @moz-wptsync-bot
Bug 1695668 [wpt PR 27837] - XMLHttpRequest: Content-Length tests, a=…
a34ae82 
…testonly

Automatic update from web-platform-tests
XMLHttpRequest: Content-Length tests

Some tests with progress events, Content-Length, and service workers.

For whatwg/fetch#604, whatwg/fetch#1184, and whatwg/xhr#317.
--

wpt-commits: 0adf967c6d70747d3f77573a7f7f2353d82142c1
wpt-pr: 27837
Bishwarupjee pushed a commit to Bishwarupjee/xhr that referenced this issue Jan 31, 2024
@annevk
Use extract a length from Fetch
8eb4db5 
The only information browsers use for progress events for responses is the Content-Length header.

Align on that and only use body's length concept for uploads.

Tests: web-platform-tests/wpt#27837.

Fetch PRs: whatwg/fetch#1183 & whatwg/fetch#1184.

Closes whatwg/fetch#604.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
clarification Standard could be clearer
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jakearchibald @wanderview @annevk and others