Skip to content

The fetch pull algorithm does not pass proper bytesWritten to ReadableByteStreamControllerRespond #1610

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.

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

Closed
saschanaz opened this issue Feb 21, 2023 · 3 comments · Fixed by #1628

Comments

@saschanaz
Copy link
Member

saschanaz commented Feb 21, 2023

(Talking about https://fetch.spec.whatwg.org/#concept-http-network-fetch step 10 that is modified by #1593)

Implementing the algorithm as-is forces the size of resulting array buffer view be equal to that of the byob request view.

The caller may do some subarraying, but maybe the enqueue algorithm should receive bytesWritten as an argument?

@saschanaz
Copy link
Member Author

saschanaz commented Feb 21, 2023

The caller may do some subarraying

It's complicated when the view is not Uint8Array, so better not. This shouldn't be problem actually.

@domenic
Copy link
Member

domenic commented Feb 22, 2023

I think you are talking about the following case:

  • current BYOB request view is non-null
  • current BYOB request view's byte length > available
  • Thus, bytes's length = available is less than current BYOB request view's byte length, and only bytes's length is written

Indeed the caller could create a new Uint8Array wrapping the underlying AB, but that is kind of annoying, and there's no nice Web IDL helper for it. So yeah maybe Streams should provide more help here.

Ideas:

  • An optional bytesWritten argument, as you suggest
  • Something that abstracts most of steps 10.2.3.1-8 for the caller?

@saschanaz
Copy link
Member Author

Something that abstracts most of steps 10.2.3.1-8 for the caller?

Gecko has an internal implementation that converts any internal byte stream to ReadableStream (that is also used for Fetch), so having an abstraction in the Streams spec would probably better match with Gecko.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants