diff --git a/docs/project/bindgen.mdx b/docs/project/bindgen.mdx index acbf0a0f6887..6da6a8b96de5 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. 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-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..bc28b3f0577a 100644 --- a/src/codegen/bindgen-lib-internal.ts +++ b/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; } diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 3a723f1dbc53..ec91e63288c9 100644 --- a/src/js/internal-for-testing.ts +++ b/src/js/internal-for-testing.ts @@ -324,6 +324,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 ab11410ef1cf..4ed79120dd93 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(); bun_boringssl::c::ERR_clear_error(); 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..8638131eae62 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: "" }, diff --git a/src/runtime/node/node_crypto_binding.rs b/src/runtime/node/node_crypto_binding.rs index e5903a6cb196..e9135b73f649 100644 --- a/src/runtime/node/node_crypto_binding.rs +++ b/src/runtime/node/node_crypto_binding.rs @@ -712,16 +712,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; @@ -730,9 +720,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) 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-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..9a54f0695483 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, [], ["size"], ["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 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"); }); 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..fb54ab6ac173 --- /dev/null +++ b/test/js/node/crypto/scrypt-option-checks.fixture.js @@ -0,0 +1,137 @@ +// 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]], + ]; + 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)", () => 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 ee17a729eb16..cbf1987efaba 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` @@ -173,3 +174,114 @@ test("scrypt passes null as the error of a successful derivation", async () => { scrypt("password", "salt", 4, { N: 2 }, (...args) => resolve(args)); expect(await promise).toEqual([null, scryptSync("password", "salt", 4, { N: 2 })]); }); + +// 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. +// 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" } + 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) + 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 }); + }); +}