whatwg / webidl

Web IDL Standard
https://webidl.spec.whatwg.org/
Other
410 stars 164 forks source link

"byte length of a buffer source type" needs updating for resizable and detached buffers #1385

Open bakkot opened 10 months ago

bakkot commented 10 months ago

What is the issue with the Web IDL Standard?

As of this proposal / PR, the [[ByteLength]] slot on TypedArrays and DataViews can be ~auto~ instead of an integer. Consumers of the "byte length of a buffer source" operation (like encodeInto) probably want TypedArrayByteLength or GetViewByteLength instead of the raw value of the [[ByteLength]] slot.

Also, when getting the length of an ArrayBuffer, this should probably use the ArrayBufferByteLength abstract operation, not raw access of the slot.

There's some other raw uses of the [[ByteLength]] slot, like in "write a byte sequence bytes into an ArrayBufferView", which should probably also be updated.

cc @syg

annevk commented 10 months ago

encodeInto() doesn't currently accept resizable buffers though, right? It does make sense to update these algorithms for when we get methods that want to accept them though.

bakkot commented 10 months ago

Huh, yeah, I didn't realize that "Uint8Array" meant specifically a Uint8Array backed by a non-resizable buffer by default, so this is not actually an issue at the moment. I'll close this but feel free to reopen if you want to use this to track.

bakkot commented 10 months ago

Hm, actually, this is already an issue because of detached buffers. Reading the [[ByteLength]] slot of a (> 0 size) TypedArray which is backed by a detached ArrayBuffer will give you a positive integer, which means that the "byte length" will be a positive integer, which breaks at least encodeInto.

Consider writing the first byte into a Uint8Array backed by a detached buffer.

annevk commented 10 months ago

If destination's byte length - written is false we'd break and return, no? Anyway, keeping this open to tidy all these things up seems worth it.

bakkot commented 10 months ago

Sorry, that should read "true". On the first byte, written is 0, destination's byte length the original size of the array, and the number of bytes in result is 1 between and 4 (I assume; anyway it's some finite number). So as long as the TA originally had at least 4 bytes, that check passes and we don't break.

annevk commented 10 months ago

It seems per https://software.hixie.ch/utilities/js/live-dom-viewer/?%3Cscript%3E%0As%20%3D%20new%20Uint8Array(new%20ArrayBuffer(8))%3B%0Aw(s.buffer.byteLength)%0Aw(s.byteLength)%3B%0ApostMessage(s%2C%20%22*%22%2C%20[s.buffer])%3B%0Aw(s.buffer.byteLength)%0Aw(s.byteLength)%3B%0Aw(new%20TextEncoder().encodeInto(%22fd%22%2C%20s))%3B%0A%3C%2Fscript%3E implementations already do the right thing here, but this is indeed a bug.

syg commented 10 months ago

Good catch. These should be checked for using https://tc39.es/ecma262/#sec-istypedarrayoutofbounds for simpler reasoning at the boundary, even if resizable buffers are currently disallowed.

bakkot commented 10 months ago

Is the intention to treat detached buffers as zero-length? That's kind of weird. I've copied that behavior into .fromBase64Into, but maybe it would be better to throw, like TA.prototype.set.

annevk commented 10 months ago

I think throwing a TypeError would be better. Not sure if encodeInto() can still be changed, but I'd be willing to try. It seems somewhat unlikely folks rely on it not throwing, but who knows.