fix: DataView initialization in form-utils.js:deserialize_binary_form #14970
Conversation
When you call get_buffer, it returns a Uint8Array. In many JavaScript runtime environments (like Cloudflare Workers, Node.js, or modern browsers), streams often allocate chunks from a shared memory pool (slab allocation). This means the Uint8Array you get back is a view into a much larger ArrayBuffer, and it often has a non-zero byteOffset. When you pass header.buffer to DataView, it creates a view starting at the very beginning (byte 0) of the underlying memory allocation, completely ignoring the byteOffset of your header array. Because the header data usually lives at a specific offset inside that buffer, your code reads 4 bytes from the wrong location (the start of the memory slab), interprets that garbage data as a 32-bit integer, and gets 16793089 (in my test case). It then tries to read ~16MB of data, runs out of stream, and throws "data too short". You must pass the byteOffset and byteLength to the DataView constructor to ensure it looks at the correct slice of memory. I have confirmed on my code that this fix resolves my problem.
|
|
Thank you @touzoku. Yesterday, after updating |
|
This looks good, although I think the comment is unnecessary |
removed it |
teemingc
changed the title
|
Thanks for this! Can you add a test and a changeset? Maybe even a short comment above the changed line why those additional arguments are needed? |
|
I'd love to merge this -- but we at least need a changeset. It'd be great to get a regression test in as well, though I'd settle for just the changeset if figuring out how to test it is too difficult. |
|
@elliott-with-the-longest-name-on-github I opened #15028 for that :) |
5 participants
When you call get_buffer, it returns a Uint8Array. In many JavaScript runtime environments (like Cloudflare Workers, Node.js, or modern browsers), streams often allocate chunks from a shared memory pool (slab allocation). This means the Uint8Array you get back is a view into a much larger ArrayBuffer, and it often has a non-zero byteOffset.
When you pass header.buffer to DataView, it creates a view starting at the very beginning (byte 0) of the underlying memory allocation, completely ignoring the byteOffset of your header array.
Because the header data usually lives at a specific offset inside that buffer, your code reads 4 bytes from the wrong location (the start of the memory slab), interprets that garbage data as a 32-bit integer, and gets 16793089 (in my test case). It then tries to read ~16MB of data, runs out of stream, and throws "data too short".
You must pass the byteOffset and byteLength to the DataView constructor to ensure it looks at the correct slice of memory.
I have confirmed on my code that this fix resolves my problem.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits