From c4d60b695717cd5ba840643b8f59561160c45ed2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:20:29 +0000 Subject: [PATCH 1/5] node:crypto: refuse both names of a scrypt option when the second is read `crypto.scrypt` and `crypto.scryptSync` give `N`, `r` and `p` a second name each: `cost`, `blockSize` and `parallelization`. Node refuses an options object that has both names of one option when it reads the second name. That is before it validates the value, before it reads a later option, and before it checks the callback. The generated converter of `ScryptOptions` validated all seven members, and the Rust body tested the pairs after that. So a wrong second value, a wrong later option or a wrong callback was reported in place of `ERR_INCOMPATIBLE_OPTION_PAIR`. bindgen: a dictionary member takes `exclusive: true` with one name in `altNames`. The converter reads each name once and converts what it holds under that name into the one field. It throws `ERR_INCOMPATIBLE_OPTION_PAIR` when the second name is given after the first. `ScryptOptions` now has one field for each of `N`, `r` and `p`. `crypto.scrypt(password, salt, keylen, x)` takes `x` as the callback again, whatever it is, and reads none of its members, as Node does. --- docs/project/bindgen.mdx | 6 +- src/codegen/bindgen-emit.ts | 30 +++- src/codegen/bindgen-lib-internal.ts | 23 ++- src/js/internal-for-testing.ts | 1 + src/jsc/JSGlobalObject.rs | 12 -- src/jsc/bindgen_test.bind.ts | 7 + src/jsc/bindgen_test.rs | 18 ++- src/runtime/node/node_crypto_binding.bind.ts | 18 +-- src/runtime/node/node_crypto_binding.rs | 25 ++- ...escribes-fourth-argument-without-fifth.mjs | 21 +++ ...scribes-option-refused-before-callback.mjs | 21 +++ .../scrypt-reads-no-option-without-fifth.mjs | 26 ++++ ...rypt-refuses-two-names-before-callback.mjs | 27 ++++ test/internal/bindgen.test.ts | 85 ++++++++++- .../crypto/scrypt-option-checks.fixture.js | 142 ++++++++++++++++++ test/js/node/crypto/scrypt.test.ts | 124 ++++++++++++++- 16 files changed, 536 insertions(+), 50 deletions(-) create mode 100644 test/aot/parallel/builtin-modules/scrypt-describes-fourth-argument-without-fifth.mjs create mode 100644 test/aot/parallel/builtin-modules/scrypt-describes-option-refused-before-callback.mjs create mode 100644 test/aot/parallel/builtin-modules/scrypt-reads-no-option-without-fifth.mjs create mode 100644 test/aot/parallel/builtin-modules/scrypt-refuses-two-names-before-callback.mjs create mode 100644 test/js/node/crypto/scrypt-option-checks.fixture.js diff --git a/docs/project/bindgen.mdx b/docs/project/bindgen.mdx index acbf0a0f6887..008db791e1ad 100644 --- a/docs/project/bindgen.mdx +++ b/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. An object that gives both throws +`ERR_INCOMPATIBLE_OPTION_PAIR` when the second name 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. diff --git a/src/codegen/bindgen-emit.ts b/src/codegen/bindgen-emit.ts index 5c55cdb93cbb..4e6ca65358bb 100644 --- a/src/codegen/bindgen-emit.ts +++ b/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}` : ""} { diff --git a/src/codegen/bindgen-lib-internal.ts b/src/codegen/bindgen-lib-internal.ts index 832a8c653746..6afe2af7e5e1 100644 --- a/src/codegen/bindgen-lib-internal.ts +++ b/src/codegen/bindgen-lib-internal.ts @@ -2484,8 +2484,14 @@ 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`. Both are read, + * and each is converted and reported under its own name. An object that gives both throws + * `ERR_INCOMPATIBLE_OPTION_PAIR` when the second is read, before it is converted. + */ + exclusive?: boolean; /** As a function's `expected`, for this member. */ expected?: readonly string[] | string; } @@ -2496,6 +2502,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 +2515,16 @@ 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, `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 +2609,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; } diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 8a6c6774d464..8e20be2419df 100644 --- a/src/js/internal-for-testing.ts +++ b/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"), diff --git a/src/jsc/JSGlobalObject.rs b/src/jsc/JSGlobalObject.rs index 5bcf6a2fd0b5..9002a5039d69 100644 --- a/src/jsc/JSGlobalObject.rs +++ b/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 { diff --git a/src/jsc/bindgen_test.bind.ts b/src/jsc/bindgen_test.bind.ts index 640da9692a05..89ee5aaa1beb 100644 --- a/src/jsc/bindgen_test.bind.ts +++ b/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". */ diff --git a/src/jsc/bindgen_test.rs b/src/jsc/bindgen_test.rs index 485ede471193..0abfabc593d8 100644 --- a/src/jsc/bindgen_test.rs +++ b/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 { 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 } diff --git a/src/runtime/node/node_crypto_binding.bind.ts b/src/runtime/node/node_crypto_binding.bind.ts index 17c408769f97..29b70c29b3af 100644 --- a/src/runtime/node/node_crypto_binding.bind.ts +++ b/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, }, diff --git a/src/runtime/node/node_crypto_binding.rs b/src/runtime/node/node_crypto_binding.rs index 996da64d9424..b18c5465afae 100644 --- a/src/runtime/node/node_crypto_binding.rs +++ b/src/runtime/node/node_crypto_binding.rs @@ -707,16 +707,6 @@ pub(crate) struct Argon2 { impl Scrypt { fn new(global: &JSGlobalObject, keylen: u32, options: &ScryptOptions) -> JsResult { - 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, 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, diff --git a/test/aot/parallel/builtin-modules/scrypt-describes-fourth-argument-without-fifth.mjs b/test/aot/parallel/builtin-modules/scrypt-describes-fourth-argument-without-fifth.mjs new file mode 100644 index 000000000000..16e8ba729328 --- /dev/null +++ b/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(); diff --git a/test/aot/parallel/builtin-modules/scrypt-describes-option-refused-before-callback.mjs b/test/aot/parallel/builtin-modules/scrypt-describes-option-refused-before-callback.mjs new file mode 100644 index 000000000000..78f162c45178 --- /dev/null +++ b/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(); diff --git a/test/aot/parallel/builtin-modules/scrypt-reads-no-option-without-fifth.mjs b/test/aot/parallel/builtin-modules/scrypt-reads-no-option-without-fifth.mjs new file mode 100644 index 000000000000..68aa08ca2b25 --- /dev/null +++ b/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(); diff --git a/test/aot/parallel/builtin-modules/scrypt-refuses-two-names-before-callback.mjs b/test/aot/parallel/builtin-modules/scrypt-refuses-two-names-before-callback.mjs new file mode 100644 index 000000000000..07e96614e8c0 --- /dev/null +++ b/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(); diff --git a/test/internal/bindgen.test.ts b/test/internal/bindgen.test.ts index b3dc688619d8..2f3d7d725aa0 100644 --- a/test/internal/bindgen.test.ts +++ b/test/internal/bindgen.test.ts @@ -193,6 +193,16 @@ itAlways("mistakes in a schema are errors when it loads", () => { ); } expect(() => b.dictionary({ size: { type: range, expected: ["number"] } })).toThrow("expected: size is a range"); + for (const altNames of [undefined, [], ["length", "count"]]) { + expect(() => b.dictionary({ size: { type: b.u8.optional, altNames, exclusive: true } })).toThrow( + "exclusive: size has one other name in altNames", + ); + } + for (const type of [b.u8, b.u8.nullable, b.u8.default(0), b.string.optional, b.callable.optional]) { + expect(() => b.dictionary({ size: { type, altNames: ["length"], exclusive: true } })).toThrow( + "exclusive: size is .optional, and not a string, bytes, an instance or a JS value", + ); + } const point = b.dictionary({ x: b.f64 }); for (const later of [b.jsArray, b.callable, b.bufferSource, b.array(b.f64)]) { expect(() => b.union({ point, later, text: b.string })).toThrow("later is never chosen"); @@ -1084,6 +1094,76 @@ describe("the members of a dictionary", () => { `The "size" argument must be of type number. Received null`, ); }); + + const bothNames = (first: string, second: string) => + expect.objectContaining({ + name: "TypeError", + code: "ERR_INCOMPATIBLE_OPTION_PAIR", + message: `Option "${first}" cannot be used in combination with option "${second}"`, + }); + + it("exclusive: either name fills the field, each is reported as itself, and both are refused", () => { + expect(bindgen.exclusiveEcho({ size: 3, ratio: 0.5 })).toStrictEqual({ size: 3, ratio: 0.5 }); + expect(bindgen.exclusiveEcho({ length: 4, scale: 0.25 })).toStrictEqual({ size: 4, ratio: 0.25 }); + expect(bindgen.exclusiveEcho({ size: undefined, length: 4 })).toStrictEqual({ size: 4 }); + expect(bindgen.exclusiveEcho({ size: 3, length: undefined })).toStrictEqual({ size: 3 }); + + expect(() => bindgen.exclusiveEcho({ size: "3" })).toThrow("TestExclusive.size must be"); + expect(() => bindgen.exclusiveEcho({ length: "4" })).toThrow("TestExclusive.length must be"); + expect(() => bindgen.exclusiveEcho({ ratio: "x", scale: 1 })).toThrow( + `The "TestExclusive.ratio" property must be of type number. Received type string ('x')`, + ); + expect(() => bindgen.exclusiveEcho({ scale: "x" })).toThrow( + `The "TestExclusive.scale" property must be of type number. Received type string ('x')`, + ); + + // The second name is refused for what it is, not for what it holds. + for (const length of [4, 0, "4", null, 1.5, 256, {}]) { + expect(() => bindgen.exclusiveEcho({ size: 3, length })).toThrow(bothNames("size", "length")); + } + expect(() => bindgen.exclusiveEcho({ size: 0, length: 4 })).toThrow(bothNames("size", "length")); + expect(() => bindgen.exclusiveEcho({ ratio: 1, scale: "x" })).toThrow(bothNames("ratio", "scale")); + }); + + it("exclusive: each name is read once, in order, and none after the one that is refused", () => { + const log: string[] = []; + const logged = (members: Record) => + Object.defineProperties( + {}, + Object.fromEntries( + ["last", "scale", "ratio", "length", "size"].map(key => [ + key, + { + get() { + log.push(key); + return members[key]; + }, + enumerable: true, + }, + ]), + ), + ); + expect(bindgen.exclusiveEcho(logged({ length: 4, ratio: 0.5, last: 1 }))).toStrictEqual({ + size: 4, + ratio: 0.5, + last: 1, + }); + expect(log.splice(0)).toEqual(["size", "length", "ratio", "scale", "last"]); + + expect(() => bindgen.exclusiveEcho(logged({ size: 3, length: "4", scale: 1 }))).toThrow( + bothNames("size", "length"), + ); + expect(log.splice(0)).toEqual(["size", "length"]); + + expect(() => bindgen.exclusiveEcho(logged({ size: "3", length: 4 }))).toThrow("TestExclusive.size must be"); + expect(log.splice(0)).toEqual(["size"]); + }); +}); + +itAlways("a dictionary with an exclusive member can refuse an object its members would take", () => { + const ratio = { type: b.f64.loose.optional, altNames: ["scale"] }; + expect(b.dictionary({ ratio }, { nonObjectAsEmpty: true }).canRefuse).toBe(false); + expect(b.dictionary({ ratio: { ...ratio, exclusive: true } }, { nonObjectAsEmpty: true }).canRefuse).toBe(true); }); describe("b.callable, b.jsArray, b.object and b.undefined", () => { @@ -2774,10 +2854,11 @@ describe("classes", () => { it("estimated_size is reported to the GC", () => { Bun.gc(true); const before = heapStats().extraMemorySize; - const size = 1024 * 1024; + const size = 64 * 1024 * 1024; const note = new Note("text", size); Bun.gc(true); - expect(heapStats().extraMemorySize - before).toBeGreaterThanOrEqual(size); + // The second collection also frees the code of functions that last ran 5 seconds ago: far less than half of this. + expect(heapStats().extraMemorySize - before).toBeGreaterThan(size / 2); expect(note.text).toBe("text"); }); diff --git a/test/js/node/crypto/scrypt-option-checks.fixture.js b/test/js/node/crypto/scrypt-option-checks.fixture.js new file mode 100644 index 000000000000..ba32a7f4ae81 --- /dev/null +++ b/test/js/node/crypto/scrypt-option-checks.fixture.js @@ -0,0 +1,142 @@ +// Calls crypto.scryptSync and crypto.scrypt with options and callbacks that Node.js refuses, and with a few that it +// takes, and prints how each call ends. Run under Node.js and under Bun: the output must be identical. +const crypto = require("node:crypto"); +const { promisify } = require("node:util"); + +const describeError = e => `${e.name} [${e.code}]: ${e.message}`; + +function syncOutcome(...args) { + try { + return `key ${crypto.scryptSync("p", "s", ...args).toString("hex")}`; + } catch (e) { + return describeError(e); + } +} + +// What the call throws, or else what the callback gets. +function asyncOutcome(...args) { + const { promise, resolve } = Promise.withResolvers(); + const callback = (error, key) => + resolve(error ? `calls back with ${describeError(error)}` : `key ${key.toString("hex")}`); + try { + crypto.scrypt("p", "s", ...args.map(arg => (arg === asyncOutcome.callback ? callback : arg))); + } catch (e) { + resolve(describeError(e)); + } + return promise; +} +asyncOutcome.callback = Symbol("callback"); + +// The names of the options that the call reads, in order. Node.js reads twice an option that is given: it is printed once. +function logged(members) { + const read = []; + const options = new Proxy(members, { + get(target, key, receiver) { + if (typeof key === "string" && key !== "constructor" && read.at(-1) !== key) read.push(key); + return Reflect.get(target, key, receiver); + }, + }); + return [options, () => (read.length > 0 ? read.join(", ") : "none")]; +} + +(async () => { + const lines = []; + + // [label, options]: scryptSync(p, s, 4, options) and scrypt(p, s, 4, options, callback). + const rows = [ + // Both names of one option. The second is refused for being there, whatever it holds. + ['{ N: 2, cost: "x" }', { N: 2, cost: "x" }], + ["{ N: 2, cost: null }", { N: 2, cost: null }], + ["{ N: 2, cost: -1 }", { N: 2, cost: -1 }], + ["{ N: 2, cost: 1.5 }", { N: 2, cost: 1.5 }], + ["{ N: 2, cost: NaN }", { N: 2, cost: NaN }], + ["{ N: 2, cost: {} }", { N: 2, cost: {} }], + ["{ N: 2, cost: true }", { N: 2, cost: true }], + ["{ N: 2, cost: 2 }", { N: 2, cost: 2 }], + ["{ N: 0, cost: 2 }", { N: 0, cost: 2 }], + ['{ r: 2, blockSize: "x" }', { r: 2, blockSize: "x" }], + ["{ r: 2, blockSize: -1 }", { r: 2, blockSize: -1 }], + ["{ r: 0, blockSize: null }", { r: 0, blockSize: null }], + ['{ p: 1, parallelization: "x" }', { p: 1, parallelization: "x" }], + ["{ p: 1, parallelization: 1.5 }", { p: 1, parallelization: 1.5 }], + + // Both names, and a later option that is wrong. + ['{ N: 2, cost: 2, r: "x" }', { N: 2, cost: 2, r: "x" }], + ['{ r: 2, blockSize: 2, p: "x" }', { r: 2, blockSize: 2, p: "x" }], + ["{ p: 1, parallelization: 1, maxmem: -1 }", { p: 1, parallelization: 1, maxmem: -1 }], + ["{ N: 2, cost: 2, r: 2, blockSize: 2 }", { N: 2, cost: 2, r: 2, blockSize: 2 }], + + // Both names, and an earlier option that is wrong. + ['{ N: "x", cost: 2 }', { N: "x", cost: 2 }], + ['{ r: 2, blockSize: 2, N: "x" }', { r: 2, blockSize: 2, N: "x" }], + ["{ p: 1, parallelization: 1, blockSize: -1 }", { p: 1, parallelization: 1, blockSize: -1 }], + + // One name. + ['{ cost: "x" }', { cost: "x" }], + ["{ blockSize: -1 }", { blockSize: -1 }], + ["{ parallelization: 1.5 }", { parallelization: 1.5 }], + + // A name that holds undefined is not given. + ["{ N: 2, r: 1, p: 1 }", { N: 2, r: 1, p: 1 }], + ["{ cost: 2, blockSize: 1, parallelization: 1 }", { cost: 2, blockSize: 1, parallelization: 1 }], + [ + "{ N: 2, cost: undefined, r: undefined, blockSize: 1, p: 1, parallelization: undefined }", + { N: 2, cost: undefined, r: undefined, blockSize: 1, p: 1, parallelization: undefined }, + ], + ]; + for (const [label, options] of rows) { + lines.push(label); + const sync = syncOutcome(4, options); + const async = await asyncOutcome(4, options, asyncOutcome.callback); + if (sync === async) lines.push(` ${sync}`); + else lines.push(` scryptSync: ${sync}`, ` scrypt: ${async}`); + } + + // [label, the arguments after the salt, the options among them that are logged] + const calls = [ + // keylen is checked before the options. + ['scryptSync(p, s, "x", { N: 2, cost: 2 })', syncOutcome, ["x"], { N: 2, cost: 2 }], + // The options are read up to the one that is refused. + ['scryptSync(p, s, 4, { N: 2, cost: "x", r: 1 })', syncOutcome, [4], { N: 2, cost: "x", r: 1 }], + [ + 'scryptSync(p, s, 4, { N: 2, r: 2, blockSize: 2, p: "x" })', + syncOutcome, + [4], + { N: 2, r: 2, blockSize: 2, p: "x" }, + ], + ["scryptSync(p, s, 4, { N: 2, r: 1, p: 1 })", syncOutcome, [4], { N: 2, r: 1, p: 1 }], + // The callback is checked after the options. + ["scrypt(p, s, 4, { N: 2, cost: 2 }, 5)", asyncOutcome, [4], { N: 2, cost: 2 }, [5]], + ['scrypt(p, s, 4, { N: 2, cost: "x" }, null)', asyncOutcome, [4], { N: 2, cost: "x" }, [null]], + ['scrypt(p, s, 4, { N: "x" }, 5)', asyncOutcome, [4], { N: "x" }, [5]], + ["scrypt(p, s, 4, { N: 2 }, 5)", asyncOutcome, [4], { N: 2 }, [5]], + // Without a fifth argument the fourth is the callback, and nothing of it is read. + ['scrypt(p, s, 4, { N: 2, cost: "x" })', asyncOutcome, [4], { N: 2, cost: "x" }], + ['scrypt(p, s, 4, { N: 2, cost: "x" }, undefined)', asyncOutcome, [4], { N: 2, cost: "x" }, [undefined]], + ]; + for (const [label, outcome, before, members, after = []] of calls) { + const [options, read] = logged(members); + lines.push(label, ` ${await outcome(...before, options, ...after)}`, ` reads: ${read()}`); + } + + const others = [ + ["scrypt(p, s, 4, 5)", () => asyncOutcome(4, 5)], + ["scrypt(p, s, 4, null)", () => asyncOutcome(4, null)], + ["scrypt(p, s, 4)", () => asyncOutcome(4)], + // Options that are undefined or null are no options. + ["scrypt(p, s, 4, undefined, callback)", () => asyncOutcome(4, undefined, asyncOutcome.callback)], + ["scrypt(p, s, 4, null, callback)", () => asyncOutcome(4, null, asyncOutcome.callback)], + ["scrypt(p, s, 4, callback)", () => asyncOutcome(4, asyncOutcome.callback)], + [ + 'promisify(scrypt)(p, s, 4, { N: 2, cost: "x" })', + () => + promisify(crypto.scrypt)("p", "s", 4, { N: 2, cost: "x" }).then( + key => `key ${key.toString("hex")}`, + e => `rejects with ${describeError(e)}`, + ), + ], + ]; + for (const [label, outcome] of others) lines.push(label, ` ${await outcome()}`); + + console.log(lines.join("\n")); +})(); diff --git a/test/js/node/crypto/scrypt.test.ts b/test/js/node/crypto/scrypt.test.ts index 0e0ecf8b77e9..bc7d5a446da4 100644 --- a/test/js/node/crypto/scrypt.test.ts +++ b/test/js/node/crypto/scrypt.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, nodeExeAtLeast, tempDir } from "harness"; import { scrypt, scryptSync } from "node:crypto"; +import { join } from "node:path"; // When `crypto.scrypt` fails to allocate the output buffer (OOM for a huge // `keylen`), `CryptoJob.init` takes the error path. Previously the `errdefer` @@ -167,3 +168,124 @@ test("scrypt reports its arguments and options as Node does", () => { 'The "callback" argument must be of type function. Received type number (5)', ); }); + +// Node refuses the second name of an option where it reads it: before it validates what the name holds, before it reads +// a later option, and before it checks the callback. Without a fifth argument, the fourth is the callback. +// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/crypto/scrypt.js#L40-L50 and #L90-L129 +// The fixture prints how each call ends. The expected text is the output of node v26.3.0. +const optionChecksFixture = join(import.meta.dir, "scrypt-option-checks.fixture.js"); +const optionChecksExpected = `{ N: 2, cost: "x" } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: 2, cost: null } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: 2, cost: -1 } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: 2, cost: 1.5 } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: 2, cost: NaN } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: 2, cost: {} } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: 2, cost: true } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: 2, cost: 2 } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: 0, cost: 2 } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ r: 2, blockSize: "x" } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "r" cannot be used in combination with option "blockSize" +{ r: 2, blockSize: -1 } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "r" cannot be used in combination with option "blockSize" +{ r: 0, blockSize: null } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "r" cannot be used in combination with option "blockSize" +{ p: 1, parallelization: "x" } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "p" cannot be used in combination with option "parallelization" +{ p: 1, parallelization: 1.5 } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "p" cannot be used in combination with option "parallelization" +{ N: 2, cost: 2, r: "x" } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ r: 2, blockSize: 2, p: "x" } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "r" cannot be used in combination with option "blockSize" +{ p: 1, parallelization: 1, maxmem: -1 } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "p" cannot be used in combination with option "parallelization" +{ N: 2, cost: 2, r: 2, blockSize: 2 } + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +{ N: "x", cost: 2 } + TypeError [ERR_INVALID_ARG_TYPE]: The "N" argument must be of type number. Received type string ('x') +{ r: 2, blockSize: 2, N: "x" } + TypeError [ERR_INVALID_ARG_TYPE]: The "N" argument must be of type number. Received type string ('x') +{ p: 1, parallelization: 1, blockSize: -1 } + RangeError [ERR_OUT_OF_RANGE]: The value of "blockSize" is out of range. It must be >= 0 && <= 4294967295. Received -1 +{ cost: "x" } + TypeError [ERR_INVALID_ARG_TYPE]: The "cost" argument must be of type number. Received type string ('x') +{ blockSize: -1 } + RangeError [ERR_OUT_OF_RANGE]: The value of "blockSize" is out of range. It must be >= 0 && <= 4294967295. Received -1 +{ parallelization: 1.5 } + RangeError [ERR_OUT_OF_RANGE]: The value of "parallelization" is out of range. It must be an integer. Received 1.5 +{ N: 2, r: 1, p: 1 } + key 48b0d2a8 +{ cost: 2, blockSize: 1, parallelization: 1 } + key 48b0d2a8 +{ N: 2, cost: undefined, r: undefined, blockSize: 1, p: 1, parallelization: undefined } + key 48b0d2a8 +scryptSync(p, s, "x", { N: 2, cost: 2 }) + TypeError [ERR_INVALID_ARG_TYPE]: The "keylen" argument must be of type number. Received type string ('x') + reads: none +scryptSync(p, s, 4, { N: 2, cost: "x", r: 1 }) + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" + reads: N, cost +scryptSync(p, s, 4, { N: 2, r: 2, blockSize: 2, p: "x" }) + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "r" cannot be used in combination with option "blockSize" + reads: N, cost, r, blockSize +scryptSync(p, s, 4, { N: 2, r: 1, p: 1 }) + key 48b0d2a8 + reads: N, cost, r, blockSize, p, parallelization, maxmem +scrypt(p, s, 4, { N: 2, cost: 2 }, 5) + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" + reads: N, cost +scrypt(p, s, 4, { N: 2, cost: "x" }, null) + TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" + reads: N, cost +scrypt(p, s, 4, { N: "x" }, 5) + TypeError [ERR_INVALID_ARG_TYPE]: The "N" argument must be of type number. Received type string ('x') + reads: N +scrypt(p, s, 4, { N: 2 }, 5) + TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received type number (5) + reads: N, cost, r, blockSize, p, parallelization, maxmem +scrypt(p, s, 4, { N: 2, cost: "x" }) + TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received an instance of Object + reads: none +scrypt(p, s, 4, { N: 2, cost: "x" }, undefined) + TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received an instance of Object + reads: none +scrypt(p, s, 4, 5) + TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received type number (5) +scrypt(p, s, 4, null) + TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received null +scrypt(p, s, 4) + TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received undefined +scrypt(p, s, 4, undefined, callback) + key 43297877 +scrypt(p, s, 4, null, callback) + key 43297877 +scrypt(p, s, 4, callback) + key 43297877 +promisify(scrypt)(p, s, 4, { N: 2, cost: "x" }) + rejects with TypeError [ERR_INCOMPATIBLE_OPTION_PAIR]: Option "N" cannot be used in combination with option "cost" +`; + +for (const [name, runtime] of [ + ["bun", bunExe()], + ["node", nodeExeAtLeast("26.3.0")], +] as const) { + test.skipIf(!runtime)(`scrypt checks its options and its callback in the order of node v26 (${name})`, async () => { + await using proc = Bun.spawn({ + cmd: [runtime!, optionChecksFixture], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: optionChecksExpected, stderr: "", exitCode: 0 }); + }); +} From ffd81551157736041216c548d04151f177fc878a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 30 Oct 2026 02:52:26 +0000 Subject: [PATCH 2/5] bindgen: refuse an `exclusive` member whose other name is its own A member that lists its own name in `altNames` would read that name twice and refuse every object that gives it. The schema is now refused when it loads. The documentation of `exclusive` says that a name that holds `undefined` is not given. --- docs/project/bindgen.mdx | 4 ++-- src/codegen/bindgen-lib-internal.ts | 9 ++++++--- test/internal/bindgen.test.ts | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/project/bindgen.mdx b/docs/project/bindgen.mdx index 008db791e1ad..6da6a8b96de5 100644 --- a/docs/project/bindgen.mdx +++ b/docs/project/bindgen.mdx @@ -402,8 +402,8 @@ whose type does not accept `undefined` is required. A member can be written 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. An object that gives both throws -`ERR_INCOMPATIBLE_OPTION_PAIR` when the second name is read: before its value is converted, and before a later member is read. +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. diff --git a/src/codegen/bindgen-lib-internal.ts b/src/codegen/bindgen-lib-internal.ts index 6afe2af7e5e1..48424577357b 100644 --- a/src/codegen/bindgen-lib-internal.ts +++ b/src/codegen/bindgen-lib-internal.ts @@ -2488,8 +2488,8 @@ export interface MemberOptions { altNames?: readonly string[]; /** * The one name in `altNames` is a second name that Node gives the option, as `cost` is of scrypt's `N`. Both are read, - * and each is converted and reported under its own name. An object that gives both throws - * `ERR_INCOMPATIBLE_OPTION_PAIR` when the second is read, before it is converted. + * and each is converted and reported under its own name. If neither holds `undefined`, the second throws + * `ERR_INCOMPATIBLE_OPTION_PAIR` when it is read, before it is converted. */ exclusive?: boolean; /** As a function's `expected`, for this member. */ @@ -2516,7 +2516,10 @@ export class Member { if (options.expected !== undefined) refuseExpectedOfRange(name, options.type); this.expected = options.expected; this.exclusive = options.exclusive ?? false; - assert(!this.exclusive || this.names.length === 2, `exclusive: ${name} has one other name in altNames`); + 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 || diff --git a/test/internal/bindgen.test.ts b/test/internal/bindgen.test.ts index 2f3d7d725aa0..2f9b69038629 100644 --- a/test/internal/bindgen.test.ts +++ b/test/internal/bindgen.test.ts @@ -193,7 +193,7 @@ itAlways("mistakes in a schema are errors when it loads", () => { ); } expect(() => b.dictionary({ size: { type: range, expected: ["number"] } })).toThrow("expected: size is a range"); - for (const altNames of [undefined, [], ["length", "count"]]) { + for (const altNames of [undefined, [], ["size"], ["length", "count"]]) { expect(() => b.dictionary({ size: { type: b.u8.optional, altNames, exclusive: true } })).toThrow( "exclusive: size has one other name in altNames", ); From 298fedfe1757de2feae68ba3d7aa2acd079dbe6d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 30 Oct 2026 02:56:57 +0000 Subject: [PATCH 3/5] bindgen: keep the GC test within 8 MiB of the reported size `estimated_size is reported to the GC` asked for more than half of the size of the note. It now asks for all of it but 8 MiB, which is many times what a collection frees of old code between the two measurements. The description of `exclusive` is one line. --- src/codegen/bindgen-lib-internal.ts | 6 +----- test/internal/bindgen.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/codegen/bindgen-lib-internal.ts b/src/codegen/bindgen-lib-internal.ts index 48424577357b..bc28b3f0577a 100644 --- a/src/codegen/bindgen-lib-internal.ts +++ b/src/codegen/bindgen-lib-internal.ts @@ -2486,11 +2486,7 @@ export interface MemberOptions { rustName?: string; /** 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`. Both are read, - * and each is converted and reported under its own name. If neither holds `undefined`, the second throws - * `ERR_INCOMPATIBLE_OPTION_PAIR` when it is read, before it is converted. - */ + /** 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; diff --git a/test/internal/bindgen.test.ts b/test/internal/bindgen.test.ts index 2f9b69038629..9a54f0695483 100644 --- a/test/internal/bindgen.test.ts +++ b/test/internal/bindgen.test.ts @@ -2857,8 +2857,8 @@ describe("classes", () => { const size = 64 * 1024 * 1024; const note = new Note("text", size); Bun.gc(true); - // The second collection also frees the code of functions that last ran 5 seconds ago: far less than half of this. - expect(heapStats().extraMemorySize - before).toBeGreaterThan(size / 2); + // The second collection also frees code that last ran 5 seconds ago, which comes off the total: some hundred KiB. + expect(heapStats().extraMemorySize - before).toBeGreaterThan(size - 8 * 1024 * 1024); expect(note.text).toBe("text"); }); From 803b2f68e95754d78674ef16d54a9eecc07a7594 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 30 Oct 2026 08:08:21 +0000 Subject: [PATCH 4/5] ci: retrigger From 2a25c792676b5419d5d298634cc4ab4c37f69be6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 31 Oct 2026 22:26:19 +0000 Subject: [PATCH 5/5] node:crypto: keep the overloads of scrypt as on main The second overload took `b.any`, so that a call without a fifth argument reported its fourth argument as the callback, as Node does. With `b.any` there, the table of built-ins no longer says that scrypt describes an option that it refuses. An executable built with `--aot` then leaves out a `constructor` getter that the error message reads. #46540 accepted that difference from Node for a call that leaves the callback out. The overloads and `scrypt_with_defaults` are main's again. The fixture and the AOT programs lose their rows for such calls. --- src/runtime/node/node_crypto_binding.bind.ts | 8 +++--- src/runtime/node/node_crypto_binding.rs | 9 +------ ...escribes-fourth-argument-without-fifth.mjs | 21 --------------- .../scrypt-reads-no-option-without-fifth.mjs | 26 ------------------- .../crypto/scrypt-option-checks.fixture.js | 5 ---- test/js/node/crypto/scrypt.test.ts | 14 ++-------- 6 files changed, 6 insertions(+), 77 deletions(-) delete mode 100644 test/aot/parallel/builtin-modules/scrypt-describes-fourth-argument-without-fifth.mjs delete mode 100644 test/aot/parallel/builtin-modules/scrypt-reads-no-option-without-fifth.mjs diff --git a/src/runtime/node/node_crypto_binding.bind.ts b/src/runtime/node/node_crypto_binding.bind.ts index 29b70c29b3af..8638131eae62 100644 --- a/src/runtime/node/node_crypto_binding.bind.ts +++ b/src/runtime/node/node_crypto_binding.bind.ts @@ -152,17 +152,15 @@ export const crypto = b.namespace({ getHashes: { returns: b.array(b.string.utf8) }, scrypt: [ { - /** `.default({})` so that an `undefined` fourth argument does not choose the other overload. */ - args: { ...passwordAndSaltHandles, keylen, options: ScryptOptions.default({}), callback: b.callable }, + args: { ...passwordAndSaltHandles, keylen, options: ScryptOptions, 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.any }, - expected: passwordAndSaltNames, + args: { ...passwordAndSaltHandles, keylen, callback: b.callable }, + expected: { ...passwordAndSaltNames, callback: ["function"] }, rustPath: "crate::node::crypto::scrypt_with_defaults", ...js, }, diff --git a/src/runtime/node/node_crypto_binding.rs b/src/runtime/node/node_crypto_binding.rs index 482e315bcd3b..1723a3ee844e 100644 --- a/src/runtime/node/node_crypto_binding.rs +++ b/src/runtime/node/node_crypto_binding.rs @@ -1060,15 +1060,8 @@ pub(crate) fn scrypt_with_defaults<'a>( password: StringOrJsBytes<'a>, salt: StringOrJsBytes<'a>, keylen: u32, - callback: JsLocal<'a>, + callback: JsLocal<'a, Callable>, ) -> 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, diff --git a/test/aot/parallel/builtin-modules/scrypt-describes-fourth-argument-without-fifth.mjs b/test/aot/parallel/builtin-modules/scrypt-describes-fourth-argument-without-fifth.mjs deleted file mode 100644 index 16e8ba729328..000000000000 --- a/test/aot/parallel/builtin-modules/scrypt-describes-fourth-argument-without-fifth.mjs +++ /dev/null @@ -1,21 +0,0 @@ -// 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(); diff --git a/test/aot/parallel/builtin-modules/scrypt-reads-no-option-without-fifth.mjs b/test/aot/parallel/builtin-modules/scrypt-reads-no-option-without-fifth.mjs deleted file mode 100644 index 68aa08ca2b25..000000000000 --- a/test/aot/parallel/builtin-modules/scrypt-reads-no-option-without-fifth.mjs +++ /dev/null @@ -1,26 +0,0 @@ -// 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(); diff --git a/test/js/node/crypto/scrypt-option-checks.fixture.js b/test/js/node/crypto/scrypt-option-checks.fixture.js index ba32a7f4ae81..fb54ab6ac173 100644 --- a/test/js/node/crypto/scrypt-option-checks.fixture.js +++ b/test/js/node/crypto/scrypt-option-checks.fixture.js @@ -110,9 +110,6 @@ function logged(members) { ['scrypt(p, s, 4, { N: 2, cost: "x" }, null)', asyncOutcome, [4], { N: 2, cost: "x" }, [null]], ['scrypt(p, s, 4, { N: "x" }, 5)', asyncOutcome, [4], { N: "x" }, [5]], ["scrypt(p, s, 4, { N: 2 }, 5)", asyncOutcome, [4], { N: 2 }, [5]], - // Without a fifth argument the fourth is the callback, and nothing of it is read. - ['scrypt(p, s, 4, { N: 2, cost: "x" })', asyncOutcome, [4], { N: 2, cost: "x" }], - ['scrypt(p, s, 4, { N: 2, cost: "x" }, undefined)', asyncOutcome, [4], { N: 2, cost: "x" }, [undefined]], ]; for (const [label, outcome, before, members, after = []] of calls) { const [options, read] = logged(members); @@ -120,8 +117,6 @@ function logged(members) { } const others = [ - ["scrypt(p, s, 4, 5)", () => asyncOutcome(4, 5)], - ["scrypt(p, s, 4, null)", () => asyncOutcome(4, null)], ["scrypt(p, s, 4)", () => asyncOutcome(4)], // Options that are undefined or null are no options. ["scrypt(p, s, 4, undefined, callback)", () => asyncOutcome(4, undefined, asyncOutcome.callback)], diff --git a/test/js/node/crypto/scrypt.test.ts b/test/js/node/crypto/scrypt.test.ts index 7531f328846e..cbf1987efaba 100644 --- a/test/js/node/crypto/scrypt.test.ts +++ b/test/js/node/crypto/scrypt.test.ts @@ -176,8 +176,8 @@ test("scrypt passes null as the error of a successful derivation", async () => { }); // Node refuses the second name of an option where it reads it: before it validates what the name holds, before it reads -// a later option, and before it checks the callback. Without a fifth argument, the fourth is the callback. -// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/crypto/scrypt.js#L40-L50 and #L90-L129 +// a later option, and before it checks the callback. +// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/crypto/scrypt.js#L46-L50 and #L90-L129 // The fixture prints how each call ends. The expected text is the output of node v26.3.0. const optionChecksFixture = join(import.meta.dir, "scrypt-option-checks.fixture.js"); const optionChecksExpected = `{ N: 2, cost: "x" } @@ -258,16 +258,6 @@ scrypt(p, s, 4, { N: "x" }, 5) scrypt(p, s, 4, { N: 2 }, 5) TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received type number (5) reads: N, cost, r, blockSize, p, parallelization, maxmem -scrypt(p, s, 4, { N: 2, cost: "x" }) - TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received an instance of Object - reads: none -scrypt(p, s, 4, { N: 2, cost: "x" }, undefined) - TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received an instance of Object - reads: none -scrypt(p, s, 4, 5) - TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received type number (5) -scrypt(p, s, 4, null) - TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received null scrypt(p, s, 4) TypeError [ERR_INVALID_ARG_TYPE]: The "callback" argument must be of type function. Received undefined scrypt(p, s, 4, undefined, callback)