[M] Accept any BufferSource in TextDecoder.decode - #3
Merged
Merged
Conversation
The polyfill for `TextDecoder` read `input.length`. An `ArrayBuffer` and a `DataView` have no `length` property. The loop never ran, and the method returned an empty string. The method did not throw, so the caller lost the data without a signal. The WHATWG encoding standard defines the input as a `BufferSource`. A `BufferSource` is an `ArrayBuffer` or a view on an `ArrayBuffer`. This change converts the input to a `Uint8Array` first. The conversion keeps the byte offset and the byte length of a view, so a subarray still decodes the correct bytes. An input that is not a `BufferSource` now throws a `TypeError`. The method also builds the intermediate string in chunks of 8192 bytes with `String.fromCharCode.apply`. The previous code called `String.fromCharCode` one time for each byte. A benchmark in the sandbox decoded 600 KB of JSON. The time was 131 ms before this change and 20 ms after this change. This change also increments the package version to 3.2.1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
This change is part of the following stack: Change managed by git-spice. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
TextDecoder.decodereturned an empty string for anArrayBufferand for aDataView. The polyfill readinput.length. Neither type has alengthproperty, so the loop never ran. The method did not throw, so a caller lost
the data without a signal.
The backend runs customer-supplied and agent-authored TypeScript in this
sandbox. A script that calls
new TextDecoder().decode(someArrayBuffer)getsan empty string today, and the failure appears far away from the cause.
The change
The WHATWG encoding standard defines the input as a
BufferSource, which isan
ArrayBufferor a view on anArrayBuffer. The new functiontoUint8Arrayconverts the input first. The conversion keeps the byte offsetand the byte length of a view, so
new Uint8Array(buffer, 6)still decodesthe correct bytes. An input that is not a
BufferSourcenow throws aTypeErrorinstead of a silent empty string.The method also builds the intermediate string in chunks of 8192 bytes with
String.fromCharCode.apply. The previous code calledString.fromCharCodeone time for each byte. The chunked loop keeps the same
escapeanddecodeURIComponentpair, so the output and the error behaviour do notchange.
The measurement
A benchmark in the sandbox decoded 600 KB, three runs, in milliseconds:
A hand-written UTF-8 decoder removes the Annex B function
escape, but it isslower and it changes the behaviour for invalid UTF-8. This change keeps the
escapepair.The tests
src/test/sync/util.test.tsandsrc/test/async/util.test.tseach get 12cases: an
ArrayBuffer, aDataView, aUint8Array, a view with a byteoffset, a view with a shorter byte length, a signed
Int8Arrayview,multibyte characters, an empty input, a payload larger than one chunk, and
the two
TypeErrorcases. Five of them failed before the change.The version
package.jsonmoves to 3.2.1. The fork needs av3.2.1GitHub Release afterthis merges, so that
@loop-payments/ts-sandboxcan take the fix.🤖 Generated with Claude Code