fix: return a more-complete 304 from TransformingTransport.RoundTrip

to address the segfault we're seeing in production. The necessary
conditions for the issue are:

1. The 303 must return a location URI that changes every time, so that
   the real location of the image is never cached. As an example, if the
   remove service redirects to S3, the presence of the
   `X-Amz-Security-Token` accomplishes this.

2. The image responses must match by Etag, so that
   imageProxy.should304() returns true causing
   TranformingTransport.RoundTrip() to return a bare 304 response.

If those conditions are met, then the bare 304 Response returned will
be used and read by one of the callers. Specifically, the lack of a
Body causes a segfault.

So let's make it more like a real Response and use http.NoBody so when
it's used it doesn't cause things to explode.
This commit is contained in:
Mike Dalessio 2025-03-01 12:32:27 -05:00 committed by Will Norris
parent 2254a1f2ff
commit df0b6d337a
2 changed files with 80 additions and 7 deletions

View file

@ -573,7 +573,14 @@ func (t *TransformingTransport) RoundTrip(req *http.Request) (*http.Response, er
if should304(req, resp) {
// bare 304 response, full response will be used from cache
return &http.Response{StatusCode: http.StatusNotModified}, nil
return &http.Response{
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
Status: fmt.Sprintf("%d %s", http.StatusNotModified, http.StatusText(http.StatusNotModified)),
StatusCode: http.StatusNotModified,
Body: http.NoBody,
}, nil
}
b, err := io.ReadAll(resp.Body)