Skip to content
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.

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

Remove user credentials in URLs when converting to a string #3513

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion httpx/_urlparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ class ParseResult(typing.NamedTuple):

@property
def authority(self) -> str:
# Do not include the userinfo here, to avoid leaking credentials
return "".join(
[
f"{self.userinfo}@" if self.userinfo else "",
f"[{self.host}]" if ":" in self.host else self.host,
f":{self.port}" if self.port is not None else "",
]
Expand Down
25 changes: 1 addition & 24 deletions httpx/_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,30 +375,7 @@ def __str__(self) -> str:
return str(self._uri_reference)

def __repr__(self) -> str:
scheme, userinfo, host, port, path, query, fragment = self._uri_reference

if ":" in userinfo:
# Mask any password component.
userinfo = f'{userinfo.split(":")[0]}:[secure]'

authority = "".join(
[
f"{userinfo}@" if userinfo else "",
f"[{host}]" if ":" in host else host,
f":{port}" if port is not None else "",
]
)
url = "".join(
[
f"{self.scheme}:" if scheme else "",
f"//{authority}" if authority else "",
path,
f"?{query}" if query is not None else "",
f"#{fragment}" if fragment is not None else "",
]
)

return f"{self.__class__.__name__}({url!r})"
return f"{self.__class__.__name__}({str(self._uri_reference)!r})"
Copy link
Author

Choose a reason for hiding this comment

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

This seems simpler, rather than re-implementing the logic already in _urlparse.py. The user credentials are sent in a header rather than the URL anyway.


@property
def raw(self) -> tuple[bytes, bytes, int, bytes]: # pragma: nocover
Expand Down
2 changes: 1 addition & 1 deletion tests/client/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ async def test_auth_disable_per_request() -> None:

def test_auth_hidden_url() -> None:
url = "http://example-username:[email protected]/"
expected = "URL('http://example-username:[secure]@example.org/')"
expected = "URL('http://example.org/')"
assert url == httpx.URL(url)
assert expected == repr(httpx.URL(url))

Expand Down
3 changes: 2 additions & 1 deletion tests/models/test_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ def test_response_json():


def test_raise_for_status():
request = httpx.Request("GET", "https://example.org")
# The credentials here should be removed from all exception strings below
request = httpx.Request("GET", "https://user:[email protected]")

# 2xx status codes are not an error.
response = httpx.Response(200, request=request)
Expand Down
7 changes: 5 additions & 2 deletions tests/models/test_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,10 @@ def test_url_copywith_authority_subcomponents():
}
url = httpx.URL("https://example.org")
new = url.copy_with(**copy_with_kwargs)
assert str(new) == "https://username:[email protected]:444"
assert str(new) == "https://example.net:444"
assert new.username == "username"
assert new.password == "password"
assert new.userinfo == b"username:password"


def test_url_copywith_netloc():
Expand All @@ -610,7 +613,7 @@ def test_url_copywith_userinfo_subcomponents():
}
url = httpx.URL("https://example.org")
new = url.copy_with(**copy_with_kwargs)
assert str(new) == "https://tom%40example.org:abc123%40%20%@example.org"
assert str(new) == "https://example.org"
assert new.username == "[email protected]"
assert new.password == "abc123@ %"
assert new.userinfo == b"tom%40example.org:abc123%40%20%"
Expand Down
20 changes: 20 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,26 @@ def test_logging_request(server, caplog):
]


def test_logging_request_with_auth(server, caplog):
caplog.set_level(logging.INFO)
with httpx.Client() as client:
credentials = {
"username": "user",
"password": "abc123%",
}
url = server.url.copy_with(**credentials)
response = client.get(url)
assert response.status_code == 200

assert caplog.record_tuples == [
(
"httpx",
logging.INFO,
'HTTP Request: GET http://127.0.0.1:8000/ "HTTP/1.1 200 OK"',
)
]


def test_logging_redirect_chain(server, caplog):
caplog.set_level(logging.INFO)
with httpx.Client(follow_redirects=True) as client:
Expand Down