Skip to content

fix: implement rfc3986 dot-segment removal and percent-encoding normalization - #152

Open
gaoflow wants to merge 1 commit into
sabre-io:masterfrom
gaoflow:fix-rfc3986-dot-segments-and-pct-normalization
Open

fix: implement rfc3986 dot-segment removal and percent-encoding normalization#152
gaoflow wants to merge 1 commit into
sabre-io:masterfrom
gaoflow:fix-rfc3986-dot-segments-and-pct-normalization

Conversation

@gaoflow

@gaoflow gaoflow commented Jul 26, 2026

Copy link
Copy Markdown

resolve() and normalize() each carried their own hand-written ./.. loop, and neither is the remove_dot_segments routine from rfc3986 §5.2.4. Both now call one helper transcribed from it.

A trailing . or .. segment dropped the required slash — §5.2.4 steps 2B/2C replace a final /. or /.. with /. resolve('http://a/b/c/d;p?q', '..') returned http://a/b, want http://a/b/; normalize('http://e.org/a/b/..') returned http://e.org/a, want http://e.org/a/.

A reference with its own authority inherited the base path and port — §5.2.2 does not consult the base when R.authority is defined. resolve('http://a/b/c/d;p?q', '//g') returned http://g/b/c/d;p, but the §5.4.1 table in the RFC gives "//g" = "http://g". resolve('https://a:8080/x/y', '//g/h') returned https://g:8080/h, want https://g/h.

normalize() percent-encoded valid path charactersrawurlencode(rawurldecode($segment)) escapes the 11 sub-delims plus : and @, all of which are valid pchar (§3.3), so http://a/b/c/d;p?q became http://a/b/c/d%3Bp?q and /a;b compared equal to /a%3Bb, which §2.2 says are not equivalent. The ltrim() also collapsed //a to /a and the unconditional '/'.implode() rooted rootless paths (urn:isbn:0451450523urn:/isbn%3A0451450523). Triplets are now upper-cased and decoded only when the octet is unreserved (§6.2.2.1, §6.2.2.2); bytes that are not pchar are still encoded, as before.

The §5.4.1 and §5.4.2 examples are now a data provider — 41 of the 42, with the empty reference left out because #145 covers it. Plus normalization vectors for the 13 characters, the trailing dot segments, %2E, empty segments and rootless paths.

Four ResolveTest rows change, all of them //example.net with no path. They arrived in 0d4c9a3 next to a ?:?? falsiness fix for a base path of '0', and they pin the base-path inheritance above; the four sibling rows using //example.net/ are untouched, and I added two rows so a '0' base path stays covered. Cross-checked against urllib.parse.urljoin, Go net/url and Addressable — all three drop the base path for //g. phpunit, phpstan and php-cs-fixer are green locally.

…lization

resolve() and normalize() each carried their own hand-written "." / ".."
loop, and neither was the remove_dot_segments routine from rfc3986 section
5.2.4. Both now call one helper transcribed from it.

- A trailing "." or ".." segment lost the "/" that steps 2B and 2C leave
  behind, so resolve('http://a/b/c/d;p?q', '..') returned 'http://a/b'.
- A reference with its own authority inherited the base path and port.
  Section 5.2.2 does not consult the base when R.authority is defined, and
  the section 5.4.1 table gives "//g" = "http://g".
- normalize() ran rawurlencode(rawurldecode($segment)), which escapes the
  sub-delims plus ":" and "@" even though all of them are valid pchar
  (section 3.3). Section 2.2 protects reserved characters from
  normalization, so this reported /a;b and /a%3Bb as equal. Its ltrim() and
  unconditional '/'.implode() also collapsed empty segments and rooted
  rootless paths.

The rfc3986 section 5.4.1 and 5.4.2 examples are now a data provider.
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.

1 participant