From ce1981c52519e5fdb8e674b1635d31ea89fd02a6 Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 30 Nov 2025 16:58:58 -0800 Subject: [PATCH] fix(node:assert): handle Number and Boolean wrappers in deepStrictEqual (#25201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fixes `assert.deepStrictEqual()` to properly compare Number and Boolean wrapper objects - Previously, `new Number(1)` and `new Number(2)` were incorrectly considered equal because they have no enumerable properties - Now correctly extracts and compares internal values using `JSC::sameValue()`, then falls through to check own properties ## Test plan - [x] Run `bun bd test test/regression/issue/24045.test.ts` - all 6 tests pass - [x] Verify tests fail with system Bun (`USE_SYSTEM_BUN=1`) to confirm fix validity - [x] Verified behavior matches Node.js exactly (see table below) ## Node.js Compatibility | Test Case | Node.js | Bun | |-----------|---------|-----| | Different Number values (`new Number(1)` vs `new Number(2)`) | throws | throws | | Same Number values (`new Number(1)` vs `new Number(1)`) | equal | equal | | 0 vs -0 (`new Number(0)` vs `new Number(-0)`) | throws | throws | | NaN equals NaN (`new Number(NaN)` vs `new Number(NaN)`) | equal | equal | | Different Boolean values (`new Boolean(true)` vs `new Boolean(false)`) | throws | throws | | Same Boolean values | equal | equal | | Number wrapper vs primitive (`new Number(1)` vs `1`) | throws | throws | | Number vs Boolean wrapper | throws | throws | | Same value, different own properties | throws | throws | | Same value, same own properties | equal | equal | | Different own property values | throws | throws | ## Example Before (bug): ```javascript assert.deepStrictEqual(new Number(1), new Number(2)); // passes incorrectly ``` After (fixed): ```javascript assert.deepStrictEqual(new Number(1), new Number(2)); // throws AssertionError ``` Closes #24045 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Bot Co-authored-by: Claude Co-authored-by: Jarred Sumner --- src/bun.js/bindings/bindings.cpp | 13 +++- test/regression/issue/24045.test.ts | 105 ++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 test/regression/issue/24045.test.ts diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 3be3fad2c9..80a6de32d5 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -1555,9 +1555,20 @@ std::optional specialObjectsDequal(JSC::JSGlobalObject* globalObject, Mark auto* gp2 = jsDynamicCast(c2); return gp1->target()->m_globalThis == gp2->target()->m_globalThis; } - default: { + case NumberObjectType: + case BooleanObjectType: { + // Number and Boolean wrapper objects must be the same type and have the same internal value + if (c1Type != c2Type) return false; + JSValue val1 = jsCast(c1)->internalValue(); + JSValue val2 = jsCast(c2)->internalValue(); + bool same = JSC::sameValue(globalObject, val1, val2); + RETURN_IF_EXCEPTION(scope, {}); + if (!same) return false; + // Fall through to check own properties break; } + default: + break; } return std::nullopt; } diff --git a/test/regression/issue/24045.test.ts b/test/regression/issue/24045.test.ts new file mode 100644 index 0000000000..74db1ffafa --- /dev/null +++ b/test/regression/issue/24045.test.ts @@ -0,0 +1,105 @@ +import assert from "assert/strict"; +import { expect, test } from "bun:test"; + +test("assert.deepStrictEqual() should compare Number wrapper object values - issue #24045", () => { + // Different values should throw + expect(() => { + assert.deepStrictEqual(new Number(1), new Number(2)); + }).toThrow("Expected values to be strictly deep-equal"); + + // Same values should not throw + expect(() => { + assert.deepStrictEqual(new Number(1), new Number(1)); + }).not.toThrow(); + + // Edge cases + // 0 and -0 should be different in strict mode + expect(() => { + assert.deepStrictEqual(new Number(0), new Number(-0)); + }).toThrow("Expected values to be strictly deep-equal"); + + // NaN should equal NaN + expect(() => { + assert.deepStrictEqual(new Number(NaN), new Number(NaN)); + }).not.toThrow(); + + expect(() => { + assert.deepStrictEqual(new Number(Infinity), new Number(-Infinity)); + }).toThrow("Expected values to be strictly deep-equal"); +}); + +test("assert.deepStrictEqual() should compare Boolean wrapper object values - issue #24045", () => { + // Different values should throw + expect(() => { + assert.deepStrictEqual(new Boolean(true), new Boolean(false)); + }).toThrow("Expected values to be strictly deep-equal"); + + // Same values should not throw + expect(() => { + assert.deepStrictEqual(new Boolean(true), new Boolean(true)); + }).not.toThrow(); + + expect(() => { + assert.deepStrictEqual(new Boolean(false), new Boolean(false)); + }).not.toThrow(); +}); + +test("assert.deepStrictEqual() should not compare Number wrapper with primitive", () => { + // Wrapper objects should not equal primitives in strict mode + expect(() => { + assert.deepStrictEqual(new Number(1), 1); + }).toThrow("Expected values to be strictly deep-equal"); + + expect(() => { + assert.deepStrictEqual(1, new Number(1)); + }).toThrow("Expected values to be strictly deep-equal"); +}); + +test("assert.deepStrictEqual() should not compare Boolean wrapper with primitive", () => { + // Wrapper objects should not equal primitives in strict mode + expect(() => { + assert.deepStrictEqual(new Boolean(true), true); + }).toThrow("Expected values to be strictly deep-equal"); + + expect(() => { + assert.deepStrictEqual(false, new Boolean(false)); + }).toThrow("Expected values to be strictly deep-equal"); +}); + +test("assert.deepStrictEqual() should not compare Number and Boolean wrappers", () => { + // Different wrapper types should not be equal even with truthy/falsy values + expect(() => { + assert.deepStrictEqual(new Number(1), new Boolean(true)); + }).toThrow("Expected values to be strictly deep-equal"); + + expect(() => { + assert.deepStrictEqual(new Number(0), new Boolean(false)); + }).toThrow("Expected values to be strictly deep-equal"); +}); + +test("assert.deepStrictEqual() should check own properties on wrapper objects", () => { + // Same internal value but different own properties should not be equal + const num1 = new Number(42); + const num2 = new Number(42); + (num1 as any).customProp = "hello"; + + expect(() => { + assert.deepStrictEqual(num1, num2); + }).toThrow("Expected values to be strictly deep-equal"); + + // Same internal value and same own properties should be equal + (num2 as any).customProp = "hello"; + expect(() => { + assert.deepStrictEqual(num1, num2); + }).not.toThrow(); + + // Different own property values should not be equal + const bool1 = new Boolean(true); + const bool2 = new Boolean(true); + (bool1 as any).foo = 1; + (bool2 as any).foo = 2; + + expect(() => { + assert.deepStrictEqual(bool1, bool2); + }).toThrow("Expected values to be strictly deep-equal"); +});