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

Make encodeInto() throw when given a detached buffer #324

Open
annevk opened this issue Jan 24, 2024 · 5 comments
Open

Make encodeInto() throw when given a detached buffer #324

annevk opened this issue Jan 24, 2024 · 5 comments
Labels
needs implementer interest Moving the issue forward requires implementers to express interest topic: api

Comments

@annevk
Copy link
Member

annevk commented Jan 24, 2024

In whatwg/webidl#1385 we discovered some specification UB. However, that also raised the question as to whether this method should perhaps be throwing an exception when given a detached buffer.

I tend to think it should and hopefully there's no callers relying on it not throwing as it's still early days?

cc @hsivonen @inexorabletash @youennf

@inexorabletash
Copy link
Member

cc @ricea

@ricea
Copy link
Collaborator

ricea commented Jan 25, 2024

Chrome accidentally does nothing successfully.

I think we should throw. My guess is that no-one is intentionally using the current behaviour.

@domenic
Copy link
Member

domenic commented Jan 25, 2024

Is the goal to add a one-off check to Encoding, or to throw at the Web IDL layer? The latter would be kind of nice, but perhaps scarier.

@annevk
Copy link
Member Author

annevk commented Jan 25, 2024

Throwing at the IDL layer would change the exception for everything calling into StructuredSerializeWithTransfer at least. Not sure that's really possible. My idea was to add one-off checks and also to assert that the buffer is not detached in various IDL algorithms so callers are "forced" to deal with it.

@annevk
Copy link
Member Author

annevk commented Feb 1, 2024

This also applies to TextDecoder's decode() and looking at https://webidl.spec.whatwg.org/#dfn-get-buffer-source-copy the handling of detached buffers is pretty intentional at least for that and APIs like it.

I suspect it's all aligned with the original non-throwy view APIs. This makes me a bit more worried about introducing a throw here and there as it seems unlikely we will be able to be consistent.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs implementer interest Moving the issue forward requires implementers to express interest topic: api
Development

No branches or pull requests

4 participants