From c58e7d13caff802c188b851e18c0a107ce6ccd03 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 24 Jan 2025 14:45:29 -0500 Subject: [PATCH 1/4] integrity: refine Accept header handling See https://github.com/pypi/warehouse/issues/17084. Signed-off-by: William Woodruff --- tests/unit/api/test_integrity.py | 61 ++++++++++++++++++++++++++++---- warehouse/api/integrity.py | 15 ++++---- warehouse/api/simple.py | 2 +- 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/tests/unit/api/test_integrity.py b/tests/unit/api/test_integrity.py index 10251e6c2ac4..c39500891a4f 100644 --- a/tests/unit/api/test_integrity.py +++ b/tests/unit/api/test_integrity.py @@ -16,13 +16,53 @@ from warehouse.api import integrity -def test_select_content_type(db_request): - db_request.accept = "application/json" +@pytest.mark.parametrize( + ("accept", "expected"), + [ + # Simple cases + ( + "application/vnd.pypi.integrity.v1+json", + integrity.MIME_PYPI_INTEGRITY_V1_JSON, + ), + ("application/json", integrity.MIME_APPLICATION_JSON), + # No accept header means we give the user our first offer + (None, integrity.MIME_PYPI_INTEGRITY_V1_JSON), + # Accept header contains only things we don't offer + ("text/xml", None), + ("application/octet-stream", None), + ("text/xml, application/octet-stream", None), + # Accept header contains both things we offer and things we don't; + # we pick our matching offer even if the q-value is lower + ( + "text/xml, application/vnd.pypi.integrity.v1+json", + integrity.MIME_PYPI_INTEGRITY_V1_JSON, + ), + ( + "application/vnd.pypi.integrity.v1+json; q=0.1, text/xml", + integrity.MIME_PYPI_INTEGRITY_V1_JSON, + ), + # Accept header contains multiple things we offer with the same q-value; + # we pick our preferred offer + ( + "application/json, application/vnd.pypi.integrity.v1+json", + integrity.MIME_PYPI_INTEGRITY_V1_JSON, + ), + ( + "application/vnd.pypi.integrity.v1+json; q=0.5, application/json; q=0.5", + integrity.MIME_PYPI_INTEGRITY_V1_JSON, + ), + # Accept header contains multiple things we offer; we pick our + # offer based on the q-value + ( + "application/vnd.pypi.integrity.v1+json; q=0.1, application/json", + integrity.MIME_APPLICATION_JSON, + ), + ], +) +def test_select_content_type(db_request, accept, expected): + db_request.accept = accept - assert ( - integrity._select_content_type(db_request) - == integrity.MIME_PYPI_INTEGRITY_V1_JSON - ) + assert integrity._select_content_type(db_request) == expected # Backstop; can be removed/changed once this view supports HTML. @@ -37,6 +77,15 @@ def test_provenance_for_file_bad_accept(db_request, content_type): assert response.json == {"message": "Request not acceptable"} +def test_provenance_for_file_accept_multiple(db_request, monkeypatch): + db_request.accept = "text/html, application/vnd.pypi.integrity.v1+json; q=0.9" + file = pretend.stub(provenance=None, filename="fake-1.2.3.tar.gz") + + response = integrity.provenance_for_file(file, db_request) + assert response.status_code == 404 + assert response.json == {"message": "No provenance available for fake-1.2.3.tar.gz"} + + def test_provenance_for_file_not_enabled(db_request, monkeypatch): monkeypatch.setattr(db_request, "flags", pretend.stub(enabled=lambda *a: True)) diff --git a/warehouse/api/integrity.py b/warehouse/api/integrity.py index cecf3bc33d0f..8c5a13b764ea 100644 --- a/warehouse/api/integrity.py +++ b/warehouse/api/integrity.py @@ -21,23 +21,26 @@ from warehouse.utils.cors import _CORS_HEADERS MIME_TEXT_HTML = "text/html" +MIME_APPLICATION_JSON = "application/json" MIME_PYPI_INTEGRITY_V1_HTML = "application/vnd.pypi.integrity.v1+html" MIME_PYPI_INTEGRITY_V1_JSON = "application/vnd.pypi.integrity.v1+json" -def _select_content_type(request: Request) -> str: +def _select_content_type(request: Request) -> str | None: offers = request.accept.acceptable_offers( [ # JSON currently has the highest priority. MIME_PYPI_INTEGRITY_V1_JSON, - MIME_TEXT_HTML, - MIME_PYPI_INTEGRITY_V1_HTML, + MIME_APPLICATION_JSON, + # Ensures that we fall back to the first offer if + # the client does not provide an Accept header. + "identity", ] ) - # Default case: JSON. + # Client provided an Accept header, but none of the offers matched. if not offers: - return MIME_PYPI_INTEGRITY_V1_JSON + return None else: return offers[0][0] @@ -63,7 +66,7 @@ def provenance_for_file(file: File, request: Request): # Determine our response content-type. For the time being, only the JSON # type is accepted. request.response.content_type = _select_content_type(request) - if request.response.content_type != MIME_PYPI_INTEGRITY_V1_JSON: + if not request.response.content_type: return HTTPNotAcceptable(json={"message": "Request not acceptable"}) if request.flags.enabled(AdminFlagValue.DISABLE_PEP740): diff --git a/warehouse/api/simple.py b/warehouse/api/simple.py index a7882af67c4b..0d836fff5692 100644 --- a/warehouse/api/simple.py +++ b/warehouse/api/simple.py @@ -50,7 +50,7 @@ def _select_content_type(request: Request) -> str: ] ) - # Default case, we want to return whatevr we want to return + # Default case, we want to return whatever we want to return # by default when there is no Accept header. if not offers: return MIME_TEXT_HTML From dfd4d78ed225c88f88e2ff4a916530456d338f8e Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Fri, 24 Jan 2025 16:49:50 -0500 Subject: [PATCH 2/4] remove unneeded identity fallback Signed-off-by: William Woodruff --- warehouse/api/integrity.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/warehouse/api/integrity.py b/warehouse/api/integrity.py index 8c5a13b764ea..b71d6235dd79 100644 --- a/warehouse/api/integrity.py +++ b/warehouse/api/integrity.py @@ -32,9 +32,6 @@ def _select_content_type(request: Request) -> str | None: # JSON currently has the highest priority. MIME_PYPI_INTEGRITY_V1_JSON, MIME_APPLICATION_JSON, - # Ensures that we fall back to the first offer if - # the client does not provide an Accept header. - "identity", ] ) From 488d28959aadf6c9228e728a7bd57223071fb02a Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Mon, 27 Jan 2025 12:05:52 -0500 Subject: [PATCH 3/4] remove unused MIME types Signed-off-by: William Woodruff --- warehouse/api/integrity.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/warehouse/api/integrity.py b/warehouse/api/integrity.py index b71d6235dd79..8e8c6bf18db4 100644 --- a/warehouse/api/integrity.py +++ b/warehouse/api/integrity.py @@ -20,9 +20,7 @@ from warehouse.packaging.models import File from warehouse.utils.cors import _CORS_HEADERS -MIME_TEXT_HTML = "text/html" MIME_APPLICATION_JSON = "application/json" -MIME_PYPI_INTEGRITY_V1_HTML = "application/vnd.pypi.integrity.v1+html" MIME_PYPI_INTEGRITY_V1_JSON = "application/vnd.pypi.integrity.v1+json" From 4ef5f08b88bc945bc85eb5bc8f0146b9a2f20ca0 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Mon, 27 Jan 2025 12:07:47 -0500 Subject: [PATCH 4/4] remove HTML mime type uses Signed-off-by: William Woodruff --- tests/unit/api/test_integrity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/api/test_integrity.py b/tests/unit/api/test_integrity.py index c39500891a4f..dcfd82aa0137 100644 --- a/tests/unit/api/test_integrity.py +++ b/tests/unit/api/test_integrity.py @@ -68,7 +68,7 @@ def test_select_content_type(db_request, accept, expected): # Backstop; can be removed/changed once this view supports HTML. @pytest.mark.parametrize( "content_type", - [integrity.MIME_TEXT_HTML, integrity.MIME_PYPI_INTEGRITY_V1_HTML], + ["text/html", "application/vnd.pypi.integrity.v1+html"], ) def test_provenance_for_file_bad_accept(db_request, content_type): db_request.accept = content_type