From 1efe2f279cf17d59abbb2ab1cb99da24c278aed7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 10 Nov 2026 09:02:26 +0000 Subject: [PATCH 1/2] node:assert, node:util: compare RegExp lastIndex with === in deep equality assert.deepStrictEqual, util.isDeepStrictEqual and assert.partialDeepStrictEqual compared the lastIndex of two RegExps with SameValue. Node's areSimilarRegExps() uses ===. So a lastIndex of -0 was unequal to 0, and NaN was equal to NaN. Both native sites now call JSC::JSValue::strictEqual. --- src/jsc/bindings/bindings.cpp | 5 ++- src/jsc/modules/NodeUtilTypesModule.cpp | 5 ++- test/js/node/assert/assert.test.cjs | 37 ++++++++++++++++++ test/js/node/assert/deep-equal.test.ts | 52 +++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 4 deletions(-) diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 31ba33ed07ef..b349481587ae 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -1793,8 +1793,9 @@ static std::optional specialObjectsDequalSlow(const DeepEqualsMode& mode, return false; } if (mode.checkPrototypes) { - // node also compares `lastIndex` and own enumerable properties. - bool sameLastIndex = JSC::sameValue(globalObject, left->getLastIndex(), right->getLastIndex()); + // node compares `lastIndex` with ===, not Object.is, then own enumerable properties: + // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 + bool sameLastIndex = JSC::JSValue::strictEqual(globalObject, left->getLastIndex(), right->getLastIndex()); RETURN_IF_EXCEPTION(scope, {}); if (!sameLastIndex) { return false; diff --git a/src/jsc/modules/NodeUtilTypesModule.cpp b/src/jsc/modules/NodeUtilTypesModule.cpp index 925fd83a6b7f..2e4f8cdb7e21 100644 --- a/src/jsc/modules/NodeUtilTypesModule.cpp +++ b/src/jsc/modules/NodeUtilTypesModule.cpp @@ -808,8 +808,9 @@ static bool compareBranch(JSC::JSGlobalObject* globalObject, JSC::MarkedArgument if (actualRegExp->regExp()->pattern() != expectedRegExp->regExp()->pattern() || actualRegExp->regExp()->flags() != expectedRegExp->regExp()->flags()) return false; - // node also compares lastIndex strictly (comparisons.js isRegExp arm). - bool lastIndexEqual = JSC::sameValue(globalObject, actualRegExp->getLastIndex(), expectedRegExp->getLastIndex()); + // node compares lastIndex with ===, not Object.is: + // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 + bool lastIndexEqual = JSC::JSValue::strictEqual(globalObject, actualRegExp->getLastIndex(), expectedRegExp->getLastIndex()); RETURN_IF_EXCEPTION(scope, false); if (!lastIndexEqual) return false; diff --git a/test/js/node/assert/assert.test.cjs b/test/js/node/assert/assert.test.cjs index a8a2043b730b..fdccf2c57246 100644 --- a/test/js/node/assert/assert.test.cjs +++ b/test/js/node/assert/assert.test.cjs @@ -92,6 +92,43 @@ describe("assert.partialDeepStrictEqual", () => { const arr = [[]]; assert.partialDeepStrictEqual([arr], arr); }); + + // Node's areSimilarRegExps() compares lastIndex with ===, so -0 equals 0 and NaN differs from NaN. + // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 + test.each([ + ["at the top level", re => re], + ["in an array", re => [re]], + ["in an object property", re => ({ re })], + ["in a Map value", re => new Map([["k", re]])], + ["in a Map key", re => new Map([[re, 1]])], + ["in a Set", re => new Set([re])], + ["in an Error cause", re => new Error("e", { cause: re })], + ])("RegExp lastIndex is compared with === %s", (_name, wrap) => { + const withLastIndex = lastIndex => Object.assign(/a/g, { lastIndex }); + const partial = (actual, expected) => { + try { + assert.partialDeepStrictEqual(wrap(actual), wrap(expected)); + return "equal"; + } catch (e) { + return e.code; + } + }; + const sameObject = withLastIndex(NaN); + expect({ + negativeZeroAndZero: partial(withLastIndex(-0), withLastIndex(0)), + zeroAndNegativeZero: partial(withLastIndex(0), withLastIndex(-0)), + nanAndNaN: partial(withLastIndex(NaN), withLastIndex(NaN)), + threeAndZero: partial(withLastIndex(3), withLastIndex(0)), + // The identity shortcut answers before lastIndex is read. + sameObjectWithNaN: partial(sameObject, sameObject), + }).toEqual({ + negativeZeroAndZero: "equal", + zeroAndNegativeZero: "equal", + nanAndNaN: "ERR_ASSERTION", + threeAndZero: "ERR_ASSERTION", + sameObjectWithNaN: "equal", + }); + }); }); describe("AssertionError diff rendering", () => { diff --git a/test/js/node/assert/deep-equal.test.ts b/test/js/node/assert/deep-equal.test.ts index abb31a1a0e3d..e02311bacce0 100644 --- a/test/js/node/assert/deep-equal.test.ts +++ b/test/js/node/assert/deep-equal.test.ts @@ -666,6 +666,22 @@ const cases: Case[] = [ loose: false, looseBug: "reports equal", }, + // lastIndex is compared with ===, not Object.is, in every mode. + { + name: "regexps with lastIndex -0 and 0", + a: () => Object.assign(/a/g, { lastIndex: -0 }), + b: () => /a/g, + strict: true, + loose: true, + }, + { + name: "regexps with lastIndex NaN", + a: () => Object.assign(/a/g, { lastIndex: NaN }), + b: () => Object.assign(/a/g, { lastIndex: NaN }), + strict: false, + loose: false, + looseBug: "reports equal", + }, // Error. { @@ -1162,6 +1178,42 @@ describe("util.isDeepStrictEqual", () => { expect(calls).toBe(2); }); + // Node's areSimilarRegExps() compares lastIndex with ===, so -0 equals 0 and NaN differs from NaN. + // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 + describe("RegExp lastIndex", () => { + const withLastIndex = (lastIndex: number) => Object.assign(/a/g, { lastIndex }); + const positions: Array<[string, (re: RegExp) => unknown]> = [ + ["at the top level", re => re], + ["in an array", re => [re]], + ["in an object property", re => ({ re })], + ["in a Map value", re => new Map([["k", re]])], + ["in a Map key", re => new Map([[re, 1]])], + ["in a Set", re => new Set([re])], + ["in an Error cause", re => new Error("e", { cause: re })], + ]; + + test.each(positions)("is compared with === %s", (_name, wrap) => { + const sameObject = withLastIndex(NaN); + for (const skipPrototype of [false, true]) { + const equal = (a: RegExp, b: RegExp) => util.isDeepStrictEqual(wrap(a), wrap(b), skipPrototype as any); + expect({ + skipPrototype, + negativeZeroAndZero: equal(withLastIndex(-0), withLastIndex(0)), + zeroAndNegativeZero: equal(withLastIndex(0), withLastIndex(-0)), + nanAndNaN: equal(withLastIndex(NaN), withLastIndex(NaN)), + // The identity shortcut answers before lastIndex is read. + sameObjectWithNaN: equal(sameObject, sameObject), + }).toEqual({ + skipPrototype, + negativeZeroAndZero: true, + zeroAndNegativeZero: true, + nanAndNaN: false, + sameObjectWithNaN: true, + }); + } + }); + }); + // The third argument was added in Node v26. describe("skipPrototype", () => { class Foo { From df4bb80d8f6e65912e418ff04dcb962fbd4fed58 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 10 Nov 2026 17:41:20 +0000 Subject: [PATCH 2/2] test: pin the RegExp lastIndex cases that reach the Set and Map walks Write the nesting tests in deep-equal.test.ts with a plain loop, test() and node:assert. Add a "0" against 0 case. Add two rows for Sets and Maps that hold RegExps whose lastIndex differs only by the sign of zero. --- test/js/node/assert/assert.test.cjs | 20 ++--- test/js/node/assert/deep-equal.test.ts | 107 ++++++++++++++++++------- 2 files changed, 88 insertions(+), 39 deletions(-) diff --git a/test/js/node/assert/assert.test.cjs b/test/js/node/assert/assert.test.cjs index fdccf2c57246..03b0449bd165 100644 --- a/test/js/node/assert/assert.test.cjs +++ b/test/js/node/assert/assert.test.cjs @@ -85,15 +85,7 @@ describe("assert.partialDeepStrictEqual", () => { assert.partialDeepStrictEqual({ arr: [shared, shared] }, { arr: [{ x: 1 }] }); }); - test("a descendant actual identical to an ancestor expected is not mistaken for a cycle", () => { - const e = { k: {} }; - assert.partialDeepStrictEqual({ k: e }, e); - - const arr = [[]]; - assert.partialDeepStrictEqual([arr], arr); - }); - - // Node's areSimilarRegExps() compares lastIndex with ===, so -0 equals 0 and NaN differs from NaN. + // Node's areSimilarRegExps() compares lastIndex with ===: -0 equals 0, NaN differs from NaN, "0" differs from 0. // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 test.each([ ["at the top level", re => re], @@ -118,6 +110,7 @@ describe("assert.partialDeepStrictEqual", () => { negativeZeroAndZero: partial(withLastIndex(-0), withLastIndex(0)), zeroAndNegativeZero: partial(withLastIndex(0), withLastIndex(-0)), nanAndNaN: partial(withLastIndex(NaN), withLastIndex(NaN)), + stringZeroAndZero: partial(withLastIndex("0"), withLastIndex(0)), threeAndZero: partial(withLastIndex(3), withLastIndex(0)), // The identity shortcut answers before lastIndex is read. sameObjectWithNaN: partial(sameObject, sameObject), @@ -125,10 +118,19 @@ describe("assert.partialDeepStrictEqual", () => { negativeZeroAndZero: "equal", zeroAndNegativeZero: "equal", nanAndNaN: "ERR_ASSERTION", + stringZeroAndZero: "ERR_ASSERTION", threeAndZero: "ERR_ASSERTION", sameObjectWithNaN: "equal", }); }); + + test("a descendant actual identical to an ancestor expected is not mistaken for a cycle", () => { + const e = { k: {} }; + assert.partialDeepStrictEqual({ k: e }, e); + + const arr = [[]]; + assert.partialDeepStrictEqual([arr], arr); + }); }); describe("AssertionError diff rendering", () => { diff --git a/test/js/node/assert/deep-equal.test.ts b/test/js/node/assert/deep-equal.test.ts index e02311bacce0..dc492f10699e 100644 --- a/test/js/node/assert/deep-equal.test.ts +++ b/test/js/node/assert/deep-equal.test.ts @@ -658,14 +658,6 @@ const cases: Case[] = [ // RegExp. { name: "two equal regexps", a: () => /a/g, b: () => /a/g, strict: true, loose: true }, { name: "regexps with different flags", a: () => /a/g, b: () => /a/i, strict: false, loose: false }, - { - name: "regexps with different lastIndex", - a: () => Object.assign(/a/g, { lastIndex: 3 }), - b: () => /a/g, - strict: false, - loose: false, - looseBug: "reports equal", - }, // lastIndex is compared with ===, not Object.is, in every mode. { name: "regexps with lastIndex -0 and 0", @@ -682,6 +674,54 @@ const cases: Case[] = [ loose: false, looseBug: "reports equal", }, + // A lastIndex of 0 and of -0 make two deep-equal members. The Set and Map walks take the + // first deep-equal member and do not pair members one to one. + // https://github.com/oven-sh/bun/issues/28760 + { + name: "sets of three regexps with lastIndex 0, -0, 1 and 0, 1, 1", + a: () => + new Set([ + Object.assign(/a/g, { lastIndex: 0 }), + Object.assign(/a/g, { lastIndex: -0 }), + Object.assign(/a/g, { lastIndex: 1 }), + ]), + b: () => + new Set([ + Object.assign(/a/g, { lastIndex: 0 }), + Object.assign(/a/g, { lastIndex: 1 }), + Object.assign(/a/g, { lastIndex: 1 }), + ]), + strict: false, + loose: false, + strictBug: "reports equal", + looseBug: "reports equal", + }, + // https://github.com/oven-sh/bun/issues/34830 + { + name: "maps with two regexp keys whose lastIndex is 0 and -0", + a: () => + new Map([ + [Object.assign(/a/g, { lastIndex: 0 }), 1], + [Object.assign(/a/g, { lastIndex: -0 }), 2], + ]), + b: () => + new Map([ + [Object.assign(/a/g, { lastIndex: 0 }), 1], + [Object.assign(/a/g, { lastIndex: -0 }), 2], + ]), + strict: true, + loose: true, + strictBug: "reports not equal", + looseBug: "reports not equal", + }, + { + name: "regexps with different lastIndex", + a: () => Object.assign(/a/g, { lastIndex: 3 }), + b: () => /a/g, + strict: false, + loose: false, + looseBug: "reports equal", + }, // Error. { @@ -1178,10 +1218,10 @@ describe("util.isDeepStrictEqual", () => { expect(calls).toBe(2); }); - // Node's areSimilarRegExps() compares lastIndex with ===, so -0 equals 0 and NaN differs from NaN. + // Node's areSimilarRegExps() compares lastIndex with ===: -0 equals 0, NaN differs from NaN, "0" differs from 0. // https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152 describe("RegExp lastIndex", () => { - const withLastIndex = (lastIndex: number) => Object.assign(/a/g, { lastIndex }); + const withLastIndex = (lastIndex: unknown) => Object.assign(/a/g, { lastIndex }); const positions: Array<[string, (re: RegExp) => unknown]> = [ ["at the top level", re => re], ["in an array", re => [re]], @@ -1192,26 +1232,33 @@ describe("util.isDeepStrictEqual", () => { ["in an Error cause", re => new Error("e", { cause: re })], ]; - test.each(positions)("is compared with === %s", (_name, wrap) => { - const sameObject = withLastIndex(NaN); - for (const skipPrototype of [false, true]) { - const equal = (a: RegExp, b: RegExp) => util.isDeepStrictEqual(wrap(a), wrap(b), skipPrototype as any); - expect({ - skipPrototype, - negativeZeroAndZero: equal(withLastIndex(-0), withLastIndex(0)), - zeroAndNegativeZero: equal(withLastIndex(0), withLastIndex(-0)), - nanAndNaN: equal(withLastIndex(NaN), withLastIndex(NaN)), - // The identity shortcut answers before lastIndex is read. - sameObjectWithNaN: equal(sameObject, sameObject), - }).toEqual({ - skipPrototype, - negativeZeroAndZero: true, - zeroAndNegativeZero: true, - nanAndNaN: false, - sameObjectWithNaN: true, - }); - } - }); + for (const [name, wrap] of positions) { + test(`is compared with === ${name}`, () => { + const sameObject = withLastIndex(NaN); + for (const skipPrototype of [false, true]) { + const equal = (a: RegExp, b: RegExp) => util.isDeepStrictEqual(wrap(a), wrap(b), skipPrototype as any); + assert.deepStrictEqual( + { + skipPrototype, + negativeZeroAndZero: equal(withLastIndex(-0), withLastIndex(0)), + zeroAndNegativeZero: equal(withLastIndex(0), withLastIndex(-0)), + nanAndNaN: equal(withLastIndex(NaN), withLastIndex(NaN)), + stringZeroAndZero: equal(withLastIndex("0"), withLastIndex(0)), + // The identity shortcut answers before lastIndex is read. + sameObjectWithNaN: equal(sameObject, sameObject), + }, + { + skipPrototype, + negativeZeroAndZero: true, + zeroAndNegativeZero: true, + nanAndNaN: false, + stringZeroAndZero: false, + sameObjectWithNaN: true, + }, + ); + } + }); + } }); // The third argument was added in Node v26.