Skip to content

refactor: build manifestTypes from named media-type constants (de-duplicate literals)#140

Open
m-ferrero wants to merge 1 commit into
cloudflare:mainfrom
aggrega-ai:refactor/manifest-media-type-consts
Open

refactor: build manifestTypes from named media-type constants (de-duplicate literals)#140
m-ferrero wants to merge 1 commit into
cloudflare:mainfrom
aggrega-ai:refactor/manifest-media-type-consts

Conversation

@m-ferrero

Copy link
Copy Markdown

Problem

The manifest content-negotiation accept-list manifestTypes (src/registry/http.ts) is built from bare string literals, but named constants for two of those exact media types already exist and are bypassed:

  • application/vnd.docker.distribution.manifest.list.v2+json — already held by dockerManifestListContentType in src/manifest.ts (and used there to validate a manifest's mediaType).
  • application/vnd.oci.image.index.v1+json — already held by ociImageIndexContentType in src/registry/r2.ts, which is even imported into http.ts and used for index handling — yet the accept-list re-types the literal instead of using it.
  • application/vnd.oci.image.manifest.v1+json has no named constant at all.

These strings are the OCI/Docker content-negotiation contract and must stay byte-identical to the spec. With the same value living in two places, editing one (say, as the spec evolves) silently diverges from the other: the index-handling path would use the updated constant while the accept-list kept the stale literal, so a client sending that type would be rejected at negotiation even though the handler was ready for it — a failure a happy-path test using one consistent string would not catch.

Solution

Add a single src/media-types.ts (a zero-import leaf module) that defines every manifest media-type constant exactly once, and build manifestTypes from those constants. The existing consumers — src/manifest.ts, src/registry/r2.ts, src/registry/http.ts, src/router.ts — all import from that one source.

Pure, behavior-preserving refactor: the string values and the array order are unchanged (order matters — Accept is emitted as manifestTypes.join(", ")), as const is kept so the ManifestType union resolves to the identical set, and no runtime behavior changes.

Tests

Adds test/media-types.test.ts asserting manifestTypes holds the expected set in the expected order, plus each constant's value.

Part of #134.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant