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: #35093 Refs: #21525 Refs: #20816
nodejs · nodejs-github-bot · Jul 10, 2024 · Jun 3, 2024 · Jun 3, 2024 · 87c5136313c866895879e22b2c0b8d332e7df983
commit 87c5136313c866895879e22b2c0b8d332e7df983
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -1010,7 +1010,8 @@ An invalid message length was provided.
 added: v15.0.0
 -->
 
-Invalid scrypt algorithm parameters were provided.
+One or more [`crypto.scrypt()`][] or [`crypto.scryptSync()`][] parameters are
+outside their legal range.
 
 <a id="ERR_CRYPTO_INVALID_STATE"></a>
 
@@ -1070,13 +1071,6 @@ A crypto operation failed for an otherwise unspecified reason.
 The PBKDF2 algorithm failed for unspecified reasons. OpenSSL does not provide
 more details and therefore neither does Node.js.
 
-<a id="ERR_CRYPTO_SCRYPT_INVALID_PARAMETER"></a>
-
-### `ERR_CRYPTO_SCRYPT_INVALID_PARAMETER`
-
-One or more [`crypto.scrypt()`][] or [`crypto.scryptSync()`][] parameters are
-outside their legal range.
-
 <a id="ERR_CRYPTO_SCRYPT_NOT_SUPPORTED"></a>
 
 ### `ERR_CRYPTO_SCRYPT_NOT_SUPPORTED`
@@ -3207,6 +3201,18 @@ The UTF-16 encoding was used with [`hash.digest()`][]. While the
 causing the method to return a string rather than a `Buffer`, the UTF-16
 encoding (e.g. `ucs` or `utf16le`) is not supported.
 
+<a id="ERR_CRYPTO_SCRYPT_INVALID_PARAMETER"></a>
+
+### `ERR_CRYPTO_SCRYPT_INVALID_PARAMETER`
+
+<!-- YAML
+removed: REPLACEME
+-->
+
+An incompatible combination of options was passed to [`crypto.scrypt()`][] or
+[`crypto.scryptSync()`][]. New versions of Node.js use the error code
+[`ERR_INCOMPATIBLE_OPTION_PAIR`][] instead, which is consistent with other APIs.
+
 <a id="ERR_FS_INVALID_SYMLINK_TYPE"></a>
 
 ### `ERR_FS_INVALID_SYMLINK_TYPE`
@@ -3981,6 +3987,7 @@ An error occurred trying to allocate memory. This should never happen.
 [`--no-addons`]: cli.md#--no-addons
 [`--unhandled-rejections`]: cli.md#--unhandled-rejectionsmode
 [`Class: assert.AssertionError`]: assert.md#class-assertassertionerror
+[`ERR_INCOMPATIBLE_OPTION_PAIR`]: #err_incompatible_option_pair
 [`ERR_INVALID_ARG_TYPE`]: #err_invalid_arg_type
 [`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: #err_missing_message_port_in_transfer_list
 [`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`]: #err_missing_transferable_in_transfer_list

diff --git a/lib/internal/crypto/scrypt.js b/lib/internal/crypto/scrypt.js
@@ -21,8 +21,8 @@ const {
 
 const {
   codes: {
-    ERR_CRYPTO_SCRYPT_INVALID_PARAMETER,
     ERR_CRYPTO_SCRYPT_NOT_SUPPORTED,
+    ERR_INCOMPATIBLE_OPTION_PAIR,
   },
 } = require('internal/errors');
 
@@ -92,7 +92,7 @@ function check(password, salt, keylen, options) {
       validateUint32(N, 'N');
     }
     if (options.cost !== undefined) {
-      if (has_N) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
+      if (has_N) throw new ERR_INCOMPATIBLE_OPTION_PAIR('N', 'cost');
       N = options.cost;
       validateUint32(N, 'cost');
     }
@@ -102,7 +102,7 @@ function check(password, salt, keylen, options) {
       validateUint32(r, 'r');
     }
     if (options.blockSize !== undefined) {
-      if (has_r) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
+      if (has_r) throw new ERR_INCOMPATIBLE_OPTION_PAIR('r', 'blockSize');
       r = options.blockSize;
       validateUint32(r, 'blockSize');
     }
@@ -112,7 +112,7 @@ function check(password, salt, keylen, options) {
       validateUint32(p, 'p');
     }
     if (options.parallelization !== undefined) {
-      if (has_p) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
+      if (has_p) throw new ERR_INCOMPATIBLE_OPTION_PAIR('p', 'parallelization');
       p = options.parallelization;
       validateUint32(p, 'parallelization');
     }

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -1164,7 +1164,6 @@ E('ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE',
   'Invalid key object type %s, expected %s.', TypeError);
 E('ERR_CRYPTO_INVALID_STATE', 'Invalid state for operation %s', Error);
 E('ERR_CRYPTO_PBKDF2_ERROR', 'PBKDF2 error', Error);
-E('ERR_CRYPTO_SCRYPT_INVALID_PARAMETER', 'Invalid scrypt parameter', Error);
 E('ERR_CRYPTO_SCRYPT_NOT_SUPPORTED', 'Scrypt algorithm not supported', Error);
 // Switch to TypeError. The current implementation does not seem right.
 E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign', Error);

diff --git a/test/parallel/test-crypto-scrypt.js b/test/parallel/test-crypto-scrypt.js
@@ -93,12 +93,16 @@ const good = [
   },
 ];
 
-// Test vectors that should fail.
+// Test vectors that contain invalid parameters.
 const bad = [
   { N: 1, p: 1, r: 1 },         // N < 2
   { N: 3, p: 1, r: 1 },         // Not power of 2.
-  { N: 1, cost: 1 },            // Both N and cost
-  { p: 1, parallelization: 1 }, // Both p and parallelization
+];
+
+// Test vectors that contain incompatible options.
+const incompatibleOptions = [
+  { N: 1, cost: 1 },             // Both N and cost
+  { p: 1, parallelization: 1 },  // Both p and parallelization
   { r: 1, blockSize: 1 },        // Both r and blocksize
 ];
 
@@ -176,6 +180,18 @@ for (const options of bad) {
                 expected);
 }
 
+for (const options of incompatibleOptions) {
+  const [short, long] = Object.keys(options).sort((a, b) => a.length - b.length);
+  const expected = {
+    message: `Option "${short}" cannot be used in combination with option "${long}"`,
+    code: 'ERR_INCOMPATIBLE_OPTION_PAIR',
+  };
+  assert.throws(() => crypto.scrypt('pass', 'salt', 1, options, () => {}),
+                expected);
+  assert.throws(() => crypto.scryptSync('pass', 'salt', 1, options),
+                expected);
+}
+
 for (const options of toobig) {
   const expected = {
     message: /Invalid scrypt param/