Skip to content

don't lower case-sensitive user info embedded in URLs - #466

Open
ogalland-tob wants to merge 2 commits into
psf:masterfrom
trail-of-forks:ptp/fix-5
Open

don't lower case-sensitive user info embedded in URLs#466
ogalland-tob wants to merge 2 commits into
psf:masterfrom
trail-of-forks:ptp/fix-5

Conversation

@ogalland-tob

Copy link
Copy Markdown
Contributor

CacheControl lowercases URL user information when constructing cache keys. Requests preserves its case and generates different Basic Authorization headers, allowing a response for one credential spelling to satisfy another.

This PR prevents the lowering of userinfo in URL and introduces a unit test for this case.


scheme = scheme.lower()
authority = authority.lower()
userinfo, separator, host = authority.rpartition("@")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little worried about this being conformant -- does RFC 3986 or similar guarantee that partitioning on the last @ is correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I was assuming there could only be one @ in the authority, but I haven't checked against the RFC. Will take a look now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per RFC 3986, authority = [ userinfo "@" ] host [ ":" port ]. afaiu @ is excluded from all of these symbols. The @ character is only included in gen-delims, which does not show up in any of userinfo, host, port.

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.

2 participants