diff --git a/tests/unit/forklift/test_decorators.py b/tests/unit/forklift/test_decorators.py index 3d696c52cd9f..0550c95f1a42 100644 --- a/tests/unit/forklift/test_decorators.py +++ b/tests/unit/forklift/test_decorators.py @@ -5,7 +5,6 @@ import pretend import pytest -from pyramid.httpexceptions import HTTPBadRequest, HTTPForbidden from pyramid.testing import DummyRequest from webob.multidict import MultiDict @@ -13,6 +12,71 @@ from warehouse.forklift import decorators +class TestUploadMetrics: + @pytest.mark.parametrize("filetype", [None, "bdist_foo"]) + def test_successful_view(self, filetype): + m = pretend.stub(increment=pretend.call_recorder(lambda n, tags=None: None)) + request = pretend.stub(metrics=m, POST={"filetype": filetype}) + response = pretend.stub() + + @decorators.upload_metrics + def wrapped(context, request): + return response + + tags = {f"filetype:{filetype}"} if filetype else set() + + assert wrapped(pretend.stub(), request) is response + assert request.metrics.increment.calls == [ + pretend.call("warehouse.upload.attempt", tags=tags), + pretend.call("warehouse.upload.ok", tags=tags), + ] + + @pytest.mark.parametrize("filetype", [None, "bdist_foo"]) + def test_forklift_error(self, filetype): + m = pretend.stub(increment=pretend.call_recorder(lambda n, tags=None: None)) + request = pretend.stub(metrics=m, POST={"filetype": filetype}) + + @decorators.upload_metrics + def wrapped(context, request): + raise decorators.InvalidTupleFieldError(field="foo") + + with pytest.raises(decorators.InvalidTupleFieldError): + wrapped(pretend.stub(), request) + + tags = {f"filetype:{filetype}"} if filetype else set() + + assert request.metrics.increment.calls == [ + pretend.call("warehouse.upload.attempt", tags=tags), + pretend.call( + "warehouse.upload.failed", tags={"reason:field-is-tuple", "field:foo"} + ), + ] + + @pytest.mark.parametrize("filetype", [None, "bdist_foo"]) + def test_unknown_error(self, filetype): + m = pretend.stub(increment=pretend.call_recorder(lambda n, tags=None: None)) + request = pretend.stub(metrics=m, POST={"filetype": filetype}) + + class SomeError(Exception): + pass + + @decorators.upload_metrics + def wrapped(context, request): + raise SomeError("blah") + + with pytest.raises(SomeError): + wrapped(pretend.stub(), request) + + tags = {f"filetype:{filetype}"} if filetype else set() + + assert request.metrics.increment.calls == [ + pretend.call("warehouse.upload.attempt", tags=tags), + pretend.call( + "warehouse.upload.failed", tags={"reason:unknown-error"} | tags + ), + ] + + class TestSanitizeRequest: def test_removes_unknowns(self): req = DummyRequest( @@ -21,6 +85,9 @@ def test_removes_unknowns(self): "foo": "UNKNOWN", "bar": " UNKNOWN ", "real": "value", + "content": cgi.FieldStorage( + headers={"content-type": "application/octet-stream"}, + ), } ) ) @@ -28,13 +95,24 @@ def test_removes_unknowns(self): @decorators.sanitize def wrapped(context, request): + # Remove the content field as it doesn't compare correctly. + del request.POST["content"] assert MultiDict({"real": "value"}) == request.POST return resp assert wrapped(pretend.stub(), req) is resp def test_escapes_nul_characters(self): - req = DummyRequest(post=MultiDict({"summary": "I want to go to the \x00"})) + req = DummyRequest( + post=MultiDict( + { + "summary": "I want to go to the \x00", + "content": cgi.FieldStorage( + headers={"content-type": "application/octet-stream"}, + ), + } + ) + ) resp = pretend.stub() @decorators.sanitize @@ -46,18 +124,36 @@ def wrapped(context, request): def test_fails_with_fieldstorage(self): req = DummyRequest(post=MultiDict({"keywords": cgi.FieldStorage()})) - req.metrics = pretend.stub(increment=lambda key, tags: None) @decorators.sanitize def wrapped(context, request): pytest.fail("wrapped view should not have been called") - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(decorators.InvalidTupleFieldError) as excinfo: wrapped(pretend.stub(), req) - resp = excinfo.value - assert resp.status_code == 400 - assert resp.status == "400 keywords: Should not be a tuple." + assert excinfo.value.values == {"field": "keywords"} + + def test_fails_without_file(self): + req = DummyRequest(post=MultiDict({})) + + @decorators.sanitize + def wrapped(context, request): + pytest.fail("wrapped view should not have been called") + + with pytest.raises(decorators.NoFileUploadError): + wrapped(pretend.stub(), req) + + @pytest.mark.parametrize("content_type", [None, "image/foobar"]) + def test_fails_invalid_content_type(self, content_type): + req = DummyRequest(post=MultiDict({"content": pretend.stub(type=content_type)})) + + @decorators.sanitize + def wrapped(context, request): + pytest.fail("wrapped view should not have been called") + + with pytest.raises(decorators.InvalidContentTypeError): + wrapped(pretend.stub(), req) class TestEnsureUploadsAllowed: @@ -90,58 +186,36 @@ def wrapped(context, request): assert wrapped(pretend.stub(), req) is resp @pytest.mark.parametrize( - ("flag", "error", "help_url"), + ("flag", "error_type"), [ ( AdminFlagValue.READ_ONLY, - "Read-only mode: Uploads are temporarily disabled.", - "", + decorators.ReadOnlyError, ), ( AdminFlagValue.DISALLOW_NEW_UPLOAD, - ( - "New uploads are temporarily disabled. " - "See /help/url/ for more information." - ), - "/help/url/", + decorators.UploadsDisabledError, ), ], ) - def test_disallowed_with_admin_flags(self, flag, error, help_url): + def test_disallowed_with_admin_flags(self, flag, error_type): req = DummyRequest() req.flags = pretend.stub(enabled=lambda f: f is flag) - req.help_url = lambda *a, **k: help_url - req.metrics = pretend.stub(increment=lambda key, tags: None) @decorators.ensure_uploads_allowed def wrapped(context, request): pytest.fail("wrapped view should not have been called") - with pytest.raises(HTTPForbidden) as excinfo: + with pytest.raises(error_type): wrapped(pretend.stub(), req) - resp = excinfo.value - - assert resp.status_code == 403 - assert resp.status == f"403 {error}" - def test_fails_without_identity(self): req = DummyRequest() req.flags = pretend.stub(enabled=lambda f: False) - req.help_url = lambda *a, **k: "/path/to/help/" - req.metrics = pretend.stub(increment=lambda key, tags: None) @decorators.ensure_uploads_allowed def wrapped(context, request): pytest.fail("wrapped view should not have been called") - with pytest.raises(HTTPForbidden) as excinfo: + with pytest.raises(decorators.MissingIdentityError): wrapped(pretend.stub(), req) - - resp = excinfo.value - - assert resp.status_code == 403 - assert resp.status == ( - "403 Invalid or non-existent authentication information. " - "See /path/to/help/ for more information." - ) diff --git a/tests/unit/forklift/test_utils.py b/tests/unit/forklift/test_errors.py similarity index 79% rename from tests/unit/forklift/test_utils.py rename to tests/unit/forklift/test_errors.py index e9619be8f236..565d3b4fa3f3 100644 --- a/tests/unit/forklift/test_utils.py +++ b/tests/unit/forklift/test_errors.py @@ -4,18 +4,18 @@ from pyramid.httpexceptions import HTTPBadRequest -from warehouse.forklift import utils +from warehouse.forklift import errors class TestExcWithMessage: def test_exc_with_message(self): - exc = utils._exc_with_message(HTTPBadRequest, "My Test Message.") + exc = errors._exc_with_message(HTTPBadRequest, "My Test Message.") assert isinstance(exc, HTTPBadRequest) assert exc.status_code == 400 assert exc.status == "400 My Test Message." def test_exc_with_exotic_message(self): - exc = utils._exc_with_message( + exc = errors._exc_with_message( HTTPBadRequest, "look at these wild chars: аä‗" ) assert isinstance(exc, HTTPBadRequest) @@ -26,8 +26,8 @@ def test_exc_with_missing_message(self, monkeypatch): sentry_sdk = pretend.stub( capture_message=pretend.call_recorder(lambda message: None) ) - monkeypatch.setattr(utils, "sentry_sdk", sentry_sdk) - exc = utils._exc_with_message(HTTPBadRequest, "") + monkeypatch.setattr(errors, "sentry_sdk", sentry_sdk) + exc = errors._exc_with_message(HTTPBadRequest, "") assert isinstance(exc, HTTPBadRequest) assert exc.status_code == 400 assert exc.status == "400 Bad Request" diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index 4645ce7609c1..976a59ffd1b8 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -5,7 +5,6 @@ import hashlib import io import json -import re import tarfile import tempfile import zipfile @@ -20,7 +19,7 @@ import pytest from pypi_attestations import Attestation, Envelope, VerificationMaterial -from pyramid.httpexceptions import HTTPBadRequest, HTTPForbidden, HTTPTooManyRequests +from pyramid.httpexceptions import HTTPBadRequest from sqlalchemy import and_, event, exists from sqlalchemy.orm import joinedload from trove_classifiers import classifiers @@ -31,6 +30,7 @@ from warehouse.accounts.utils import UserContext from warehouse.attestations.interfaces import IIntegrityService from warehouse.classifiers.models import Classifier +from warehouse.errors import WarehouseDenied from warehouse.forklift import legacy, metadata from warehouse.macaroons import IMacaroonService, caveats, security_policy from warehouse.metrics import IMetricsService @@ -648,25 +648,13 @@ def test_is_duplicate_false(self, pyramid_config, db_request): class TestFileUpload: @pytest.mark.parametrize("version", ["2", "3", "-1", "0", "dog", "cat"]) - def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): - pyramid_request.POST["protocol_version"] = version - pyramid_request.flags = pretend.stub(enabled=lambda *a: False) - pyramid_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") - - user = UserFactory.create(with_verified_primary_email=True) - pyramid_config.testing_securitypolicy(identity=user) - pyramid_request.user = user - - with pytest.raises(HTTPBadRequest) as excinfo: - legacy.file_upload(pyramid_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == "400 Unknown protocol version." + def test_fails_invalid_version(self, version): + with pytest.raises(legacy.InvalidProtocolVersionError) as excinfo: + legacy.file_upload(pretend.stub(POST={"protocol_version": version})) + assert excinfo.value.values == {"version": version} @pytest.mark.parametrize( - ("post_data", "message"), + ("post_data", "error_type", "expected"), [ # metadata_version errors. ( @@ -677,9 +665,8 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): "filetype": "sdist", "pyversion": "source", }, - "None is not a valid metadata version. See " - "https://packaging.python.org/specifications/core-metadata for more " - "information.", + legacy.InvalidCoreMetadataError, + {"msg": "None is not a valid metadata version."}, ), ( { @@ -690,27 +677,25 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): "filetype": "sdist", "pyversion": "source", }, - "'-1' is not a valid metadata version. See " - "https://packaging.python.org/specifications/core-metadata for more " - "information.", + legacy.InvalidCoreMetadataError, + {"msg": "'-1' is not a valid metadata version."}, ), # name errors. ( - {"metadata_version": "1.2"}, - "'' is an invalid value for Name. " - "Error: This field is required. " - "See " - "https://packaging.python.org/specifications/core-metadata" - " for more information.", + {}, + legacy.InvalidCoreMetadataError, + {"msg": "None is an invalid value for Name: This field is required."}, ), ( - {"metadata_version": "1.2", "name": "foo-"}, - "'foo-' is an invalid value for Name. " - "Error: Start and end with a letter or numeral containing " - "only ASCII numeric and '.', '_' and '-'. " - "See " - "https://packaging.python.org/specifications/core-metadata" - " for more information.", + {"name": "foo-"}, + legacy.InvalidCoreMetadataError, + { + "msg": ( + "'foo-' is an invalid value for Name: Start and end with a " + "letter or numeral containing only ASCII numeric and '.', '_' " + "and '-'." + ), + }, ), # version errors. ( @@ -721,9 +706,8 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): "md5_digest": "bad", "filetype": "sdist", }, - "'version' is a required field. See " - "https://packaging.python.org/specifications/core-metadata for " - "more information.", + legacy.InvalidCoreMetadataError, + {"msg": "'version' is a required field."}, ), ( { @@ -733,9 +717,8 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): "md5_digest": "bad", "filetype": "sdist", }, - "'dog' is invalid for 'version'. See " - "https://packaging.python.org/specifications/core-metadata for " - "more information.", + legacy.InvalidCoreMetadataError, + {"msg": "'dog' is invalid for 'version'."}, ), ( { @@ -745,74 +728,74 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): "md5_digest": "bad", "filetype": "sdist", }, - "'1.0.dev.a1' is invalid for 'version'. See " - "https://packaging.python.org/specifications/core-metadata for " - "more information.", + legacy.InvalidCoreMetadataError, + {"msg": "'1.0.dev.a1' is invalid for 'version'."}, ), # filetype/pyversion errors. ( { - "metadata_version": "1.2", "name": "example", "version": "1.0", "md5_digest": "bad", }, - "Invalid value for filetype. Error: This field is required.", + legacy.InvalidUploadMetadataError, + {"field": "filetype", "msg": "This field is required."}, ), ( { - "metadata_version": "1.2", "name": "example", - "version": "1.0", "filetype": "bdist_wheel", - "content": "fake binary content", }, - "Invalid value for pyversion. " - "Error: Python version is required for binary distribution uploads.", + legacy.InvalidUploadMetadataError, + { + "field": "pyversion", + "msg": ( + "Python version is required for binary distribution uploads." + ), + }, ), ( { - "metadata_version": "1.2", "name": "example", - "version": "1.0", "filetype": "bdist_wat", "pyversion": "1.0", "md5_digest": "bad", }, - "Invalid value for filetype. Error: Use a known file type.", + legacy.InvalidUploadMetadataError, + {"field": "filetype", "msg": "Use a known file type."}, ), ( { - "metadata_version": "1.2", "name": "example", - "version": "1.0", "filetype": "sdist", "pyversion": "1.0", }, - "Invalid value for pyversion. " - "Error: Use 'source' as Python version for an sdist.", + legacy.InvalidUploadMetadataError, + { + "field": "pyversion", + "msg": "Use 'source' as Python version for an sdist.", + }, ), # digest errors. ( { - "metadata_version": "1.2", "name": "example", - "version": "1.0", "filetype": "sdist", - "content": "fake binary content", }, - "Error: Include at least one message digest.", + legacy.MissingUploadMetadataError, + {"msg": "Include at least one message digest."}, ), ( { - "metadata_version": "1.2", "name": "example", - "version": "1.0", "filetype": "sdist", "sha256_digest": "an invalid sha256 digest", }, - "Invalid value for sha256_digest. " - "Error: Use a valid, hex-encoded, SHA256 message digest.", + legacy.InvalidUploadMetadataError, + { + "field": "sha256_digest", + "msg": "Use a valid, hex-encoded, SHA256 message digest.", + }, ), # summary errors ( @@ -824,9 +807,8 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): "md5_digest": "a fake md5 digest", "summary": "A" * 513, }, - "'summary' field must be 512 characters or less. See " - "https://packaging.python.org/specifications/core-metadata for more " - "information.", + legacy.InvalidCoreMetadataError, + {"msg": "'summary' field must be 512 characters or less."}, ), ( { @@ -837,9 +819,8 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): "md5_digest": "a fake md5 digest", "summary": "A\nB", }, - "'summary' must be a single line. See " - "https://packaging.python.org/specifications/core-metadata for more " - "information.", + legacy.InvalidCoreMetadataError, + {"msg": "'summary' must be a single line."}, ), # local version error ( @@ -850,15 +831,14 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): "md5_digest": "bad", "filetype": "sdist", }, - "The use of local versions in '1.0+local' is not allowed. " - "See https://packaging.python.org/en/latest/specifications/" - "version-specifiers/#local-version-identifiers for more information.", + legacy.InvalidLocalVersionError, + {"msg": "The use of local versions in '1.0+local' is not allowed."}, ), ], ) @pytest.mark.filterwarnings("ignore:Creating a LegacyVersion.*:DeprecationWarning") def test_fails_invalid_post_data( - self, pyramid_config, db_request, post_data, message + self, pyramid_config, db_request, post_data, error_type, expected ): user = UserFactory.create() EmailFactory.create(user=user) @@ -866,13 +846,9 @@ def test_fails_invalid_post_data( db_request.user = user db_request.POST = MultiDict(post_data) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(error_type) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == f"400 {message}" + assert excinfo.value.values == expected @pytest.mark.parametrize("name", ["requirements.txt", "rrequirements.txt"]) def test_fails_with_invalid_names(self, pyramid_config, db_request, name): @@ -963,25 +939,11 @@ def test_fails_with_ultranormalized_names( ) @pytest.mark.parametrize( - ("description_content_type", "description", "message"), - [ - ( - "text/x-rst", - ".. invalid-directive::", - "400 The description failed to render for 'text/x-rst'. " - "See /the/help/url/ for more information.", - ), - ( - None, - ".. invalid-directive::", - "400 The description failed to render in the default format " - "of reStructuredText. " - "See /the/help/url/ for more information.", - ), - ], + ("description_content_type", "description"), + [("text/x-rst", ".. invalid-directive::"), (None, ".. invalid-directive::")], ) def test_fails_invalid_render( - self, pyramid_config, db_request, description_content_type, description, message + self, pyramid_config, db_request, description_content_type, description ): user = UserFactory.create() EmailFactory.create(user=user) @@ -1007,19 +969,12 @@ def test_fails_invalid_render( if description_content_type is not None: db_request.POST.add("description_content_type", description_content_type) - db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") - - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidDescriptionError) as excinfo: legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == message - - assert db_request.help_url.calls == [ - pretend.call(_anchor="description-content-type") - ] + excinfo.value.values == { + "content_type": description_content_type or "text/x-rst" + } @pytest.mark.parametrize( "name", @@ -1085,29 +1040,6 @@ def test_fails_with_stdlib_names(self, pyramid_config, db_request, name): "for more information." ) - def test_upload_fails_without_file(self, pyramid_config, db_request): - user = UserFactory.create() - EmailFactory.create(user=user) - pyramid_config.testing_securitypolicy(identity=user) - db_request.user = user - db_request.POST = MultiDict( - { - "metadata_version": "1.2", - "name": "example", - "version": "1.0", - "filetype": "sdist", - "md5_digest": "a fake md5 digest", - } - ) - - with pytest.raises(HTTPBadRequest) as excinfo: - legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == "400 Upload payload does not have a file." - @pytest.mark.parametrize("macaroon_in_user_context", [True, False]) @pytest.mark.parametrize( "digests", @@ -1304,56 +1236,6 @@ def storage_service_store(path, file_path, *, meta): pretend.call(uploaded_file.id), ] - assert db_request.metrics.increment.calls == [ - pretend.call("warehouse.upload.attempt"), - pretend.call("warehouse.upload.ok", tags=["filetype:sdist"]), - ] - - @pytest.mark.parametrize("content_type", [None, "image/foobar"]) - def test_upload_fails_invalid_content_type( - self, tmpdir, monkeypatch, pyramid_config, db_request, content_type - ): - monkeypatch.setattr(tempfile, "tempdir", str(tmpdir)) - - user = UserFactory.create() - EmailFactory.create(user=user) - pyramid_config.testing_securitypolicy(identity=user) - db_request.user = user - project = ProjectFactory.create() - release = ReleaseFactory.create(project=project, version="1.0") - RoleFactory.create(user=user, project=project) - - db_request.db.add(Classifier(classifier="Environment :: Other Environment")) - - filename = "{}-{}.tar.gz".format( - project.normalized_name.replace("-", "_"), release.version - ) - - db_request.POST = MultiDict( - { - "metadata_version": "1.2", - "name": project.name, - "version": release.version, - "filetype": "sdist", - "pyversion": "source", - "md5_digest": _TAR_GZ_PKG_MD5, - "content": pretend.stub( - filename=filename, - file=io.BytesIO(_TAR_GZ_PKG_TESTDATA), - type=content_type, - ), - } - ) - db_request.POST.extend([("classifiers", "Environment :: Other Environment")]) - - with pytest.raises(HTTPBadRequest) as excinfo: - legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == "400 Invalid distribution file." - def test_upload_fails_with_legacy_type(self, pyramid_config, db_request): user = UserFactory.create() EmailFactory.create(user=user) @@ -1383,16 +1265,12 @@ def test_upload_fails_with_legacy_type(self, pyramid_config, db_request): } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidUploadMetadataError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert ( - resp.status - == "400 Invalid value for filetype. Error: Use a known file type." - ) + assert excinfo.value.values == { + "field": "filetype", + "msg": "Use a known file type.", + } def test_upload_fails_with_legacy_ext(self, pyramid_config, db_request): user = UserFactory.create() @@ -1422,18 +1300,9 @@ def test_upload_fails_with_legacy_ext(self, pyramid_config, db_request): } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.UnknownFileExtensionError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 Invalid file extension: Use .tar.gz, .whl or .zip " - "extension. See https://www.python.org/dev/peps/pep-0527 " - "and https://peps.python.org/pep-0715/ for more information" - ) - def test_upload_fails_for_second_sdist(self, pyramid_config, db_request): user = UserFactory.create() pyramid_config.testing_securitypolicy(identity=user) @@ -1467,14 +1336,9 @@ def test_upload_fails_for_second_sdist(self, pyramid_config, db_request): } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.DuplicateSDistError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == "400 Only one sdist may be uploaded per release." - def test_upload_fails_with_invalid_classifier(self, pyramid_config, db_request): user = UserFactory.create() pyramid_config.testing_securitypolicy(identity=user) @@ -1504,33 +1368,23 @@ def test_upload_fails_with_invalid_classifier(self, pyramid_config, db_request): ) db_request.POST.extend([("classifiers", "Invalid :: Classifier")]) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidCoreMetadataError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 'Invalid :: Classifier' is not a valid classifier. See " - "https://packaging.python.org/specifications/core-metadata for more " - "information." - ) + assert excinfo.value.values == { + "msg": "'Invalid :: Classifier' is not a valid classifier." + } @pytest.mark.parametrize( ("deprecated_classifiers", "expected"), [ ( {"AA :: BB": ["CC :: DD"]}, - "400 The classifier 'AA :: BB' has been deprecated, use one of " - "['CC :: DD'] instead. See " - "https://packaging.python.org/specifications/core-metadata for more " - "information.", + "The classifier 'AA :: BB' has been deprecated, use one of " + "['CC :: DD'] instead.", ), ( {"AA :: BB": []}, - "400 The classifier 'AA :: BB' has been deprecated. See " - "https://packaging.python.org/specifications/core-metadata for more " - "information.", + "The classifier 'AA :: BB' has been deprecated.", ), ], ) @@ -1572,13 +1426,9 @@ def test_upload_fails_with_deprecated_classifier( db_request.POST.extend([("classifiers", classifier.classifier)]) db_request.route_url = pretend.call_recorder(lambda *a, **kw: "/url") - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidCoreMetadataError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == expected + assert excinfo.value.values == {"msg": expected} @pytest.mark.parametrize( "digests", @@ -1639,17 +1489,9 @@ def test_upload_fails_with_invalid_digest( ) db_request.POST.update(digests) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.DigestMismatchError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 The digest supplied does not match a digest calculated " - "from the uploaded file." - ) - def test_upload_fails_with_invalid_file(self, pyramid_config, db_request): user = UserFactory.create() pyramid_config.testing_securitypolicy(identity=user) @@ -1676,13 +1518,12 @@ def test_upload_fails_with_invalid_file(self, pyramid_config, db_request): } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidDistFileError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ("400 Invalid distribution file. File is not a zipfile") + assert excinfo.value.values == { + "msg": "File is not a zipfile", + "filetype": "sdist", + } def test_upload_fails_end_of_file_error( self, pyramid_config, db_request, project_service @@ -1721,13 +1562,10 @@ def test_upload_fails_end_of_file_error( }.get(svc) db_request.user_agent = "warehouse-tests/6.6.6" - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidDistFileError) as excinfo: legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ("400 Invalid distribution file. File is not a tarfile") + excinfo.value.values == {"msg": "File is not a tarfile"} def test_upload_fails_with_too_large_file(self, pyramid_config, db_request): user = UserFactory.create() @@ -1756,26 +1594,10 @@ def test_upload_fails_with_too_large_file(self, pyramid_config, db_request): ), } ) - db_request.user_docs_url = pretend.call_recorder( - lambda *a, **kw: "/the/help/url/" - ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.FileTooLargeError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert db_request.user_docs_url.calls == [ - pretend.call( - "/project-management/storage-limits", - anchor="requesting-a-file-size-limit-increase", - ) - ] - assert resp.status_code == 400 - assert resp.status == ( - "400 File too large. Limit for project 'foobar' is 100 MB. " - "See /the/help/url/ for more information." - ) + assert excinfo.value.values == {"project": project.name, "limit": 100} def test_upload_fails_with_too_large_project_size_default_limit( self, pyramid_config, db_request @@ -1810,27 +1632,10 @@ def test_upload_fails_with_too_large_project_size_default_limit( ), } ) - db_request.user_docs_url = pretend.call_recorder( - lambda *a, **kw: "/the/help/url/" - ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.ProjectTooLargeError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert db_request.user_docs_url.calls == [ - pretend.call( - "/project-management/storage-limits", - anchor="requesting-a-project-size-limit-increase", - ) - ] - assert resp.status_code == 400 - assert resp.status == ( - "400 Project size too large." - " Limit for project 'foobar' total size is 10 GB. " - "See /the/help/url/" - ) + assert excinfo.value.values == {"project": project.name, "limit": 10} def test_upload_fails_with_too_large_project_size_custom_limit( self, pyramid_config, db_request @@ -1870,27 +1675,10 @@ def test_upload_fails_with_too_large_project_size_custom_limit( ), } ) - db_request.user_docs_url = pretend.call_recorder( - lambda *a, **kw: "/the/help/url/" - ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.ProjectTooLargeError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert db_request.user_docs_url.calls == [ - pretend.call( - "/project-management/storage-limits", - anchor="requesting-a-project-size-limit-increase", - ) - ] - assert resp.status_code == 400 - assert resp.status == ( - "400 Project size too large." - " Limit for project 'foobar' total size is 10 GB. " - "See /the/help/url/" - ) + assert excinfo.value.values == {"project": project.name, "limit": 10} def test_upload_succeeds_custom_project_size_limit( self, @@ -2055,21 +1843,9 @@ def mock_open(file, mode="r", *args, **kwargs): monkeypatch.setattr("builtins.open", mock_open) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.FilenameTooLongError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == f"400 Filename is too long: '{filename}'" - - assert db_request.metrics.increment.calls == [ - pretend.call("warehouse.upload.attempt"), - pretend.call( - "warehouse.upload.failed", - tags=["reason:filename-too-long", "filetype:bdist_wheel"], - ), - ] + assert excinfo.value.values == {"filename": filename, "filetype": "bdist_wheel"} def test_upload_fails_with_previously_used_filename( self, pyramid_config, db_request @@ -2101,20 +1877,10 @@ def test_upload_fails_with_previously_used_filename( ) db_request.db.add(Filename(filename=filename)) - db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.FilenameReusedError): legacy.file_upload(db_request) - resp = excinfo.value - - assert db_request.help_url.calls == [pretend.call(_anchor="file-name-reuse")] - assert resp.status_code == 400 - assert resp.status == ( - "400 This filename was previously used by a file that has since been " - "deleted. Use a different version. See /the/help/url/ for more information." - ) - def test_upload_noop_with_existing_filename_same_content( self, pyramid_config, db_request ): @@ -2206,19 +1972,11 @@ def test_upload_fails_with_existing_filename_diff_content( path=f"source/{project.name[0]}/{project.name}/{filename}", ) ) - db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") - with pytest.raises(HTTPBadRequest) as excinfo: - legacy.file_upload(db_request) - resp = excinfo.value + with pytest.raises(legacy.FileAlreadyExistsError) as excinfo: + legacy.file_upload(db_request) - assert db_request.help_url.calls == [pretend.call(_anchor="file-name-reuse")] - assert resp.status_code == 400 - assert resp.status == ( - f"400 File already exists ({filename!r}, " - f"with blake2_256 hash {blake2_256_digest!r}). " - "See /the/help/url/ for more information." - ) + excinfo.value.values == {"filename": filename, "blake2_256": blake2_256_digest} def test_upload_fails_with_diff_filename_same_blake2( self, pyramid_config, db_request @@ -2264,20 +2022,13 @@ def test_upload_fails_with_diff_filename_same_blake2( path=f"source/{project.name[0]}/{project.name}/{filename}", ) ) - db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.FileAlreadyExistsError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert db_request.help_url.calls == [pretend.call(_anchor="file-name-reuse")] - assert resp.status_code == 400 - assert resp.status == ( - f"400 File already exists ({db_request.POST['content'].filename!r}, " - f"with blake2_256 hash {blake2_256_digest!r}). " - "See /the/help/url/ for more information." - ) + assert excinfo.value.values == { + "filename": f"{project.name}-fake.tar.gz", + "blake2_256": blake2_256_digest, + } @pytest.mark.parametrize( ("filename", "filetype", "project_name"), @@ -2340,37 +2091,32 @@ def test_upload_fails_with_wrong_filename_project_name( ), } ) - db_request.help_url = lambda **kw: "/the/help/url/" - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.UnnormalizedFilenameError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 Start filename for {!r} with {!r}.".format( - project.name, - project.normalized_name.replace("-", "_"), - ) - ) + assert excinfo.value.values == { + "project": project.name, + "normalized": project.normalized_name.replace("-", "_"), + "filetype": filetype, + } @pytest.mark.parametrize( - ("filename", "status"), + ("filename", "exc_type", "expected"), [ ( "wutang-6.6.6.tar.gz", - "400 Filename 'wutang-6.6.6.tar.gz' is invalid, should be " - "'wutang-1.2.3.tar.gz'.", + legacy.UnnormalizedFilenameError, + {"project": "wutang", "normalized": "wutang", "filetype": "sdist"}, ), ( "wutang-6.6.6-py3-none-any.whl", - "400 Version in filename should be '1.2.3' not '6.6.6'.", + legacy.InvalidFilenameVersionError, + {"expected": "1.2.3", "filetype": "bdist_wheel", "found": "6.6.6"}, ), ], ) def test_upload_fails_with_wrong_filename_version( - self, monkeypatch, pyramid_config, db_request, filename, status + self, monkeypatch, pyramid_config, db_request, filename, exc_type, expected ): user = UserFactory.create() pyramid_config.testing_securitypolicy(identity=user) @@ -2408,15 +2154,10 @@ def test_upload_fails_with_wrong_filename_version( ), } ) - db_request.help_url = lambda **kw: "/the/help/url/" - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(exc_type) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == status + assert excinfo.value.values == expected @pytest.mark.parametrize( ("filetype", "extension"), @@ -2461,17 +2202,9 @@ def test_upload_fails_with_invalid_filetype( } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidFileExtensionError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - f"400 Invalid file extension: Extension {extension} is invalid for " - f"filetype {filetype}. See https://www.python.org/dev/peps/pep-0527 " - "for more information." - ) + assert excinfo.value.values == {"extension": extension, "filetype": filetype} def test_upload_fails_with_invalid_extension(self, pyramid_config, db_request): user = UserFactory.create() @@ -2501,18 +2234,9 @@ def test_upload_fails_with_invalid_extension(self, pyramid_config, db_request): } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.UnknownFileExtensionError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 Invalid file extension: Use .tar.gz, .whl or .zip " - "extension. See https://www.python.org/dev/peps/pep-0527 " - "and https://peps.python.org/pep-0715/ for more information" - ) - @pytest.mark.parametrize("character", ["/", "\\"]) def test_upload_fails_with_unsafe_filename( self, pyramid_config, db_request, character @@ -2544,14 +2268,9 @@ def test_upload_fails_with_unsafe_filename( } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.PathlikeFilenameError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == "400 Cannot upload a file with '/' or '\\' in the name." - @pytest.mark.parametrize("character", [*(chr(x) for x in range(32)), chr(127)]) def test_upload_fails_with_disallowed_in_filename( self, pyramid_config, db_request, character @@ -2583,17 +2302,9 @@ def test_upload_fails_with_disallowed_in_filename( } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.UnprintableFilenameError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 Cannot upload a file with non-printable characters (ordinals 0-31) " - "or the DEL character (ordinal 127) in the name." - ) - def test_upload_fails_without_user_permission(self, pyramid_config, db_request): user1 = UserFactory.create() EmailFactory.create(user=user1) @@ -2624,21 +2335,50 @@ def test_upload_fails_without_user_permission(self, pyramid_config, db_request): } ) - db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") - - with pytest.raises(HTTPForbidden) as excinfo: + with pytest.raises(legacy.UserPermissionError) as excinfo: legacy.file_upload(db_request) + assert excinfo.value.values == { + "project": project.name, + "username": user2.username, + } - resp = excinfo.value + def test_upload_fails_with_permission_reason(self, pyramid_config, db_request): + user1 = UserFactory.create() + EmailFactory.create(user=user1) + user2 = UserFactory.create() + EmailFactory.create(user=user2) + project = ProjectFactory.create() + release = ReleaseFactory.create(project=project, version="1.0") + RoleFactory.create(user=user1, project=project) - assert db_request.help_url.calls == [pretend.call(_anchor="project-name")] - assert resp.status_code == 403 - assert resp.status == ( - f"403 The user '{user2.username}' " - f"isn't allowed to upload to project '{project.name}'. " - "See /the/help/url/ for more information." + filename = "{}-{}.tar.gz".format( + project.normalized_name.replace("-", "_"), release.version ) + pyramid_config.testing_securitypolicy(identity=user2, permissive=False) + db_request.user = user2 + db_request.POST = MultiDict( + { + "metadata_version": "1.2", + "name": project.name, + "version": release.version, + "filetype": "sdist", + "md5_digest": "nope!", + "content": pretend.stub( + filename=filename, + file=io.BytesIO(b"a" * (warehouse.constants.MAX_FILESIZE + 1)), + type="application/tar", + ), + } + ) + db_request.has_permission = lambda perm, ctx: WarehouseDenied( + "bad stuff", reason="a reason for stuff" + ) + + with pytest.raises(legacy.PermissionError) as excinfo: + legacy.file_upload(db_request) + assert excinfo.value.values == {"msg": "bad stuff"} + def test_upload_fails_without_oidc_publisher_permission( self, pyramid_config, db_request ): @@ -2668,19 +2408,9 @@ def test_upload_fails_without_oidc_publisher_permission( } ) - db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") - - with pytest.raises(HTTPForbidden) as excinfo: + with pytest.raises(legacy.TokenPermissionError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert db_request.help_url.calls == [pretend.call(_anchor="project-name")] - assert resp.status_code == 403 - assert resp.status == ( - f"403 The given token isn't allowed to upload to project '{project.name}'. " - "See /the/help/url/ for more information." - ) + assert excinfo.value.values == {"project": project.name} def test_upload_fails_with_unverified_email_after_permission_check( self, pyramid_config, db_request, mocker @@ -2716,20 +2446,10 @@ def test_upload_fails_with_unverified_email_after_permission_check( ), } ) - db_request.help_url = mocker.Mock(return_value="/the/help/url/") - with pytest.raises(HTTPForbidden) as excinfo: + with pytest.raises(legacy.UnverifiedEmailError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 403 - assert resp.status == ( - f"403 User {user.username!r}, associated with the API token used, " - "does not have a verified primary email address. Please add a " - "verified primary email before attempting to upload to PyPI. " - "See /the/help/url/ for more information." - ) + assert excinfo.value.values == {"username": user.username} def test_upload_fails_without_two_factor_after_permission_check( self, pyramid_config, db_request, mocker @@ -2762,20 +2482,10 @@ def test_upload_fails_without_two_factor_after_permission_check( ), } ) - db_request.help_url = mocker.Mock(return_value="/the/help/url/") - with pytest.raises(HTTPForbidden) as excinfo: + with pytest.raises(legacy.MissingTwoFactorError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 403 - assert resp.status == ( - f"403 User {user.username!r}, associated with the API token used, " - "does not have two-factor authentication enabled. Please enable " - "two-factor authentication before attempting to upload to PyPI. " - "See /the/help/url/ for more information." - ) + assert excinfo.value.values == {"username": user.username} def test_upload_permission_error_surfaces_before_email_check( self, pyramid_config, db_request, mocker @@ -2813,19 +2523,13 @@ def test_upload_permission_error_surfaces_before_email_check( ), } ) - db_request.help_url = mocker.Mock(return_value="/the/help/url/") - with pytest.raises(HTTPForbidden) as excinfo: + with pytest.raises(legacy.UserPermissionError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 403 - assert resp.status == ( - f"403 The user {intruder.username!r} " - f"isn't allowed to upload to project '{project.name}'. " - "See /the/help/url/ for more information." - ) + assert excinfo.value.values == { + "username": intruder.username, + "project": project.name, + } def test_upload_new_project_fails_with_unverified_email( self, pyramid_config, db_request, mocker @@ -2855,18 +2559,10 @@ def test_upload_new_project_fails_with_unverified_email( ) db_request.help_url = mocker.Mock(return_value="/the/help/url/") - with pytest.raises(HTTPForbidden) as excinfo: + with pytest.raises(legacy.UnverifiedEmailError) as excinfo: legacy.file_upload(db_request) + assert excinfo.value.values == {"username": user.username} - resp = excinfo.value - - assert resp.status_code == 403 - assert resp.status == ( - f"403 User {user.username!r}, associated with the API token used, " - "does not have a verified primary email address. Please add a " - "verified primary email before attempting to upload to PyPI. " - "See /the/help/url/ for more information." - ) # The project must not have been created. assert ( db_request.db.query(Project) @@ -2946,16 +2642,11 @@ def test_upload_attestation_fails_without_oidc_publisher( }.get(svc) db_request.user_agent = "warehouse-tests/6.6.6" - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidAttestationsError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 Invalid attestations supplied during upload: " - "Attestations are only supported when using Trusted Publishing" - ) + assert excinfo.value.values == { + "msg": "Attestations are only supported when using Trusted Publishing" + } @pytest.mark.parametrize( "plat", @@ -3141,11 +2832,6 @@ def storage_service_store(path, file_path, *, meta): ) ] - assert db_request.metrics.increment.calls == [ - pretend.call("warehouse.upload.attempt"), - pretend.call("warehouse.upload.ok", tags=["filetype:bdist_wheel"]), - ] - @pytest.mark.parametrize( ("project_name", "version"), [ @@ -3491,29 +3177,20 @@ def storage_service_store(path, file_path, *, meta): ] @pytest.mark.parametrize( - ("filename", "expected"), + ("filename"), [ - ( - "foo-1.0.whl", - "400 Invalid wheel filename (wrong number of parts): 'foo-1.0'", - ), - ( - "foo-1.0-q-py3-none-any.whl", - "400 Invalid build number: q in 'foo-1.0-q-py3-none-any'", - ), - ( - "foo-0.0.4test1-py3-none-any.whl", - "400 Invalid wheel filename (invalid version): " - "'foo-0.0.4test1-py3-none-any'", - ), - ( - "something.tar.gz", - "400 Invalid source distribution filename: something.tar.gz", - ), + "foo-1.0.whl", + "foo-1.0-q-py3-none-any.whl", + "foo-0.0.4test1-py3-none-any.whl", + "something.tar.gz", ], ) def test_upload_fails_with_invalid_filename( - self, monkeypatch, pyramid_config, db_request, filename, expected + self, + monkeypatch, + pyramid_config, + db_request, + filename, ): user = UserFactory.create() pyramid_config.testing_securitypolicy(identity=user) @@ -3548,28 +3225,27 @@ def test_upload_fails_with_invalid_filename( legacy, "_is_valid_dist_file", lambda *a, **kw: (True, None) ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidFilenameError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == expected + assert excinfo.value.values == { + "filename": filename, + "filetype": "bdist_wheel" if filename.endswith(".whl") else "sdist", + } @pytest.mark.parametrize( - "plat", + ("plat", "expected"), [ - "linux_x86_64", - "linux_x86_64.win32", - "macosx_9_2_x86_64", - "macosx_11_2_arm64", - "macosx_16_2_arm64", - "macosx_10_15_amd64", - "macosx_27_0_arm64", + ("linux_x86_64", "linux_x86_64"), + ("linux_x86_64.win32", "linux_x86_64"), + ("macosx_9_2_x86_64", "macosx_9_2_x86_64"), + ("macosx_11_2_arm64", "macosx_11_2_arm64"), + ("macosx_16_2_arm64", "macosx_16_2_arm64"), + ("macosx_10_15_amd64", "macosx_10_15_amd64"), + ("macosx_27_0_arm64", "macosx_27_0_arm64"), ], ) def test_upload_fails_with_unsupported_wheel_plat( - self, monkeypatch, pyramid_config, db_request, plat + self, monkeypatch, pyramid_config, db_request, plat, expected ): user = UserFactory.create() pyramid_config.testing_securitypolicy(identity=user) @@ -3601,15 +3277,9 @@ def test_upload_fails_with_unsupported_wheel_plat( legacy, "_is_valid_dist_file", lambda *a, **kw: (True, None) ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidPlatformTagError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert re.match( - "400 Binary wheel .* has an unsupported platform tag .*", resp.status - ) + assert excinfo.value.values == {"filename": filename, "tag": expected} def test_upload_fails_with_missing_record_wheel( self, monkeypatch, pyramid_config, db_request @@ -3652,15 +3322,15 @@ def test_upload_fails_with_missing_record_wheel( legacy, "_is_valid_dist_file", lambda *a, **kw: (True, None) ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.MissingRecordFileError) as excinfo: legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert re.match( - "400 Wheel .* does not contain the required RECORD file: .*", resp.status - ) + expected_filename = project.normalized_name.replace("-", "_") + assert excinfo.value.values == { + "filename": filename, + "record_filename": f"{expected_filename}-1.0.dist-info/RECORD", + "filetype": "bdist_wheel", + } def test_upload_warns_with_mismatched_wheel_and_zip_contents( self, monkeypatch, pyramid_config, db_request @@ -4072,15 +3742,12 @@ def test_upload_fails_with_missing_metadata_wheel( legacy, "_is_valid_dist_file", lambda *a, **kw: (True, None) ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.MissingMetadataFileError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert re.match( - "400 Wheel .* does not contain the required METADATA file: .*", resp.status - ) + assert excinfo.value.values == { + "filename": filename, + "metadata_filename": f"{project_name}-{release.version}.dist-info/METADATA", + } def test_upload_updates_existing_project_name(self, pyramid_config, db_request): user = UserFactory.create() @@ -4479,14 +4146,9 @@ def test_upload_fails_attestation_error( ) monkeypatch.setattr(HasEvents, "record_event", record_event) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidAttestationsError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status.startswith("400 Invalid attestations") - @pytest.mark.parametrize( ("url", "expected"), [ @@ -5146,21 +4808,9 @@ def test_upload_fails_nonuser_identity_cannot_create_project( }.get(svc) db_request.user_agent = "warehouse-tests/6.6.6" - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.NonUserIdentityError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 Non-user identities cannot create new projects. " - "This was probably caused by successfully using a pending " - "publisher but specifying the project name incorrectly (either " - "in the publisher or in your project's metadata). Please ensure " - "that both match. " - "See: https://docs.pypi.org/trusted-publishers/troubleshooting/" - ) - @pytest.mark.parametrize( ("failing_limiter", "remote_addr"), [ @@ -5211,14 +4861,9 @@ def test_upload_new_project_fails_ratelimited( }.get(svc) db_request.user_agent = "warehouse-tests/6.6.6" - with pytest.raises(HTTPTooManyRequests) as excinfo: + with pytest.raises(legacy.ProjectCreationRateLimitedError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 429 - assert resp.status == ("429 Too many new projects created") - def test_upload_succeeds_creates_project( self, pyramid_config, db_request, project_service ): @@ -5583,19 +5228,18 @@ def test_upload_fails_pep427( IProjectService: project_service, }.get(svc) db_request.user_agent = "warehouse-tests/6.6.6" - db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") monkeypatch.setattr( legacy, "_is_valid_dist_file", lambda *a, **kw: (True, None) ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.UnnormalizedFilenameError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == status + assert excinfo.value.values == { + "project": project.name, + "normalized": project.normalized_name.replace("-", "_"), + "filetype": "bdist_wheel", + } @pytest.mark.parametrize( ("filename", "version", "expected"), @@ -5651,21 +5295,18 @@ def test_upload_fails_pep625( IProjectService: project_service, }.get(svc) db_request.user_agent = "warehouse-tests/6.6.6" - db_request.help_url = pretend.call_recorder(lambda **kw: "/the/help/url/") monkeypatch.setattr( legacy, "_is_valid_dist_file", lambda *a, **kw: (True, None) ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.UnnormalizedFilenameError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - f"400 Filename {filename!r} is invalid, should be {expected!r}." - ) + assert excinfo.value.values == { + "project": project.name, + "normalized": project.normalized_name.replace("-", "_"), + "filetype": "sdist", + } @pytest.mark.parametrize( ("version", "expected_version", "filetype", "mimetype"), @@ -5945,16 +5586,14 @@ def test_upload_fails_missing_license_file_metadata_2_4( IFileStorage: storage_service, }.get(svc) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.MissingLicenseFileError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - f"400 License-File LICENSE does not exist " - f"in distribution file {filename} at {license_filename}" - ) + assert excinfo.value.values == { + "filename": filename, + "license_file": "LICENSE", + "target_file": license_filename, + "filetype": filetype, + } def test_upload_fails_when_license_and_license_expression_are_present( self, @@ -6001,18 +5640,14 @@ def test_upload_fails_when_license_and_license_expression_are_present( IFileStorage: storage_service, }.get(svc) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidCoreMetadataError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 License is deprecated when License-Expression is present. " - "Only License-Expression should be present. " - "See https://packaging.python.org/specifications/core-metadata " - "for more information." - ) + assert excinfo.value.values == { + "msg": ( + "License is deprecated when License-Expression is present. Only " + "License-Expression should be present." + ) + } def test_upload_for_organization_owned_project_succeeds( self, pyramid_config, db_request, monkeypatch @@ -6112,19 +5747,9 @@ def test_upload_for_company_organization_owned_project_fails_without_subscriptio } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.OrgInactiveError): legacy.file_upload(db_request) - resp = excinfo.value - - assert resp.status_code == 400 - assert resp.status == ( - "400 Organization account owning this project is inactive. " - "This may be due to inactive billing for Company Organizations, " - "or administrator intervention for Community Organizations. " - "Please contact support+orgs@pypi.org." - ) - def test_upload_with_organization_file_size_limit_succeeds( self, pyramid_config, db_request, monkeypatch ): @@ -6599,12 +6224,15 @@ def test_upload_fails_with_yara_match( } ) - with pytest.raises(HTTPBadRequest) as excinfo: + with pytest.raises(legacy.InvalidDistFileError) as excinfo: legacy.file_upload(db_request) - - resp = excinfo.value - assert resp.status_code == 400 - assert "PyArmor-encrypted content is not allowed" in resp.status + assert excinfo.value.values == { + "msg": ( + "PyArmor-encrypted content is not allowed. See " + "https://pypi.org/policy/terms-of-use/ for more information." + ), + "filetype": "bdist_wheel", + } def test_submit(pyramid_request): diff --git a/warehouse/forklift/decorators.py b/warehouse/forklift/decorators.py index c410d531b905..f9f938df735a 100644 --- a/warehouse/forklift/decorators.py +++ b/warehouse/forklift/decorators.py @@ -2,10 +2,103 @@ import cgi -from pyramid.httpexceptions import HTTPBadRequest, HTTPForbidden +from pyramid.httpexceptions import HTTPForbidden from warehouse.admin.flags import AdminFlagValue -from warehouse.forklift.utils import _exc_with_message +from warehouse.forklift.errors import ForkliftError + + +class InvalidTupleFieldError( + ForkliftError, + message="{field}: Should not be a tuple.", + tags={"reason:field-is-tuple", "field:{field}"}, +): + pass + + +class NoFileUploadError( + ForkliftError, + message="Upload payload does not have a file.", + tags={"reason:no-file"}, +): + pass + + +class InvalidContentTypeError( + ForkliftError, + message="Invalid distribution file.", + tags={"reason:invalid-content-type"}, +): + pass + + +class ReadOnlyError( + ForkliftError, + error_type=HTTPForbidden, + message="Read-only mode: Uploads are temporarily disabled.", + tags={"reason:read-only"}, +): + pass + + +class UploadsDisabledError( + ForkliftError, + error_type=HTTPForbidden, + message="New uploads are temporarily disabled.", + help_anchor="admin-intervention", + tags={"reason:uploads-disabled"}, +): + pass + + +class MissingIdentityError( + ForkliftError, + error_type=HTTPForbidden, + message="Invalid or non-existent authentication information.", + help_anchor="invalid-auth", + tags={"reason:no-identity"}, +): + pass + + +def upload_metrics(wrapped): + + def wrapper(context, request): + # We'll manually pull this out of the request because we don't want to invoke + # the forms machinery or anything. This may end up meaning we get an invalid + # value, but that's OK it's just the metrics tagging so not the end of the + # world. + # + # NOTE: We'll only use these tags for cases where we can't expect the properly + # validated value to have already been included. + tags = { + f"{k}:{v}" for (k, v) in [("filetype", request.POST.get("filetype"))] if v + } + + # Log an attempt to upload + request.metrics.increment("warehouse.upload.attempt", tags=tags) + + try: + response = wrapped(context, request) + except ForkliftError as exc: + # We've got an error that we know how to get the tags out of it, so we'll + # increment a failure with those tags. + request.metrics.increment("warehouse.upload.failed", tags=exc.tags) + raise + except Exception: + # If we get any other kind of exception, then we'll mark a failed + # upload as well, but we don't know what the error type is. + request.metrics.increment( + "warehouse.upload.failed", tags=tags | {"reason:unknown-error"} + ) + raise + else: + # Log a successful upload + request.metrics.increment("warehouse.upload.ok", tags=tags) + + return response + + return wrapper def sanitize(wrapped): @@ -42,13 +135,17 @@ def wrapper(context, request): for field in set(request.POST) - {"content", "gpg_signature"}: values = request.POST.getall(field) if any(isinstance(value, cgi.FieldStorage) for value in values): - request.metrics.increment( - "warehouse.upload.failed", - tags=["reason:field-is-tuple", f"field:{field}"], - ) - raise _exc_with_message( - HTTPBadRequest, f"{field}: Should not be a tuple." - ) + raise InvalidTupleFieldError(field=field) + + # Ensure that we have file data in the request. + if "content" not in request.POST: + raise NoFileUploadError + + # Check the content type of what is being uploaded + if not request.POST["content"].type or request.POST["content"].type.startswith( + "image/" + ): + raise InvalidContentTypeError # Otherwise, we'll just dispatch to our underlying view return wrapped(context, request) @@ -66,40 +163,17 @@ def wrapper(context, request): # The very first thing we want to check, is whether we're currently in # read only mode, because if we're in read only mode nothing else matters. if request.flags.enabled(AdminFlagValue.READ_ONLY): - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:read-only"] - ) - raise _exc_with_message( - HTTPForbidden, "Read-only mode: Uploads are temporarily disabled." - ) + raise ReadOnlyError # After that, we want to check if we're disallowing new uploads, which is # functionally the same as read only mode, but only for the upload endpoint. if request.flags.enabled(AdminFlagValue.DISALLOW_NEW_UPLOAD): - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:uploads-disabled"] - ) - raise _exc_with_message( - HTTPForbidden, - "New uploads are temporarily disabled. " - "See {projecthelp} for more information.".format( - projecthelp=request.help_url(_anchor="admin-intervention") - ), - ) + raise UploadsDisabledError # Before we do anything else, if there isn't an authenticated identity with # this request, then we'll go ahead and bomb out. if request.identity is None: - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:no-identity"] - ) - raise _exc_with_message( - HTTPForbidden, - "Invalid or non-existent authentication information. " - "See {projecthelp} for more information.".format( - projecthelp=request.help_url(_anchor="invalid-auth") - ), - ) + raise MissingIdentityError # Otherwise, we'll just dispatch to our underlying view return wrapped(context, request) diff --git a/warehouse/forklift/errors.py b/warehouse/forklift/errors.py new file mode 100644 index 000000000000..1ae458fb9aee --- /dev/null +++ b/warehouse/forklift/errors.py @@ -0,0 +1,147 @@ +# SPDX-License-Identifier: Apache-2.0 + +from collections.abc import Iterable +from typing import Any + +import sentry_sdk + +from pyramid.httpexceptions import HTTPBadRequest, HTTPException +from pyramid.view import exception_view_config + + +# We don't subclass from a HTTP Exception here, because pyramid will automatically +# turn those into a response for us, and we don't want that to happen "silently" +# for these exceptions. +# +# In other words, a ForkliftError is only intended to be raised from within +# forklift, and forklift should always have an exception view applied that does the +# translation to what Pyramid expects, so these should never reach Pyramid itself, +# and if they do, then something has gone wrong and we should be alerted to that +# fact. +# +# All of our ForkliftErrors have certain values that are set at the class +# level (like a message, tags to emit in the metrics, etc) and accept arbitrary +# keyword arguments which will be formatted into those values at runtime. +# +# This means we can have a message (or a tag, or whatever) that says something +# like `message = "this is an error with {foo}` and you can pass in foo as a +# keyword argument to the ForkliftError. +class ForkliftError(Exception): + message: str + tags: set[str] + values: dict[str, Any] + + _message_template: str + _tag_templates: set[str] + _http_exception_type: type[HTTPException] + _help_anchor: str | None + _help_url: str | None + _user_docs_path: str | None + _user_docs_anchor: str | None + + # We use an __init_subclass__ hook to make it impossible for a subclass to + # forget to specify either a message or a set of tags, which they could do + # if we just used "normal" class variables. + # + # This also means that our class variables that we use to smuggle these values + # into the __init__ and as_http_exception methods don't need nice human readable + # names, because they're just part of the internal details of this class. + def __init_subclass__( + cls, + *, + message: str, + tags: Iterable[str], + error_type: type[HTTPException] = HTTPBadRequest, + help_anchor=None, + user_docs_path=None, + user_docs_anchor=None, + help_url=None, + **kwargs, + ): + # This is the template that we'll use to render the base "message" of + # the error, which will get treated as if the user did Foo(message), but + # with the string template expanded. + cls._message_template = message + + # This is the set of tags that we'll use when emitting metrics, again + # this will end up expanded with the kwargs. + cls._tag_templates = set(tags) + + # This is the HttpException that this exception will actually raise when + # it gets translated from a ForkliftException to a HttpException. + cls._http_exception_type = error_type + + # If there is a help anchor (which is pointing to the standad help url) + # associated with this, then when we translate this error we'll also + # automatically include a link to the help url in the message as well. + cls._help_anchor = help_anchor + + # We also have user docs, and we point some of our errors to that. + cls._user_docs_path = user_docs_path + cls._user_docs_anchor = user_docs_anchor + + # If there is a help url, then we assume it is an already resolved url + # that we can include in the response to direct the user towards help. + cls._help_url = help_url + + # Let the normal behavior finish initializing the subclass. + super().__init_subclass__(**kwargs) + + def __init__(self, *args, **kwargs): + # We're going to generate a "real" message by using the kwards to format + # against the message template stored on the class, and then pass that + # into the parent class as if it was called like Foo("result of template"). + self.message = self._message_template.format(**kwargs) + + # Like message, our tags are also formatted by the kwargs, but we're just + # going to store them ourselves because our parent class doesn't have any + # concept of tags that we'd want to mimic. + self.tags = {t.format(**kwargs) for t in self._tag_templates} + + # Store our values that were passed in here so they're easy to introspect + # later. + self.values = kwargs + + # We do this last, so that all of our pre-formatting has already taken + # into effect before this happens. + super().__init__(self.message, *args) + + def as_http_exception(self, request): + # Append the standard "see X for more info" message to our message if there + # a help url associated with this error. + # TODO: Handle _help_url, and multiples and _user_docs_anchor, _user_docs_path + message = self.message + if self._help_anchor is not None: + message += ( + f" See {request.help_url(_anchor=self._help_anchor)} for more " + "information." + ) + + # Actually construct the "real" http error, using our now finalized message. + return _exc_with_message(self._http_exception_type, message) + + +# This is kept as a stand alone function to support the handful of non-upload endpoints +# in Forklift, at least until we have a better pattern for handling metrics that would +# let us re-use the translate_error machinery without emitting spurious metrics. +def _exc_with_message(exc, message, **kwargs): + # TODO: Is this something we still need to worry about? + if not message: + sentry_sdk.capture_message("Attempting to _exc_with_message without a message") + + # The crappy old API that PyPI offered uses the status to pass down + # messages to the client, so we'll construct a normal http exception, + # but then set the "status" to our error message as well. + resp = exc(detail=message, **kwargs) + # We need to guard against characters outside of iso-8859-1 per RFC. + # Specifically here, where user-supplied text may appear in the message, + # which our WSGI server may not appropriately handle (indeed gunicorn does not). + status_message = message.encode("iso-8859-1", "replace").decode("iso-8859-1") + resp.status = f"{resp.status_code} {status_message}" + return resp + + +@exception_view_config(ForkliftError, route_name="forklift.legacy.file_upload") +def forklift_error(exc, request): + request.metrics.increment("warehouse.upload.failed", tags=exc.tags) + return exc.as_http_exception(request) diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index fad63b3e0d9c..8d29049d5d17 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -7,18 +7,13 @@ import tempfile import zipfile -import packaging.requirements -import packaging.specifiers import packaging.utils import packaging.version import packaging_legacy.version import sentry_sdk -import wtforms -import wtforms.validators from pypi_attestations import Attestation, Distribution from pyramid.httpexceptions import ( - HTTPBadRequest, HTTPException, HTTPForbidden, HTTPGone, @@ -43,9 +38,13 @@ ) from warehouse.events.tags import EventTag from warehouse.forklift import metadata -from warehouse.forklift.decorators import ensure_uploads_allowed, sanitize +from warehouse.forklift.decorators import ( + ensure_uploads_allowed, + sanitize, + upload_metrics, +) +from warehouse.forklift.errors import ForkliftError, _exc_with_message from warehouse.forklift.forms import UploadForm, _filetype_extension_mapping -from warehouse.forklift.utils import _exc_with_message from warehouse.macaroons.models import Macaroon from warehouse.packaging.interfaces import IFileStorage, IProjectService from warehouse.packaging.metadata_verification import verify_email, verify_url @@ -87,6 +86,364 @@ # existing descriptions. MAX_DESCRIPTION_LENGTH_TO_BIGQUERY_IN_BYTES = 40000 + +class InvalidProtocolVersionError( + ForkliftError, + message="Unknown protocol version: {version!r}", + tags={"reason:unsupported-protocol-version"}, +): + pass + + +class NonUserIdentityError( + ForkliftError, + error_type=HTTPForbidden, + message=( + "Non-user identities cannot create new projects. " + "This was probably caused by successfully using a pending " + "publisher but specifying the project name incorrectly " + "(either in the publisher or in your project's metadata). " + "Please ensure that both match." + ), + help_url="https://docs.pypi.org/trusted-publishers/troubleshooting/", + tags={"reason:non-user-identity"}, +): + pass + + +class UserPermissionError( + ForkliftError, + error_type=HTTPForbidden, + message="The user {username!r} isn't allowed to upload to project {project!r}.", + help_anchor="project-name", + tags={"reason:permission-denied"}, +): + pass + + +class TokenPermissionError( + ForkliftError, + error_type=HTTPForbidden, + message="The given token isn't allowed to upload to project {project!r}.", + help_anchor="project-name", + tags={"reason:permission-denied"}, +): + pass + + +class PermissionError( + ForkliftError, + error_type=HTTPForbidden, + # NOTE: This looks wonky, but it's done so the permissions + # system can pass us a message itself that we'll use. + message="{msg}", + tags={"reason:permission-denied"}, +): + pass + + +class UnverifiedEmailError( + ForkliftError, + error_type=HTTPForbidden, + message=( + "User {username!r} does not have a verified primary email address. " + "Please add a verified primary email before attempting to " + "upload to PyPI." + ), + help_anchor="verified-email", + tags={"reason:unverified-email"}, +): + pass + + +class MissingTwoFactorError( + ForkliftError, + error_type=HTTPForbidden, + message=( + "User {username!r} does not have two-factor authentication enabled. " + "Please enable two-factor authentication before attempting to " + "upload to PyPI." + ), + help_anchor="two-factor-authentication", + tags={"reason:no-2fa"}, +): + pass + + +class OrgInactiveError( + ForkliftError, + error_type=HTTPForbidden, + message=( + "Organization account owning this project is inactive. " + "This may be due to inactive billing for Company " + "Organizations, or administrator intervention for Community " + "Organizations. Please contact support+orgs@pypi.org." + ), + tags={"reason:org-not-active"}, +): + pass + + +class InvalidDescriptionError( + ForkliftError, + message="The description failed to render as {content_type!r}.", + help_anchor="description-content-type", + tags={"reason:invalid-description"}, +): + pass + + +class UnprintableFilenameError( + ForkliftError, + message=( + "Cannot upload a file with non-printable " + "characters (ordinals 0-31) or the DEL character " + "(ordinal 127) in the name." + ), + tags={"reason:invalid-filename"}, +): + pass + + +class PathlikeFilenameError( + ForkliftError, + message="Cannot upload a file with '/' or '\\' in the name.", + tags={"reason:invalid-filename"}, +): + pass + + +class InvalidFileExtensionError( + ForkliftError, + message=( + "Invalid file extension: Extension {extension} is " + "invalid for filetype {filetype}. See " + "https://www.python.org/dev/peps/pep-0527 for more " + "information." + ), + tags={"reason:invalid-filename"}, +): + pass + + +class UnknownFileExtensionError( + ForkliftError, + message=( + "Invalid file extension: Use .tar.gz, .whl or .zip " + "extension. See " + "https://www.python.org/dev/peps/pep-0527 and " + "https://peps.python.org/pep-0715/ for more " + "information." + ), + tags={"reason:invalid-filename"}, +): + pass + + +class InvalidFilenameError( + ForkliftError, + message="Invalid filename: {filename}", + tags={"reason:invalid-filename", "filetype:{filetype}"}, +): + pass + + +class UnnormalizedFilenameError( + ForkliftError, + message=( + "Filename for {project!r} should contain the normalized project " + "name {normalized!r}." + ), + tags={"reason:invalid-filename-normalized", "filetype:{filetype}"}, +): + pass + + +class InvalidFilenameVersionError( + ForkliftError, + message="Version in filename should be {expected!r} not {found!r}.", + tags={"reason:invalid-filename-version", "filetype:{filetype}"}, +): + pass + + +class InvalidPlatformTagError( + ForkliftError, + message="Binary wheel {filename!r} has an unsupported platform tag {tag!r}.", + tags={"reason:unsupported-platform-tag"}, +): + pass + + +class MissingRecordFileError( + ForkliftError, + message=( + "Wheel {filename!r} does not contain the required RECORD file: " + "{record_filename}." + ), + tags={"reason:missing-record-file", "filetype:{filetype}"}, +): + pass + + +class MissingMetadataFileError( + ForkliftError, + message=( + "Wheel {filename!r} does not contain the required METADATA file: " + "{metadata_filename}." + ), + tags={"reason:missing-metadata-file", "filetype:bdist_wheel"}, +): + pass + + +class FileTooLargeError( + ForkliftError, + message="File too large. Limit for project {project!r} is {limit}MB.", + user_docs_path="/project-management/storage-limits", + user_docs_anchor="requesting-a-file-size-limit-increase", + tags={"reason:file-too-large"}, +): + pass + + +class ProjectTooLargeError( + ForkliftError, + message="Project size too large. Limit for project {project!r} is {limit}GB.", + user_docs_path="/project-management/storage-limits", + user_docs_anchor="requesting-a-project-size-limit-increase", + tags={"reason:project-too-large"}, +): + pass + + +class FilenameTooLongError( + ForkliftError, + message="Filename is too long: {filename!r}", + tags={"reason:filename-too-long", "filetype:{filetype}"}, +): + pass + + +class DigestMismatchError( + ForkliftError, + message=( + "The digest supplied does not match a digest calculated from the uploaded file." + ), + tags={"reason:digest-mismatch"}, +): + pass + + +class FileAlreadyExistsError( + ForkliftError, + # Note: Changing this error message to something that doesn't + # start with "File already exists" will break the + # --skip-existing functionality in twine + # ref: https://github.com/pypi/warehouse/issues/3482 + # ref: https://github.com/pypa/twine/issues/332 + message="File already exists ({filename!r}, with blake2_256 hash {blake2_256!r}).", + help_anchor="file-name-reuse", + tags={"reason:duplicate-file"}, +): + pass + + +class FilenameReusedError( + ForkliftError, + message=( + "This filename was previously used by a file that has " + "since been deleted. Use a different version." + ), + help_anchor="file-name-reuse", + tags={"reason:filename-reuse"}, +): + pass + + +class DuplicateSDistError( + ForkliftError, + message="Only one sdist may be uploaded per release.", + tags={"reason:duplicate-sdist"}, +): + pass + + +class InvalidDistFileError( + ForkliftError, + message="Invalid distribution file: {msg}", + tags={ + "reason:invalid-distribution-file", + "filetype:{filetype}", + "message:{msg}", + }, +): + pass + + +class MissingLicenseFileError( + ForkliftError, + message=( + "License-File {license_file} does not exist in " + "distribution file {filename} at {target_file}" + ), + tags={"reason:missing-license-file", "filetype:{filetype}"}, +): + pass + + +class InvalidAttestationsError( + ForkliftError, + message="Invalid attestations supplied during upload: {msg}", + tags={"reason:invalid-attestations"}, +): + pass + + +class ProjectCreationRateLimitedError( + ForkliftError, + error_type=HTTPTooManyRequests, + message="Too many new projects created", + tags={"reason:rate-limited"}, +): + pass + + +class InvalidCoreMetadataError( + ForkliftError, + message="{msg}", + help_url="https://packaging.python.org/specifications/core-metadata", + tags={"reason:invalid-metadata"}, +): + pass + + +class InvalidLocalVersionError( + ForkliftError, + message="{msg}", + help_url=( + "https://packaging.python.org/en/latest/specifications/" + "version-specifiers/#local-version-identifiers" + ), + tags={"reason:invalid-metadata"}, +): + pass + + +class InvalidUploadMetadataError( + ForkliftError, + message="Invalid value for {field}. Error: {msg}", + tags={"reason:invalid-form-data"}, +): + pass + + +class MissingUploadMetadataError( + ForkliftError, message="Error: {msg}", tags={"reason:invalid-form-data"} +): + pass + + # Wheel platform checking # Note: defining new platform ABI compatibility tags that don't @@ -235,39 +592,19 @@ def _validate_filename(filename, filetype): # or completely by mistake. disallowed = [*(chr(x) for x in range(32)), chr(127)] if [char for char in filename if char in disallowed]: - raise _exc_with_message( - HTTPBadRequest, - ( - "Cannot upload a file with " - "non-printable characters (ordinals 0-31) " - "or the DEL character (ordinal 127) " - "in the name." - ), - ) + raise UnprintableFilenameError # Make sure that the filename does not contain any path separators. if "/" in filename or "\\" in filename: - raise _exc_with_message( - HTTPBadRequest, "Cannot upload a file with '/' or '\\' in the name." - ) + raise PathlikeFilenameError # Make sure the filename ends with an allowed extension. if m := _dist_file_re.match(filename): extension = m.group("extension") if extension not in _filetype_extension_mapping[filetype]: - raise _exc_with_message( - HTTPBadRequest, - f"Invalid file extension: Extension {extension} is invalid for " - f"filetype {filetype}. See https://www.python.org/dev/peps/pep-0527 " - "for more information.", - ) + raise InvalidFileExtensionError(extension=extension, filetype=filetype) else: - raise _exc_with_message( - HTTPBadRequest, - "Invalid file extension: Use .tar.gz, .whl or .zip " - "extension. See https://www.python.org/dev/peps/pep-0527 " - "and https://peps.python.org/pep-0715/ for more information", - ) + raise UnknownFileExtensionError def _is_valid_dist_file(filename, filetype, metrics, *, scan=True): @@ -522,36 +859,10 @@ def _ensure_user_can_upload(request: Request) -> None: return if not (request.user.primary_email and request.user.primary_email.verified): - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:unverified-email"] - ) - raise _exc_with_message( - HTTPForbidden, - ( - "User {!r}, associated with the API token used, does not " - "have a verified primary email address. Please add a " - "verified primary email before attempting to upload to " - "PyPI. See {project_help} for more information." - ).format( - request.user.username, - project_help=request.help_url(_anchor="verified-email"), - ), - ) from None + raise UnverifiedEmailError(username=request.user.username) if not request.user.has_two_factor: - request.metrics.increment("warehouse.upload.failed", tags=["reason:no-2fa"]) - raise _exc_with_message( - HTTPForbidden, - ( - "User {!r}, associated with the API token used, does not " - "have two-factor authentication enabled. Please enable " - "two-factor authentication before attempting to upload to " - "PyPI. See {project_help} for more information." - ).format( - request.user.username, - project_help=request.help_url(_anchor="two-factor-authentication"), - ), - ) from None + raise MissingTwoFactorError(username=request.user.username) @view_config( @@ -561,62 +872,45 @@ def _ensure_user_can_upload(request: Request) -> None: require_methods=["POST"], has_translations=True, permit_duplicate_post_keys=True, - decorator=[sanitize, ensure_uploads_allowed], + decorator=[upload_metrics, sanitize, ensure_uploads_allowed], ) def file_upload(request): - # Log an attempt to upload - request.metrics.increment("warehouse.upload.attempt") - # This is a list of warnings that we'll emit *IF* the request is successful. warnings: list[str] = [] # We require protocol_version 1, it's the only supported version however # passing a different version should raise an error. - if request.POST.get("protocol_version", "1") != "1": - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:unsupported-protocol-version"] - ) - raise _exc_with_message(HTTPBadRequest, "Unknown protocol version.") + if (version := request.POST.get("protocol_version", "1")) != "1": + raise InvalidProtocolVersionError(version=version) # Validate and process the incoming file data. form = UploadForm(request.POST) if not form.validate(): + # We'll only return a single field's worth of errors, so we'll see if our more + # important fields are involved in the errors, and if so we'll select that + # field as the one whose error we'll return. for field_name in _error_message_order: if field_name in form.errors: break + # Otherwise we'll just take the first field alphabetically as the field that + # we'll return an error from. else: field_name = sorted(form.errors.keys())[0] if field_name in form: field = form[field_name] - if field.description and isinstance(field, wtforms.StringField): - error_message = ( - "{value!r} is an invalid value for {field}. ".format( - value=( - field.data[:30] + "..." + field.data[-30:] - if field.data and len(field.data) > 60 - else field.data or "" - ), - field=field.description, - ) - + f"Error: {form.errors[field_name][0]} " - + "See " - "https://packaging.python.org/specifications/core-metadata" - + " for more information." - ) - else: - error_message = ( - f"Invalid value for {field_name}. Error: " - f"{form.errors[field_name][0]}" + if field.description: + raise InvalidCoreMetadataError( + msg=( + f"{field.data!r} is an invalid value for " + f"{field.description}: {form.errors[field_name][0]}" + ), ) - else: - error_message = f"Error: {form.errors[field_name][0]}" - - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:invalid-form-data"] - ) - raise _exc_with_message(HTTPBadRequest, error_message) + raise InvalidUploadMetadataError( + field=field_name, msg=form.errors[field_name][0] + ) + raise MissingUploadMetadataError(msg=form.errors[field_name][0]) # Get a validated Metadata object from the form data. # TODO: We should eventually extract this data out of the artifact and use that, @@ -641,30 +935,11 @@ def file_upload(request): # for that field error = errors[field_name][0] error_msg = str(error) - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:invalid-metadata"] - ) - _see_url = ( - "https://packaging.python.org/en/latest/specifications/" - "version-specifiers/#local-version-identifiers" - if field_name == "version" - and any("use of local versions" in str(e) for e in errors["version"]) - else "https://packaging.python.org/specifications/core-metadata" + if isinstance(error, metadata.InvalidLocalVersion): + raise InvalidLocalVersionError(msg=error_msg) + raise InvalidCoreMetadataError( + msg=error_msg + ("." if not error_msg.endswith(".") else "") ) - raise _exc_with_message( - HTTPBadRequest, - " ".join( - [ - error_msg + ("." if not error_msg.endswith(".") else ""), - f"See {_see_url} for more information.", - ] - ), - ) - - # Ensure that we have file data in the request. - if "content" not in request.POST: - request.metrics.increment("warehouse.upload.failed", tags=["reason:no-file"]) - raise _exc_with_message(HTTPBadRequest, "Upload payload does not have a file.") # Set a statement timeout for this connection to prevent long-running # transactions from holding database locks indefinitely. The upload @@ -693,20 +968,7 @@ def file_upload(request): # produce a valid API token, but the project lookup above uses (2) # and will fail because (1) != (2). if not request.user: - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:non-user-identity"] - ) - raise _exc_with_message( - HTTPBadRequest, - ( - "Non-user identities cannot create new projects. " - "This was probably caused by successfully using a pending " - "publisher but specifying the project name incorrectly (either " - "in the publisher or in your project's metadata). Please ensure " - "that both match. " - "See: https://docs.pypi.org/trusted-publishers/troubleshooting/" - ), - ) + raise NonUserIdentityError # Enforce email/2FA prerequisites before creating a brand new project, # so we don't leave an empty project record behind on rejection. @@ -718,71 +980,38 @@ def file_upload(request): project = project_service.create_project( form.name.data, request.user, request ) + # TODO: The project service raises a lot of different HTTPExceptions, ideally + # it wouldn't do that and would raise regular exceptions, and it would + # be up to the handlers to translate those. However, changing that would + # require changes to several areas of warehouse, so we'll leave that for + # a future change. except HTTPException as exc: request.metrics.increment( "warehouse.upload.failed", tags=["reason:project-creation-failed"] ) raise _exc_with_message(exc.__class__, exc.detail) from None except RateLimiterException: - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:rate-limited"] - ) - msg = "Too many new projects created" - raise _exc_with_message(HTTPTooManyRequests, msg) + raise ProjectCreationRateLimitedError # Check that the identity has permission to do things to this project, if this # is a new project this will act as a sanity check for the role we just # added above. allowed = request.has_permission(Permissions.ProjectsUpload, project) if not allowed: - reason = getattr(allowed, "reason", None) + if getattr(allowed, "reason", None) is not None: + raise PermissionError(msg=allowed.msg) if request.user: - msg = ( - ( - "The user '{}' isn't allowed to upload to project '{}'. " - "See {} for more information." - ).format( - request.user.username, - project.name, - request.help_url(_anchor="project-name"), - ) - if reason is None - else allowed.msg + raise UserPermissionError( + project=project.name, username=request.user.username ) - else: - msg = ( - ( - "The given token isn't allowed to upload to project '{}'. " - "See {} for more information." - ).format( - project.name, - request.help_url(_anchor="project-name"), - ) - if reason is None - else allowed.msg - ) - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:permission-denied"] - ) - raise _exc_with_message(HTTPForbidden, msg) + raise TokenPermissionError(project=project.name) _ensure_user_can_upload(request) # If organization owned project, check if the organization is active. # Inactive organizations cannot upload new releases to their projects. if project.organization and not project.organization.good_standing: - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:org-not-active"] - ) - raise _exc_with_message( - HTTPBadRequest, - ( - "Organization account owning this project is inactive. " - "This may be due to inactive billing for Company Organizations, " - "or administrator intervention for Community Organizations. " - "Please contact support+orgs@pypi.org." - ), - ) + raise OrgInactiveError # If this is a user identity (i.e: API token) but there exists # a trusted publisher for this project, send an email warning that an @@ -814,6 +1043,7 @@ def file_upload(request): project_id=project.id, ) ) + # Update name if it differs but is still equivalent. We don't need to check if # they are equivalent when normalized because that's already been done when we # queried for the project. @@ -832,26 +1062,7 @@ def file_upload(request): # Uploading should prevent broken rendered descriptions. if rendered is None: - if meta.description_content_type: - message = ( - "The description failed to render for " - f"'{description_content_type}'." - ) - else: - message = ( - "The description failed to render " - "in the default format of reStructuredText." - ) - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:invalid-description"] - ) - raise _exc_with_message( - HTTPBadRequest, - "{message} See {projecthelp} for more information.".format( - message=message, - projecthelp=request.help_url(_anchor="description-content-type"), - ), - ) from None + raise InvalidDescriptionError(content_type=description_content_type) # Verify any verifiable URLs project_urls = ( @@ -907,9 +1118,9 @@ def file_upload(request): else verify_email(email=maintainer_email, project=project) ) + is_new_release = False + canonical_version = packaging.utils.canonicalize_version(meta.version) try: - is_new_release = False - canonical_version = packaging.utils.canonicalize_version(meta.version) release = ( request.db.query(Release) .filter( @@ -1035,19 +1246,8 @@ def file_upload(request): filename = request.POST["content"].filename # Ensure the filename doesn't contain any characters that are too 🌶️spicy🥵 - # TODO: refactor to accept `request` and emit metrics, or return a list of errors - # and handle them here and emit metrics. _validate_filename(filename, filetype=form.filetype.data) - # Check the content type of what is being uploaded - if not request.POST["content"].type or request.POST["content"].type.startswith( - "image/" - ): - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:invalid-content-type"] - ) - raise _exc_with_message(HTTPBadRequest, "Invalid distribution file.") - # The project may or may not have a file size specified on the project, if # it does then it may or may not be smaller or larger than our global file # size limits. Additionally, if the project belongs to an organization, @@ -1072,30 +1272,14 @@ def file_upload(request): for chunk in iter(lambda: request.POST["content"].file.read(8096), b""): file_size += len(chunk) if file_size > file_size_limit: - raise _exc_with_message( - HTTPBadRequest, - "File too large. " - f"Limit for project {project.name!r} is " - f"{file_size_limit // ONE_MIB} MB. " - "See " - + request.user_docs_url( - "/project-management/storage-limits", - anchor="requesting-a-file-size-limit-increase", - ) - + " for more information.", + raise FileTooLargeError( + project=project.name, limit=file_size_limit // ONE_MIB ) if file_size + project.total_size > project_size_limit: - raise _exc_with_message( - HTTPBadRequest, - "Project size too large. Limit for " - f"project {project.name!r} total size is " - f"{project_size_limit // ONE_GIB} GB. " - "See " - + request.user_docs_url( - "/project-management/storage-limits", - anchor="requesting-a-project-size-limit-increase", - ), + raise ProjectTooLargeError( + project=project.name, limit=project_size_limit // ONE_GIB ) + fp.write(chunk) for hasher in file_hashes.values(): hasher.update(chunk) @@ -1115,14 +1299,7 @@ def file_upload(request): for digest_name, digest_value in file_hashes.items() if getattr(form, f"{digest_name}_digest").data ): - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:digest-mismatch"] - ) - raise _exc_with_message( - HTTPBadRequest, - "The digest supplied does not match a digest calculated " - "from the uploaded file.", - ) + raise DigestMismatchError # Check to see if the file that was uploaded exists already or not. is_duplicate = _is_duplicate_file(request.db, filename, file_hashes) @@ -1130,37 +1307,15 @@ def file_upload(request): request.tm.doom() return HTTPOk() if is_duplicate is not None: - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:duplicate-file"] - ) - raise _exc_with_message( - HTTPBadRequest, - # Note: Changing this error message to something that doesn't - # start with "File already exists" will break the - # --skip-existing functionality in twine - # ref: https://github.com/pypi/warehouse/issues/3482 - # ref: https://github.com/pypa/twine/issues/332 - "File already exists " - f"({filename!r}, with blake2_256 hash {file_hashes['blake2_256']!r})." - " See " - + request.help_url(_anchor="file-name-reuse") - + " for more information.", + raise FileAlreadyExistsError( + filename=filename, blake2_256=file_hashes["blake2_256"] ) # Check to see if the file that was uploaded exists in our filename log if request.db.query( request.db.query(Filename).filter(Filename.filename == filename).exists() ).scalar(): - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:filename-reuse"] - ) - raise _exc_with_message( - HTTPBadRequest, - "This filename was previously used by a file that has since been " - "deleted. Use a different version. See " - + request.help_url(_anchor="file-name-reuse") - + " for more information.", - ) + raise FilenameReusedError # Check to see if uploading this file would create a duplicate sdist # for the current release. @@ -1172,12 +1327,7 @@ def file_upload(request): .exists() ).scalar() ): - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:duplicate-sdist"] - ) - raise _exc_with_message( - HTTPBadRequest, "Only one sdist may be uploaded per release." - ) + raise DuplicateSDistError # Check the file to make sure it is a valid distribution file. _scan = not request.flags.enabled(AdminFlagValue.DISABLE_UPLOAD_SCANNING) @@ -1192,17 +1342,7 @@ def file_upload(request): scan=_scan, ) if not _valid: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:invalid-distribution-file", - f"filetype:{form.filetype.data}", - f"message:{_msg}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, f"Invalid distribution file. {_msg}" - ) + raise InvalidDistFileError(filetype=form.filetype.data, msg=_msg) # Check that the sdist filename is correct if form.filetype.data == "sdist": @@ -1212,13 +1352,8 @@ def file_upload(request): packaging.utils.parse_sdist_filename(filename) ) except packaging.utils.InvalidSdistFilename: - request.metrics.increment( - "warehouse.upload.failed", - tags=["reason:invalid-filename", f"filetype:{form.filetype.data}"], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Invalid source distribution filename: {filename}", + raise InvalidFilenameError( + filename=filename, filetype=form.filetype.data ) # The previous function fails to accomodate the edge case where @@ -1252,17 +1387,10 @@ def file_upload(request): packaging.utils.canonicalize_name(name_from_filename) != project.normalized_name ): - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:invalid-filename-normalized", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Start filename for {project.name!r} with " - f"{project.normalized_name.replace('-', '_')!r}.", + raise UnnormalizedFilenameError( + project=project.name, + normalized=project.normalized_name.replace("-", "_"), + filetype=form.filetype.data, ) # PEP 625: Enforcement of source distribution filename @@ -1272,14 +1400,10 @@ def file_upload(request): expected_filename = f"{expected_name}-{expected_version}.tar.gz" if filename != expected_filename: - request.metrics.increment( - "warehouse.upload.failed", - tags=["reason:invalid-sdist-filename-normalization"], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Filename {filename!r} is invalid, should be " - f"{expected_filename!r}.", + raise UnnormalizedFilenameError( + project=project.name, + normalized=project.normalized_name.replace("-", "_"), + filetype=form.filetype.data, ) filename = os.path.basename(temporary_filename) @@ -1297,17 +1421,11 @@ def file_upload(request): try: tar.getmember(target_file) except KeyError: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:missing-license-file", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - f"License-File {license_file} does not exist in " - f"distribution file {filename} at {target_file}", + raise MissingLicenseFileError( + filename=filename, + license_file=license_file, + target_file=target_file, + filetype=form.filetype.data, ) # Check that if it's a binary wheel, it's on a supported platform @@ -1316,40 +1434,20 @@ def file_upload(request): name, version, ___, tags = packaging.utils.parse_wheel_filename( filename ) - except packaging.utils.InvalidWheelFilename as e: - request.metrics.increment( - "warehouse.upload.failed", - tags=["reason:invalid-filename", f"filetype:{form.filetype.data}"], - ) - raise _exc_with_message( - HTTPBadRequest, - str(e), + except packaging.utils.InvalidWheelFilename: + raise InvalidFilenameError( + filename=filename, filetype=form.filetype.data ) for tag in tags: if not _valid_platform_tag(tag.platform): - request.metrics.increment( - "warehouse.upload.failed", - tags=["reason:unsupported-platform-tag"], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Binary wheel '{filename}' has an unsupported " - f"platform tag '{tag.platform}'.", - ) + raise InvalidPlatformTagError(filename=filename, tag=tag.platform) if (canonical_name := packaging.utils.canonicalize_name(meta.name)) != name: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:invalid-filename-normalized", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Start filename for {project.name!r} with " - f"{canonical_name.replace('-', '_')!r}.", + raise UnnormalizedFilenameError( + project=project.name, + normalized=canonical_name.replace("-", "_"), + filetype=form.filetype.data, ) # The parse_wheel_filename function does not enforce lowercasing, @@ -1363,32 +1461,17 @@ def file_upload(request): # https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode normalized_name = project.normalized_name.replace("-", "_") if name_from_filename != normalized_name: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:invalid-filename-projectname", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Filename {filename!r} should contain the normalized " - f"project name {normalized_name!r}, not " - f"{name_from_filename!r}.", + raise UnnormalizedFilenameError( + project=project.name, + normalized=normalized_name, + filetype=form.filetype.data, ) if meta.version != version: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:invalid-filename-version", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Version in filename should be {str(meta.version)!r} not " - f"{str(version)!r}.", + raise InvalidFilenameVersionError( + expected=str(meta.version), + found=str(version), + filetype=form.filetype.data, ) filename = os.path.basename(temporary_filename) @@ -1411,36 +1494,20 @@ def file_upload(request): try: zfp.read(license_filename) except KeyError: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:missing-license-file", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - f"License-File {license_file} does not exist in " - f"distribution file {filename} at {license_filename}", + raise MissingLicenseFileError( + filename=filename, + license_file=license_file, + target_file=license_filename, + filetype=form.filetype.data, ) try: validate_record(temporary_filename) except MissingWheelRecordError: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:missing-record-file", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - "Wheel '{filename}' does not contain the required " - "RECORD file: {record_filename}".format( - filename=filename, - record_filename=f"{name}-{version}.dist-info/RECORD", - ), + raise MissingRecordFileError( + filename=filename, + record_filename=f"{name}-{version}.dist-info/RECORD", + filetype=form.filetype.data, ) except InvalidWheelRecordError: send_wheel_record_mismatch_email( @@ -1460,32 +1527,15 @@ def file_upload(request): with zipfile.ZipFile(temporary_filename) as zfp: wheel_metadata_contents = zfp.read(metadata_filename) except KeyError: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:missing-metadata-file", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Wheel '{filename}' does not contain the required " - f"METADATA file: {metadata_filename}", + raise MissingMetadataFileError( + filename=filename, metadata_filename=metadata_filename ) try: with open(temporary_filename + ".metadata", "wb") as fp: fp.write(wheel_metadata_contents) except OSError: - request.metrics.increment( - "warehouse.upload.failed", - tags=[ - "reason:filename-too-long", - f"filetype:{form.filetype.data}", - ], - ) - raise _exc_with_message( - HTTPBadRequest, - f"Filename is too long: '{filename}'", + raise FilenameTooLongError( + filename=filename, filetype=form.filetype.data ) metadata_file_hashes = { @@ -1514,13 +1564,7 @@ def file_upload(request): Distribution(name=filename, digest=file_hashes["sha256"]), ) except AttestationUploadError as e: - request.metrics.increment( - "warehouse.upload.failed", tags=["reason:invalid-attestations"] - ) - raise _exc_with_message( - HTTPBadRequest, - f"Invalid attestations supplied during upload: {e}", - ) + raise InvalidAttestationsError(msg=str(e)) # Store the information about the file in the database. file_ = File( @@ -1725,11 +1769,6 @@ def file_upload(request): if request.registry.settings.get("warehouse.release_files_table") is not None: request.task(update_bigquery_release_files).delay(dist_metadata) - # Log a successful upload - request.metrics.increment( - "warehouse.upload.ok", tags=[f"filetype:{form.filetype.data}"] - ) - # Dispatch our task to sync this to cache as soon as possible request.task(sync_file_to_cache).delay(file_.id) diff --git a/warehouse/forklift/metadata.py b/warehouse/forklift/metadata.py index cdbe895e1328..2fb18822e371 100644 --- a/warehouse/forklift/metadata.py +++ b/warehouse/forklift/metadata.py @@ -70,6 +70,10 @@ class NoMetadataError(Exception): pass +class InvalidLocalVersion(InvalidMetadata): + pass + + def parse( content: bytes | None, *, form_data: MultiDict | None = None, backfill: bool = False ) -> Metadata: @@ -115,7 +119,7 @@ def _validate_metadata(metadata: Metadata, *, backfill: bool = False): # We don't allow the use of the "local version" field when releasing to PyPI if metadata.version.local: errors.append( - InvalidMetadata( + InvalidLocalVersion( "version", f"The use of local versions in '{metadata.version}' is not allowed.", ) diff --git a/warehouse/forklift/utils.py b/warehouse/forklift/utils.py deleted file mode 100644 index 52aa8a14f0c3..000000000000 --- a/warehouse/forklift/utils.py +++ /dev/null @@ -1,18 +0,0 @@ -# SPDX-License-Identifier: Apache-2.0 - -import sentry_sdk - - -def _exc_with_message(exc, message, **kwargs): - if not message: - sentry_sdk.capture_message("Attempting to _exc_with_message without a message") - - # The crappy old API that PyPI offered uses the status to pass down - # messages to the client. So this function will make that easier to do. - resp = exc(detail=message, **kwargs) - # We need to guard against characters outside of iso-8859-1 per RFC. - # Specifically here, where user-supplied text may appear in the message, - # which our WSGI server may not appropriately handle (indeed gunicorn does not). - status_message = message.encode("iso-8859-1", "replace").decode("iso-8859-1") - resp.status = f"{resp.status_code} {status_message}" - return resp