From aaa827f90eb76561abff8de47de4631774838b51 Mon Sep 17 00:00:00 2001 From: dave caruso Date: Fri, 8 Dec 2023 16:32:35 -0800 Subject: [PATCH] fix(node:module): allow file url strings in createRequire (#7533) * fix(node:module): allow file url strings in createRequire * add a non-happy path test :) --- src/bun.js/bindings/CommonJSModuleRecord.cpp | 2 ++ src/bun.js/modules/NodeModuleModule.h | 20 ++++++++++--- test/js/bun/resolve/import-meta.test.js | 30 ++++++++++++++++++++ test/js/bun/resolve/with space/hello.js | 1 + 4 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 test/js/bun/resolve/with space/hello.js diff --git a/src/bun.js/bindings/CommonJSModuleRecord.cpp b/src/bun.js/bindings/CommonJSModuleRecord.cpp index 39356e4519..6507d24c6b 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.cpp +++ b/src/bun.js/bindings/CommonJSModuleRecord.cpp @@ -1050,6 +1050,8 @@ std::optional createCommonJSModule( JSObject* JSCommonJSModule::createBoundRequireFunction(VM& vm, JSGlobalObject* lexicalGlobalObject, const WTF::String& pathString) { + ASSERT(!pathString.startsWith("file://"_s)); + auto* globalObject = jsCast(lexicalGlobalObject); JSString* filename = JSC::jsStringWithCache(vm, pathString); diff --git a/src/bun.js/modules/NodeModuleModule.h b/src/bun.js/modules/NodeModuleModule.h index 00807521c3..cf8ad2be4e 100644 --- a/src/bun.js/modules/NodeModuleModule.h +++ b/src/bun.js/modules/NodeModuleModule.h @@ -180,14 +180,26 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeModuleCreateRequire, if (callFrame->argumentCount() < 1) { throwTypeError(globalObject, scope, "createRequire() requires at least one argument"_s); - RELEASE_AND_RETURN(scope, JSC::JSValue::encode(JSC::jsUndefined())); + RELEASE_AND_RETURN(scope, JSC::JSValue::encode({})); } auto val = callFrame->uncheckedArgument(0).toWTFString(globalObject); + + if (val.startsWith("file://"_s)) { + WTF::URL url(val); + if (!url.isValid()) { + throwTypeError(globalObject, scope, makeString("createRequire() was given an invalid URL '"_s, url.string(), "'"_s));; + RELEASE_AND_RETURN(scope, JSValue::encode({})); + } + if (!url.protocolIsFile()) { + throwTypeError(globalObject, scope, "createRequire() does not support non-file URLs"_s); + RELEASE_AND_RETURN(scope, JSValue::encode({})); + } + val = url.fileSystemPath(); + } + RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode(JSC::jsUndefined())); - RELEASE_AND_RETURN( - scope, JSValue::encode(Bun::JSCommonJSModule::createBoundRequireFunction( - vm, globalObject, val))); + RELEASE_AND_RETURN(scope, JSValue::encode(Bun::JSCommonJSModule::createBoundRequireFunction(vm, globalObject, val))); } extern "C" JSC::EncodedJSValue Resolver__nodeModulePathsForJS(JSGlobalObject *, CallFrame *); diff --git a/test/js/bun/resolve/import-meta.test.js b/test/js/bun/resolve/import-meta.test.js index e0e3bb11e2..dc76a1e194 100644 --- a/test/js/bun/resolve/import-meta.test.js +++ b/test/js/bun/resolve/import-meta.test.js @@ -34,6 +34,36 @@ it("Module.createRequire", () => { expect(Module.createRequire(new URL(import.meta.url)).resolve(import.meta.path)).toBe(import.meta.path); }); +it("Module.createRequire works with a file url", () => { + const require = Module.createRequire(import.meta.url); + expect(require.resolve(import.meta.path)).toBe(path); + expect(require.resolve("./" + import.meta.file)).toBe(path); + const { resolve } = require; + expect(resolve("./" + import.meta.file)).toBe(path); +}); + +it("Module.createRequire works with a file url with a space", () => { + const path = join(import.meta.dir, "with space/hello.js"); + const require = Module.createRequire(new URL("./with space/nonexist.js", import.meta.url).toString()); + expect(require.resolve(import.meta.path)).toBe(import.meta.path); + expect(require.resolve("./hello")).toBe(path); + const { resolve } = require; + expect(resolve("./hello")).toBe(path); +}); + +it("Module.createRequire does not use file url as the referrer (err message check)", () => { + const require = Module.createRequire(import.meta.url); + try { + require("whaaat"); + expect.unreachable(); + } catch (e) { + expect(e.name).not.toBe("UnreachableError"); + expect(e.message).not.toInclude("file:///"); + expect(e.message).toInclude('"whaaat"'); + expect(e.message).toInclude('"' + import.meta.path + '"'); + } +}); + it("require with a query string works on dynamically created content", () => { rmSync("/tmp/bun-test-import-meta-dynamic-dir", { recursive: true, diff --git a/test/js/bun/resolve/with space/hello.js b/test/js/bun/resolve/with space/hello.js new file mode 100644 index 0000000000..842e368a0a --- /dev/null +++ b/test/js/bun/resolve/with space/hello.js @@ -0,0 +1 @@ +export default 2;