Improve redirect error message for trailing slash (#812)

* Reformat existing message

* Make existing test more specific

* Add failing test

* Add error message for trailing slash

* Reword existing message

* Add changelog entry

changelog/812.bugfix.rst

@@ -0,0 +1,2 @@
+Add a helpful error message when an upload fails due to missing a trailing
+slash in the URL.

tests/test_register.py

@@ -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
         """
@@ -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,
@@ -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)
 
 

tests/test_upload.py

@@ -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
         """
@@ -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(
@@ -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

twine/exceptions.py

@@ -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):