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.

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

Redact URL to hide password #6295

Merged
merged 10 commits into from
Mar 1, 2019
Merged

Redact URL to hide password #6295

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
00b872f
Redact URL to hide password
expobrain Feb 23, 2019
34b123d
Use already existent utility function ot hide password
expobrain Feb 24, 2019
5c232bc
Updated unit test
expobrain Feb 24, 2019
ddb7320
Pass index url straight to the command
expobrain Feb 24, 2019
70dee95
Moved test into unit
expobrain Feb 25, 2019
c33535b
Use caplog instead of mock
expobrain Feb 27, 2019
1214941
Added NEWS file
expobrain Feb 27, 2019
5bfb47c
Improved unit test
expobrain Feb 28, 2019
137a7ab
Better unit test comment
expobrain Mar 1, 2019
b37ad3f
Better NEWS files content
expobrain Mar 1, 2019
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 src/pip/_internal/index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def _get_html_response(url, session):
if _is_url_like_archive(url):
_ensure_html_response(url, session=session)

logger.debug('Getting page %s', url)
logger.debug('Getting page %s', redact_password_from_url(url))

resp = session.get(
url,
Expand Down
28 changes: 28 additions & 0 deletions tests/unit/test_index_html_page.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,34 @@ def test_get_html_response_no_head(url):
]


def test_get_html_response_dont_log_clear_text_password(caplog):
"""
`_get_html_response()` shouldn't send a HEAD request if the URL does not
look like an archive, only the GET request that retrieves data.
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this docstring was copied without updating.

"""
password = "my_password"
url_template = "https://user:{}@example.com/simple/"
Copy link
Member

Choose a reason for hiding this comment

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

You won't need these two variables (see below).

session = mock.Mock(PipSession)

# Mock the headers dict to ensure it is accessed.
session.get.return_value = mock.Mock(headers=mock.Mock(**{
"get.return_value": "text/html",
}))

caplog.set_level(logging.DEBUG)

resp = _get_html_response(url_template.format(password), session=session)
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using url_template, it would be simpler and more direct just to pass https://user:my_password@example.com/simple/.


assert resp is not None

assert len(caplog.records) == 1
record = caplog.records[0]
assert record.levelname == 'DEBUG'
assert record.message.splitlines() == [
"Getting page {}".format(url_template.format("****")),
Copy link
Member

Choose a reason for hiding this comment

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

Again, you can just check that actual string instead of using nested format strings, so "Getting page https://user:****@example.com/simple/"... (It's generally better to be more explicit in test code.)

]


@pytest.mark.parametrize(
"url, vcs_scheme",
[
Expand Down