Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/js/builtins/CommonJS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down
18 changes: 16 additions & 2 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <ext>: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
Expand Down
132 changes: 131 additions & 1 deletion test/js/bun/resolve/import-query.test.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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": ``,
Expand Down
Loading