From b55a43b7c677cc2228b5ad2034fe46b99799ce83 Mon Sep 17 00:00:00 2001 From: Ben Bucksch Date: Wed, 20 May 2020 14:13:29 +0200 Subject: [PATCH 1/5] crypto: fix wrong error message When calling `crypto.sign()`, if the `key` parameter object is missing the `key` property, the error message is wrong. Before the fix: TypeError [ERR_INVALID_ARG_TYPE]: The "key" argument must be of type string or an instance of Buffer, TypedArray, DataView, or KeyObject. Received an instance of Object Expected: TypeError [ERR_INVALID_ARG_TYPE]: The "key.key property" argument must be of type string or an instance of Buffer, TypedArray, DataView, or KeyObject. Received undefined Fixes: https://github.com/nodejs/node/issues/33480 This seems like a copy&paste bug. Somebody copied from the end of the function, where this is correct, to here, where it's wrong. --- lib/internal/crypto/keys.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index c0a133d5d429a3..644f6a39e5ecc1 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -268,10 +268,10 @@ function prepareAsymmetricKey(key, ctx) { // Either PEM or DER using PKCS#1 or SPKI. if (!isStringOrBuffer(data)) { throw new ERR_INVALID_ARG_TYPE( - 'key', + 'key.key property', ['string', 'Buffer', 'TypedArray', 'DataView', ...(ctx !== kCreatePrivate ? ['KeyObject'] : [])], - key); + data); } const isPublic = From 0fb7721285a3d4f9a2f7cd7533cd009d555c98c1 Mon Sep 17 00:00:00 2001 From: Ben Bucksch Date: Wed, 20 May 2020 15:11:41 +0200 Subject: [PATCH 2/5] Adapt test to new error message --- test/parallel/test-crypto-sign-verify.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js index b70bfccae47eef..eff271faebf570 100644 --- a/test/parallel/test-crypto-sign-verify.js +++ b/test/parallel/test-crypto-sign-verify.js @@ -394,7 +394,7 @@ assert.throws( const errObj = { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "key" argument must be of type string or an instance of ' + + message: 'The "key.key property" argument must be of type string or an instance of ' + 'Buffer, TypedArray, DataView, or KeyObject.' + common.invalidArgTypeHelper(input) }; From 8b0621738a85f8c54a780940c0e43d818f8e0038 Mon Sep 17 00:00:00 2001 From: Ben Bucksch Date: Wed, 20 May 2020 16:40:00 +0200 Subject: [PATCH 3/5] Review comment --- lib/internal/crypto/keys.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index 644f6a39e5ecc1..bfb30dda3fd752 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -268,7 +268,7 @@ function prepareAsymmetricKey(key, ctx) { // Either PEM or DER using PKCS#1 or SPKI. if (!isStringOrBuffer(data)) { throw new ERR_INVALID_ARG_TYPE( - 'key.key property', + 'key.key', ['string', 'Buffer', 'TypedArray', 'DataView', ...(ctx !== kCreatePrivate ? ['KeyObject'] : [])], data); From 2148e9af33409bc813255bb4b1bc39494f32ff82 Mon Sep 17 00:00:00 2001 From: Ben Bucksch Date: Wed, 20 May 2020 16:40:15 +0200 Subject: [PATCH 4/5] Adapt tests --- test/parallel/test-crypto-sign-verify.js | 32 +++++++++++++++++------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js index eff271faebf570..2516a3ace40cbd 100644 --- a/test/parallel/test-crypto-sign-verify.js +++ b/test/parallel/test-crypto-sign-verify.js @@ -390,13 +390,19 @@ assert.throws( verify.update(new clazz()); }); - [1, {}, [], Infinity].forEach((input) => { + [{}, [], 1, Infinity].forEach((input) => { + let prop = '"key" argument'; + let value = input; + if (typeof input === 'object') { + prop = '"key.key" property'; + value = undefined; + } const errObj = { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', - message: 'The "key.key property" argument must be of type string or an instance of ' + - 'Buffer, TypedArray, DataView, or KeyObject.' + - common.invalidArgTypeHelper(input) + message: `The ${prop} must be of type string or ` + + 'an instance of Buffer, TypedArray, DataView, or KeyObject.' + + common.invalidArgTypeHelper(value) }; assert.throws(() => sign.sign(input), errObj); @@ -478,25 +484,33 @@ assert.throws( [1, {}, [], true, Infinity].forEach((input) => { const data = Buffer.alloc(1); const sig = Buffer.alloc(1); - const received = common.invalidArgTypeHelper(input); const errObj = { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError', message: 'The "data" argument must be an instance of Buffer, ' + - `TypedArray, or DataView.${received}` + 'TypedArray, or DataView.' + + common.invalidArgTypeHelper(input) }; assert.throws(() => crypto.sign(null, input, 'asdf'), errObj); assert.throws(() => crypto.verify(null, input, 'asdf', sig), errObj); - errObj.message = 'The "key" argument must be of type string or an instance ' + - `of Buffer, TypedArray, DataView, or KeyObject.${received}`; + let prop = '"key" argument'; + let value = input; + if (typeof input === 'object') { + prop = '"key.key" property'; + value = undefined; + } + errObj.message = `The ${prop} must be of type string or ` + + 'an instance of Buffer, TypedArray, DataView, or KeyObject.' + + common.invalidArgTypeHelper(value); assert.throws(() => crypto.sign(null, data, input), errObj); assert.throws(() => crypto.verify(null, data, input, sig), errObj); errObj.message = 'The "signature" argument must be an instance of ' + - `Buffer, TypedArray, or DataView.${received}`; + 'Buffer, TypedArray, or DataView.' + + common.invalidArgTypeHelper(input); assert.throws(() => crypto.verify(null, data, 'test', input), errObj); }); From c4a3d0c22ee1069036557d1327317c19c4ce3c48 Mon Sep 17 00:00:00 2001 From: Ben Bucksch Date: Wed, 20 May 2020 18:13:21 +0200 Subject: [PATCH 5/5] NIT: Remove unnecessary change --- test/parallel/test-crypto-sign-verify.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js index 2516a3ace40cbd..ff410dcf00fa6a 100644 --- a/test/parallel/test-crypto-sign-verify.js +++ b/test/parallel/test-crypto-sign-verify.js @@ -390,7 +390,7 @@ assert.throws( verify.update(new clazz()); }); - [{}, [], 1, Infinity].forEach((input) => { + [1, {}, [], Infinity].forEach((input) => { let prop = '"key" argument'; let value = input; if (typeof input === 'object') {