Skip to content

fix: normalized Windows drive letter must be exactly two code points#1156

Merged
anonrig merged 1 commit into
ada-url:mainfrom
spokodev:fix/normalized-drive-letter-exact-length
Jul 3, 2026
Merged

fix: normalized Windows drive letter must be exactly two code points#1156
anonrig merged 1 commit into
ada-url:mainfrom
spokodev:fix/normalized-drive-letter-exact-length

Conversation

@spokodev

Copy link
Copy Markdown
Contributor

is_normalized_windows_drive_letter accepted any input of length >= 2 beginning with <ASCII alpha>:. Per the URL Standard a normalized Windows drive letter is a Windows drive letter, which is two code points, so the check should require exactly two.

The loose length let shorten a url's path treat a longer first segment that merely starts with <alpha>: as a drive letter and skip popping it on ..:

ada::parse("file:c:x/..")    -> "file:///c:x/"    (expected "file:///")
ada::parse("file:u:p@h/..")  -> "file:///u:p@h/"  (expected "file:///")

A real drive letter is unaffected: file:c:/.. still yields file:///c:/.

The fix changes >= 2 to == 2. All three call sites (shorten_path and the base-path drive-letter check in parser.cpp) pass a single path segment that should match only an exact drive letter, so the tighter check is correct for each.

Verified against the full WHATWG URL web-platform-tests urltestdata (no regressions) and added a regression test.

@lemire

lemire commented Jun 23, 2026

Copy link
Copy Markdown
Member

running tests

`is_normalized_windows_drive_letter` accepted any input of length >= 2 that
started with `<ASCII alpha>:`. Per the URL standard a normalized Windows drive
letter is a Windows drive letter, i.e. exactly two code points. The loose check
made `shorten_path` (file scheme) treat longer first segments such as `c:x` or
`u:p@h` as drive letters and refuse to pop them on `..`:

  parse("file:c:x/..")   -> "file:///c:x/"   (expected "file:///")
  parse("file:u:p@h/..") -> "file:///u:p@h/" (expected "file:///")

A genuine drive letter (`file:c:/..` -> "file:///c:/") is unaffected. Verified
against the full WHATWG url web-platform-tests (urltestdata, 0 regressions).
@anonrig anonrig force-pushed the fix/normalized-drive-letter-exact-length branch from a096666 to 1e7308d Compare July 1, 2026 21:08
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.78%. Comparing base (1245a1e) to head (1e7308d).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
include/ada/checkers-inl.h 0.00% 0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1156      +/-   ##
==========================================
- Coverage   59.80%   59.78%   -0.02%     
==========================================
  Files          37       37              
  Lines        6127     6127              
  Branches     2978     2978              
==========================================
- Hits         3664     3663       -1     
  Misses        618      618              
- Partials     1845     1846       +1     

☔ 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.

@anonrig anonrig merged commit 3cafc48 into ada-url:main Jul 3, 2026
53 of 54 checks passed
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