node:crypto: refuse both names of a scrypt option when the second is read, with a bindgen exclusive member

docs/project/bindgen.mdx

@@ -399,7 +399,11 @@ export const BracesOptions = b.dictionary({
 Each member is read, then converted, in declaration order, once. A member
 whose type does not accept `undefined` is required. A member can be written
 `{ type, rustName, altNames, expected }` to rename its field, to accept other spellings, or to report a wrong type as
-a function's `expected` does.
+a function's `expected` does. What another spelling holds is reported under the member's own name.
+`exclusive: true` is for an option that Node gives two names and refuses under both, as `cost` is the second name of
+scrypt's `N`: `N: { type: b.u32.validateInteger().optional, altNames: ["cost"], exclusive: true }`. Each name is read once, and
+what it holds is converted and reported under that name, into the one field. If neither holds `undefined`, the second throws
+`ERR_INCOMPATIBLE_OPTION_PAIR` when it is read: before its value is converted, and before a later member is read.
 A dictionary whose members can all be absent implements `Default`. As an argument, with
 `.default()` or without, the struct stays in the frame of the thunk and the body borrows it:
 `options: &BracesOptions`, or `Option<&BracesOptions>` with one of the modifiers that make it an `Option`. As a member, with `.withValue`, as the value of a setter, or inside a union or an array, it is held by value.

src/codegen/bindgen-emit.ts

@@ -23,6 +23,7 @@ import {
   isIfPassed,
   kindTests,
   LocalStringType,
+  type Member,
   memberIdentifier,
   type Method,
   type ModuleDefinition,
@@ -583,11 +584,12 @@ function cppContext(
 function cppSourceForDictionary(type: DictionaryType): string {
   if (!type.accepted) return "";
   let nameIndex = 0;
-  const members = type.members.map((member, i) => {
-    const [first, ...others] = member.names.map(
+  /** Reads `names` in order while the value is `undefined`, and converts what it finds as `names[0]`. `refusal` is C++ that runs ahead of the conversion of a value that is not `undefined`. */
+  const readMember = (member: Member, i: number, names: readonly string[], refusal?: string) => {
+    const [first, ...others] = names.map(
       () => `members.get(${nameIndex++});\n${cppReturnIfFailed(`!!value${i}`, "false")}`,
     );
-    const qualified = type.reportedAs ? `${type.reportedAs}.${member.name}` : member.name;
+    const qualified = type.reportedAs ? `${type.reportedAs}.${names[0]}` : names[0];
     const lines: string[] = [];
     const ctx = cppContext(lines, i, qualified, member.expected, `value${i}`);
     lines.push(
@@ -599,8 +601,25 @@ function cppSourceForDictionary(type: DictionaryType): string {
     ${ctx}.throwRequired(*globalObject, scope);
     return false;
 }`);
-    lines.push(member.type.cppConvertInto(`result.${member.fieldName}`, `value${i}`, ctx, qualified, "false"));
+    const convert = member.type.cppConvertInto(`result.${member.fieldName}`, `value${i}`, ctx, qualified, "false");
+    lines.push(refusal ? `if (!value${i}.isUndefined()) {\n${indent(`${refusal}\n${convert}`, 4)}\n}` : convert);
     return `    {\n${indent(lines.join("\n"), 8)}\n    }`;
+  };
+  const members = type.members.flatMap((member, i) => {
+    if (!member.exclusive) return readMember(member, i, member.names);
+    const [first, second] = member.names;
+    return [
+      readMember(member, i, [first]),
+      readMember(
+        member,
+        i,
+        [second],
+        `if (result.${member.fieldName}) [[unlikely]] {
+    Bun::ERR::INCOMPATIBLE_OPTION_PAIR(scope, globalObject, ${asciiLiteral(first)}, ${asciiLiteral(second)});
+    return false;
+}`,
+      ),
+    ];
   });
   const count = type.names.length;
   return `
@@ -2473,7 +2492,8 @@ export function dts(files: readonly SchemaFile[]): string {
   const declarations = files.flatMap(file =>
     file.types.map(type => {
       if (type instanceof DictionaryType) {
-        return `  interface ${type.name} {\n${type.members.map(m => member(m.name, m.type.takesUndefined, m.type.tsType)).join("\n")}\n  }`;
+        const names = (m: Member) => (m.exclusive ? m.names : [m.name]);
+        return `  interface ${type.name} {\n${type.members.flatMap(m => names(m).map(name => member(name, m.type.takesUndefined, m.type.tsType))).join("\n")}\n  }`;
       }
       if (!(type instanceof ClassType)) return "";
       return `  interface ${type.name}${type.base ? ` extends ${type.base.name}` : ""} {

src/codegen/bindgen-lib-internal.ts

@@ -2484,8 +2484,10 @@ export interface MemberOptions {
   type: Type;
   /** Name of the field in the Rust struct, and in the C++ one that mirrors it. Defaults to the JS name in snake_case. */
   rustName?: string;
-  /** Other JS names, tried in order while the value is `undefined`. */
+  /** Other JS names, tried in order while the value is `undefined`. What any of them holds is reported under the member's own name. */
   altNames?: readonly string[];
+  /** The one name in `altNames` is a second name that Node gives the option, as `cost` is of scrypt's `N`. Each is read and reported as itself, and if neither holds `undefined` the second throws `ERR_INCOMPATIBLE_OPTION_PAIR` before it is converted. */
+  exclusive?: boolean;
   /** As a function's `expected`, for this member. */
   expected?: readonly string[] | string;
 }
@@ -2496,6 +2498,7 @@ export class Member {
   readonly fieldName: string;
   readonly type: Type;
   readonly expected?: readonly string[] | string;
+  readonly exclusive: boolean;
   constructor(name: string, spec: Type | MemberOptions) {
     const options: MemberOptions = spec instanceof Type ? { type: spec } : spec;
     this.names = [name, ...(options.altNames ?? [])];
@@ -2508,6 +2511,19 @@ export class Member {
     assert(options.expected === undefined || options.expected.length > 0, `expected: ${name} expects nothing`);
     if (options.expected !== undefined) refuseExpectedOfRange(name, options.type);
     this.expected = options.expected;
+    this.exclusive = options.exclusive ?? false;
+    assert(
+      !this.exclusive || (this.names.length === 2 && this.names[1] !== name),
+      `exclusive: ${name} has one other name in altNames`,
+    );
+    // The converter asks the field whether the first name filled it, which a std::optional answers.
+    assert(
+      !this.exclusive ||
+        (this.type instanceof OptionalType &&
+          this.type.modifier === "optional" &&
+          this.type.inner.nullRepresentation === "optional"),
+      `exclusive: ${name} is .optional, and not a string, bytes, an instance or a JS value`,
+    );
   }
   get name(): string {
     return this.names[0];
@@ -2592,6 +2608,10 @@ export class DictionaryType extends NamedType {
   get refuses(): string | null {
     return this.nonObjectAsEmpty ? null : nonObjects;
   }
+  /** An object that gives both names of an `exclusive` member is refused, whatever the member's type takes. */
+  get canRefuse(): boolean {
+    return super.canRefuse || this.members.some(member => member.exclusive);
+  }
   get borrowedByBody(): boolean {
     return true;
   }

src/js/internal-for-testing.ts

@@ -315,6 +315,7 @@ export const bindgen = $bindgen("bindgen_test.bind.ts", "add") && {
   testNamespace: $bindgen("bindgen_test.bind.ts", "testNamespace"),
   nodeOptionsSize: $bindgen("bindgen_test.bind.ts", "nodeOptionsSize"),
   bareOptionsSize: $bindgen("bindgen_test.bind.ts", "bareOptionsSize"),
+  exclusiveEcho: $bindgen("bindgen_test.bind.ts", "exclusiveEcho"),
   timeValue: $bindgen("bindgen_test.bind.ts", "timeValue"),
   textIfPassed: $bindgen("bindgen_test.bind.ts", "textIfPassed"),
   isPassed: $bindgen("bindgen_test.bind.ts", "isPassed"),

src/jsc/JSGlobalObject.rs

@@ -502,18 +502,6 @@ impl JSGlobalObject {
         Ok(str)
     }
 
-    pub fn throw_incompatible_option_pair(&self, opt1: &[u8], opt2: &[u8]) -> JsError {
-        self.err(
-            JscError::INCOMPATIBLE_OPTION_PAIR,
-            format_args!(
-                "Option \"{}\" cannot be used in combination with option \"{}\"",
-                bstr::BStr::new(opt1),
-                bstr::BStr::new(opt2)
-            ),
-        )
-        .throw()
-    }
-
     pub fn throw_invalid_scrypt_params(&self) -> JsError {
         let err = bun_boringssl::c::ERR_peek_last_error();
         if err != 0 {

src/jsc/bindgen_test.bind.ts

@@ -505,6 +505,13 @@ export const TestNodeOptions = b.dictionary(
 export const TestBareOptions = b.dictionary({ size: b.u32.validateInteger(1, 10).optional }, { reportedAs: "" });
 export const nodeOptionsSize = b.fn({ args: { options: TestNodeOptions }, returns: b.u32.optional });
 export const bareOptionsSize = b.fn({ args: { options: TestBareOptions }, returns: b.u32.optional });
+/** `length` is a second name of `size`, and `scale` of `ratio`. */
+export const TestExclusive = b.dictionary({
+  size: { type: b.u8.optional, altNames: ["length"], exclusive: true },
+  ratio: { type: b.f64.optional, altNames: ["scale"], exclusive: true, expected: ["number"] },
+  last: b.u8.optional,
+});
+export const exclusiveEcho = b.fn({ args: { options: TestExclusive }, returns: TestExclusive });
 
 export const timeValue = b.fn({ args: { time: b.f64.allowDate }, returns: b.f64 });
 /** "none" without the argument. `undefined` is converted, to "undefined". */

src/jsc/bindgen_test.rs

@@ -10,11 +10,11 @@ use crate::bindgen::{
 };
 use crate::generated_bindgen::bindgen_test::{
     Bun__BindgenTest__doubled, TestAbsence, TestAnything, TestBare, TestBareOptions, TestBracket,
-    TestChoice, TestColor, TestColorOrRest, TestFailure, TestFlag, TestHandle, TestNames,
-    TestNodeOptions, TestObjectOrRest, TestOptions, TestOptionsEcho, TestOtherAsNullOptions,
-    TestOuter, TestTextOrBytes, TestValue, js_TestCounter, js_TestError, js_TestMemberFunctions,
-    js_TestNonFinal, js_TestNote, js_TestRefCounted, js_TestRefCountedNote, js_TestShape,
-    js_TestSquare,
+    TestChoice, TestColor, TestColorOrRest, TestExclusive, TestFailure, TestFlag, TestHandle,
+    TestNames, TestNodeOptions, TestObjectOrRest, TestOptions, TestOptionsEcho,
+    TestOtherAsNullOptions, TestOuter, TestTextOrBytes, TestValue, js_TestCounter, js_TestError,
+    js_TestMemberFunctions, js_TestNonFinal, js_TestNote, js_TestRefCounted, js_TestRefCountedNote,
+    js_TestShape, js_TestSquare,
 };
 use crate::{JSGlobalObject, JSString, JSValue, JsClass, JsResult};
 use bun_ptr::{AsCtxPtr as _, RefPtr};
@@ -971,6 +971,14 @@ pub(crate) fn bare_options_size(options: &TestBareOptions) -> Option<u32> {
     options.size
 }
 
+pub(crate) fn exclusive_echo(options: &TestExclusive) -> TestExclusive {
+    TestExclusive {
+        size: options.size,
+        ratio: options.ratio,
+        last: options.last,
+    }
+}
+
 pub(crate) fn time_value(time: f64) -> f64 {
     time
 }

src/runtime/node/node_crypto_binding.bind.ts

@@ -37,14 +37,12 @@ const randomUUID = {
 } as const;
 
 const optionalUint32 = b.u32.validateInteger().optional;
+/** Node refuses the second name of an option where it reads it: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/crypto/scrypt.js#L91-L100 */
 export const ScryptOptions = b.dictionary(
   {
-    N: optionalUint32,
-    cost: optionalUint32,
-    r: optionalUint32,
-    blockSize: optionalUint32,
-    p: optionalUint32,
-    parallelization: optionalUint32,
+    N: { type: optionalUint32, altNames: ["cost"], exclusive: true },
+    r: { type: optionalUint32, altNames: ["blockSize"], exclusive: true },
+    p: { type: optionalUint32, altNames: ["parallelization"], exclusive: true },
     maxmem: b.u64.validateInteger().optional,
   },
   { allowArray: true, nonObjectAsEmpty: true, reportedAs: "" },
@@ -154,15 +152,17 @@ export const crypto = b.namespace({
   getHashes: { returns: b.array(b.string.utf8) },
   scrypt: [
     {
-      args: { ...passwordAndSaltHandles, keylen, options: ScryptOptions, callback: b.callable },
+      /** `.default({})` so that an `undefined` fourth argument does not choose the other overload. */
+      args: { ...passwordAndSaltHandles, keylen, options: ScryptOptions.default({}), callback: b.callable },
       expected: { ...passwordAndSaltNames, callback: ["function"] },
       rustPath: "crate::node::crypto::scrypt",
       length: 5,
       ...js,
     },
+    /** Without a fifth argument the fourth is the callback, whatever it is: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/crypto/scrypt.js#L40-L44. The body reports one that is no function. */
     {
-      args: { ...passwordAndSaltHandles, keylen, callback: b.callable },
-      expected: { ...passwordAndSaltNames, callback: ["function"] },
+      args: { ...passwordAndSaltHandles, keylen, callback: b.any },
+      expected: passwordAndSaltNames,
       rustPath: "crate::node::crypto::scrypt_with_defaults",
       ...js,
     },

src/runtime/node/node_crypto_binding.rs

@@ -707,16 +707,6 @@ pub(crate) struct Argon2<Bytes> {
 
 impl Scrypt {
     fn new(global: &JSGlobalObject, keylen: u32, options: &ScryptOptions) -> JsResult<Self> {
-        if options.n.is_some() && options.cost.is_some() {
-            return Err(global.throw_incompatible_option_pair(b"N", b"cost"));
-        }
-        if options.r.is_some() && options.block_size.is_some() {
-            return Err(global.throw_incompatible_option_pair(b"r", b"blockSize"));
-        }
-        if options.p.is_some() && options.parallelization.is_some() {
-            return Err(global.throw_incompatible_option_pair(b"p", b"parallelization"));
-        }
-
         const N_DEFAULT: u32 = 16384;
         const R_DEFAULT: u32 = 8;
         const P_DEFAULT: u32 = 1;
@@ -725,9 +715,9 @@ impl Scrypt {
         let or_default = |value: Option<u32>, default| value.filter(|&n| n != 0).unwrap_or(default);
 
         let ctx = Scrypt {
-            n: or_default(options.n.or(options.cost), N_DEFAULT),
-            r: or_default(options.r.or(options.block_size), R_DEFAULT),
-            p: or_default(options.p.or(options.parallelization), P_DEFAULT),
+            n: or_default(options.n, N_DEFAULT),
+            r: or_default(options.r, R_DEFAULT),
+            p: or_default(options.p, P_DEFAULT),
             maxmem: options
                 .maxmem
                 .filter(|&maxmem| maxmem != 0)
@@ -1070,8 +1060,15 @@ pub(crate) fn scrypt_with_defaults<'a>(
     password: StringOrJsBytes<'a>,
     salt: StringOrJsBytes<'a>,
     keylen: u32,
-    callback: JsLocal<'a, Callable>,
+    callback: JsLocal<'a>,
 ) -> JsResult<()> {
+    let Some(callback) = callback.as_callable() else {
+        return Err(cx.global().throw_invalid_argument_type_value(
+            b"callback",
+            b"function",
+            callback.to_js(),
+        ));
+    };
     scrypt(
         cx,
         password,

test/aot/parallel/builtin-modules/scrypt-describes-fourth-argument-without-fifth.mjs

@@ -0,0 +1,21 @@
+// Which code of scrypt's fourth argument runs when it is no function and no fifth follows: it is the callback, and it is described.
+import { scrypt } from "node:crypto";
+function run() {
+  const box = { field: 1 };
+  let count = 0;
+  const fourth = {
+    get constructor() {
+      count++;
+      box.field = 2;
+      return { name: "Named" };
+    },
+  };
+  let message = "no error";
+  try {
+    scrypt("a", "b", 8, fourth);
+  } catch (error) {
+    message = error.message;
+  }
+  console.log("scrypt callback |", message, "|", count, box.field);
+}
+run();

test/aot/parallel/builtin-modules/scrypt-describes-option-refused-before-callback.mjs

@@ -0,0 +1,21 @@
+// Which code of a refused member of scrypt's options runs when a callback follows: the callee is known, and the values are local.
+import { scrypt } from "node:crypto";
+function run() {
+  const box = { field: 1 };
+  let count = 0;
+  const member = {
+    get constructor() {
+      count++;
+      box.field = 2;
+      return { name: "Named" };
+    },
+  };
+  let message = "no error";
+  try {
+    scrypt("a", "b", 8, { N: member }, () => {});
+  } catch (error) {
+    message = error.message;
+  }
+  console.log("scrypt option |", message, "|", count, box.field);
+}
+run();

test/aot/parallel/builtin-modules/scrypt-reads-no-option-without-fifth.mjs

@@ -0,0 +1,26 @@
+// scrypt with an undefined fifth argument takes the fourth as the callback and reads no option of it.
+import { scrypt } from "node:crypto";
+function run() {
+  const box = { field: 1 };
+  let count = 0;
+  const options = {
+    get N() {
+      count++;
+      box.field = 2;
+      return 2;
+    },
+    get cost() {
+      count++;
+      box.field = 3;
+      return "x";
+    },
+  };
+  let message = "no error";
+  try {
+    scrypt("a", "b", 8, options, undefined);
+  } catch (error) {
+    message = error.code + " " + error.message;
+  }
+  console.log("scrypt fifth undefined |", message, "|", count, box.field);
+}
+run();

test/aot/parallel/builtin-modules/scrypt-refuses-two-names-before-callback.mjs

@@ -0,0 +1,27 @@
+// scrypt refuses both names of an option when it reads the second, before it reads a later one and before it looks at the callback.
+import { scrypt } from "node:crypto";
+function run() {
+  const read = [];
+  const options = {
+    get N() {
+      read.push("N");
+      return 2;
+    },
+    get cost() {
+      read.push("cost");
+      return "x";
+    },
+    get r() {
+      read.push("r");
+      return 1;
+    },
+  };
+  let message = "no error";
+  try {
+    scrypt("a", "b", 8, options, 5);
+  } catch (error) {
+    message = error.code + " " + error.message;
+  }
+  console.log("scrypt pair |", message, "|", read.join());
+}
+run();