WTF: URL::path() keeps a first path segment that starts with a dot in host-less URLs (URL("foo:/.a/b").path() was "a/b")

[robobun]

Problem

• For a URL with no host, URL::path() drops the first two characters of a first path segment that starts with a dot:

  URL("foo:/.a/b").path()          "a/b"          (spec, Node, Deno: "/.a/b"; the serialization "foo:/.a/b" is right)
  URL("git:/.git/config").path()   "git/config"
  URL("foo:/..a").path()           ".a"
  

  In Bun this is new URL("foo:/.a/b").pathname, and every URLPattern with a custom scheme and a path starting with a dot (new URLPattern("myapp://host/.well-known/:f") compiles its pathname as well-known/:f and matches nothing), because the URLPattern pathname canonicalizer for non-special schemes parses the text through a host-less dummy URL and reads path() back. Same in upstream WebKit main and in every Bun release. Found by a differential run of URL and URLPattern against Node and Deno.

• Cause: URL::pathStart() (Source/WTF/wtf/URL.cpp) skips a /. that directly follows the scheme without checking what comes after it. That /. is meant to be the prefix URLParser::addNonSpecialDotSlash() inserts in front of a path starting with // in a host-less URL (foo:/.//b, so the path does not parse back as an authority), but the check also matches a real first segment such as .a or .git. pathStart() is also the offset used by lastPathComponent(), protocolHostAndPort(), hasPath(), setPath() and setHostAndPort(), so on these URLs lastPathComponent() of foo:/.a was empty, protocolHostAndPort() was foo:/., and setHostAndPort("h:1") spliced the string into the invalid foo://h:1a (in Bun, url.host = "h:1" on such a URL was silently ignored).

Fix

• pathStart() now also requires the character after /. to be / before skipping it. addNonSpecialDotSlash() only ever inserts the prefix in front of //, and copyURLPartsUntil() already removes it with this same three-character test, so the two sides now agree. The test is sufficient because the parser removes single-dot segments: after parsing, a path can never consist of a bare . segment, so /. followed by anything other than / is always the start of a real segment. URLs with a host (foo://h/.a, foo:///.a, every special scheme) never took this branch and are unchanged; foo:/.//b still reports //b.
• Tests: Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp (Idempotence) gets rows for dotted first segments with and without a host, with query and fragment, with %2F, /.//.a, /./.a, and relative resolution against and onto such URLs; Tests/WTF/URL.cpp gets PathStartingWithDotWithoutHost covering path(), lastPathComponent(), setPath() and setHostAndPort(), plus two protocolHostAndPort() rows.
• Verification: the fork's CI does not build TestWebKitAPI, so both test files were compiled against the autobuild-eeab0404 debug ASAN prebuilt (the commit this branch is based on, and Bun's current pin) together with the bundled gtest and linked once with the unmodified libWTF.a and once with this URL.cpp compiled in front of it. Before: 3 of the 29 WTF_URL.* / WTF_URLParser.* tests fail (URLProtocolHostAndPort 1 expectation, PathStartingWithDotWithoutHost 11, Idempotence 182, all on the new rows). After: all 29 pass, including the existing URL corpus in both files.

Background

• A WHATWG URL serializer writes a host-less, non-opaque URL whose path starts with an empty segment as scheme:/. followed by the path (foo:/.//b for the path //b); without the /., re-parsing foo://b would read b as a host. The path getter has to leave that prefix out, and only that prefix.
• WTF's URL stores the serialized string plus component offsets. The path has no stored start offset; pathStart() derives it from the end of the host and port, which is where the /. special case lives, and every accessor or setter that needs the start of the path calls it.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Limit details: You’ve used all 5 included reviews currently available under your plan.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4ae5840c-aa70-4986-8d11-aa2102241494

📥 Commits

Reviewing files that changed from the base of the PR and between eeab040 and 8da5349.

📒 Files selected for processing (3)

• Source/WTF/wtf/URL.cpp
• Tools/TestWebKitAPI/Tests/WTF/URL.cpp
• Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp

---

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

[claude]

LGTM — tightens the /. skip in URL::pathStart() to only match the serialization prefix, with thorough test coverage.

What was reviewed:

• Verified needsNonSpecialDotSlash() only fires when the path already starts with //, so the inserted prefix is always /./ followed by / — the new three-character check can't miss it.
• Confirmed copyURLPartsUntil() (URLParser.cpp:1293-1296) already uses the identical /-.-/ test to strip the prefix, so both sides now agree.
• Checked the bounds change (start + 2 < length): safe because the prefix always has at least four chars after scheme:; strings like foo:/. alone can't survive parsing (single-dot segments are removed).

Extended reasoning...

Overview

This PR fixes a bug in URL::pathStart() (Source/WTF/wtf/URL.cpp) where the /. serialization prefix inserted by URLParser::addNonSpecialDotSlash() was being over-matched. The old code skipped any /. right after the scheme in a host-less URL, which incorrectly ate the first two characters of real path segments starting with a dot (e.g. foo:/.well-known → path well-known). The fix adds one more character check (m_string[start + 2] == '/') so only the actual inserted prefix /.// is skipped. The other two files are test additions covering the fixed behavior and confirming the /.// case still works.

Security risks

None identified. The change strictly narrows an existing condition — it can only cause pathStart() to skip fewer characters than before, never more. The only theoretical risk would be failing to skip a legitimate prefix, but I verified against needsNonSpecialDotSlash() (URLParser.cpp:3391-3396) that the prefix is only ever inserted in front of a path that already starts with //, so [start+2] is always / when the prefix is present. This makes URL component extraction more spec-compliant, which is a net security positive for anything relying on path() / pathname.

Level of scrutiny

Moderate. URL parsing is spec-driven and used everywhere, but this is a two-line logic change plus a comment, in a well-understood accessor. The change is a strict narrowing of a boolean condition and mirrors an identical check that already exists in copyURLPartsUntil() (URLParser.cpp:1293-1296). The PR description is precise about the WHATWG spec background, the root cause, and why the new condition is sufficient (parser removes bare . segments so a real segment can never be exactly .).

Other factors

• The bounds check moved from start + 1 < length to start + 2 < length, correctly guarding the new [start + 2] access.
• Test coverage is extensive: 15 new checkURL/checkRelativeURL rows in Idempotence (each of which also runs the tab-insertion fuzz), a dedicated PathStartingWithDotWithoutHost test exercising path(), lastPathComponent(), setPath(), and setHostAndPort() on both dotted-segment and /.// URLs, plus two protocolHostAndPort() rows.
• The author verified the tests against a before/after ASAN build and reported the expected pass/fail delta.
• No prior human reviews or outstanding comments on the PR.

[github-actions]

Preview Builds

Commit	Release	Date
8da5349b	autobuild-preview-pr-460-8da5349b	2026-08-18 05:18:22 UTC
e932620f	autobuild-preview-pr-460-e932620f	2026-08-17 19:39:45 UTC

[robobun]

Bun side, pinned to the preview build of this PR with the URL and URLPattern tests: oven-sh/bun#39457. Once this merges, that PR gets re-pinned to the merged commit.

[claude]

LGTM — tightens the /. prefix check in pathStart() to require a following /, matching the parser's own insertion/removal logic.

What was reviewed:

• Confirmed needsNonSpecialDotSlash() only fires when the path starts with //, so the inserted prefix is always /.//… — the added m_string[start + 2] == '/' check is a precise discriminator.
• Confirmed copyURLPartsUntil() in URLParser.cpp already uses this exact three-character test to strip the prefix; the two sides now agree.
• Bounds check updated from start + 1 to start + 2 to cover the new index; single-dot segments are removed during parsing so foo:/. and foo:/./x cannot survive to reach this branch.
• Test rows cover .a, ..a, %2F, query/fragment, hosted/hostless, setPath/setHostAndPort, and relative resolution — the intended foo:/.//b → //b case is preserved.

Extended reasoning...

Overview

One-line logic change in Source/WTF/wtf/URL.cpp URL::pathStart(): the existing /.-skip now also requires the following character to be /. The bounds check is bumped from start + 1 < length to start + 2 < length accordingly, and a four-line comment explains the invariant. The other two files add ~90 lines of test rows in Tools/TestWebKitAPI/Tests/WTF/URL.cpp and URLParser.cpp.

Correctness

I cross-checked the invariant against URLParser.cpp:

• needsNonSpecialDotSlash() (lines 3391–3396) returns true only when m_string[pathStart] == '/' && m_string[pathStart + 1] == '/' in a non-special host-less URL, so addNonSpecialDotSlash() always produces the sequence /.//…. Any /. at path start in a parsed URL that is NOT followed by / is therefore a real first segment.
• The parser's own prefix-removal in copyURLPartsUntil() (lines 1292–1299) already uses the identical three-character test '/' && '.' && '/'; this fix brings the accessor into agreement with the parser.
• The parser normalises single-dot segments away (isSingleDotPathSegment/consumeSingleDotPathSegment), so a bare . segment can never survive parsing — /./x becomes /x and /. becomes /. This means the three-character check has no false positives (a real . segment followed by / cannot exist) and no false negatives (the inserted prefix is always followed by /).

The change strictly narrows the skip condition, so behaviour is unchanged for every URL where the old code was already correct (including the foo:/.//b → path //b case, which is retained in the tests).

Security risks

URL parsing is security-adjacent, but this change only affects the path accessor on already-parsed non-special host-less URLs — the serialization and offsets stored by the parser were already correct. The fix makes path() stop dropping the leading two characters of a real segment, aligning with the WHATWG spec and Node/Deno. It cannot cause a path to be interpreted as shorter than before; it only stops truncating. No auth, origin, or host logic is touched.

Level of scrutiny

Low-to-medium. The functional change is a single added boolean term in a condition plus a matching bounds check, backed by a clear parser invariant that I verified in the source. Test coverage is thorough (dotted first segments with/without host, query/fragment, %2F, nested /.//.a, relative resolution both directions, and the setter paths that also read pathStart()). The PR description documents before/after test runs against the ASAN prebuilt with all 29 URL tests passing.

Other factors

There is a linked Bun-side PR (oven-sh/bun#39457) with URL/URLPattern tests pinned to this preview build. No prior reviewer comments to address; CodeRabbit was rate-limited and did not review.