mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
fix(site): avoid duplicating bin download headers (#22981)
## Summary - avoid duplicating preset headers when cachecompress serves compressed `/bin/*` responses - add a cachecompress regression test for preset `X-Original-Content-Length` and `ETag` headers - strengthen site binary tests to assert those headers stay single-valued ## Problem `site/bin.go` sets `X-Original-Content-Length` and `ETag` on the real response writer before delegating. `cachecompress` then snapshotted those headers and replayed them with `Header().Add(...)`, which duplicated them on compressed responses. For `coder-desktop-macos`, duplicate `X-Original-Content-Length` values can collapse into a comma-separated string and fail `Int64` parsing, causing the file size to show as `Unknown`. ## Testing - `/usr/local/go/bin/go test ./coderd/cachecompress -run 'TestCompressorPresetHeaders|TestCompressorHeadings' -count=1` - `/usr/local/go/bin/go test ./site -run TestServingBin -count=1` - `PATH=/usr/local/go/bin:$PATH make lint/go` ## Notes - Skipped full `make pre-commit` with explicit approval because local environment/tooling blocked it (Node version/path interaction in generated site targets, plus missing local tools before setup).
This commit is contained in:
@@ -240,9 +240,7 @@ func (c *Compressor) serveRef(w http.ResponseWriter, r *http.Request, headers ht
|
||||
}
|
||||
|
||||
for key, values := range headers {
|
||||
for _, value := range values {
|
||||
w.Header().Add(key, value)
|
||||
}
|
||||
w.Header()[key] = values
|
||||
}
|
||||
w.Header().Set("Content-Encoding", cref.key.encoding)
|
||||
w.Header().Add("Vary", "Accept-Encoding")
|
||||
|
||||
@@ -155,6 +155,41 @@ type nopEncoder struct {
|
||||
|
||||
func (nopEncoder) Close() error { return nil }
|
||||
|
||||
func TestCompressorPresetHeaders(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
logger := testutil.Logger(t)
|
||||
tempDir := t.TempDir()
|
||||
cacheDir := filepath.Join(tempDir, "cache")
|
||||
err := os.MkdirAll(cacheDir, 0o700)
|
||||
require.NoError(t, err)
|
||||
srcDir := filepath.Join(tempDir, "src")
|
||||
err = os.MkdirAll(srcDir, 0o700)
|
||||
require.NoError(t, err)
|
||||
err = os.WriteFile(filepath.Join(srcDir, "file.html"), []byte("textstring"), 0o600)
|
||||
require.NoError(t, err)
|
||||
|
||||
compressor := NewCompressor(logger, 5, cacheDir, http.FS(os.DirFS(srcDir)))
|
||||
|
||||
for range 2 {
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
req := httptest.NewRequestWithContext(ctx, "GET", "/file.html", nil)
|
||||
req.Header.Set("Accept-Encoding", "gzip")
|
||||
|
||||
respRec := httptest.NewRecorder()
|
||||
respRec.Header().Set("X-Original-Content-Length", "10")
|
||||
respRec.Header().Set("ETag", `"abc123"`)
|
||||
|
||||
compressor.ServeHTTP(respRec, req)
|
||||
resp := respRec.Result()
|
||||
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
require.Equal(t, []string{"10"}, resp.Header.Values("X-Original-Content-Length"))
|
||||
require.Equal(t, []string{`"abc123"`}, resp.Header.Values("ETag"))
|
||||
require.NoError(t, resp.Body.Close())
|
||||
}
|
||||
}
|
||||
|
||||
// nolint: tparallel // we want to assert the state of the cache, so run synchronously
|
||||
func TestCompressorHeadings(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
+3
-1
@@ -562,7 +562,7 @@ func TestServingBin(t *testing.T) {
|
||||
}
|
||||
|
||||
if tr.wantEtag != "" {
|
||||
assert.NotEmpty(t, resp.Header.Get("ETag"), "etag header is empty")
|
||||
assert.Equal(t, []string{tr.wantEtag}, resp.Header.Values("ETag"), "etag header values did not match")
|
||||
assert.Equal(t, tr.wantEtag, resp.Header.Get("ETag"), "etag did not match")
|
||||
}
|
||||
|
||||
@@ -570,6 +570,8 @@ func TestServingBin(t *testing.T) {
|
||||
// This is a custom header that we set to help the
|
||||
// client know the size of the decompressed data. See
|
||||
// the comment in site.go.
|
||||
headerValues := resp.Header.Values("X-Original-Content-Length")
|
||||
assert.Len(t, headerValues, 1, "X-Original-Content-Length should have exactly one value")
|
||||
headerStr := resp.Header.Get("X-Original-Content-Length")
|
||||
assert.NotEmpty(t, headerStr, "X-Original-Content-Length header is empty")
|
||||
originalSize, err := strconv.Atoi(headerStr)
|
||||
|
||||
Reference in New Issue
Block a user