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

Improve redirect error message for trailing slash #812

Merged
merged 6 commits into from
Sep 17, 2021
Merged
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: 2 additions & 0 deletions changelog/812.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add a helpful error message when an upload fails due to missing a trailing
slash in the URL.
15 changes: 6 additions & 9 deletions tests/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def register_settings(make_settings):
return make_settings(
"""
[pypi]
repository: https://test.pypi.org/legacy
repository: https://test.pypi.org/legacy/
username:foo
password:bar
"""
Expand Down Expand Up @@ -46,7 +46,7 @@ def test_successful_register(register_settings):
def test_exception_for_redirect(register_settings):
"""Raise an exception when repository URL results in a redirect."""
repository_url = register_settings.repository_config["repository"]
redirect_url = "https://malicious.website.org/danger"
redirect_url = "https://malicious.website.org/danger/"

stub_response = pretend.stub(
is_redirect=True,
Expand All @@ -60,13 +60,10 @@ def test_exception_for_redirect(register_settings):

register_settings.create_repository = lambda: stub_repository

err_msg = (
f"{repository_url} attempted to redirect to {redirect_url}.\n"
f"If you trust these URLs, set {redirect_url} as your repository URL.\n"
f"Aborting."
)

with pytest.raises(exceptions.RedirectDetected, match=err_msg):
with pytest.raises(
exceptions.RedirectDetected,
match=rf"{repository_url}.+{redirect_url}.+\nIf you trust these URLs",
):
register.register(register_settings, helpers.WHEEL_FIXTURE)


Expand Down
38 changes: 31 additions & 7 deletions tests/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,39 @@ def test_deprecated_repo(make_settings):
)


def test_exception_for_redirect(make_settings):
@pytest.mark.parametrize(
"repository_url, redirect_url, message_match",
[
(
"https://test.pypi.org/legacy",
"https://test.pypi.org/legacy/",
(
r"https://test.pypi.org/legacy.+https://test.pypi.org/legacy/"
r".+\nYour repository URL is missing a trailing slash"
),
),
(
"https://test.pypi.org/legacy/",
"https://malicious.website.org/danger/",
(
r"https://test.pypi.org/legacy/.+https://malicious.website.org/danger/"
r".+\nIf you trust these URLs"
),
),
],
)
def test_exception_for_redirect(
repository_url,
redirect_url,
message_match,
make_settings,
):
# Not using fixtures because this setup is significantly different

upload_settings = make_settings(
"""
f"""
[pypi]
repository: https://test.pypi.org/legacy
repository: {repository_url}
username:foo
password:bar
"""
Expand All @@ -263,7 +289,7 @@ def test_exception_for_redirect(make_settings):
stub_response = pretend.stub(
is_redirect=True,
status_code=301,
headers={"location": "https://test.pypi.org/legacy/"},
headers={"location": redirect_url},
)

stub_repository = pretend.stub(
Expand All @@ -272,11 +298,9 @@ def test_exception_for_redirect(make_settings):

upload_settings.create_repository = lambda: stub_repository

with pytest.raises(exceptions.RedirectDetected) as err:
with pytest.raises(exceptions.RedirectDetected, match=message_match):
upload.upload(upload_settings, [helpers.WHEEL_FIXTURE])

assert "https://test.pypi.org/legacy/" in err.value.args[0]


def test_prints_skip_message_for_uploaded_package(
upload_settings, stub_repository, capsys
Expand Down
21 changes: 11 additions & 10 deletions twine/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,18 @@ class RedirectDetected(TwineException):

@classmethod
def from_args(cls, repository_url: str, redirect_url: str) -> "RedirectDetected":
msg = "\n".join(
[
"{} attempted to redirect to {}.".format(repository_url, redirect_url),
"If you trust these URLs, set {} as your repository URL.".format(
redirect_url
),
"Aborting.",
]
)
if redirect_url == f"{repository_url}/":
return cls(
f"{repository_url} attempted to redirect to {redirect_url}.\n"
f"Your repository URL is missing a trailing slash. "
"Please add it and try again.",
)

return cls(msg)
return cls(
f"{repository_url} attempted to redirect to {redirect_url}.\n"
f"If you trust these URLs, set {redirect_url} as your repository URL "
"and try again.",
)


class PackageNotFound(TwineException):
Expand Down