fix: forward complete Accept header set to upstream#59
Merged
Conversation
The proxy read r.Header.Get("Accept"), which returns only the first of
multiple Accept request headers. Clients that emit more than one Accept
(notably Bazel, where Java's HttpURLConnection injects a default
"text/html, image/gif, image/jpeg, */*" first and rules_oci appends the
real media-type Accept second) had their real Accept dropped before the
upstream fetch.
This breaks OCI-format manifests on registries that strictly content-
negotiate (nvcr.io), which return 404 unless Accept explicitly contains
application/vnd.oci.image.manifest.v1+json -- they do not honor */*.
Surfaced when NVIDIA's dcgm-exporter switched from Docker-schema2 to OCI
image manifests in v4.4.2.
Join all Accept values per RFC 9110 §5.3 (multiple field lines are
semantically one comma-separated header) via a shared acceptHeader
helper, used for both the forwarded value and the OCI cache variant so
the cache key matches what is sent upstream.
Lint verified clean (golangci-lint, 0 issues) under go 1.26.4; committed
with --no-verify because the prek golangci-lint hook runs GOTOOLCHAIN=local
against a stale go 1.26.3 base toolchain and fails at package-loading,
unrelated to this change.
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
1 participant
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.
server.goreadr.Header.Get("Accept"), which returns only the first of multipleAcceptrequest headers, and forwarded that single value upstream. Clients that send more than oneAcceptlost the rest. Bazel does this: Java'sHttpURLConnectioninjects a defaultAccept: text/html, image/gif, image/jpeg, */*first, andrules_ociappends the real media-typeAcceptsecond.This breaks OCI-format manifests on registries that strictly content-negotiate. A by-digest manifest GET to nvcr.io returns 404 unless
Acceptcontains the manifest's exact stored media type (application/vnd.oci.image.manifest.v1+json); it does not honor*/*. The proxy forwarded only the Java default, so nvcr.io 404'd. It surfaced when NVIDIA's dcgm-exporter moved from Docker-schema2 to OCI image manifests in v4.4.2, where abazel buildof anoci_pulltarget failed at fetch.Change
acceptHeaderhelper that joins allAcceptfield lines into one comma-separated value, per RFC 9110 §5.3.The cache variant now keys on the real negotiated
Acceptset instead of the constant Java default, so OCI pulls no longer collide into a single variant bucket per URL.Tests
Two regression tests for multiple inbound
Acceptheaders:ociAwareKeyFunc's variant, and end-to-end upstream forwarding. Both assert the second value survives.