Skip to content

Don't wrap exception views #16759

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

Merged
merged 6 commits into from
Sep 19, 2024
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
138 changes: 138 additions & 0 deletions tests/functional/forklift/test_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
4 changes: 2 additions & 2 deletions tests/functional/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
4 changes: 3 additions & 1 deletion tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
]

Expand Down
15 changes: 10 additions & 5 deletions warehouse/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -287,8 +292,6 @@ def wrapped(context, request):

return wrapped

return view


reject_duplicate_post_keys_view.options = {"permit_duplicate_post_keys"} # type: ignore

Expand Down Expand Up @@ -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)
Expand Down