Crypto buffers #4179
Crypto buffers #4179
Conversation
| this._binding = new binding.DiffieHellmanGroup(name); | ||
| } | ||
|
|
||
| DiffieHellmanGroup.prototype.generateKeys = |
There was a problem hiding this comment.
May be just put prototypes of both to some variables to reduce repetition count?
There was a problem hiding this comment.
Yeah, that can be organized better, you're right.
|
Generally, it appears to be correct to me, but you've copy-pasted a lot of js code which made crypto.js even more unreadable that it is now, otherwise lgtm (though would be good if someone else will review it too /cc @piscisaureus @bnoordhuis ) |
| @@ -87,14 +87,15 @@ Returned by `crypto.createHash`. | |||
|
|
|||
| Updates the hash content with the given `data`, the encoding of which is given | |||
| in `input_encoding` and can be `'buffer'`, `'utf8'`, `'ascii'` or `'binary'`. | |||
| Defaults to `'binary'`. | |||
| Defaults to `'buffer'`. | |||
There was a problem hiding this comment.
I'm not a huge fan of us exposing this internal 'buffer' encoding in the docs. I think instead we should say something like "Defaults to returning a Buffer instance", and just never mention 'buffer'.
| The crypto module requires OpenSSL to be available on the underlying platform. | ||
| It offers a way of encapsulating secure credentials to be used as part | ||
| of a secure HTTPS net or http connection. | ||
| The crypto module requires OpenSSL to be available on the underlying |
There was a problem hiding this comment.
Outdated now that we bundle openssl?
|
@isaacs I landed some Diffie-Hellman bug fixes in v0.8 that you probably should merge before you land this (or merge into master first, then merge master into your branch). |
crypto: Hash and Hmac default to buffers crypto: Move Cipher encoding logic to JS crypto: Move Cipheriv encoding logic to JS crypto: Move Decipher encoding logic to JS crypto: Move Decipheriv into JS, default to buffers crypto: Move Sign class to JS crypto: Better encoding handling in Hash.update crypto: Move Verify class to JS crypto: Move DiffieHellman to JS, default to buffers crypto: Move DiffieHellmanGroup to JS, default to buffers Also, create a test for this feature
This is a flag to make it easier for users to upgrade through the breaking crypto change, and easier for us to switch it back if it's a problem. Explicitly set default encoding to 'buffer' in other tests, in case it ever changes back.
Of course, the commits should be squashed, but I figured it might be good to see them split out, for easier reviewing in a more atomic way.
This makes the node crypto module default to using Buffers. There is a flag that can be set to make it use 'binary' encoded strings by default, for ease of upgrading. All of the buffer/string handling code is moved into JS, so it can use the same Buffer implementation as everything else.
It would be nice if the crypto functions used Buffers instead of SlowBuffers, but that's a much more involved change.