-Infinity, Infinity and NaN should fail validation #532
Conversation
|
@MitMaro @ziluvatar may I ask you to review this change? |
There was a problem hiding this comment.
Thanks for this!
It may take a while to get this merged since it's technically a breaking change. But that's up to @ziluvatar , as I'm just a contributor like you. :)
test/claim-exp.test.js
Outdated
| it('should throw exception when given -Infinity', function () { | ||
| expect(() => signWithExpiresIn({exp: -Infinity})).to.throw( | ||
| '"exp" should be a number of seconds' | ||
| ); |
There was a problem hiding this comment.
This test case can be moved into the array in the 'jwt.sign "exp" claim validation' block.
test/claim-exp.test.js
Outdated
| it('should throw exception when given Infinity', function () { | ||
| expect(() => signWithExpiresIn({exp: Infinity})).to.throw( | ||
| '"exp" should be a number of seconds' | ||
| ); |
There was a problem hiding this comment.
Ditto to the above comment, this test case can be moved into the array in the 'jwt.sign "exp" claim validation' block.
test/claim-exp.test.js
Outdated
| it('should throw exception when given NaN', function () { | ||
| expect(() => signWithExpiresIn({exp: NaN})).to.throw( | ||
| '"exp" should be a number of seconds' | ||
| ); |
There was a problem hiding this comment.
Same thing here, this test case can be moved into the array in the 'jwt.sign "exp" claim validation' block.
test/claim-iat.test.js
Outdated
| expect(() => signWithIssueAtSync({iat: NaN})).to.throw( | ||
| '"iat" should be a number of seconds' | ||
| ); | ||
| }); |
There was a problem hiding this comment.
These test can be moved to the array above in the 'jwt.sign "iat" claim validation' block.
test/claim-nbf.test.js
Outdated
| it('should throw exception when given -Infinity', function () { | ||
| expect(() => signWithNotBefore({nbf: -Infinity})).to.throw( | ||
| '"nbf" should be a number of seconds' | ||
| ); |
There was a problem hiding this comment.
After the previous comments, this might not be a surprise. This and the following tests can be moved into the jwt.sign "nbf" claim validation block above.
|
@MitMaro thanks a lot for your comments, I updated tests and resolved existing merge conflicts. As to change itself, yes, I understand that it's quite BC but anyway that something that should be done, so I am fine if it takes a bit longer :) |
Fixes #500