mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
fix(napi): napi_typeof returns napi_object for String objects (#25365)
## Summary
- Fix `napi_typeof` to return `napi_object` for boxed String objects
(`new String("hello")`) instead of incorrectly returning `napi_string`
- Add regression test for boxed primitive objects (String, Number,
Boolean)
The issue was that `StringObjectType` and `DerivedStringObjectType` JSC
cell types were falling through to return `napi_string`, but these
represent object wrappers around strings, not primitive strings.
## Test plan
- [x] `bun bd test test/napi/napi.test.ts -t "napi_typeof"` passes
- [x] Test fails with `USE_SYSTEM_BUN=1` (confirming the bug exists in
released version)
Fixes #25351
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -2403,6 +2403,8 @@ extern "C" napi_status napi_typeof(napi_env env, napi_value val,
|
||||
return napi_clear_last_error(env);
|
||||
case JSC::DerivedStringObjectType:
|
||||
case JSC::StringObjectType:
|
||||
*result = napi_object;
|
||||
return napi_clear_last_error(env);
|
||||
case JSC::StringType:
|
||||
*result = napi_string;
|
||||
return napi_clear_last_error(env);
|
||||
|
||||
@@ -650,19 +650,19 @@ nativeTests.test_ref_deleted_in_async_finalize = () => {
|
||||
asyncFinalizeAddon.create_ref();
|
||||
};
|
||||
|
||||
nativeTests.test_reference_unref_in_finalizer = async (gc) => {
|
||||
nativeTests.test_reference_unref_in_finalizer = async gc => {
|
||||
// Create objects with finalizers that will call napi_reference_unref when GC'd
|
||||
let objects = testReferenceUnrefInFinalizer.test_reference_unref_in_finalizer();
|
||||
|
||||
|
||||
// Clear the reference to allow GC
|
||||
objects = null;
|
||||
|
||||
|
||||
// Force GC multiple times to ensure finalizers run
|
||||
if (gc) {
|
||||
gc();
|
||||
gc();
|
||||
}
|
||||
|
||||
|
||||
// Allocate large ArrayBuffers to trigger GC pressure
|
||||
const buffers = [];
|
||||
for (let i = 0; i < 100; i++) {
|
||||
@@ -671,54 +671,54 @@ nativeTests.test_reference_unref_in_finalizer = async (gc) => {
|
||||
gc();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// Wait for async operations
|
||||
await new Promise(resolve => setTimeout(resolve, 50));
|
||||
|
||||
|
||||
// Force final GC
|
||||
if (gc) {
|
||||
gc();
|
||||
gc();
|
||||
}
|
||||
|
||||
|
||||
// Get stats to verify finalizers were called
|
||||
const stats = testReferenceUnrefInFinalizer.get_stats();
|
||||
console.log(`Finalizers called: ${stats.finalizersCalled}, Unrefs succeeded: ${stats.unrefsSucceeded}`);
|
||||
|
||||
|
||||
if (stats.finalizersCalled === 0) {
|
||||
throw new Error("No finalizers were called - test did not properly trigger GC");
|
||||
}
|
||||
|
||||
|
||||
if (stats.unrefsSucceeded === 0) {
|
||||
throw new Error("No napi_reference_unref calls succeeded");
|
||||
}
|
||||
|
||||
|
||||
console.log("SUCCESS: napi_reference_unref worked in finalizers without crashing");
|
||||
};
|
||||
|
||||
nativeTests.test_reference_unref_in_finalizer_experimental = async (gc) => {
|
||||
nativeTests.test_reference_unref_in_finalizer_experimental = async gc => {
|
||||
// This test is expected to CRASH when the finalizer runs
|
||||
// The experimental NAPI module enforces GC checks and will abort the process
|
||||
console.log("WARNING: This test will crash the process - this is expected behavior!");
|
||||
|
||||
|
||||
// Create objects with finalizers that will call napi_reference_unref when GC'd
|
||||
let objects = testReferenceUnrefInFinalizerExperimental.test_reference_unref_in_finalizer_experimental();
|
||||
|
||||
|
||||
// Clear the reference to allow GC
|
||||
objects = null;
|
||||
|
||||
|
||||
// Force GC to trigger the finalizers - this should crash the process
|
||||
if (gc) {
|
||||
gc();
|
||||
gc();
|
||||
}
|
||||
|
||||
|
||||
// Allocate memory to ensure GC runs
|
||||
for (let i = 0; i < 5; i++) {
|
||||
new ArrayBuffer(10 * 1024 * 1024);
|
||||
if (gc) gc();
|
||||
}
|
||||
|
||||
|
||||
// If we get here, the test has FAILED - the process should have crashed
|
||||
console.log("ERROR: Process did not crash as expected!");
|
||||
console.log("ERROR: The GC check for experimental modules is NOT working!");
|
||||
@@ -731,19 +731,21 @@ nativeTests.test_create_bigint_words = () => {
|
||||
|
||||
nativeTests.test_bigint_word_count = () => {
|
||||
// Test with a 2-word BigInt
|
||||
const bigint = 0x123456789ABCDEF0123456789ABCDEFn;
|
||||
const bigint = 0x123456789abcdef0123456789abcdefn;
|
||||
const result = nativeTests.test_bigint_actual_word_count(bigint);
|
||||
|
||||
|
||||
console.log(`BigInt: ${bigint.toString(16)}`);
|
||||
console.log(`Queried word count: ${result.queriedWordCount}`);
|
||||
console.log(`Actual word count: ${result.actualWordCount}`);
|
||||
console.log(`Sign bit: ${result.signBit}`);
|
||||
|
||||
|
||||
// Both counts should be 2 for this BigInt
|
||||
if (result.queriedWordCount === 2 && result.actualWordCount === 2) {
|
||||
console.log("✅ PASS: Word count correctly returns 2");
|
||||
} else {
|
||||
console.log(`❌ FAIL: Expected word count 2, got queried=${result.queriedWordCount}, actual=${result.actualWordCount}`);
|
||||
console.log(
|
||||
`❌ FAIL: Expected word count 2, got queried=${result.queriedWordCount}, actual=${result.actualWordCount}`,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -751,16 +753,18 @@ nativeTests.test_ref_unref_underflow = () => {
|
||||
// Test that napi_reference_unref properly handles refCount == 0
|
||||
const obj = { test: "value" };
|
||||
const result = nativeTests.test_reference_unref_underflow(obj);
|
||||
|
||||
|
||||
console.log(`First unref count: ${result.firstUnrefCount}`);
|
||||
console.log(`Second unref status: ${result.secondUnrefStatus}`);
|
||||
|
||||
|
||||
// First unref should succeed and return count of 0
|
||||
// Second unref should fail with napi_generic_failure (status = 1)
|
||||
if (result.firstUnrefCount === 0 && result.secondUnrefStatus === 1) {
|
||||
console.log("✅ PASS: Reference unref correctly prevents underflow");
|
||||
} else {
|
||||
console.log(`❌ FAIL: Expected firstUnrefCount=0, secondUnrefStatus=1, got ${result.firstUnrefCount}, ${result.secondUnrefStatus}`);
|
||||
console.log(
|
||||
`❌ FAIL: Expected firstUnrefCount=0, secondUnrefStatus=1, got ${result.firstUnrefCount}, ${result.secondUnrefStatus}`,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -841,4 +845,54 @@ nativeTests.test_cleanup_hook_modification_during_iteration = () => {
|
||||
addon.test();
|
||||
};
|
||||
|
||||
// Test for napi_typeof with boxed primitive objects (String, Number, Boolean)
|
||||
// See: https://github.com/oven-sh/bun/issues/25351
|
||||
nativeTests.test_napi_typeof_boxed_primitives = () => {
|
||||
// napi_valuetype enum values (from node_api_types.h):
|
||||
// napi_undefined = 0, napi_null = 1, napi_boolean = 2, napi_number = 3,
|
||||
// napi_string = 4, napi_symbol = 5, napi_object = 6, napi_function = 7,
|
||||
// napi_external = 8, napi_bigint = 9
|
||||
|
||||
const napi_string = 4;
|
||||
const napi_object = 6;
|
||||
|
||||
// Primitive string should be napi_string
|
||||
const primitiveStringType = nativeTests.napi_get_typeof("hello");
|
||||
assert.strictEqual(
|
||||
primitiveStringType,
|
||||
napi_string,
|
||||
`primitive string should be napi_string (${napi_string}), got ${primitiveStringType}`,
|
||||
);
|
||||
console.log("PASS: primitive string returns napi_string");
|
||||
|
||||
// String object should be napi_object
|
||||
const stringObjectType = nativeTests.napi_get_typeof(new String("hello"));
|
||||
assert.strictEqual(
|
||||
stringObjectType,
|
||||
napi_object,
|
||||
`String object should be napi_object (${napi_object}), got ${stringObjectType}`,
|
||||
);
|
||||
console.log("PASS: String object returns napi_object");
|
||||
|
||||
// Number object should be napi_object
|
||||
const numberObjectType = nativeTests.napi_get_typeof(new Number(42));
|
||||
assert.strictEqual(
|
||||
numberObjectType,
|
||||
napi_object,
|
||||
`Number object should be napi_object (${napi_object}), got ${numberObjectType}`,
|
||||
);
|
||||
console.log("PASS: Number object returns napi_object");
|
||||
|
||||
// Boolean object should be napi_object
|
||||
const booleanObjectType = nativeTests.napi_get_typeof(new Boolean(true));
|
||||
assert.strictEqual(
|
||||
booleanObjectType,
|
||||
napi_object,
|
||||
`Boolean object should be napi_object (${napi_object}), got ${booleanObjectType}`,
|
||||
);
|
||||
console.log("PASS: Boolean object returns napi_object");
|
||||
|
||||
console.log("All boxed primitive tests passed!");
|
||||
};
|
||||
|
||||
module.exports = nativeTests;
|
||||
|
||||
@@ -1825,6 +1825,36 @@ static napi_value test_napi_empty_buffer_info(const Napi::CallbackInfo &info) {
|
||||
return ok(env);
|
||||
}
|
||||
|
||||
// Test for napi_typeof with boxed primitive objects (String, Number, Boolean)
|
||||
// See: https://github.com/oven-sh/bun/issues/25351
|
||||
static napi_value napi_get_typeof(const Napi::CallbackInfo &info) {
|
||||
Napi::Env env = info.Env();
|
||||
|
||||
if (info.Length() < 1) {
|
||||
printf("FAIL: Expected 1 argument\n");
|
||||
return env.Undefined();
|
||||
}
|
||||
|
||||
napi_value value = info[0];
|
||||
napi_valuetype type;
|
||||
napi_status status = napi_typeof(env, value, &type);
|
||||
|
||||
if (status != napi_ok) {
|
||||
printf("FAIL: napi_typeof failed with status %d\n", status);
|
||||
return env.Undefined();
|
||||
}
|
||||
|
||||
napi_value result;
|
||||
status = napi_create_int32(env, static_cast<int32_t>(type), &result);
|
||||
|
||||
if (status != napi_ok) {
|
||||
printf("FAIL: napi_create_int32 failed\n");
|
||||
return env.Undefined();
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
void register_standalone_tests(Napi::Env env, Napi::Object exports) {
|
||||
REGISTER_FUNCTION(env, exports, test_issue_7685);
|
||||
REGISTER_FUNCTION(env, exports, test_issue_11949);
|
||||
@@ -1857,6 +1887,7 @@ void register_standalone_tests(Napi::Env env, Napi::Object exports) {
|
||||
REGISTER_FUNCTION(env, exports, test_napi_freeze_seal_indexed);
|
||||
REGISTER_FUNCTION(env, exports, test_napi_create_external_buffer_empty);
|
||||
REGISTER_FUNCTION(env, exports, test_napi_empty_buffer_info);
|
||||
REGISTER_FUNCTION(env, exports, napi_get_typeof);
|
||||
}
|
||||
|
||||
} // namespace napitests
|
||||
|
||||
@@ -763,6 +763,18 @@ describe("cleanup hooks", () => {
|
||||
// Bun has special handling for isEmpty() that Node doesn't have
|
||||
expect(output).toContain("napi_typeof");
|
||||
});
|
||||
|
||||
it("should return napi_object for boxed primitives (String, Number, Boolean)", async () => {
|
||||
// Regression test for https://github.com/oven-sh/bun/issues/25351
|
||||
// napi_typeof was incorrectly returning napi_string for String objects (new String("hello"))
|
||||
// when it should return napi_object (matching JavaScript's typeof behavior)
|
||||
const output = await checkSameOutput("test_napi_typeof_boxed_primitives", []);
|
||||
expect(output).toContain("PASS: primitive string returns napi_string");
|
||||
expect(output).toContain("PASS: String object returns napi_object");
|
||||
expect(output).toContain("PASS: Number object returns napi_object");
|
||||
expect(output).toContain("PASS: Boolean object returns napi_object");
|
||||
expect(output).toContain("All boxed primitive tests passed!");
|
||||
});
|
||||
});
|
||||
|
||||
describe("napi_object_freeze and napi_object_seal", () => {
|
||||
|
||||
Reference in New Issue
Block a user