Skip to content
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

Include headers when serving blob through proxy #4273

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mikelr
Copy link

@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>
@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 a review from squizzi May 4, 2024 14:15
@milosgajdos milosgajdos added this to the Registry/3.0.0 milestone May 4, 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.

None yet

3 participants