refactor: build manifestTypes from named media-type constants (de-duplicate literals)#140
Open
m-ferrero wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 bydockerManifestListContentTypeinsrc/manifest.ts(and used there to validate a manifest'smediaType).application/vnd.oci.image.index.v1+json— already held byociImageIndexContentTypeinsrc/registry/r2.ts, which is even imported intohttp.tsand used for index handling — yet the accept-list re-types the literal instead of using it.application/vnd.oci.image.manifest.v1+jsonhas 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 buildmanifestTypesfrom 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 —
Acceptis emitted asmanifestTypes.join(", ")),as constis kept so theManifestTypeunion resolves to the identical set, and no runtime behavior changes.Tests
Adds
test/media-types.test.tsassertingmanifestTypesholds the expected set in the expected order, plus each constant's value.Part of #134.