url: validate percent-decoding and platform path rules in fileURLToPath

[robobun]

Fixes #29174. Supersedes #29176, which fixed the percent-decoding half only (and gave the URIError a code that Node does not set).

Reproduction

import { fileURLToPath } from "node:url";

fileURLToPath("file:///%c3%28");
// node: URIError: URI malformed
// bun:  "" <- empty path

fileURLToPath("file:///%");
// node: URIError: URI malformed
// bun:  "/%"

fileURLToPath("file:///C:/a%5Cb", { windows: true });
// node: TypeError [ERR_INVALID_FILE_URL_PATH]: File URL path must not include encoded \ or / characters
// bun:  "/C:/a\b"

Returning "" instead of throwing turns a malformed URL into "the current directory" for whatever fs call receives it, and the unvalidated pass-throughs produce paths containing characters Node's contract guarantees cannot appear.

Cause

functionFileURLToPath handed the URL pathname to WTF::URL::fileSystemPath():

• Its percent-decoder is lenient. % not followed by two hex digits is copied through verbatim, and escapes that decode to invalid UTF-8 produce a null WTF::String (WebKit's own FIXME: This returns a null string when we encounter an invalid UTF-8 sequence. Is that OK?), which surfaces in JS as "". Node decodes with decodeURIComponent, which throws URIError: URI malformed for both.
• It is compiled for the host platform only, so the windows option was ignored: the %5C check, the drive-letter check, and the / -> \ conversion never ran on POSIX, and the POSIX host check never ran on Windows.

Fix

Implement Node's getPathFromURLWin32 / getPathFromURLPosix directly in the binding and drop the fileSystemPath() call:

• decodeFileURLPath decodes the pathname with ECMA-262 Decode semantics (the algorithm behind decodeURIComponent), throwing URIError: URI malformed on a truncated or non-hex escape, and on bytes that are not well-formed UTF-8 (lone continuation bytes, truncated sequences, overlong encodings, encoded surrogates, code points above U+10FFFF).
• options.windows selects the platform rules at runtime, defaulting to the host platform, matching fileURLToPath(path, { windows }).
• UNC server names go through IDNA-to-Unicode, as Node does. Bun::domainToUnicode is factored out of node:url's domainToUnicode binding so the two share one implementation.

Error messages and the ERR_INVALID_FILE_URL_PATH / ERR_INVALID_FILE_URL_HOST codes now match Node exactly, including File URL path must be absolute (previously must be an absolute path).

Deletes the commented-out fileURLToPath shim in src/js/node/url.ts, which existed to patch in the Windows path checking the binding now does itself, along with the Char.PERCENT constant it was the last reference to.

One deliberate deviation

Bun's URL parser accepts IDN hosts Node's rejects outright (new URL("file://xn--/foo") throws ERR_INVALID_URL in Node, parses in Bun), and uidna_nameToUnicode cannot decode those. A literal port of Node's `\\\\${domainToUnicode(hostname)}${pathname}` would emit \\\foo for such a host, a UNC path with an empty server name, because Node's domainToUnicode returns '' on failure. The raw host is kept instead. For every host Node can parse, the result is identical to Node.

pathToFileURL still ignores options.windows. That is a pre-existing gap on main, not something this change introduces, and it is left for a separate fix.

Fallout: a latent sqlite bug this surfaced

fileURLToPath("file:///\\\\server\\share\\db") now throws on Windows, because the URL carries no host and the drive-letter check rejects ///server/share/db. Node throws the same ERR_INVALID_FILE_URL_PATH. That throw newly activates a fallback in parseDefinitelySqliteUrl (src/js/internal/sql/shared.ts) which had never been exercised on any platform, and which sliced off file:// and returned /\\server\share\db with a leading slash no Windows path wants. It now slices the matched prefix and, on Windows only, drops that slash when a windows-rooted path follows it, either a UNC share or a drive letter. Two shapes reach the fallback because this PR made them throw: file:///\\\\server\\share\\db (no host, so the drive-letter check rejects it) and file:///C:/50%off.db (a literal % is now a URIError, where the old lenient decoder passed it through and returned C:\\50%off.db). Posix-shaped URLs like file:///tmp/db reach the same fallback on Windows too, since they carry no drive letter, and they keep the slash they start with. The strip mirrors what fileURLToPath itself removes for those paths when it accepts them.

Every file:// case in sqlite-url-parsing.test.ts was simulated under Windows semantics before the change landed, which this PR makes possible: fileURLToPath(url, { windows: true }) exercises the real win32 branch from any host, and the fallback is pure string arithmetic on top of it.

The fallback is best effort and stays that way. It slices a fixed prefix and so has always ignored the URL's authority: new SQL("file://localhost/tmp/db") returned localhost/tmp/db on Windows long before this change, because the path is not drive-rooted and fileURLToPath rejected it then too. Making fileURLToPath stricter widens the set of URLs that reach the fallback, so that pre-existing blind spot is now also reachable for URLs that carry an authority and a malformed escape (file://server/share/50%off.db). Fixing that properly means giving the sqlite layer its own lenient URL-to-path conversion rather than piggybacking on fileURLToPath, which is a separate change; reusing the parsed pathname here would break the file://.hidden.db cases the suite already asserts. What this PR does fix is the class that is actually unopenable on Windows: a leading slash in front of a drive letter or UNC root.

Verification

bun bd test test/js/node/url/url-fileurltopath.test.js test/js/bun/util/fileUrl.test.js — 50 pass, 0 fail. The new cases all fail on main.

Beyond the unit tests, a differential against Node v26 over a generated grammar of file URLs (hosts x path segments x escape sequences x query/fragment x {windows: true|false} x string/URL input) compared 30,824 cases. Every result is byte-identical to Node except 362 that hinge on C|:

The C| cases are a pre-existing URL parser divergence, not fileURLToPath

new URL("file:///../C|").pathname           // node: "/C:"     bun: "/C|"
new URL("file://localhost/C|/foo").pathname // node: "/C:/foo" bun: "/C|/foo"
new URL("file:///C|/foo").pathname          // node: "/C:/foo" bun: "/C:/foo"  (agree)

WTF's URL parser only applies the WHATWG "normalized windows drive letter" rule in some positions. That reproduces on released Bun and is untouched by this change, so it is left for a separate fix.

[robobun]

^{Updated 11:09 AM PT - Jul 5th, 2026}

❌ @robobun, your commit 07c0929 has 1 failures in Build #68589 (All Failures):

• test/js/bun/util/v8-heap-snapshot.test.ts - SIGKILL on 🐧 25.04 x64

---

🧪   To try this PR locally:

bunx bun-pr 33373

That installs a local version of the PR into your bun-33373 executable, so you can run:

bun-33373 --bun

[coderabbitai]

Walkthrough

This PR updates fileURLToPath to accept a windows option and stricter percent-decoding, refactors domainToUnicode into a shared helper, and adjusts related tests and SQLite URL fallback behavior.

Changes

fileURLToPath windows option and strict decoding

Layer / File(s)	Summary
Type declaration update for fileURLToPath options packages/bun-types/bun.d.ts	Adds JSDoc and overload signature for options?: { windows?: boolean } on fileURLToPath.
Native decoding and validation rewrite src/jsc/bindings/BunObject.cpp	Adds decodeFileURLPath for percent-escape validation and UTF-8 decoding, and rewrites functionFileURLToPath to compute windows/platform defaults, validate host and encoded separators, and enforce drive-letter or UNC path shapes.
Test coverage for percent-decoding and windows mode test/js/bun/util/fileUrl.test.js, test/js/node/url/url-fileurltopath.test.js	Updates one Bun expectation and adds coverage for malformed percent-encoding, decoded %25, and windows: true/false plus defaulting behavior.
SQLite file URL fallback src/js/internal/sql/shared.ts	Updates parseDefinitelySqliteUrl to slice from the matched protocol prefix and adjust Windows paths that begin with /\.
Node URL cleanup src/js/node/url.ts	Removes a commented-out fileURLToPath patch snippet and deletes the PERCENT enum entry.

domainToUnicode helper extraction

Layer / File(s)	Summary
Domain conversion helper src/jsc/bindings/NodeURL.h, src/jsc/bindings/NodeURL.cpp	Adds domainToUnicode, moves UIDNA conversion into the helper, and updates jsDomainToUnicode to return an empty string on failure or the converted Unicode string on success.

Related Issues: #29174
Suggested Labels: bug, node.js compatibility, url
Suggested Reviewers: Jarred-Sumner, paperclover

Sequence Diagram(s)

sequenceDiagram
  participant JS as JavaScript caller
  participant functionFileURLToPath
  participant decodeFileURLPath

  JS->>functionFileURLToPath: fileURLToPath(url, options)
  functionFileURLToPath->>functionFileURLToPath: determine windows/platformName from options or build target
  alt windows mode
    functionFileURLToPath->>functionFileURLToPath: reject encoded / or \
    functionFileURLToPath->>functionFileURLToPath: replace / with \
  else posix mode
    functionFileURLToPath->>functionFileURLToPath: validate host against localhost
    functionFileURLToPath->>functionFileURLToPath: reject encoded /
  end
  functionFileURLToPath->>decodeFileURLPath: decode path
  decodeFileURLPath-->>functionFileURLToPath: decoded string or throw URI malformed
  functionFileURLToPath->>functionFileURLToPath: enforce drive-letter/UNC shape or return path
  functionFileURLToPath-->>JS: return decoded path or throw

Loading

Poem

Slashes decode, percent signs fall,
Windows or POSIX answers the call.
Hosts and paths are checked with care,
URIError waits there.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes implement the issue's Node-compatible malformed percent-decoding and UTF-8 rejection in fileURLToPath.
Out of Scope Changes check	✅ Passed	The extra NodeURL and sqlite fallback edits are tied to the new fileURLToPath semantics and documented fallout, not unrelated scope.
Title check	✅ Passed	The title clearly summarizes the main change: stricter fileURLToPath decoding and runtime platform path rules.
Description check	✅ Passed	The description is detailed and includes the change rationale plus verification, even though it doesn't use the repo's exact section headings.

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Thanks for the punycode fix — the domainToUnicode refactor and new UNC test cover exactly what I flagged. I'm deferring on approval since this is a full C++ rewrite of fileURLToPath's decoding/validation across all platforms and worth a human pass.

Extended reasoning...

Overview

Rewrites functionFileURLToPath in BunObject.cpp to implement Node's getPathFromURLWin32/getPathFromURLPosix directly instead of delegating to WTF::URL::fileSystemPath(). Adds a hand-rolled percent-decoder (decodeFileURLPath) with ECMA-262 Decode semantics, makes options.windows select platform rules at runtime, and factors domainToUnicode out of NodeURL.cpp into a reusable helper (used for UNC server names — the fix for my earlier comment). Also touches type declarations, deletes dead commented code from src/js/node/url.ts, and adds ~100 lines of tests.

Prior feedback

My previous review flagged that the Windows UNC branch used the punycode-encoded host directly. Commit 8439e38 addresses this exactly as suggested: Bun::domainToUnicode() is now a standalone helper, the UNC branch calls it with a fallback to the raw host on failure, and there's a dedicated decodes punycode UNC server names test block covering both Unicode and pre-encoded xn-- inputs plus an IPv4 host. That concern is resolved.

Security risks

fileURLToPath converts untrusted URL input into filesystem paths, so the encoded-separator checks and drive-letter validation are security-adjacent. The change actually tightens behavior (throwing where the old code silently returned "" or passed malformed escapes through), and the %2f/%5c rejection matches Node's guarantee that the result cannot contain smuggled separators. I don't see a new attack surface, but the hand-written UTF-8 validation via String::fromUTF8 and the ASSERT(input.containsOnlyASCII()) (debug-only) after /→\ replacement are the kind of details a maintainer should sanity-check.

Level of scrutiny

Medium-high. This is ~120 lines of new C++ in JSC bindings replacing a WebKit builtin, changing user-visible error messages and error classes on every platform, and the options.get() path can invoke user getters. Node/Web compat changes in this repo are explicitly called out as needing careful review. It's well-tested (including a claimed 21k-case differential against Node), but it's not a mechanical change.

Other factors

Test coverage is thorough — malformed escapes, overlong/surrogate/out-of-range UTF-8, both platform modes, encoded-separator rejection, drive-letter validation, punycode UNC hosts, and options-defaulting including primitive/null second args. The jsDomainToUnicode refactor preserves the pre-existing forbidden-host-code-point filter and 8-bit early-return, so node:url's domainToUnicode export should behave identically.

[robobun]

Thanks — and Windows CI just earned its keep on exactly one of the two things you flagged, so here are answers to both plus what it caught.

String::fromUTF8 strictness. Not hand-written: it forwards to StringImpl::create(std::span<const char8_t>), which runs simdutf::convert_utf8_to_utf16 and returns a null string on any invalid sequence. Checked empirically against the adversarial cases rather than trusting the name — overlong (%C0%80), encoded surrogate (%ED%A0%80), beyond U+10FFFF (%F5%80%80%80), lone continuation (%80), truncated (%c3) all reject, and all of them are in the test file. WebKit's own decodeEscapeSequencesFromParsedURL relies on the same null-on-invalid contract, with a FIXME noting it.

The ASSERT(containsOnlyASCII()). It restates the invariant WebKit asserts on this exact value (url.path()) in decodeEscapeSequencesFromParsedURL and fileSystemPathWindows: the WHATWG parser percent-encodes every non-ASCII byte in the path, so escapes are the only source of non-ASCII. Worst case if it were ever violated is a truncated code unit producing invalid UTF-8, which then throws URIError — wrong message, not memory unsafety. The /→\ replacement preserves ASCII-ness, so it holds on both branches. Verified under the assert-enabled debug build that non-ASCII paths round-trip (fileURLToPath("file:///café/naïve/🚀")).

What CI caught. test/js/sql/sqlite-url-parsing.test.ts failed on all three Windows lanes, and it was real:

Windows: file:/// with UNC path
Expected: "\\server\share\test.db"
Received: "/\\server\share\test.db"

file:///\\server\share\test.db parses to pathname ///server/share/test.db with no host, so the drive-letter check rejects it. Node throws the identical ERR_INVALID_FILE_URL_PATH here, so the new behavior is right — but the throw newly activated a fallback in parseDefinitelySqliteUrl that had never run on any platform, and it sliced file:// (7) while the test sliced file:/// (8), leaving a stray leading slash. Fixed in 53beb1f by slicing the matched prefix and dropping that slash on Windows, which is what fileURLToPath does when it accepts the URL, and what this returned before the change. Details in the PR body.

[claude]

Thanks for the punycode fix — that thread is resolved and the differential now covers IDN hosts. No further issues found, but since this is a full native rewrite of fileURLToPath with cross-platform path-shape changes and a knock-on SQL fallback fix, it's worth a human pass before merge.

Extended reasoning...

Overview

This PR rewrites functionFileURLToPath in src/jsc/bindings/BunObject.cpp, replacing the WTF::URL::fileSystemPath() call with a hand-rolled implementation of Node's getPathFromURLWin32/getPathFromURLPosix. It adds a new decodeFileURLPath percent-decoder with ECMA-262 Decode semantics, factors Bun::domainToUnicode out of NodeURL.cpp for shared use, wires up the previously-ignored options.windows flag, updates the bun.d.ts signature, deletes dead shim code from src/js/node/url.ts, and patches a newly-exercised fallback in src/js/internal/sql/shared.ts. Test coverage is thorough (~100 new lines across two files) and backed by a 30k-case differential against Node v26.

Prior feedback

My earlier inline comment about the Windows UNC branch not running the hostname through domainToUnicode was addressed in 8439e38: the ICU call is now shared via Bun::domainToUnicode, with a fallback to the raw host when decoding fails (a deliberate, documented deviation for hosts Bun's parser accepts but Node's rejects). A dedicated test and an expanded IDN-host fuzz grammar cover it. The thread is resolved.

Security risks

fileURLToPath sits on the boundary between URLs and filesystem paths, so changes here are security-adjacent (path traversal, encoded-separator smuggling). The direction of the change is strictly tightening: malformed escapes now throw instead of silently returning "" or passing bytes through, and encoded / / \ are rejected per platform exactly as Node does. The ASSERT(input.containsOnlyASCII()) in decodeFileURLPath holds because the WHATWG parser percent-encodes the pathname; if it didn't, the static_cast<Latin1Character> would truncate, but that path is guarded by the contains('%') fast-path returning early for unescaped input anyway. I don't see a new attack surface, but path-shape logic (drive-letter check, UNC prefixing, leading-slash stripping in the SQL fallback) is exactly the kind of code where a second pair of eyes is cheap insurance.

Level of scrutiny

Medium-high. This is ~120 lines of new native C++ in a function that backs import.meta, module resolution, and node:url, with per-platform branches and a behavioral change on Windows (some inputs that previously returned a path now throw ERR_INVALID_FILE_URL_PATH, matching Node). The domainToUnicode refactor is behavior-preserving for node:url callers but moves a 2KB stack buffer into a helper now called from a second site. The SQL fallback change is small but was never previously exercised.

Other factors

The PR is exceptionally well-documented and well-tested; the author responded to review feedback promptly and re-ran the differential. The bug-hunting system found nothing on the current revision. I'm deferring rather than approving solely because a native rewrite of cross-platform path handling with downstream fallout is beyond the "simple/mechanical" bar for bot approval, not because of any specific concern.

[robobun]

Agreed on deferring to a human, and your instinct about the SQL fallback being "never previously exercised" was the right thing to be nervous about: my first attempt at it was wrong and Windows CI caught it.

53beb1ff87 dropped the leading slash for any Windows fallback path. But posix-shaped URLs reach that fallback on Windows too (no drive letter, so fileURLToPath rejects them), and they need to keep the slash they start with:

file:///tmp/test.db        -> want "/tmp/test.db",  got "tmp/test.db"
file:///home/user/test.db  -> want "/home/user/test.db"
file:///var/lib/test.db    -> want "/var/lib/test.db"

The only path shape whose fileURLToPath result actually moved from a value to a throw is the UNC one, so 6a02e6037e narrows the strip to a leading /\.

Rather than guess a third time, I simulated every file:// case in sqlite-url-parsing.test.ts under Windows semantics on Linux. That is possible now precisely because windows became a runtime flag in this PR: fileURLToPath(url, { windows: true }) exercises the real win32 branch, and the fallback is pure string arithmetic on top of it.

fallback rule	failures / 23 cases
original (slice(7))	1 (the UNC case)
53beb1ff87 (drop any leading /)	3 (the posix cases above)
6a02e6037e (drop / only before \)	0

The narrowed rule also reproduces bun's pre-change Windows output exactly: fileSystemPathWindows skipped the leading slash and isAbsolutePath accepted \\server\share\test.db, so nothing regresses. Confirmed the bundler folds the branch away on posix (if (false) ;), leaving str.slice(prefix.length) byte-identical to the original.

On your two other notes: the decodeFileURLPath fast path does return early for unescaped input, as you spotted, so the static_cast is only ever reached for a pathname containing % — which the WHATWG parser guarantees is ASCII. And domainToUnicode's 2 KB buffer is char16_t hostnameBuffer[2048] on the stack of the helper, same as before the extraction; it's just now reached from two call sites instead of one.

[robobun]

CI status: the diff is green, the two red lanes are unrelated

Build #68589 is the first build to run this branch to completion: 284 passed, 2 failed. Both failures are independent of this change, and I am not pushing a ci: retrigger for them because neither is the kind of thing a re-roll fixes.

1. darwin 26 aarch64 - test-bun (shard 1/2) — infrastructure, zero tests ran

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.
Refusing to continue with a partial download (would silently fall back to the wrong binary).

The upstream darwin aarch64 - build-bun step passed with exit 0 and published its artifacts; the test shard simply could not download the binary inside 120s. The same timeout hit this fleet on build #68559 too (that time it was shard 0/2, this time shard 1/2), so it is a recurring agent/S3 condition rather than a one-off.

2. ubuntu 25.04 x64 - test-bun — test/js/bun/util/v8-heap-snapshot.test.ts, SIGKILL

Pre-existing, verified rather than assumed. I checked out origin/main into src/ and packages/, rebuilt, and ran it: 3 pass / 3 fail, identical to this branch. In the same CI build the test passed on 199 of 200 linux test shards and was OOM-killed on exactly one, which is what a memory-hungry heap-snapshot test does on a loaded box. This PR does not touch functionGenerateHeapSnapshot.

What did pass

All three Windows test lanes are green (2019 x64, 2019 x64-baseline, 11 aarch64). That is the platform that caught both of the sqlite-fallback bugs earlier in this PR, so the green is load-bearing: test/js/sql/sqlite-url-parsing.test.ts no longer appears in any failure, and the fileURLToPath windows branch is confirmed against a real Windows build rather than only the { windows: true } simulation I used while developing.

A maintainer can retry those two jobs in a couple of seconds, which is a better use of the fleet than rebuilding ~290 jobs on a roughly one-in-three chance of dodging the same darwin timeout.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-05, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. The linked issue (#29174) stays open. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.