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

nacl.setReturnType and generic byte array acceptance #79

Closed
wants to merge 9 commits into from

Conversation

RainaBatwing
Copy link

This change allows high level nacl functions to accept any byte array implementing an interface similar to Uint8Array and node-like Buffers, including the correct 8-bit truncating behaviour.

It also adds a new public high level method nacl.setReturnType(constructor) which can be used to change the class tweetnacl uses to return byte arrays from high level functions.

Resolves issue #59

@dchest
Copy link
Owner

dchest commented May 18, 2015

Thanks, and sorry that it's taking me so long to reply.

I was thinking about this, and realized that I don't like the "asymmetry" of API: that we can pass anything to functions (which satisfies the API), but with setReturnType, only get the type we set. It feels not right to me. I think it would be a good idea to instead introduce setByteArrayType (I think this name is better than setBufferType) as originally planned (which will do the interface checks), and then check that we use this exact type everywhere: in arguments and for returned arrays. What do you think? I can tweak your code for this or you can do this if you have time.

Thanks again.

@RainaBatwing
Copy link
Author

My concern with a change like that is that some other crypto-related libraries like your own blake2s-js return Uint8Arrays, even on node-like platforms. It seems counter to the goals of this change to force any situation where a user is pointlessly required to convert one data type to another by copying. It hurts code clarity, which makes it easier for bugs to hide.

I can see how it feels cleaner and more pure to choose just one type, but even with a stricter mode, the tweetnacl-js library itself has some constants in Uint8Arrays internally, so it would still be mixing both types.

I'm not sure what benefit there is in strictly requiring one class for input types instead of one interface?

I agree that setByteArrayType is a better name than setBufferType

@dchest
Copy link
Owner

dchest commented Sep 17, 2015

Closing as discussed in #59. Thank you!

@dchest dchest closed this Sep 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants