[BaptisteCentreon]

When a crate is published, its dependencies come from the Cargo.toml inside the .crate tarball. For a crates.io dependency, that Cargo.toml has no registry key at all.

The old code wrote registry: null for a bare-string dependency and read a non-existent registry key for the table form. The index spec requires a non-crates.io registry to translate null into the crates.io index URL, otherwise cargo resolves those dependencies against the local registry and the build fails.

This change writes the crates.io index URL for bare-string dependencies, and reads registry-index (the packaged key that holds an index URL) for the table form, falling back to the crates.io index URL when it is absent.

[dralley]

@BaptisteCentreon There are two failing tests, do you plan on looking into that?

[dralley]

Review from Codex, acting on behalf of @dralley.

I confirmed the core fix is correct. Cargo.toml treats an omitted registry as crates.io, but the registry API/index uses null or an omitted field to mean the registry currently serving the package. Cargo's own publish code explicitly converts a crates.io dependency to the crates.io index URL when publishing to a different registry. Because Pulp derives dependencies from the packaged Cargo.toml rather than trusting the request metadata, this translation is needed here.

The failing matrix is an expected test mismatch, not evidence against the fix: the two failing functional tests compare Pulp's new dependency records byte-for-byte with the crates.io index entry, whose dependencies omit registry. Update that comparison so an omitted/null upstream registry is normalized to the crates.io index URL for this alternate-registry publish case (or give the helper an explicit expected transformation). Also add an end-to-end test that publishes a crate with an ordinary crates.io dependency, then uses Cargo to fetch/build it from Pulp.

One further concern: the new test_table_spec_registry_name_is_ignored encodes a dangerous fallback. Cargo packaging rewrites a named alternate registry to registry-index, so normal Cargo uploads are covered. But a handcrafted tarball with registry = "some-name" cannot be resolved by Pulp without the client configuration that maps that name to a URL. Silently treating it as crates.io can resolve the wrong dependency; reject it (or explicitly support it) instead.

[dralley]

Instead of passing a string, why not add __init__ to UnsupportedDependencyError and then pass name and registry as arguments? A bit more idiomatic when defining new error types.

[dralley]

So, this is now parsing the binary payload within the request, which is something we generally try to avoid - on top of that it's happening twice because it also happens within the apublish_package task

[dralley]

how does this look (should apply on top of this PR)

diff --git a/pulp_rust/app/tasks/publishing.py b/pulp_rust/app/tasks/publishing.py
index 1204a9b..a653d77 100644
--- a/pulp_rust/app/tasks/publishing.py
+++ b/pulp_rust/app/tasks/publishing.py
@@ -1,5 +1,6 @@
 import hashlib
 import struct
+import tarfile
 
 from django.db import IntegrityError
 
@@ -8,6 +9,7 @@ from pulpcore.plugin.tasking import aadd_and_remove
 
 from pulp_rust.app.models import RustContent, RustDependency, RustRepository
 from pulp_rust.app.utils import (
+    UnsupportedDependencyError,
     canonicalize_crate_name,
     extract_cargo_toml,
     extract_dependencies,
@@ -60,6 +62,21 @@ async def apublish_package(repository_pk, metadata, crate_path):
         metadata: Parsed JSON metadata from the cargo publish request.
         crate_path: Filesystem path to the .crate tarball.
     """
+    # Extract authoritative metadata from the Cargo.toml inside the .crate tarball.
+    # The publish JSON metadata is NOT authoritative - a rogue client can send metadata
+    # that doesn't match the actual package. We only use the JSON name/vers to locate the
+    # Cargo.toml within the tarball, then extract everything from the Cargo.toml itself.
+    # See: https://github.com/rust-lang/cargo/issues/14492
+    #      https://github.com/rust-lang/crates.io/pull/7238
+    try:
+        with open(crate_path, "rb") as f:
+            cargo_toml = extract_cargo_toml(f, metadata["name"], metadata["vers"])
+        deps = extract_dependencies(cargo_toml)
+    except UnsupportedDependencyError as e:
+        return {"cargo_error": str(e)}
+    except (tarfile.TarError, EOFError, KeyError, FileNotFoundError, ValueError):
+        return {"cargo_error": "invalid crate file"}
+
     repository = await RustRepository.objects.aget(pk=repository_pk)
 
     # Create the artifact from the .crate file, or reuse an existing one
@@ -73,14 +90,6 @@ async def apublish_package(repository_pk, metadata, crate_path):
     except IntegrityError:
         artifact = await Artifact.objects.aget(sha256=cksum)
 
-    # Extract authoritative metadata from the Cargo.toml inside the .crate tarball.
-    # The publish JSON metadata is NOT authoritative - a rogue client can send metadata
-    # that doesn't match the actual package. We only use the JSON name/vers to locate the
-    # Cargo.toml within the tarball, then extract everything from the Cargo.toml itself.
-    # See: https://github.com/rust-lang/cargo/issues/14492
-    #      https://github.com/rust-lang/crates.io/pull/7238
-    with artifact.file.open("rb") as f:
-        cargo_toml = extract_cargo_toml(f, metadata["name"], metadata["vers"])
     package = cargo_toml.get("package", {})
 
     name = package["name"]
@@ -89,9 +98,6 @@ async def apublish_package(repository_pk, metadata, crate_path):
     # build metadata as identical, and the index must not contain duplicates.
     vers = strip_semver_build_metadata(package["version"])
 
-    # Build dependency list from the Cargo.toml (authoritative source)
-    deps = extract_dependencies(cargo_toml)
-
     # Reuse existing content if it already exists in the domain with the same
     # checksum (e.g. from a pull-through cache or another repository's publish).
     # Content in Pulp is globally shared - the same object can belong to
diff --git a/pulp_rust/app/utils.py b/pulp_rust/app/utils.py
index 749391b..fd2df45 100644
--- a/pulp_rust/app/utils.py
+++ b/pulp_rust/app/utils.py
@@ -43,6 +43,13 @@ CRATES_IO_INDEX = "https://github.com/rust-lang/crates.io-index"
 class UnsupportedDependencyError(ValueError):
     """A Cargo.toml dependency that cannot be translated to an index entry."""
 
+    def __init__(self, name, registry):
+        self.name = name
+        self.registry = registry
+        super().__init__(
+            f"dependency `{name}` names registry `{registry}` without a `registry-index` URL"
+        )
+
 
 def parse_dep(name, spec, kind="normal", target=None):
     """Convert a single Cargo.toml dependency entry to index format."""
@@ -65,10 +72,7 @@ def parse_dep(name, spec, kind="normal", target=None):
         # `cargo package` rewrites a named registry to `registry-index`. A bare name can only
         # be resolved with the client's configuration, so treating it as crates.io could
         # point at the wrong crate.
-        raise UnsupportedDependencyError(
-            f"dependency `{name}` names registry `{spec['registry']}` without a "
-            "`registry-index` URL"
-        )
+        raise UnsupportedDependencyError(name, spec["registry"])
     dep = {
         "name": name,
         "req": _normalize_req(spec.get("version", "*")),
diff --git a/pulp_rust/app/views.py b/pulp_rust/app/views.py
index 2cfdc34..985b773 100644
--- a/pulp_rust/app/views.py
+++ b/pulp_rust/app/views.py
@@ -1,9 +1,7 @@
-import io
 import json
 import logging
 import os
 import struct
-import tarfile
 import tempfile
 import urllib.error
 import urllib.request
@@ -45,10 +43,7 @@ from pulp_rust.app.tasks import (
     parse_cargo_publish_body,
 )
 from pulp_rust.app.utils import (
-    UnsupportedDependencyError,
     canonicalize_crate_name,
-    extract_cargo_toml,
-    extract_dependencies,
     strip_semver_build_metadata,
     validate_crate_name,
     validate_crate_version,
@@ -419,13 +414,6 @@ class CargoPublishApiView(APIView):
         if error := token_scope_error(request.auth, action, distro):
             return error
 
-        try:
-            extract_dependencies(extract_cargo_toml(io.BytesIO(crate_bytes), name, vers))
-        except UnsupportedDependencyError as e:
-            return cargo_error(str(e))
-        except (tarfile.TarError, EOFError, KeyError, FileNotFoundError, ValueError):
-            return cargo_error("invalid crate file")
-
         # Write the .crate bytes to a temp file — raw bytes can't be passed
         # through dispatch() because task kwargs are stored as JSON.
         tmp = tempfile.NamedTemporaryFile(suffix=".crate", delete=False)
@@ -443,7 +431,9 @@ class CargoPublishApiView(APIView):
                     "crate_path": tmp.name,
                 },
             )
-            has_task_completed(task)
+            result = has_task_completed(task)
+            if result and (error := result.get("cargo_error")):
+                return cargo_error(error)
         finally:
             os.unlink(tmp.name)
 
@@ -589,7 +579,7 @@ def has_task_completed(task):
     Verify whether an immediate task ran properly.
 
     Returns:
-        bool: True if the task ended successfully.
+        dict | None: The result returned by a completed task.
 
     Raises:
         Exception: If an error occured during the task's runtime.
@@ -597,8 +587,9 @@ def has_task_completed(task):
 
     """
     if task.state == "completed":
+        result = task.result
         task.delete()
-        return True
+        return result
     elif task.state == "canceled":
         raise Throttled()
     else:
diff --git a/pulp_rust/tests/functional/api/test_publish.py b/pulp_rust/tests/functional/api/test_publish.py
index 4f6a681..4a245fa 100644
--- a/pulp_rust/tests/functional/api/test_publish.py
+++ b/pulp_rust/tests/functional/api/test_publish.py
@@ -205,6 +205,24 @@ def test_cargo_publish_uploads_disabled(
     assert any("does not allow uploads" in e["detail"] for e in errors)
 
 
+def test_cargo_publish_invalid_crate_file_rejected(
+    rust_repo_factory,
+    rust_distribution_factory,
+    cargo_registry_url,
+    admin_auth_headers,
+):
+    """An invalid crate file should produce Cargo's normal error response."""
+    repository = rust_repo_factory()
+    distribution = rust_distribution_factory(repository=repository.pulp_href, allow_uploads=True)
+    base = cargo_registry_url(distribution.base_path)
+
+    metadata = {"name": "invalid-crate", "vers": "0.1.0", "deps": [], "features": {}}
+    response = cargo_publish(base, metadata, b"not a crate", headers=admin_auth_headers)
+
+    assert response.status_code == 400
+    assert response.json()["errors"][0]["detail"] == "invalid crate file"
+
+
 def test_cargo_publish_named_registry_dependency_rejected(
     rust_repo_factory,
     rust_distribution_factory,

[dralley]

Also I'm very sorry for the delays. We're starting to get into the holidays and it's been very busy with outages, security issues to deal with, etc.

[pkriko]

@dralley No worries :)
Thank you for the patch!