Don't wrap exception views (#16759)

* Move view deriver lower

* Add some failing tests

* Don't wrap exception views

* Fix unit tests

Author: di

tests/functional/forklift/test_legacy.py

@@ -132,3 +132,141 @@ def test_file_upload(webtest, upload_url, additional_data):
     assert len(project.releases) == 1
     release = project.releases[0]
     assert release.version == "3.0.0"
+
+
+def test_duplicate_file_upload_error(webtest):
+    user = UserFactory.create(
+        with_verified_primary_email=True,
+        password=(  # 'password'
+            "$argon2id$v=19$m=1024,t=6,p=6$EiLE2Nsbo9S6N+acs/beGw$ccyZDCZstr1/+Y/1s3BVZ"
+            "HOJaqfBroT0JCieHug281c"
+        ),
+    )
+
+    # Construct the macaroon
+    dm = MacaroonFactory.create(
+        user_id=user.id,
+        caveats=[caveats.RequestUser(user_id=str(user.id))],
+    )
+
+    m = pymacaroons.Macaroon(
+        location="localhost",
+        identifier=str(dm.id),
+        key=dm.key,
+        version=pymacaroons.MACAROON_V2,
+    )
+    for caveat in dm.caveats:
+        m.add_first_party_caveat(caveats.serialize(caveat))
+    serialized_macaroon = f"pypi-{m.serialize()}"
+
+    credentials = base64.b64encode(f"__token__:{serialized_macaroon}".encode()).decode(
+        "utf-8"
+    )
+
+    with open("./tests/functional/_fixtures/sampleproject-3.0.0.tar.gz", "rb") as f:
+        content = f.read()
+
+    params = MultiDict(
+        {
+            ":action": "file_upload",
+            "protocol_version": "1",
+            "name": "sampleproject",
+            "sha256_digest": (
+                "117ed88e5db073bb92969a7545745fd977ee85b7019706dd256a64058f70963d"
+            ),
+            "filetype": "sdist",
+            "metadata_version": "2.1",
+            "version": "3.0.0",
+        }
+    )
+
+    webtest.post(
+        "/legacy/",
+        headers={"Authorization": f"Basic {credentials}"},
+        params=params,
+        upload_files=[("content", "sampleproject-3.0.0.tar.gz", content)],
+        status=HTTPStatus.OK,
+    )
+
+    assert user.projects
+    assert len(user.projects) == 1
+    project = user.projects[0]
+    assert project.name == "sampleproject"
+    assert project.releases
+    assert len(project.releases) == 1
+    release = project.releases[0]
+    assert release.version == "3.0.0"
+
+    # Add some duplicate keys to ensure that this doesn't result in a error due
+    # to the duplicate key detector
+    params.add("project-url", "https://example.com/foo")
+    params.add("project-url", "https://example.com/bar")
+    params.add("classifiers", "Programming Language :: Python :: 3.10")
+    params.add("classifiers", "Programming Language :: Python :: 3.11")
+
+    resp = webtest.post(
+        "/legacy/",
+        headers={"Authorization": f"Basic {credentials}"},
+        params=params,
+        upload_files=[("content", "sampleproject-3.0.1.tar.gz", content)],
+        status=HTTPStatus.BAD_REQUEST,
+    )
+    assert "File already exists" in resp.body.decode()
+
+
+def test_invalid_classifier_upload_error(webtest):
+    user = UserFactory.create(
+        with_verified_primary_email=True,
+        password=(  # 'password'
+            "$argon2id$v=19$m=1024,t=6,p=6$EiLE2Nsbo9S6N+acs/beGw$ccyZDCZstr1/+Y/1s3BVZ"
+            "HOJaqfBroT0JCieHug281c"
+        ),
+    )
+
+    # Construct the macaroon
+    dm = MacaroonFactory.create(
+        user_id=user.id,
+        caveats=[caveats.RequestUser(user_id=str(user.id))],
+    )
+
+    m = pymacaroons.Macaroon(
+        location="localhost",
+        identifier=str(dm.id),
+        key=dm.key,
+        version=pymacaroons.MACAROON_V2,
+    )
+    for caveat in dm.caveats:
+        m.add_first_party_caveat(caveats.serialize(caveat))
+    serialized_macaroon = f"pypi-{m.serialize()}"
+
+    credentials = base64.b64encode(f"__token__:{serialized_macaroon}".encode()).decode(
+        "utf-8"
+    )
+
+    with open("./tests/functional/_fixtures/sampleproject-3.0.0.tar.gz", "rb") as f:
+        content = f.read()
+
+    params = MultiDict(
+        {
+            ":action": "file_upload",
+            "protocol_version": "1",
+            "name": "sampleproject",
+            "sha256_digest": (
+                "117ed88e5db073bb92969a7545745fd977ee85b7019706dd256a64058f70963d"
+            ),
+            "filetype": "sdist",
+            "metadata_version": "2.1",
+            "version": "3.0.0",
+        }
+    )
+    params.add("classifiers", "Programming Language :: Python :: 3.10")
+    params.add("classifiers", "This :: Is :: Invalid")
+
+    resp = webtest.post(
+        "/legacy/",
+        headers={"Authorization": f"Basic {credentials}"},
+        params=params,
+        upload_files=[("content", "sampleproject-3.0.1.tar.gz", content)],
+        status=HTTPStatus.BAD_REQUEST,
+    )
+    assert "'This :: Is :: Invalid' is not a valid classifier" in resp.body.decode()

tests/functional/test_config.py

@@ -36,6 +36,6 @@ def test_rejects_duplicate_post_keys(webtest, socket_enabled):
     body.add("foo", "bar")
     body.add("foo", "baz")
 
-    resp = webtest.post("/account/login", params=body, status=HTTPStatus.BAD_REQUEST)
+    resp = webtest.post("/account/login/", params=body, status=HTTPStatus.BAD_REQUEST)
     assert "POST body may not contain duplicate keys" in resp.body.decode()
-    assert "(URL: 'http://localhost/account/login')" in resp.body.decode()
+    assert "(URL: 'http://localhost/account/login/')" in resp.body.decode()

tests/unit/test_config.py

@@ -534,7 +534,9 @@ def __init__(self):
     ]
     assert configurator_obj.add_view_deriver.calls == [
         pretend.call(
-            config.reject_duplicate_post_keys_view, under=config.viewderivers.INGRESS
+            config.reject_duplicate_post_keys_view,
+            over="rendered_view",
+            under="decorated_view",
         )
     ]
 

warehouse/config.py

@@ -25,7 +25,7 @@
 import platformdirs
 import transaction
 
-from pyramid import renderers, viewderivers
+from pyramid import renderers
 from pyramid.authorization import Allow, Authenticated
 from pyramid.config import Configurator as _Configurator
 from pyramid.exceptions import HTTPForbidden
@@ -269,7 +269,12 @@ def from_base64_encoded_json(configuration):
 
 
 def reject_duplicate_post_keys_view(view, info):
-    if not info.options.get("permit_duplicate_post_keys"):
+    if info.options.get("permit_duplicate_post_keys") or info.exception_only:
+        return view
+
+    else:
+        # If this isn't an exception or hasn't been permitted to have duplicate
+        # POST keys, wrap the view with a check
 
         @functools.wraps(view)
         def wrapped(context, request):
@@ -287,8 +292,6 @@ def wrapped(context, request):
 
         return wrapped
 
-    return view
-
 
 reject_duplicate_post_keys_view.options = {"permit_duplicate_post_keys"}  # type: ignore
 
@@ -845,7 +848,9 @@ def configure(settings=None):
     )
 
     # Reject requests with duplicate POST keys
-    config.add_view_deriver(reject_duplicate_post_keys_view, under=viewderivers.INGRESS)
+    config.add_view_deriver(
+        reject_duplicate_post_keys_view, over="rendered_view", under="decorated_view"
+    )
 
     # Enable Warehouse to serve our static files
     prevent_http_cache = config.get_settings().get("pyramid.prevent_http_cache", False)