Skip to content

Commit 634de22

Browse files
tniessenehsankhfr
tniessen
authored and
ehsankhfr
committed
crypto: remove ERR_CRYPTO_SCRYPT_INVALID_PARAMETER
It is confusing to have both ERR_CRYPTO_SCRYPT_INVALID_PARAMETER and ERR_CRYPTO_INVALID_SCRYPT_PARAMS. The former was the original error code, added in 371103d, but parameter validation gradually changed and now produces ERR_CRYPTO_INVALID_SCRYPT_PARAMS for all parameter validation errors coming from OpenSSL, as well as different error codes for validation errors coming from JavaScript. The only remaining use of ERR_CRYPTO_SCRYPT_INVALID_PARAMETER is in the validation logic that ensures that no two synonymous options were passed. We already have an error code for that particular case, ERR_INCOMPATIBLE_OPTION_PAIR, so replace these last instances of ERR_CRYPTO_SCRYPT_INVALID_PARAMETER with that error code and remove ERR_CRYPTO_SCRYPT_INVALID_PARAMETER. If there ever is need again for such an error code, we can just use ERR_CRYPTO_INVALID_SCRYPT_PARAMS. Refs: nodejs#35093 Refs: nodejs#21525 Refs: nodejs#20816 PR-URL: nodejs#53305 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent f266c5e commit 634de22

4 files changed

Lines changed: 38 additions & 16 deletions

File tree

‎doc/api/errors.md‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,8 @@ An invalid message length was provided.
10101010
added: v15.0.0
10111011
-->
10121012

1013-
Invalid scrypt algorithm parameters were provided.
1013+
One or more [`crypto.scrypt()`][] or [`crypto.scryptSync()`][] parameters are
1014+
outside their legal range.
10141015

10151016
<a id="ERR_CRYPTO_INVALID_STATE"></a>
10161017

@@ -1070,13 +1071,6 @@ A crypto operation failed for an otherwise unspecified reason.
10701071
The PBKDF2 algorithm failed for unspecified reasons. OpenSSL does not provide
10711072
more details and therefore neither does Node.js.
10721073

1073-
<a id="ERR_CRYPTO_SCRYPT_INVALID_PARAMETER"></a>
1074-
1075-
### `ERR_CRYPTO_SCRYPT_INVALID_PARAMETER`
1076-
1077-
One or more [`crypto.scrypt()`][] or [`crypto.scryptSync()`][] parameters are
1078-
outside their legal range.
1079-
10801074
<a id="ERR_CRYPTO_SCRYPT_NOT_SUPPORTED"></a>
10811075

10821076
### `ERR_CRYPTO_SCRYPT_NOT_SUPPORTED`
@@ -3271,6 +3265,18 @@ The UTF-16 encoding was used with [`hash.digest()`][]. While the
32713265
causing the method to return a string rather than a `Buffer`, the UTF-16
32723266
encoding (e.g. `ucs` or `utf16le`) is not supported.
32733267

3268+
<a id="ERR_CRYPTO_SCRYPT_INVALID_PARAMETER"></a>
3269+
3270+
### `ERR_CRYPTO_SCRYPT_INVALID_PARAMETER`
3271+
3272+
<!-- YAML
3273+
removed: REPLACEME
3274+
-->
3275+
3276+
An incompatible combination of options was passed to [`crypto.scrypt()`][] or
3277+
[`crypto.scryptSync()`][]. New versions of Node.js use the error code
3278+
[`ERR_INCOMPATIBLE_OPTION_PAIR`][] instead, which is consistent with other APIs.
3279+
32743280
<a id="ERR_FS_INVALID_SYMLINK_TYPE"></a>
32753281

32763282
### `ERR_FS_INVALID_SYMLINK_TYPE`
@@ -4046,6 +4052,7 @@ An error occurred trying to allocate memory. This should never happen.
40464052
[`--no-addons`]: cli.md#--no-addons
40474053
[`--unhandled-rejections`]: cli.md#--unhandled-rejectionsmode
40484054
[`Class: assert.AssertionError`]: assert.md#class-assertassertionerror
4055+
[`ERR_INCOMPATIBLE_OPTION_PAIR`]: #err_incompatible_option_pair
40494056
[`ERR_INVALID_ARG_TYPE`]: #err_invalid_arg_type
40504057
[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: #err_missing_message_port_in_transfer_list
40514058
[`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`]: #err_missing_transferable_in_transfer_list

‎lib/internal/crypto/scrypt.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ const {
2121

2222
const {
2323
codes: {
24-
ERR_CRYPTO_SCRYPT_INVALID_PARAMETER,
2524
ERR_CRYPTO_SCRYPT_NOT_SUPPORTED,
25+
ERR_INCOMPATIBLE_OPTION_PAIR,
2626
},
2727
} = require('internal/errors');
2828

@@ -92,7 +92,7 @@ function check(password, salt, keylen, options) {
9292
validateUint32(N, 'N');
9393
}
9494
if (options.cost !== undefined) {
95-
if (has_N) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
95+
if (has_N) throw new ERR_INCOMPATIBLE_OPTION_PAIR('N', 'cost');
9696
N = options.cost;
9797
validateUint32(N, 'cost');
9898
}
@@ -102,7 +102,7 @@ function check(password, salt, keylen, options) {
102102
validateUint32(r, 'r');
103103
}
104104
if (options.blockSize !== undefined) {
105-
if (has_r) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
105+
if (has_r) throw new ERR_INCOMPATIBLE_OPTION_PAIR('r', 'blockSize');
106106
r = options.blockSize;
107107
validateUint32(r, 'blockSize');
108108
}
@@ -112,7 +112,7 @@ function check(password, salt, keylen, options) {
112112
validateUint32(p, 'p');
113113
}
114114
if (options.parallelization !== undefined) {
115-
if (has_p) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
115+
if (has_p) throw new ERR_INCOMPATIBLE_OPTION_PAIR('p', 'parallelization');
116116
p = options.parallelization;
117117
validateUint32(p, 'parallelization');
118118
}

‎lib/internal/errors.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1168,7 +1168,6 @@ E('ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE',
11681168
'Invalid key object type %s, expected %s.', TypeError);
11691169
E('ERR_CRYPTO_INVALID_STATE', 'Invalid state for operation %s', Error);
11701170
E('ERR_CRYPTO_PBKDF2_ERROR', 'PBKDF2 error', Error);
1171-
E('ERR_CRYPTO_SCRYPT_INVALID_PARAMETER', 'Invalid scrypt parameter', Error);
11721171
E('ERR_CRYPTO_SCRYPT_NOT_SUPPORTED', 'Scrypt algorithm not supported', Error);
11731172
// Switch to TypeError. The current implementation does not seem right.
11741173
E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign', Error);

‎test/parallel/test-crypto-scrypt.js‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,16 @@ const good = [
9393
},
9494
];
9595

96-
// Test vectors that should fail.
96+
// Test vectors that contain invalid parameters.
9797
const bad = [
9898
{ N: 1, p: 1, r: 1 }, // N < 2
9999
{ N: 3, p: 1, r: 1 }, // Not power of 2.
100-
{ N: 1, cost: 1 }, // Both N and cost
101-
{ p: 1, parallelization: 1 }, // Both p and parallelization
100+
];
101+
102+
// Test vectors that contain incompatible options.
103+
const incompatibleOptions = [
104+
{ N: 1, cost: 1 }, // Both N and cost
105+
{ p: 1, parallelization: 1 }, // Both p and parallelization
102106
{ r: 1, blockSize: 1 }, // Both r and blocksize
103107
];
104108

@@ -176,6 +180,18 @@ for (const options of bad) {
176180
expected);
177181
}
178182

183+
for (const options of incompatibleOptions) {
184+
const [short, long] = Object.keys(options).sort((a, b) => a.length - b.length);
185+
const expected = {
186+
message: `Option "${short}" cannot be used in combination with option "${long}"`,
187+
code: 'ERR_INCOMPATIBLE_OPTION_PAIR',
188+
};
189+
assert.throws(() => crypto.scrypt('pass', 'salt', 1, options, () => {}),
190+
expected);
191+
assert.throws(() => crypto.scryptSync('pass', 'salt', 1, options),
192+
expected);
193+
}
194+
179195
for (const options of toobig) {
180196
const expected = {
181197
message: /Invalid scrypt params:.*memory limit exceeded/,

0 commit comments

Comments
 (0)