8000 Include headers when serving blob through proxy by mikelr · Pull Request #4273 · distribution/distribution · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Include headers when serving blob through proxy #4273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2024

Conversation

mikelr
Copy link
Contributor
@mikelr mikelr commented Feb 1, 2024

In commit 1795292 we updated ServeBlob() to use an io.MultiWriter to write simultaneously to the local store and the HTTP response.

However, copyContent was using a type assertion to only add headers if the io.Writer was a http.ResponseWriter. Therefore, this change caused us to stop sending the expected headers (i.e. Content-Length, Etag, etc.) on the first request for a blob.

Resolve the issue by explicitly passing in http.Header and setting it unconditionally.

@github-actions github-actions bot added the area/proxy Related to registry as a pull-through cache label Feb 1, 2024
In commit 1795292 we updated ServeBlob() to use an io.MultiWriter to
write simultaneously to the local store and the HTTP response.

However, copyContent was using a type assertion to only add headers if
the io.Writer was a http.ResponseWriter. Therefore, this change caused
us to stop sending the expected headers (i.e. Content-Length, Etag,
etc.) on the first request for a blob.

Resolve the issue by explicitly passing in http.Header and setting it
unconditionally.

Signed-off-by: Mikel Rychliski <mikel@mikelr.com>
@mikelr mikelr force-pushed the proxy-blob-headers branch from e82ac3b to 0418245 Compare February 2, 2024 00:34
@ialidzhikov
Copy link
Contributor

/cc @milosgajdos @Jamstah

@milosgajdos
Copy link
Member

I'm going to close this and reopen as for some reason GH doesn't allow me to approve running the CI

Unable to re-run one or more workflows because they were created over a month ago.

@milosgajdos milosgajdos closed this Mar 2, 2024
@milosgajdos milosgajdos reopened this Mar 2, 2024
Copy link
Member
@milosgajdos milosgajdos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch! Thanks for the fix.

@milosgajdos milosgajdos requested review from Jamstah and thaJeztah March 2, 2024 08:18
@milosgajdos milosgajdos requested a review from squizzi May 4, 2024 14:15
@milosgajdos milosgajdos added this to the Registry/3.0.0 milestone May 4, 2024
@milosgajdos
Copy link
Member

PTAL @thaJeztah @Jamstah @squizzi

@milosgajdos milosgajdos requested a review from corhere May 13, 2024 16:32
Jamstah
Jamstah approved these changes May 14, 2024 < B7C5 /div>
Copy link
Collaborator
@Jamstah Jamstah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot, thanks.

@milosgajdos milosgajdos merged commit 2c6b648 into distribution:main May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Related to registry as a pull-through cache
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0