webidl icon indicating copy to clipboard operation
webidl copied to clipboard

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

Open bakkot opened this issue 1 year ago • 9 comments

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

bakkot avatar Jan 21 '24 02:01 bakkot

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.

annevk avatar Jan 22 '24 07:01 annevk

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 avatar Jan 22 '24 17:01 bakkot

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.

  • "destination’s byte length − written is greater than or equal to the number of bytes in result" is true
  • So we "Write the bytes in result into destination"
  • which will "Write bytes into arrayBuffer with startingOffset set to jsView.[[ByteOffset]] + startingOffset."
  • which asserts that "bytes’s length ≤ jsArrayBuffer.[[ArrayBufferByteLength]] − startingOffset."
  • which assertion is violated: the [[ArrayBufferByteLength]] if a detached buffer is zero.

bakkot avatar Jan 22 '24 17:01 bakkot

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.

annevk avatar Jan 22 '24 18:01 annevk

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.

bakkot avatar Jan 22 '24 18:01 bakkot

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.

annevk avatar Jan 23 '24 16:01 annevk

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.

syg avatar Jan 23 '24 23:01 syg

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.

bakkot avatar Jan 23 '24 23:01 bakkot

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.

annevk avatar Jan 24 '24 07:01 annevk