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

add more checks for keylen #23

Closed
wants to merge 1 commit into from

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Feb 24, 2016

Part of #22

@fanatid
Copy link
Contributor Author

fanatid commented Feb 24, 2016

$ node -v
v5.6.0
$ node
> require('crypto').pbkdf2Sync('password', 'salt', 1, NaN)
TypeError: Bad key length
$ nvm use 0.10
$ node -v
v0.10.42
$ node
> require('crypto').pbkdf2Sync('password', 'salt', 1, NaN)
<SlowBuffer >

Different behavior for different node versions :(

@@ -3,6 +3,10 @@ var compatNode = require('../')
var compatBrowser = require('../browser')
var fixtures = require('./fixtures')

// put NaN and Infinity
fixtures.invalid[fixtures.invalid.length - 2].dkLen = NaN
fixtures.invalid[fixtures.invalid.length - 1].dkLen = Infinity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather add these fixtures inline here, then do this weird modification which is error prone for others adding tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, any ideas how tests dkLen with NaN and Infinity?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, sorry

@fanatid
Copy link
Contributor Author

fanatid commented Feb 24, 2016

updated

@dcousens
Copy link
Member

... all failed except Node 4? Interesting

@dcousens
Copy link
Member

 1) node pbkdf2 pbkdf2-compat pbkdf2Sync sha[1/224/256/384/512] should throw Bad key length:
     AssertionError: Missing expected exception..
      at Function._throws (assert.js:301:5)
      at Function.assert.throws (assert.js:318:11)
      at Context.<anonymous> (test/index.js:95:24)

@fanatid
Copy link
Contributor Author

fanatid commented Feb 24, 2016

What version of node v4 you are using? May be your version not include nodejs/node@4c8d96b ? (checks on NaN and Infinity was added there)

@dcousens
Copy link
Member

@fanatid I'm referring to the CI.
Every version except Node 4 are failing this.

@fanatid
Copy link
Contributor Author

fanatid commented Feb 24, 2016

@dcousens I'll fix with this PR after nodejs/node#5397 will be merged and released

@calvinmetcalf
Copy link
Contributor

merged as part of #27

@fanatid fanatid deleted the feature/keylen branch March 8, 2016 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants