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.

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

πŸ› Bug Report β€” Runtime APIs - WritableStream DefaultWriter await write() does not copy chunk buffer #3137

Open
pmeenan opened this issue Nov 19, 2024 · 2 comments
Labels
api bug Something isn't working streams

Comments

@pmeenan
Copy link

pmeenan commented Nov 19, 2024

If you write a buffer to a TransformStream's WritableStream with await writer.write() and then modify the buffer after the promise completes, the output stream will be corrupted.

As best as I can tell from the spec, it should be safe to modify the chunk after the promise resolves:

If chunk is mutable, producers are advised to avoid mutating it after passing it to write(), until after the promise returned by write() settles. This ensures that the underlying sink receives and processes the same value that was passed in.

As it stands currently, the only safe way to do it is to slice() the buffer to make a copy of it when you pass it to write().

This was discovered as part of working on a worker that uses wasm to compress the response streams in a passthrough transform stream. The resulting stream would be corrupted with chunks being overwritten with later data.

@jasnell
Copy link
Member

jasnell commented Nov 20, 2024

Yes, this is a known limitation of the existing implementation that we end up having to preserve for backwards compatibility :-/ ... The other approach you could take aside from slice() is to copy the input like writer.write(new Uint8Array(chunk)). We've considered adding a fix for this behind a compatibility flag but it wasn't clear if folks were actually bumping up against this or not. I think at this point it's clear we should.

@jasnell jasnell added bug Something isn't working api streams labels Nov 20, 2024
@pmeenan
Copy link
Author

pmeenan commented Nov 20, 2024

Just documenting the behavior difference on the docs would probably be enough if you need to maintain it for compatibility reasons. It's just a pain to debug when it happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api bug Something isn't working streams
Projects
None yet
Development

No branches or pull requests

2 participants