Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ogalland-tob wants to merge 2 commits into
base: master
Choose a base branch
from
+17 −1
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34eb888
don't lower case-sensitive user info embedded in URLs
ogalland-tob Aug 27, 2026
9f36172
streamline user info case test
ogalland-tob Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cachecontrol/controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ def _urlnorm(cls, uri: str) -> str:
raise Exception("Only absolute URIs are allowed. uri = %s" % uri)

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

@woodruffw woodruffw Aug 29, 2026

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?

@ogalland-tob ogalland-tob Aug 31, 2026

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.

@ogalland-tob ogalland-tob Aug 31, 2026

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.

authority = f"{userinfo}{separator}{host.lower()}"

if not path:
path = "/"
Expand Down
15 changes: 15 additions & 0 deletions tests/test_cache_control.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,21 @@ def test_no_cache_with_vary_star(self, cc):

assert not cc.cache.set.called

def test_cache_response_preserves_userinfo_case(self):
cache = DictCache({})
cc = CacheController(cache)
url = "http://Alice:Secret@localhost/"
req = DummyRequest(url=url, headers={})
resp = DummyResponse(status=200, headers={"ETag": "xyz"})
cc.cache_response(req, resp, b"")

req = DummyRequest(
url="http://alice:secret@localhost/", headers={"if-match": "xyz"}
)
result = cc.conditional_headers(req)

assert not result

def test_update_cached_response_no_local_cache(self):
"""
If the local cache doesn't have the given URL, just reuse the response
Expand Down