diff --git a/src/js/builtins/CommonJS.ts b/src/js/builtins/CommonJS.ts index b2e756e16b79..31d0360d17ba 100644 --- a/src/js/builtins/CommonJS.ts +++ b/src/js/builtins/CommonJS.ts @@ -60,7 +60,10 @@ export function overridableRequire(this: JSCommonJSModule, originalId: string, o } } - if (id.endsWith(".node")) { + // A resolved id may carry a `?query` suffix (part of the module cache key); + // match the native-addon extension against the path portion only. + const queryIndex = id.indexOf("?"); + if (queryIndex === -1 ? id.endsWith(".node") : id.endsWith(".node", queryIndex)) { return $internalRequire(id, this); } @@ -158,10 +161,14 @@ export function requireResolve( $visibility = "Private"; export function internalRequire(id: string, parent: JSCommonJSModule) { $assert($requireMap.$get(id) === undefined, "Module " + JSON.stringify(id) + " should not be in the map"); - $assert(id.endsWith(".node")); + // `id` keys the module cache and may carry a `?query` suffix; + // `process.dlopen` needs the on-disk path. + const queryIndex = id.indexOf("?"); + const filename = queryIndex === -1 ? id : id.substring(0, queryIndex); + $assert(filename.endsWith(".node")); const module = $createCommonJSModule(id, {}, true, parent); - process.dlopen(module, id); + process.dlopen(module, filename); $requireMap.$set(id, module); return module.exports; } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index cb06ec630542..93cb7a4738af 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -3086,8 +3086,22 @@ fn transpile_source_code_inner( } } - // `provideFetch()` should be called. - L::Napi => unreachable!("napi modules go through provideFetch()"), + // The `.node` fast-paths in `moduleLoaderFetch` / `overridableRequire` + // only match module keys that literally end in `.node`, so `?query` + // suffixes and `--loader :napi` mappings still reach here. + L::Napi => { + if global_object.is_null() { + return Err(bun_core::err!("NotSupported")); + } + // SAFETY: null-checked above; `global_object` is the live + // per-thread global. + let global = unsafe { &*global_object }; + Err(global + .throw_type_error(format_args!( + "To load Node-API modules, use require() or process.dlopen instead of import." + )) + .into()) + } // ──────────────────────────────────────────────────────────────────── // .wasm diff --git a/test/js/bun/resolve/import-query.test.ts b/test/js/bun/resolve/import-query.test.ts index dea10515882a..42cb4f3bd972 100644 --- a/test/js/bun/resolve/import-query.test.ts +++ b/test/js/bun/resolve/import-query.test.ts @@ -1,5 +1,5 @@ import { beforeEach, expect, test } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness"; globalThis.importQueryFixtureOrder = []; const resolvedPath = require.resolve("./import-query-fixture.ts"); const resolvedURL = Bun.pathToFileURL(resolvedPath).href; @@ -86,6 +86,136 @@ test("query string with non-ASCII specifier (static import)", async () => { expect(exitCode).toBe(0); }); +// A `.node` (Node-API addon) specifier must behave the same with and without a +// `?query` suffix. Query-suffixed spellings used to bypass the `.node` checks +// (which run before the query is stripped) and abort the process with +// `panic: entered unreachable code: napi modules go through provideFetch()`. +const NAPI_IMPORT_ERROR = "To load Node-API modules, use require() or process.dlopen instead of import."; + +test("dynamic import of a .node addon ignores a query string suffix", async () => { + using dir = tempDir("import-query-napi-dynamic", { + "addon.node": "", + "entry.mjs": ` + const out = []; + for (const spec of ["./addon.node", "./addon.node?v=1", "./addon.node?v=2"]) { + try { + await import(spec); + out.push(null); + } catch (e) { + out.push(e.constructor.name + ": " + e.message); + } + } + console.log(JSON.stringify(out)); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode, signalCode: proc.signalCode }).toEqual({ + stdout: JSON.stringify([ + `TypeError: ${NAPI_IMPORT_ERROR}`, + `TypeError: ${NAPI_IMPORT_ERROR}`, + `TypeError: ${NAPI_IMPORT_ERROR}`, + ]), + stderr: "", + exitCode: 0, + signalCode: null, + }); +}); + +test("static import of a .node addon ignores a query string suffix", async () => { + using dir = tempDir("import-query-napi-static", { + "addon.node": "", + "entry.mjs": `import "./addon.node?update=1";`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ + firstLine: normalizeBunSnapshot(stderr, String(dir)).split("\n")[0], + stdout, + exitCode, + signalCode: proc.signalCode, + }).toEqual({ + firstLine: `TypeError: ${NAPI_IMPORT_ERROR}`, + stdout: "", + exitCode: 1, + signalCode: null, + }); +}); + +test("require of a .node addon with a query string reaches process.dlopen", async () => { + using dir = tempDir("import-query-napi-require", { + "addon.node": "", + "entry.cjs": ` + const out = []; + for (const spec of ["./addon.node", "./addon.node?v=1", "./addon.node?v=2"]) { + try { + require(spec); + out.push(null); + } catch (e) { + out.push(e.constructor.name + ": " + e.message); + } + } + console.log(JSON.stringify(out)); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.cjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stderr, exitCode, signalCode: proc.signalCode }).toEqual({ stderr: "", exitCode: 0, signalCode: null }); + const [plain, withQuery, withOtherQuery] = JSON.parse(stdout.trim()); + // An empty `.node` file cannot be dlopen'd. The query-suffixed spellings must + // fail with the identical dlopen error (proving the on-disk path was + // stripped of the query), not the ESM "use require()" TypeError. + expect(plain).not.toContain("Node-API"); + expect(withQuery).toBe(plain); + expect(withOtherQuery).toBe(plain); +}); + +test("import of an extension mapped to the napi loader throws instead of crashing", async () => { + using dir = tempDir("import-query-napi-loader-flag", { + "thing.xyz": "", + "entry.mjs": ` + try { + await import("./thing.xyz"); + console.log(JSON.stringify(null)); + } catch (e) { + console.log(JSON.stringify(e.constructor.name + ": " + e.message)); + } + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "--loader=.xyz:napi", "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode, signalCode: proc.signalCode }).toEqual({ + stdout: JSON.stringify(`TypeError: ${NAPI_IMPORT_ERROR}`), + stderr: "", + exitCode: 0, + signalCode: null, + }); +}); + test("Bun.resolveSync with non-ASCII specifier and query string", async () => { using dir = tempDir("resolve-query-nonascii", { "target.js": ``,