fix(resolver): support encoded file urls (#5766)

* start working on this

* it works now

* better implementation imo

* yippee

* more tests and better unrefing

* fix leak?

10 files changed, 230 insertions, 73 deletions

diff --git a/src/bun.js/api/bun.zig b/src/bun.js/api/bun.zig
index 966c82d38..949ba402c 100644
--- a/src/bun.js/api/bun.zig
+++ b/src/bun.js/api/bun.zig
@@ -782,11 +782,17 @@ fn doResolveWithArgs(
     var errorable: ErrorableString = undefined;
     var query_string = ZigString.Empty;
 
+    const specifier_decoded = if (specifier.hasPrefixComptime("file://"))
+        bun.JSC.URL.pathFromFileURL(specifier)
+    else
+        specifier.dupeRef();
+    defer specifier_decoded.deref();
+
     if (comptime is_file_path) {
         VirtualMachine.resolveFilePathForAPI(
             &errorable,
             ctx.ptr(),
-            specifier,
+            specifier_decoded,
             from,
             &query_string,
             is_esm,
@@ -795,7 +801,7 @@ fn doResolveWithArgs(
         VirtualMachine.resolveForAPI(
             &errorable,
             ctx.ptr(),
-            specifier,
+            specifier_decoded,
             from,
             &query_string,
             is_esm,
diff --git a/src/bun.js/bindings/BunString.cpp b/src/bun.js/bindings/BunString.cpp
index 416d5d334..31b331111 100644
--- a/src/bun.js/bindings/BunString.cpp
+++ b/src/bun.js/bindings/BunString.cpp
@@ -190,22 +190,6 @@ BunString toStringRef(WTF::StringImpl* wtfString)
     return { BunStringTag::WTFStringImpl, { .wtf = wtfString } };
 }
 
-BunString fromString(WTF::String& wtfString)
-{
-    if (wtfString.isEmpty())
-        return { BunStringTag::Empty };
-
-    return { BunStringTag::WTFStringImpl, { .wtf = wtfString.impl() } };
-}
-
-BunString fromString(WTF::StringImpl* wtfString)
-{
-    if (wtfString->isEmpty())
-        return { BunStringTag::Empty };
-
-    return { BunStringTag::WTFStringImpl, { .wtf = wtfString } };
-}
-
 }
 
 extern "C" JSC::EncodedJSValue BunString__toJS(JSC::JSGlobalObject* globalObject, BunString* bunString)
@@ -252,7 +236,7 @@ extern "C" BunString BunString__fromUTF8(const char* bytes, size_t length)
 
     auto str = WTF::String::fromUTF8ReplacingInvalidSequences(reinterpret_cast<const LChar*>(bytes), length);
     str.impl()->ref();
-    return Bun::fromString(str);
+    return Bun::toString(str);
 }
 
 extern "C" BunString BunString__fromLatin1(const char* bytes, size_t length)
@@ -381,7 +365,17 @@ extern "C" BunString URL__getHref(BunString* input)
     return Bun::toStringRef(url.string());
 }
 
-extern "C" BunString URL__getHrefJoin(BunString* baseStr, BunString *relativeStr)
+extern "C" BunString URL__pathFromFileURL(BunString* input)
+{
+    auto&& str = Bun::toWTFString(*input);
+    auto url = WTF::URL(str);
+    if (!url.isValid() || url.isEmpty())
+        return { BunStringTag::Dead };
+
+    return Bun::toStringRef(url.fileSystemPath());
+}
+
+extern "C" BunString URL__getHrefJoin(BunString* baseStr, BunString* relativeStr)
 {
     auto base = Bun::toWTFString(*baseStr);
     auto relative = Bun::toWTFString(*relativeStr);
@@ -455,4 +449,4 @@ extern "C" uint32_t URL__port(WTF::URL* url)
 extern "C" BunString URL__pathname(WTF::URL* url)
 {
     return Bun::toStringRef(url->path().toStringWithoutCopying());
-}
\ No newline at end of file
+}
diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp
index a7d2bb7e5..d54800ca6 100644
--- a/src/bun.js/bindings/ZigGlobalObject.cpp
+++ b/src/bun.js/bindings/ZigGlobalObject.cpp
@@ -4042,10 +4042,27 @@ JSC::Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* globalObject,
 {
     ErrorableString res;
     res.success = false;
-    BunString keyZ = Bun::toString(globalObject, key);
-    BunString referrerZ = referrer && !referrer.isUndefinedOrNull() && referrer.isString() ? Bun::toString(globalObject, referrer) : BunStringEmpty;
+    BunString keyZ;
+    if (key.isString()) {
+        auto moduleName = jsCast<JSString*>(key)->value(globalObject);
+        if (moduleName.startsWith("file://"_s)) {
+            auto url = WTF::URL(moduleName);
+            if (url.isValid() && !url.isEmpty()) {
+                keyZ = Bun::toStringRef(url.fileSystemPath());
+            } else {
+                keyZ = Bun::toStringRef(moduleName);
+            }
+        } else {
+            keyZ = Bun::toStringRef(moduleName);
+        }
+    } else {
+        keyZ = Bun::toStringRef(globalObject, key);
+    }
+    BunString referrerZ = referrer && !referrer.isUndefinedOrNull() && referrer.isString() ? Bun::toStringRef(globalObject, referrer) : BunStringEmpty;
     ZigString queryString = { 0, 0 };
     Zig__GlobalObject__resolve(&res, globalObject, &keyZ, &referrerZ, &queryString);
+    keyZ.deref();
+    referrerZ.deref();
 
     if (res.success) {
         if (queryString.len > 0) {
@@ -4074,11 +4091,32 @@ JSC::JSInternalPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* g
 
     auto sourceURL = sourceOrigin.url();
     ErrorableString resolved;
-    auto moduleNameZ = Bun::toString(globalObject, moduleNameValue);
-    auto sourceOriginZ = sourceURL.isEmpty() ? BunStringCwd : Bun::toString(sourceURL.fileSystemPath());
+    BunString moduleNameZ;
+
+    auto moduleName = moduleNameValue->value(globalObject);
+#if BUN_DEBUG
+    auto startRefCount = moduleName.impl()->refCount();
+#endif
+    if (moduleName.startsWith("file://"_s)) {
+        auto url = WTF::URL(moduleName);
+        if (url.isValid() && !url.isEmpty()) {
+            moduleNameZ = Bun::toStringRef(url.fileSystemPath());
+        } else {
+            moduleNameZ = Bun::toStringRef(moduleName);
+        }
+    } else {
+        moduleNameZ = Bun::toStringRef(moduleName);
+    }
+    auto sourceOriginZ = sourceURL.isEmpty() ? BunStringCwd : Bun::toStringRef(sourceURL.fileSystemPath());
     ZigString queryString = { 0, 0 };
     resolved.success = false;
     Zig__GlobalObject__resolve(&resolved, globalObject, &moduleNameZ, &sourceOriginZ, &queryString);
+    moduleNameZ.deref();
+    sourceOriginZ.deref();
+#if BUN_DEBUG
+    // TODO: ASSERT doesnt work right now
+    RELEASE_ASSERT(startRefCount == moduleName.impl()->refCount());
+#endif
     if (!resolved.success) {
         throwException(scope, resolved.result.err, globalObject);
         return promise->rejectWithCaughtException(globalObject, scope);
diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig
index 60f8f9376..f274f72d7 100644
--- a/src/bun.js/bindings/bindings.zig
+++ b/src/bun.js/bindings/bindings.zig
@@ -5496,6 +5496,7 @@ pub const URL = opaque {
     extern fn URL__getHref(*String) String;
     extern fn URL__getFileURLString(*String) String;
     extern fn URL__getHrefJoin(*String, *String) String;
+    extern fn URL__pathFromFileURL(*String) String;
 
     pub fn hrefFromString(str: bun.String) String {
         JSC.markBinding(@src());
@@ -5516,6 +5517,12 @@ pub const URL = opaque {
         return URL__getFileURLString(&input);
     }
 
+    pub fn pathFromFileURL(str: bun.String) String {
+        JSC.markBinding(@src());
+        var input = str;
+        return URL__pathFromFileURL(&input);
+    }
+
     /// This percent-encodes the URL, punycode-encodes the hostname, and returns the result
     /// If it fails, the tag is marked Dead
     pub fn hrefFromJS(value: JSValue, globalObject: *JSC.JSGlobalObject) String {
diff --git a/src/bun.js/bindings/headers-handwritten.h b/src/bun.js/bindings/headers-handwritten.h
index e19be7abe..cef5d0804 100644
--- a/src/bun.js/bindings/headers-handwritten.h
+++ b/src/bun.js/bindings/headers-handwritten.h
@@ -17,10 +17,6 @@ typedef union BunStringImpl {
     void* wtf;
 } BunStringImpl;
 
-typedef struct BunString {
-    BunStringTag tag;
-    BunStringImpl impl;
-} BunString;
 #else
 typedef union BunStringImpl {
     ZigString zig;
@@ -34,13 +30,15 @@ enum class BunStringTag : uint8_t {
     StaticZigString = 3,
     Empty = 4,
 };
+#endif
 
 typedef struct BunString {
     BunStringTag tag;
     BunStringImpl impl;
-} BunString;
 
-#endif
+    inline void ref();
+    inline void deref();
+} BunString;
 
 typedef struct ZigErrorType {
     ZigErrorCode code;
@@ -348,3 +346,16 @@ class ScriptArguments;
 using ScriptArguments = Inspector::ScriptArguments;
 
 #endif
+
+ALWAYS_INLINE void BunString::ref()
+{
+    if (this->tag == BunStringTag::WTFStringImpl) {
+        this->impl.wtf->ref();
+    }
+}
+ALWAYS_INLINE void BunString::deref()
+{
+    if (this->tag == BunStringTag::WTFStringImpl) {
+        this->impl.wtf->deref();
+    }
+}
\ No newline at end of file
diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig
index caa7d3cd8..d1b87e34e 100644
--- a/src/bun.js/javascript.zig
+++ b/src/bun.js/javascript.zig
@@ -1523,7 +1523,6 @@ pub const VirtualMachine = struct {
 
     fn normalizeSpecifierForResolution(specifier_: []const u8, query_string: *[]const u8) []const u8 {
         var specifier = specifier_;
-        if (strings.hasPrefixComptime(specifier, "file://")) specifier = specifier["file://".len..];
 
         if (strings.indexOfChar(specifier, '?')) |i| {
             query_string.* = specifier[i..];
diff --git a/src/resolver/resolver.zig b/src/resolver/resolver.zig
index 4ddd97dca..8835e57ce 100644
--- a/src/resolver/resolver.zig
+++ b/src/resolver/resolver.zig
@@ -1139,41 +1139,6 @@ pub const Resolver = struct {
             return .{ .not_found = {} };
         }
 
-        if (strings.hasPrefixComptime(import_path, "file:///")) {
-            const path = import_path[7..];
-
-            if (r.opts.external.abs_paths.count() > 0 and r.opts.external.abs_paths.contains(path)) {
-                // If the string literal in the source text is an absolute path and has
-                // been marked as an external module, mark it as *not* an absolute path.
-                // That way we preserve the literal text in the output and don't generate
-                // a relative path from the output directory to that path.
-                if (r.debug_logs) |*debug| {
-                    debug.addNoteFmt("The path \"{s}\" is marked as external by the user", .{path});
-                }
-
-                return .{
-                    .success = Result{
-                        .path_pair = .{ .primary = Path.init(import_path) },
-                        .is_external = true,
-                    },
-                };
-            }
-
-            if (r.loadAsFile(path, r.extension_order)) |file| {
-                return .{
-                    .success = Result{
-                        .dirname_fd = file.dirname_fd,
-                        .path_pair = .{ .primary = Path.init(file.path) },
-                        .diff_case = file.diff_case,
-                        .file_fd = file.file_fd,
-                        .jsx = r.opts.jsx,
-                    },
-                };
-            }
-
-            return .{ .not_found = {} };
-        }
-
         // Check both relative and package paths for CSS URL tokens, with relative
         // paths taking precedence over package paths to match Webpack behavior.
         const is_package_path = isPackagePath(import_path);
diff --git a/test/js/bun/resolve/import-meta.test.js b/test/js/bun/resolve/import-meta.test.js
index a940d0c87..e0e3bb11e 100644
--- a/test/js/bun/resolve/import-meta.test.js
+++ b/test/js/bun/resolve/import-meta.test.js
@@ -102,7 +102,7 @@ it("Module.createRequire(file://url).resolve(file://url)", () => {
 
   const createdRequire = Module.createRequire(import.meta.url);
   const result1 = createdRequire.resolve("./require-json.json");
-  const result2 = createdRequire.resolve("file://./require-json.json");
+  const result2 = createdRequire.resolve(`file://${expected}`);
   expect(result1).toBe(expected);
   expect(result2).toBe(expected);
 });
diff --git a/test/js/bun/resolve/png/test-png-import.test.js b/test/js/bun/resolve/png/test-png-import.test.js
index f4a809e7a..ca2d0b9ce 100644
--- a/test/js/bun/resolve/png/test-png-import.test.js
+++ b/test/js/bun/resolve/png/test-png-import.test.js
@@ -1,7 +1,7 @@
 import { expect, test } from "bun:test";
 import { resolve } from "path";
-// import MyPNG from "./test-png.png";
+import MyPNG from "./test-png.png";
 
-test.todo("png import", () => {
+test("png import", () => {
   expect(MyPNG).toBe(resolve(__dirname, "./test-png.png"));
 });
diff --git a/test/js/bun/resolve/resolve.test.ts b/test/js/bun/resolve/resolve.test.ts
index a9272fb3f..217d3dc81 100644
--- a/test/js/bun/resolve/resolve.test.ts
+++ b/test/js/bun/resolve/resolve.test.ts
@@ -92,7 +92,6 @@ it("file url in import resolves", async () => {
   });
   writeFileSync(`${dir}/test.js`, `import {foo} from 'file://${dir}/index.js';\nconsole.log(foo);`);
 
-  console.log("dir", dir);
   const { exitCode, stdout } = Bun.spawnSync({
     cmd: [bunExe(), `${dir}/test.js`],
     env: bunEnv,
@@ -102,13 +101,97 @@ it("file url in import resolves", async () => {
   expect(stdout.toString("utf8")).toBe("1\n");
 });
 
+it("invalid file url in import throws error", async () => {
+  const dir = tempDirWithFiles("fileurl", {});
+  writeFileSync(`${dir}/test.js`, `import {foo} from 'file://\0invalid url';\nconsole.log(foo);`);
+
+  const { exitCode, stdout, stderr } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).not.toBe(0);
+  expect(stderr.toString("utf8")).toContain("file://\0invalid url");
+});
+
 it("file url in await import resolves", async () => {
   const dir = tempDirWithFiles("fileurl", {
     "index.js": "export const foo = 1;",
   });
   writeFileSync(`${dir}/test.js`, `const {foo} = await import('file://${dir}/index.js');\nconsole.log(foo);`);
 
-  console.log("dir", dir);
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe("1\n");
+});
+
+it("file url with special characters in await import resolves", async () => {
+  const filename = "🅱️ndex.js";
+  const dir = tempDirWithFiles("file url", {
+    [filename]: "export const foo = 1;",
+  });
+  writeFileSync(
+    `${dir}/test.js`,
+    `const {foo} = await import('file://${dir.replace(/ /g, "%20")}/${encodeURIComponent(
+      filename,
+    )}');\nconsole.log(foo);`,
+  );
+
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe("1\n");
+});
+
+it("file url with special characters not encoded in await import resolves", async () => {
+  const filename = "🅱️ndex.js";
+  const dir = tempDirWithFiles("file url", {
+    [filename]: "export const foo = 1;",
+  });
+  writeFileSync(`${dir}/test.js`, `const {foo} = await import('file://${dir}/${filename}');\nconsole.log(foo);`);
+
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe("1\n");
+});
+
+it("file url with special characters in import statement resolves", async () => {
+  const filename = "🅱️ndex.js";
+  const dir = tempDirWithFiles("file url", {
+    [filename]: "export const foo = 1;",
+  });
+  writeFileSync(
+    `${dir}/test.js`,
+    `import {foo} from 'file://${dir.replace(/ /g, "%20")}/${encodeURIComponent(filename)}';\nconsole.log(foo);`,
+  );
+
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe("1\n");
+});
+
+it("file url with special characters not encoded in import statement resolves", async () => {
+  const filename = "🅱️ndex.js";
+  const dir = tempDirWithFiles("file url", {
+    [filename]: "export const foo = 1;",
+  });
+  writeFileSync(`${dir}/test.js`, `import {foo} from 'file://${dir}/${filename}';\nconsole.log(foo);`);
+
   const { exitCode, stdout } = Bun.spawnSync({
     cmd: [bunExe(), `${dir}/test.js`],
     env: bunEnv,
@@ -124,7 +207,6 @@ it("file url in require resolves", async () => {
   });
   writeFileSync(`${dir}/test.js`, `const {foo} = require('file://${dir}/index.js');\nconsole.log(foo);`);
 
-  console.log("dir", dir);
   const { exitCode, stdout } = Bun.spawnSync({
     cmd: [bunExe(), `${dir}/test.js`],
     env: bunEnv,
@@ -134,6 +216,61 @@ it("file url in require resolves", async () => {
   expect(stdout.toString("utf8")).toBe("1\n");
 });
 
+it("file url with special characters in require resolves", async () => {
+  const filename = "🅱️ndex.js";
+  const dir = tempDirWithFiles("file url", {
+    [filename]: "export const foo = 1;",
+  });
+  writeFileSync(
+    `${dir}/test.js`,
+    `const {foo} = require('file://${dir.replace(/ /g, "%20")}/${encodeURIComponent(filename)}');\nconsole.log(foo);`,
+  );
+
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe("1\n");
+});
+
+it("file url in require.resolve resolves", async () => {
+  const dir = tempDirWithFiles("fileurl", {
+    "index.js": "export const foo = 1;",
+  });
+  writeFileSync(`${dir}/test.js`, `const to = require.resolve('file://${dir}/index.js');\nconsole.log(to);`);
+
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe(`${dir}/index.js\n`);
+});
+
+it("file url with special characters in require resolves", async () => {
+  const filename = "🅱️ndex.js";
+  const dir = tempDirWithFiles("file url", {
+    [filename]: "export const foo = 1;",
+  });
+  writeFileSync(
+    `${dir}/test.js`,
+    `const to = require.resolve('file://${dir.replace(/ /g, "%20")}/${encodeURIComponent(
+      filename,
+    )}');\nconsole.log(to);`,
+  );
+
+  const { exitCode, stdout } = Bun.spawnSync({
+    cmd: [bunExe(), `${dir}/test.js`],
+    env: bunEnv,
+    cwd: import.meta.dir,
+  });
+  expect(exitCode).toBe(0);
+  expect(stdout.toString("utf8")).toBe(`${dir}/${filename}\n`);
+});
+
 it("import long string should not segfault", async () => {
   try {
     await import("a".repeat(10000));