node 0.10, more checks for keylen #27
Conversation
|
I haven't got time to merge this at the moment. Once-over looks good. |
|
this does much more then simply update the keylen checks, can you add some comments in the pull to make it easeir to review the changes ? |
@calvinmetcalf if you have any questions, please ask! |
browser.js
Outdated
| } | ||
|
|
||
| if (keylen < 0 || keylen > MAX_ALLOC) { | ||
| if (keylen < 0 || isNaN(keylen) || keylen > MAX_ALLOC) { |
There was a problem hiding this comment.
probably want to do keylen !== keylen as older browsers aren't going to have isNaN
There was a problem hiding this comment.
in lodash isNaN: value != +value
https://github.com/lodash/lodash/blob/092f90d2fca959fa75d14cae8a2377a58bcb122d/lodash.js#L10534
or better will use keylen !== keylen ?
There was a problem hiding this comment.
lodash is making sure it's a number, but we already have a check to do that
fanatid
force-pushed
the
feature/node-shim
branch
from
b0d1399 to
f252609
Compare
browser.js
Outdated
| } | ||
|
|
||
| digest = digest || 'sha1' | ||
| exports._checkParameters(iterations, keylen) |
There was a problem hiding this comment.
why don't we pull check paramaters into it's own file and require it from both locations
There was a problem hiding this comment.
it's a good idea, I'll do this
|
get rid of the browser._createHmac stuff and fix the typo and I'm good to merge this thank you for the work |
fanatid
force-pushed
the
feature/node-shim
branch
2 times, most recently
from
4c71354 to
b080e6e
Compare
fanatid
force-pushed
the
feature/node-shim
branch
from
b080e6e to
93ffd51
Compare
|
@calvinmetcalf done. Thank you that checked this! |
|
Please rename |
3 participants
Another attempt (summary of #22 #23 #25 #26)