Fix fragment with path structure misparsed as a query (#172)#189
Open
gaoflow wants to merge 1 commit into
Open
Fix fragment with path structure misparsed as a query (#172)#189gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
A fragment with no '?' was treated as a query whenever it contained '=', so a fragment like '#/foo=123' was parsed as a query and its '/' was percent-encoded, round-tripping to '#%2Ffoo=123' instead of '#/foo=123'. Only treat such a fragment as a query when '=' precedes any '/' (or there is no '/'); a '/' before the first '=' signals path structure. Fragments like '#woofs=dogs' stay queries; '#/foo=123' and '#a/b=c' are now paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #172.
Summary
A URL fragment that contains path structure is silently misparsed as a query, corrupting its
/:The
/is percent-encoded to%2F— exactly the round-trip corruption reported in #172.Cause
In
Fragment.load, a fragment with no?was routed to the query parser whenever it contained an=:So
#/foo=123was treated as a query, making/fooa query key whose/is then percent-encoded on serialization. The branch had no test coverage for=-without-?inputs that also contain/.Fix
Only treat such a fragment as a query when
=precedes any/(or there is no/at all). A/before the first=signals path structure:#woofs=dogs→ query (unchanged)#key=val/ue→ query (unchanged — the/is in the value, after=)#/foo=123,#a/b=c,#/a=1&b=2→ path (the/is preserved)?-split branch (path + query) is untouched.Verification
TestFragment.test_load(the/-before-=path cases, plus a=-before-/query case to pin the unchanged behavior). They fail before the fix (the fragment parses as a query with an empty path) and pass after.test_hosts/test_netloc/test_odd_urls) are pre-existingipaddress/urllibversion drift, identical with and without this change (baseline is 69 passed / 4 failed; my new assertions account for the one-test delta).flake8clean.This pull request was prepared with the assistance of AI, under my direction and review.