-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
test: test hmac binding robustness #10923
Conversation
@@ -8,6 +8,14 @@ if (!common.hasCrypto) { | |||
} | |||
const crypto = require('crypto'); | |||
|
|||
// Test for binding layer robustness | |||
const binding = process.binding('crypto'); | |||
(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of an IIFE, can you just create a new block scope.
(() => { | ||
const h = new binding.Hmac(); | ||
// Fail to init the Hmac with an algorithm. | ||
assert.throws(() => h.update('hello'), TypeError); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been moving toward using a regular expression as the final argument to assert.throws()
. It would look something like /^TypeError: whatever the error message is$/
.
@cjihrig PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if CI is green.
{ | ||
const binding = process.binding('crypto'); | ||
const h = new binding.Hmac(); | ||
// Fail to init the Hmac with an algorithm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: It took me a little bit to realize this comment was saying "We fail to init the Hmac with an algorithm, so an attempt to update should throw." I read it as "We're going to fail (that is: throw) when we try to init the Hmac with an algorithm" and I was all like, "What? I do not understand this test. There's no algorithm specified at all!"
Maybe expand the comment a bit to something like: "Hmac.update() should throw if Hmac has not been initialized with an algorithm" or something like that?
def691f
to
71072af
Compare
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: nodejs#10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
71072af
to
b4b37e8
Compare
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: nodejs#10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: nodejs#10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: nodejs#10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: nodejs#10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: #10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: #10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: #10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The Hmac binding layer is not documented as part of the API, and is not intended to be used, but it should be robust to misuse, and contains defensive checks for misuse. This test checks that updates without init throw (as opposed to abort or misbehave in some other way). PR-URL: #10923 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The Hmac binding layer is not documented as part of the API, and is not
intended to be used, but it should be robust to misuse, and contains
defensive checks for misuse. This test checks that updates without init
throw (as opposed to abort or misbehave in some other way).
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test