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

Incorrect result from Buffer.from(...).buffer when using base64url encoding #55564

Closed
Foair opened this issue Oct 27, 2024 · 5 comments
Closed
Labels
buffer Issues and PRs related to the buffer subsystem.

Comments

@Foair
Copy link

Foair commented Oct 27, 2024

Version

v20.18.0

Platform

Microsoft Windows NT 10.0.22631.0 x64

Subsystem

No response

What steps will reproduce the bug?

When attempting to create an ArrayBuffer using Buffer.from(..., 'base64url').buffer, the output is incorrect. Instead of the expected ArrayBuffer, the function produces an unexpectedly larger ArrayBuffer.

node -p "Buffer.from('G4Zmayf9a4KOkKHBFeHHQREzRMW_6MPR', 'base64url').buffer"

Result:

ArrayBuffer {
  [Uint8Contents]: <5c 00 00 00 00 00 00 00 1b 86 66 6b 27 fd 6b 82 8e 90 a1 c1 15 e1 c7 41 11 33 44 c5 bf e8 c3 d1 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... 8092 more bytes>,
  byteLength: 8192
}

FYI: The string G4Zmayf9a4KOkKHBFeHHQREzRMW_6MPR is produced by Buffer#toString('base64url') containing some random bytes.

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

Stable

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

Should get a correctly-sized ArrayBuffer representing the base64url decoded string, like Deno does:

deno eval "import { Buffer } from 'node:buffer';
  console.log(Buffer.from('G4Zmayf9a4KOkKHBFeHHQREzRMW_6MPR', 'base64url').buffer)"
ArrayBuffer {
  [Uint8Contents]: <1b 86 66 6b 27 fd 6b 82 8e 90 a1 c1 15 e1 c7 41 11 33 44 c5 bf e8 c3 d1>,
  byteLength: 24
}

What do you see instead?

ArrayBuffer {
  [Uint8Contents]: <5c 00 00 00 00 00 00 00 1b 86 66 6b 27 fd 6b 82 8e 90 a1 c1 15 e1 c7 41 11 33 44 c5 bf e8 c3 d1 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... 8092 more bytes>,
  byteLength: 8192
}

Additional information

No response

@avivkeller
Copy link
Member

This might be a similar issue to that of #55422

@avivkeller avivkeller added the buffer Issues and PRs related to the buffer subsystem. label Oct 27, 2024
@duncpro
Copy link
Contributor

duncpro commented Oct 27, 2024

I believe this is intended behavior. The Buffer.from call is returning a "pooled buffer" wrapped in a FastBuffer. When you access FastBuffer#buffer you are accessing the underlying pooled buffer which is quite a bit larger, and contains other unrelated data.

Not accessing the FastBuffer#buffer field and instead using the returned FastBuffer itself solves this. For example, this test passes...

import { Buffer } from 'node:buffer';
import assert from 'node:assert';

const expected = Buffer.from([0x1b, 0x86, 0x66, 0x6b, 0x27, 0xfd, 0x6b, 0x82, 0x8e, 0x90, 0xa1, 0xc1, 0x15, 0xe1, 0xc7, 0x41,
    0x11, 0x33, 0x44, 0xc5, 0xbf, 0xe8, 0xc3, 0xd1]);

const actual = Buffer.from('G4Zmayf9a4KOkKHBFeHHQREzRMW_6MPR', 'base64url');

assert.deepStrictEqual(actual, expected);

We should probably not allow access to the underlying pooled buffer, to prevent confusions just like this one.

@Foair
Copy link
Author

Foair commented Oct 28, 2024

I believe this is intended behavior. The Buffer.from call is returning a "pooled buffer" wrapped in a FastBuffer. When you access FastBuffer#buffer you are accessing the underlying pooled buffer which is quite a bit larger, and contains other unrelated data.

Not accessing the Buffer#buffer field and instead using the returned FastBuffer itself solves this. For example, this test passes...

import { Buffer } from 'node:buffer';
import assert from 'node:assert';

const expected = Buffer.from([0x1b, 0x86, 0x66, 0x6b, 0x27, 0xfd, 0x6b, 0x82, 0x8e, 0x90, 0xa1, 0xc1, 0x15, 0xe1, 0xc7, 0x41,
    0x11, 0x33, 0x44, 0xc5, 0xbf, 0xe8, 0xc3, 0xd1]);

const actual = Buffer.from('G4Zmayf9a4KOkKHBFeHHQREzRMW_6MPR', 'base64url');

assert.deepStrictEqual(actual, expected);

We should probably not allow access to the underlying pooled buffer, to prevent confusions just like this one.

I didn’t come up with this myself; it’s actually from Bun’s guides, and I thought it was supposed to be a standard way to do so. Most of the time, I can just use Buffer directly, since it inherits from Uint8Array.

https://bun.sh/guides/binary/buffer-to-arraybuffer

@duncpro
Copy link
Contributor

duncpro commented Oct 28, 2024

I believe this is intended behavior. The Buffer.from call is returning a "pooled buffer" wrapped in a FastBuffer. When you access FastBuffer#buffer you are accessing the underlying pooled buffer which is quite a bit larger, and contains other unrelated data.

Not accessing the Buffer#buffer field and instead using the returned FastBuffer itself solves this. For example, this test passes...

import { Buffer } from 'node:buffer';

import assert from 'node:assert';

const expected = Buffer.from([0x1b, 0x86, 0x66, 0x6b, 0x27, 0xfd, 0x6b, 0x82, 0x8e, 0x90, 0xa1, 0xc1, 0x15, 0xe1, 0xc7, 0x41,

0x11, 0x33, 0x44, 0xc5, 0xbf, 0xe8, 0xc3, 0xd1]);

const actual = Buffer.from('G4Zmayf9a4KOkKHBFeHHQREzRMW_6MPR', 'base64url');

assert.deepStrictEqual(actual, expected);

We should probably not allow access to the underlying pooled buffer, to prevent confusions just like this one.

I didn’t come up with this myself; it’s actually from Bun’s guides, and I thought it was supposed to be a standard way to do so. Most of the time, I can just use Buffer directly, since it inherits from Uint8Array.

https://bun.sh/guides/binary/buffer-to-arraybuffer

See the Node.js docs here.

It states that the returned 'ArrayBuffer' might not correspond exactly to the 'Buffer'.

@Foair
Copy link
Author

Foair commented Oct 28, 2024

Thanks, I’ll close this since it lines up with the docs.

@Foair Foair closed this as completed Oct 28, 2024
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.
Projects
None yet
Development

No branches or pull requests

3 participants