diff --git a/src/jsc/bindings/ImportMetaObject.cpp b/src/jsc/bindings/ImportMetaObject.cpp index d01ce5d6802c..136370982f29 100644 --- a/src/jsc/bindings/ImportMetaObject.cpp +++ b/src/jsc/bindings/ImportMetaObject.cpp @@ -226,47 +226,49 @@ extern "C" JSC::EncodedJSValue functionImportMeta__resolveSyncPrivate(JSC::JSGlo if (!isESM) { if (globalObject) [[likely]] { if (globalObject->hasOverriddenModuleResolveFilenameFunction) [[unlikely]] { - auto overrideHandler = uncheckedDowncast(globalObject->m_moduleResolveFilenameFunction.getInitializedOnMainThread(globalObject)); - if (overrideHandler) [[likely]] { - ASSERT(overrideHandler->isCallable()); - JSValue parentModuleObject = globalObject->requireMap()->get(globalObject, from); - - JSValue parentID = jsUndefined(); - if (auto* parent = dynamicDowncast(parentModuleObject)) { - parentID = parent->filename(); - } else { - parentID = from; - } + JSValue overrideHandler = globalObject->m_moduleResolveFilenameOverride.get(); + JSC::CallData overrideCallData = JSC::getCallData(overrideHandler); + if (overrideCallData.type == JSC::CallData::Type::None) [[unlikely]] { + return JSC::throwVMTypeError(lexicalGlobalObject, scope, "Module._resolveFilename is not a function"_s); + } - MarkedArgumentBuffer args; - args.append(moduleName); - args.append(parentModuleObject); - auto parentIdStr = parentID.toWTFString(globalObject); - auto bunStr = Bun::toString(parentIdStr); - args.append(jsBoolean(Bun__isBunMain(lexicalGlobalObject, &bunStr))); - - // Pass options object with paths if provided - if (!userPathList.isUndefinedOrNull()) { - JSObject* options = JSC::constructEmptyObject(globalObject); - options->putDirect(vm, JSC::Identifier::fromString(vm, "paths"_s), userPathList); - args.append(options); - } + JSValue parentModuleObject = globalObject->requireMap()->get(globalObject, from); + + JSValue parentID = jsUndefined(); + if (auto* parent = dynamicDowncast(parentModuleObject)) { + parentID = parent->filename(); + } else { + parentID = from; + } - JSValue result = JSC::profiledCall(lexicalGlobalObject, ProfilingReason::API, overrideHandler, JSC::getCallData(overrideHandler), parentModuleObject, args); + MarkedArgumentBuffer args; + args.append(moduleName); + args.append(parentModuleObject); + auto parentIdStr = parentID.toWTFString(globalObject); + auto bunStr = Bun::toString(parentIdStr); + args.append(jsBoolean(Bun__isBunMain(lexicalGlobalObject, &bunStr))); + + // Pass options object with paths if provided + if (!userPathList.isUndefinedOrNull()) { + JSObject* options = JSC::constructEmptyObject(globalObject); + options->putDirect(vm, JSC::Identifier::fromString(vm, "paths"_s), userPathList); + args.append(options); + } + + JSValue result = JSC::profiledCall(lexicalGlobalObject, ProfilingReason::API, overrideHandler, overrideCallData, parentModuleObject, args); + RETURN_IF_EXCEPTION(scope, {}); + if (!isRequireDotResolve) { + JSString* string = result.toString(globalObject); RETURN_IF_EXCEPTION(scope, {}); - if (!isRequireDotResolve) { - JSString* string = result.toString(globalObject); - RETURN_IF_EXCEPTION(scope, {}); - auto str = string->value(globalObject); - RETURN_IF_EXCEPTION(scope, {}); - WTF::String prefixed = Bun::isUnprefixedNodeBuiltin(str); - if (!prefixed.isNull()) { - return JSValue::encode(jsString(vm, prefixed)); - } - return JSC::JSValue::encode(string); + auto str = string->value(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + WTF::String prefixed = Bun::isUnprefixedNodeBuiltin(str); + if (!prefixed.isNull()) { + return JSValue::encode(jsString(vm, prefixed)); } - return JSC::JSValue::encode(result); + return JSC::JSValue::encode(string); } + return JSC::JSValue::encode(result); } } diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index 35a8a4f69ec6..35765cf1088e 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -516,7 +516,11 @@ class GlobalObject : public Bun::GlobalScope { \ /* TODO: these should use LazyProperty */ \ \ - V(public, LazyPropertyOfGlobalObject, m_moduleResolveFilenameFunction) \ + V(public, LazyPropertyOfGlobalObject, m_moduleResolveFilenameFunction) \ + /* Whatever user code assigned to require("module")._resolveFilename. Like Node's plain data */ \ + /* property it holds any value; require() throws if it is not callable. Only meaningful while */ \ + /* hasOverriddenModuleResolveFilenameFunction is set. */ \ + V(public, WriteBarrier, m_moduleResolveFilenameOverride) \ V(public, LazyPropertyOfGlobalObject, m_moduleRunMainFunction) \ V(public, LazyPropertyOfGlobalObject, m_modulePrototypeUnderscoreCompileFunction) \ V(public, LazyPropertyOfGlobalObject, m_commonJSRequireESMFromHijackedExtensionFunction) \ diff --git a/src/jsc/modules/NodeModuleModule.cpp b/src/jsc/modules/NodeModuleModule.cpp index b8732d7d8fee..7cf527d8f779 100644 --- a/src/jsc/modules/NodeModuleModule.cpp +++ b/src/jsc/modules/NodeModuleModule.cpp @@ -431,6 +431,9 @@ JSC_DEFINE_CUSTOM_GETTER(nodeModuleResolveFilename, PropertyName propertyName)) { auto* globalObject = defaultGlobalObject(lexicalGlobalObject); + if (globalObject->hasOverriddenModuleResolveFilenameFunction) [[unlikely]] { + return JSValue::encode(globalObject->m_moduleResolveFilenameOverride.get()); + } return JSValue::encode( globalObject->m_moduleResolveFilenameFunction.getInitializedOnMainThread( globalObject)); @@ -443,20 +446,26 @@ JSC_DEFINE_CUSTOM_SETTER(setNodeModuleResolveFilename, { auto* globalObject = defaultGlobalObject(lexicalGlobalObject); auto value = JSValue::decode(encodedValue); - if (value.isCell()) { - bool isOriginal = false; - if (value.isCallable()) { - JSC::CallData callData = JSC::getCallData(value); + bool isOriginal = false; + if (value.isCallable()) { + JSC::CallData callData = JSC::getCallData(value); - if (callData.type == JSC::CallData::Type::Native) { - if (callData.native.function.untaggedPtr() == &jsFunctionResolveFileName) { - isOriginal = true; - } + if (callData.type == JSC::CallData::Type::Native) { + if (callData.native.function.untaggedPtr() == &jsFunctionResolveFileName) { + isOriginal = true; } } - globalObject->hasOverriddenModuleResolveFilenameFunction = !isOriginal; - globalObject->m_moduleResolveFilenameFunction.set( - lexicalGlobalObject->vm(), globalObject, value.asCell()); + } + + if (isOriginal) { + globalObject->hasOverriddenModuleResolveFilenameFunction = false; + globalObject->m_moduleResolveFilenameOverride.clear(); + } else { + // Any value is accepted, as with Node's plain data property; whether it + // is callable is checked when require() goes to call it. + globalObject->m_moduleResolveFilenameOverride.set( + lexicalGlobalObject->vm(), globalObject, value); + globalObject->hasOverriddenModuleResolveFilenameFunction = true; } return true; @@ -1152,7 +1161,7 @@ void addNodeModuleConstructorProperties(JSC::VM& vm, }); globalObject->m_moduleResolveFilenameFunction.initLater( - [](const Zig::GlobalObject::Initializer& init) { + [](const Zig::GlobalObject::Initializer& init) { JSFunction* resolveFilenameFunction = JSFunction::create( init.vm, init.owner, 2, "_resolveFilename"_s, jsFunctionResolveFileName, JSC::ImplementationVisibility::Public, diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index 9014990b19f1..356fcebf00bd 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/test/js/node/module/node-module-module.test.js @@ -478,6 +478,76 @@ console.log("survived", require("./late.js"));`, expect(await proc.exited).toBe(0); }); + test("Overwriting _resolveFilename with a non-callable makes require() throw like Node", async () => { + // Node keeps _resolveFilename as a plain data property: any value can be + // assigned and reads back, and require() throws when it goes to call it. + using dir = tempDir("resolve-filename-non-callable", { + "dep.cjs": `module.exports = "dep";`, + "main.cjs": ` + const Module = require("module"); + const original = Module._resolveFilename; + const attempt = fn => { + try { + return "returned " + String(fn()); + } catch (e) { + return e.constructor.name + ": " + e.message; + } + }; + const results = {}; + for (const [label, value] of [ + ["object", {}], + ["string", "not a function"], + ["undefined", undefined], + ["null", null], + ["number", 42], + ["symbol", Symbol("s")], + ]) { + Module._resolveFilename = value; + results[label] = { + readsBack: Object.is(Module._resolveFilename, value), + require: attempt(() => require("./dep.cjs")), + requireResolve: attempt(() => require.resolve("./dep.cjs")), + createRequire: attempt(() => Module.createRequire(__filename)("./dep.cjs")), + }; + } + // Callable objects other than plain functions are still honored. + Module._resolveFilename = new Proxy(original, {}); + results.callableProxy = attempt(() => require("./dep.cjs")); + Module._resolveFilename = original; + results.restored = { + readsBack: Module._resolveFilename === original, + require: attempt(() => require("./dep.cjs")), + }; + console.log(JSON.stringify(results)); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.cjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const notAFunction = { + readsBack: true, + require: "TypeError: Module._resolveFilename is not a function", + requireResolve: "TypeError: Module._resolveFilename is not a function", + createRequire: "TypeError: Module._resolveFilename is not a function", + }; + expect(JSON.parse(stdout)).toEqual({ + object: notAFunction, + string: notAFunction, + undefined: notAFunction, + null: notAFunction, + number: notAFunction, + symbol: notAFunction, + callableProxy: "returned dep", + restored: { readsBack: true, require: "returned dep" }, + }); + expect(exitCode).toBe(0); + }); + test("Overwriting Module.prototype.require", async () => { await using proc = Bun.spawn({ cmd: [bunExe(), "run", path.join(import.meta.dir, "modulePrototypeOverwrite.cjs")],