From 9550d8132b45fa123eeda33f132cd1e1fa6e2452 Mon Sep 17 00:00:00 2001 From: Danilo Lakovic Date: Thu, 29 Oct 2026 13:43:05 +0100 Subject: [PATCH] Announce no metadata on pull-through links A package not yet in the repository is linked as ?redirect=, and the link announced its PEP 658 metadata. pip asks for that metadata by appending ".metadata" to the whole URL, so it lands in the redirect parameter and the path still names the package. Once another client has fetched the package, the content app finds it by that path and answers the metadata request with the package itself, and pip fails its hash check. The pull-through handler's workaround for pip's URL is never reached in that case, because the path lookup comes first. Pull-through links no longer announce metadata; installers then read it from the package they download. Once the package is in the repository, its local link announces the metadata again, and that link has no query string. The metadata is still served through a pull-through link for clients holding an older page. fixes #1453 --- CHANGES/1453.bugfix | 1 + pulp_python/app/pypi/views.py | 15 ++++++- .../tests/functional/api/test_full_mirror.py | 45 ++++++++++++------- 3 files changed, 42 insertions(+), 19 deletions(-) create mode 100644 CHANGES/1453.bugfix diff --git a/CHANGES/1453.bugfix b/CHANGES/1453.bugfix new file mode 100644 index 00000000..6a6cc926 --- /dev/null +++ b/CHANGES/1453.bugfix @@ -0,0 +1 @@ +Stopped announcing PEP 658 metadata on pull-through links, whose ?redirect= query made pip ask for the metadata at the package's own path: once the package was cached, pip got the package in place of its metadata and failed its hash check. diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index 8d580a2f..c2a45e12 100644 --- a/pulp_python/app/pypi/views.py +++ b/pulp_python/app/pypi/views.py @@ -81,12 +81,18 @@ def _get_repo_version(path): return None +# Part of the ETag of the simple pages. Change it whenever the pages render differently for +# the same repository version, so a client revalidating a page cached before the change gets +# the new page instead of a 304. +SIMPLE_PAGE_REVISION = "pull-through-links-without-metadata" + + def _etag_func(request, path, **kwargs): """Compute unquoted ETag for the condition decorator. Returns None if no repo.""" repo_ver = _get_repo_version(path) if repo_ver is None: return None - raw = f"{repo_ver.number}:{repo_ver.pulp_created.isoformat()}" + raw = f"{repo_ver.number}:{repo_ver.pulp_created.isoformat()}:{SIMPLE_PAGE_REVISION}" return hashlib.sha256(raw.encode()).hexdigest()[:16] @@ -378,7 +384,12 @@ def parse_package(release_package): "url": d_url, "sha256": release_package.digests.get("sha256", ""), "requires_python": release_package.requires_python, - "metadata_sha256": (release_package.metadata_digests or {}).get("sha256"), + # No PEP 658 metadata for a file served through this redirect link: pip asks for + # it by appending ".metadata" to the whole URL, which leaves the path naming the + # file itself. Once the file is in the repository, a client holding this link is + # served the file by its path and gets it in place of the metadata. Installers + # fall back to the metadata inside the file they download anyway. + "metadata_sha256": None, "size": release_package.size, "upload_time": release_package.upload_time, "version": release_package.version, diff --git a/pulp_python/tests/functional/api/test_full_mirror.py b/pulp_python/tests/functional/api/test_full_mirror.py index e89cdac2..d1a7f1e7 100644 --- a/pulp_python/tests/functional/api/test_full_mirror.py +++ b/pulp_python/tests/functional/api/test_full_mirror.py @@ -208,36 +208,43 @@ def test_pull_through_filtering_bad_names(python_remote_factory, python_distribu @pytest.mark.parallel def test_pull_through_metadata(python_remote_factory, python_distribution_factory): """ - Tests that metadata is correctly served when using pull-through. + Tests that a pull-through link announces no metadata, but the metadata is still served. - So when requesting the metadata url according to PEP 658 you should just need to add .metadata - to the end of the url path. Since pull-through includes a redirect query parameter we need to - test adding .metadata to the end of the url path vs adding it to the end of redirect query. + pip asks for PEP 658 metadata by appending ".metadata" to the end of the url, which puts it + into the redirect query parameter and leaves the path naming the package. Once the package + is in a repository, that request is answered with the package by its path, so the link must + not announce metadata. A client holding an older page still gets the metadata either way. """ remote = python_remote_factory(includes=["pytz"]) distro = python_distribution_factory(remote=remote.pulp_href) url = f"{distro.base_url}simple/pytz/" project_page = ProjectPage.from_response(requests.get(url), "pytz") + upstream_page = ProjectPage.from_response( + requests.get(urljoin(PYPI_URL, "simple/pytz/")), "pytz" + ) filename1 = "pytz-2023.2-py2.py3-none-any.whl" filename2 = "pytz-2023.3-py2.py3-none-any.whl" package1 = next(p for p in project_page.packages if p.filename == filename1) package2 = next(p for p in project_page.packages if p.filename == filename2) - assert package1.has_metadata - assert package2.has_metadata + assert "?redirect=" in package1.url + assert not package1.has_metadata + assert not package2.has_metadata + upstream1 = next(p for p in upstream_page.packages if p.filename == filename1) + upstream2 = next(p for p in upstream_page.packages if p.filename == filename2) - # The correct way to get the metadata url: add to path (uv does this) + # Adding .metadata to the url path (uv does this) parts1 = urlsplit(package1.url) url1 = urlunsplit((parts1[0], parts1[1], parts1[2] + ".metadata", parts1[3], parts1[4])) r = requests.get(url1) assert r.status_code == 200 - assert sha256(r.content).hexdigest() == package1.metadata_digests["sha256"] + assert sha256(r.content).hexdigest() == upstream1.metadata_digests["sha256"] - # The incorrect way to get the metadata url: add to end of string (pip does this) + # Adding .metadata to the end of the url (pip does this) url2 = package2.url + ".metadata" r = requests.get(url2) assert r.status_code == 200 - assert sha256(r.content).hexdigest() == package2.metadata_digests["sha256"] + assert sha256(r.content).hexdigest() == upstream2.metadata_digests["sha256"] @pytest.mark.parallel @@ -247,7 +254,7 @@ def test_pull_through_metadata_with_repo( python_distribution_factory, pulpcore_bindings, ): - """Tests that metadata is correctly saved when using pull-through with a repository.""" + """Tests that metadata is correctly saved and announced when using pull-through with a repo.""" remote = python_remote_factory(url=PYPI_URL, includes=["pip"]) repo = python_repo_factory() distro = python_distribution_factory(repository=repo.pulp_href, remote=remote.pulp_href) @@ -256,32 +263,36 @@ def test_pull_through_metadata_with_repo( project_page = ProjectPage.from_response(requests.get(pip_url), "pip") filename = "pip-26.0.1-py3-none-any.whl" package = next(p for p in project_page.packages if p.filename == filename) - assert package.has_metadata assert "?redirect=" in package.url + assert not package.has_metadata + upstream_page = ProjectPage.from_response(requests.get(urljoin(PYPI_URL, "simple/pip/")), "pip") + upstream = next(p for p in upstream_page.packages if p.filename == filename) + metadata_sha256 = upstream.metadata_digests["sha256"] # Retrieve the metadata and assert the content was not saved to the repository parts = urlsplit(package.url) url = urlunsplit((parts[0], parts[1], parts[2] + ".metadata", parts[3], parts[4])) r = requests.get(url) assert r.status_code == 200 - assert sha256(r.content).hexdigest() == package.metadata_digests["sha256"] + assert sha256(r.content).hexdigest() == metadata_sha256 project_page = ProjectPage.from_response(requests.get(pip_url), "pip") package = next(p for p in project_page.packages if p.filename == filename) - assert package.has_metadata assert "?redirect=" in package.url + assert not package.has_metadata # Now retrieve the package and assert the content was saved with metadata r = requests.get(package.url) assert r.status_code == 200 pa = pulpcore_bindings.ArtifactsApi.list(sha256=package.digests["sha256"]) assert pa.count == 1 - ma = pulpcore_bindings.ArtifactsApi.list(sha256=package.metadata_digests["sha256"]) + ma = pulpcore_bindings.ArtifactsApi.list(sha256=metadata_sha256) assert ma.count == 1 - # Check the simple page is updated to point to the local repository + # Check the simple page is updated to point to the local repository, with the metadata project_page = ProjectPage.from_response(requests.get(pip_url), "pip") package = next(p for p in project_page.packages if p.filename == filename) assert "?redirect=" not in package.url + assert package.metadata_digests["sha256"] == metadata_sha256 r = requests.get(package.metadata_url) assert r.status_code == 200 - assert sha256(r.content).hexdigest() == package.metadata_digests["sha256"] + assert sha256(r.content).hexdigest() == metadata_sha256