fix: normalize_url preserves path/query casing, lowercasing only scheme and host - #2241
Open
enriquealonso01 wants to merge 1 commit into
Open
enriquealonso01 wants to merge 1 commit into
enriquealonso01 wants to merge 1 commit into
Conversation
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.
Summary
normalize_urllowercased the entire URL viastr(yarl_new_url).lower(). Since only the scheme and host are case-insensitive (RFC 3986 §6.2.2.1), this silently merged distinct URLs during request deduplication — e.g.https://example.com/Product/ABCandhttps://example.com/product/abccollapse into one request key, and case-significant query tokens (signed URLs, API tokens) were mangled.Changes
normalize_urlnow returnsstr(yarl_new_url)unchanged: yarl already lowercases scheme and host during parsing, while path/query/fragment keep their original casingCloses #2008
Test plan
HTTPS://EXAMPLE.COM/?KEY=VALUEcase: scheme/host lowercase, query casing preserved/Product/ABC), query token casing preserved (?token=SeCrEt)pytest tests/unit/_utils/test_requests.py— 17 passedBreaking change note
As flagged in #2008 (comment) (this is planned for v2 as breaking): anyone who relied on full-lowercase unique keys will see different keys for case-distinct URLs. Targeting
masterper the current default branch — happy to rebase onto av2branch if one exists.Submitted as part of Hacktoberfest-adjacent open-source contributions by @enriquealonso01.