Sitelet https://web.archive.org/web/20220119114142/https://github.com/nodejs/node/issues/41588
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

Start moving to Uint8Array in new APIs? #41588

Open
benjamingr opened this issue Jan 19, 2022 · 7 comments
Open

Start moving to Uint8Array in new APIs? #41588

benjamingr opened this issue Jan 19, 2022 · 7 comments

Comments

@benjamingr
Copy link
Member

@benjamingr benjamingr commented Jan 19, 2022 •

There was a suggestion by @jasnell to use Uint8Arrays in new APIs over Buffers as well as a weigh-in by @sindresorhus saying it is easier to author cross-platform APIs when using Uint8Arrays.

Here is a context #41553 (comment)

That is, the ask here is that Node.js should prefer Uint8Arrays over Buffers in new APIs.

What does everyone think? Should we stick to Buffer (which is a subclass of Uint8Array as a reminder) or prefer Uint8Arrays over buffers when possible in new APIs?

cc @nodejs/buffer @nodejs/streams

@ronag
Copy link
Member

@ronag ronag commented Jan 19, 2022

That will cause soo much confusion. I believe there are several methods/props that Buffer override and which act differently than Uint8Array.

@ronag
Copy link
Member

@ronag ronag commented Jan 19, 2022 •

In particular .slice work differently. Not sure if there are others.

@ronag
Copy link
Member

@ronag ronag commented Jan 19, 2022

I'm fine with using Uint8Array for Web apis which define the type. But for node api's I think we should stick with Buffer.

@mcollina
Copy link
Member

@mcollina mcollina commented Jan 19, 2022

It really depends on the subsystems we are targeting. It's impossible to make a generic call.

@paulmillr
Copy link

@paulmillr paulmillr commented Jan 19, 2022 •

I have a big problem with Buffers. They must be killed.

  1. buffer.slice() is mutable copy, uint8array.slice() is immutable. This is very bad. Recently got hit with a bug report. I was checking for the input to be instanceof Uint8Array and did array.slice(), then operated on the copy. But that doesn't work with buffers! The buffers were mutated even after copy. Why are they instance of Uint8Arrays if the behavior is different? paulmillr/noble-ed25519#45
  2. They are not supported in browsers and will never be. Adding an additional in-browser shim for buffers is bad.
  3. They expose private information to a global variable. Imagine you're developing some secure software. You reason about zeroing data etc. #41467
// Somewhere in your code
const privateBuf = Buffer.from(privateKey, 'hex');

// Rogue package can access
Buffer.from('1').buffer
// Which will of course show the contents of `privateBuf`
// No need in complex memory dumps!

This happens because there is 8KB shared buffer reused for all Buffer.from calls! There is zero need in making this as subtle as it is right now. Buffer.allocUnsafe seems like a good name for "using a part of global shared buffer", Buffer.from is not. Search GitHub code for the snippet and tell me how many people know about the "feature".

@benjamingr
Copy link
Member Author

@benjamingr benjamingr commented Jan 19, 2022 •

Hey @paulmillr can we avoid statements like "they must be killed"? it detracts from technical arguments you are making and anyone with a strong opinion the other way will automatically be put on a defensive rather than be in a learning mood.

(I'm going to read the rest of it just wanted to start with that quickly before others read it)

@benjamingr
Copy link
Member Author

@benjamingr benjamingr commented Jan 19, 2022

buffer.slice() is mutable copy, uint8array.slice() is immutable. This is very bad. Recently got hit with a bug report. I was checking for the input to be instanceof Uint8Array and did array.slice(), then operated on the copy. But that doesn't work with buffers! The buffers were mutated even after copy. Why are they instance of Uint8Arrays if the behavior is different? Regression in 1.5.0 paulmillr/noble-ed25519#45

FWIW: I agree having an API that behaves like a subclass but "lies" about keeping the same API structure is super-confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants