Skip to content

Collapse all leading slashes for special relative URLs#1136

Open
MerlijnW70 wants to merge 1 commit into
servo:mainfrom
MerlijnW70:fix/scheme-relative-multiple-slashes
Open

Collapse all leading slashes for special relative URLs#1136
MerlijnW70 wants to merge 1 commit into
servo:mainfrom
MerlijnW70:fix/scheme-relative-multiple-slashes

Conversation

@MerlijnW70

Copy link
Copy Markdown

Summary

Special-scheme URLs with multiple leading slashes (e.g. ///test) were parsed
incorrectly and failed with EmptyHost. Resolving new URL("///test", "http://example.org/")
returned an error instead of http://test/, disagreeing with Chrome, Firefox,
Node and Bun.

Fix

When resolving a relative reference against a special base URL (http, https,
ws, wss, ftp), the WHATWG URL standard's special authority ignore slashes
state
skips all leading slashes and backslashes before the authority.
parse_relative previously stripped only the initial // and parsed the
remainder as the authority, so ///test produced an authority of /test whose
leading slash yielded an empty host.

The fix branches on scheme_type.is_special(): a special URL now consistently
consumes every leading slash/backslash before the authority, while a non-special
URL keeps treating only the first // as the authority delimiter and the rest as
the path.

Test

No new fixtures were needed — the upstream WPT data in url/tests/urltestdata.json
already encodes the spec-mandated results. These seven ///-prefixed cases (base
http://example.org/) were removed from url/tests/expected_failures.txt and now
pass:

input resolved href
///test http://test/
///\//\//test http://test/
///example.org/path http://example.org/path
///example.org/../path http://example.org/path
///example.org/../../ http://example.org/
///example.org/../path/../../ http://example.org/
///example.org/../path/../../path http://example.org/path

The full url_wpt conformance suite passes with no other regressions (the harness
also flags "unexpected success", confirming these cases genuinely flipped from
failing to passing).

Reference

This is also the upstream cause of downstream issues such as
denoland/deno#35172, since Deno
parses URLs through this crate.

When resolving a relative reference against a special base URL (http,
https, ws, wss, ftp), the WHATWG URL standard's "special authority
ignore slashes state" skips *all* leading slashes and backslashes before
the authority. `parse_relative` only stripped the initial `//` and then
parsed the remainder as the authority, so an input such as `///test`
resolved to an authority of `/test` whose leading slash produced an empty
host and an `EmptyHost` error.

Browsers, Node and Bun all resolve `new URL("///test", "http://example.org/")`
to `http://test/`. Branch on `scheme_type.is_special()`: special URLs now
consume every leading slash/backslash (`remaining`) before the authority,
while non-special URLs keep treating only the first `//` as the authority
delimiter and the rest as path.

Drops seven `///`-prefixed http cases from the WPT expected-failures list;
they now parse to the spec-mandated result.
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@828b28c). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1136   +/-   ##
=======================================
  Coverage        ?   86.74%           
=======================================
  Files           ?       26           
  Lines           ?     5267           
  Branches        ?        0           
=======================================
  Hits            ?     4569           
  Misses          ?      698           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Manishearth

Copy link
Copy Markdown
Member

Hmm, can you help me understand how this matches the spec?

image

It seems like this only ignores a single additional slash since it consumes a slash, then goes into "ignore slashes" state, and then will validation error on the next slash.

@mrobinson

Copy link
Copy Markdown
Member

It seems that this issue in Deno is attracting many duplicate LLM-generated pull requests.

@Manishearth

Copy link
Copy Markdown
Member

If this is LLM generated we cannot accept it.

https://book.servo.org/contributing/getting-started.html#ai-contributions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants