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

Class: Buffer do not follow when the underlying ArrayBuffer is resized #52195

Closed
klebom opened this issue Mar 23, 2024 · 3 comments · Fixed by #55377
Closed

Class: Buffer do not follow when the underlying ArrayBuffer is resized #52195

klebom opened this issue Mar 23, 2024 · 3 comments · Fixed by #55377
Labels
buffer Issues and PRs related to the buffer subsystem. repro-exists Issues with reproductions. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.

Comments

@klebom
Copy link

klebom commented Mar 23, 2024

Version

v21.7.1

Platform

Microsoft Windows NT 10.0.22631.0 x64

Subsystem

Class: Buffer

What steps will reproduce the bug?

const arraybuffer = new ArrayBuffer(4, { maxByteLength: 4096 });

const uint8array = new Uint8Array(arraybuffer);

uint8array.set([0,1,2,3]);

const buffer = Buffer.from(arraybuffer);

console.error(uint8array);
console.error(buffer);

arraybuffer.resize(8);
uint8array.set([4,5,6,7],4);

uint8array[0]=0xff;

console.error(uint8array);
console.error(buffer);

Output:

Uint8Array(4) [ 0, 1, 2, 3 ]
<Buffer 00 01 02 03>
Uint8Array(8) [
  255, 1, 2, 3,
    4, 5, 6, 7
]
<Buffer ff 01 02 03>

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

Expecting buffer to be resized and output to be:

<Buffer ff 01 02 03 04 05 06 07>

What do you see instead?

<Buffer ff 01 02 03>

Additional information

If the resolution to this is that Buffer isn't going to support ArrayBuffer.resize() that should be reflected in the documentation.

@targos targos added buffer Issues and PRs related to the buffer subsystem. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. doc Issues and PRs related to the documentations. and removed doc Issues and PRs related to the documentations. labels Mar 24, 2024
@targos
Copy link
Member

targos commented Mar 24, 2024

Behavior comes from:

node/lib/buffer.js

Lines 476 to 505 in bae14b7

function fromArrayBuffer(obj, byteOffset, length) {
// Convert byteOffset to integer
if (byteOffset === undefined) {
byteOffset = 0;
} else {
byteOffset = +byteOffset;
if (NumberIsNaN(byteOffset))
byteOffset = 0;
}
const maxLength = obj.byteLength - byteOffset;
if (maxLength < 0)
throw new ERR_BUFFER_OUT_OF_BOUNDS('offset');
if (length === undefined) {
length = maxLength;
} else {
// Convert length to non-negative integer.
length = +length;
if (length > 0) {
if (length > maxLength)
throw new ERR_BUFFER_OUT_OF_BOUNDS('length');
} else {
length = 0;
}
}
return new FastBuffer(obj, byteOffset, length);
}

byteOffset and length are initially undefined, but the function ensures they are numbers, and if they are specified, the typed array's length stays fixed.

/cc @nodejs/buffer

@jasnell
Copy link
Member

jasnell commented Mar 24, 2024

Buffer has always been assumed to be non-resizable. I'm wondering if the right fix for now shouldn't be to forbid creating a Buffer from a resizable ArrayBuffer until we can make sure that all of Buffer's APIs handle it appropriately

@ronag
Copy link
Member

ronag commented Aug 26, 2024

I'm wondering if the right fix for now shouldn't be to forbid creating a Buffer from a resizable ArrayBuffer until we can make sure that all of Buffer's APIs handle it appropriately

This will break me. Please don't.

@RedYetiDev RedYetiDev added the repro-exists Issues with reproductions. label Aug 26, 2024
jasnell added a commit to jasnell/node that referenced this issue Oct 13, 2024
nodejs-github-bot pushed a commit that referenced this issue Oct 15, 2024
Fixes: #52195
PR-URL: #55377
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. repro-exists Issues with reproductions. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants