Skip to content

Commit

Permalink
Stop using real headers for side channels (#1367)
Browse files Browse the repository at this point in the history
I thought it was clever to use If-None-Match because it was pretty close
to what I wanted, but the way we're wrapping transports means that this
actually breaks if the Etag matches. Whoops!

Just change this to an invalid header so that this stops breaking, we
can fix it better later by making the cache not look like a transport.

Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
  • Loading branch information
jonjohnsonjr authored Oct 24, 2024
1 parent 395e822 commit f60d40e
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions pkg/apk/apk/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func (t *cacheTransport) get(ctx context.Context, request *http.Request, cacheFi
}

func (t *cacheTransport) fetchAndCache(ctx context.Context, request *http.Request, cacheFile string) (*http.Response, error) {
initialEtag := request.Header.Get("If-None-Match")
initialEtag := request.Header.Get("I-Cant-Believe-Its-Not-If-None-Match")
if initialEtag == "" {
resp, err := t.head(request, cacheFile)
if err != nil {
Expand All @@ -232,7 +232,7 @@ func (t *cacheTransport) fetchAndCache(ctx context.Context, request *http.Reques
// Since we already send a HEAD request to get that etag, we want to avoid sending a redundant HEAD request above if we can.
// However, we don't actually want to pass along this header because it's using the "parsed" version of the Etag from
// etagFromResponse and doesn't actually attempt to follow HTTP semantics, so we remove it here to avoid any confusion.
request.Header.Del("If-None-Match")
request.Header.Del("I-Cant-Believe-Its-Not-If-None-Match")

etagFile, err := t.get(ctx, request, cacheFile, initialEtag)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/apk/apk/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func fetchRepositoryIndex(ctx context.Context, u string, etag string, opts *inde
// also really wants to do a HEAD request in order to do etag-based caching itself. To avoid the double HEAD,
// I'm stuffing the etag into the If-None-Match header, which isn't exactly correct semantics but it rhymes.
// The alternative is to rewrite everything, which I don't have time to do right now, so it is what it is.
req.Header.Set("If-None-Match", etag)
req.Header.Set("I-Cant-Believe-Its-Not-If-None-Match", etag)
}

if opts.auth == nil {
Expand Down

0 comments on commit f60d40e

Please sign in to comment.