Bump goproxy to v1.9.0 and fix keep-alive framing desync

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/aws/aws-sdk-go-v2/config v1.32.36
 	github.com/aws/aws-sdk-go-v2/credentials v1.19.35
 	github.com/aws/aws-sdk-go-v2/service/ecr v1.60.5
-	github.com/elazarl/goproxy v0.0.0-20240726154733-8b0c20506380
+	github.com/elazarl/goproxy v1.9.0
 	github.com/evalphobia/logrus_sentry v0.8.2
 	github.com/getsentry/raven-go v0.2.0
 	github.com/jarcoal/httpmock v1.4.2

go.sum

@@ -32,6 +32,8 @@ github.com/cenk/backoff v2.2.1+incompatible h1:djdFT7f4gF2ttuzRKPbMOWgZajgesItGL
 github.com/cenk/backoff v2.2.1+incompatible/go.mod h1:7FtoeaSnHoZnmZzz47cM35Y9nSW7tNyaidugnHTaFDE=
 github.com/certifi/gocertifi v0.0.0-20191021191039-0944d244cd40 h1:xvUo53O5MRZhVMJAxWCJcS5HHrqAiAG9SJ1LpMu6aAI=
 github.com/certifi/gocertifi v0.0.0-20191021191039-0944d244cd40/go.mod h1:sGbDF6GwGcLpkNXPUTkMRoywsNa/ol15pxFe6ERfguA=
+github.com/coder/websocket v1.8.14 h1:9L0p0iKiNOibykf283eHkKUHHrpG7f65OE3BhhO7v9g=
+github.com/coder/websocket v1.8.14/go.mod h1:NX3SzP+inril6yawo5CQXx8+fk145lPDC6pumgx0mVg=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -41,10 +43,8 @@ github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBi
 github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
 github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 h1:UhxFibDNY/bfvqU5CAUmr9zpesgbU6SWc8/B4mflAE4=
 github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7/go.mod h1:cyGadeNEkKy96OOhEzfZl+yxihPEzKnqJwvfuSUqbZE=
-github.com/elazarl/goproxy v0.0.0-20240726154733-8b0c20506380 h1:1NyRx2f4W4WBRyg0Kys0ZbaNmDDzZ2R/C7DTi+bbsJ0=
-github.com/elazarl/goproxy v0.0.0-20240726154733-8b0c20506380/go.mod h1:thX175TtLTzLj3p7N/Q9IiKZ7NF+p72cvL91emV0hzo=
-github.com/elazarl/goproxy/ext v0.0.0-20190711103511-473e67f1d7d2 h1:dWB6v3RcOy03t/bUadywsbyrQwCqZeNIEX6M1OtSZOM=
-github.com/elazarl/goproxy/ext v0.0.0-20190711103511-473e67f1d7d2/go.mod h1:gNh8nYJoAm43RfaxurUnxr+N1PwuFV3ZMl/efxlIlY8=
+github.com/elazarl/goproxy v1.9.0 h1:2j3c13lD5v0QTjxphJSSIHS7w8/m/pzSHtLMPOpznC0=
+github.com/elazarl/goproxy v1.9.0/go.mod h1:THdE5ix2clxX9lZzcICPpZ67d6CdrPZxdOYsNgU5e30=
 github.com/evalphobia/logrus_sentry v0.8.2 h1:dotxHq+YLZsT1Bb45bB5UQbfCh3gM/nFFetyN46VoDQ=
 github.com/evalphobia/logrus_sentry v0.8.2/go.mod h1:pKcp+vriitUqu9KiWj/VRFbRfFNUwz95/UkgG8a6MNc=
 github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a h1:yDWHCSQ40h88yih2JAcL6Ls/kVkSE8GFACTGVnMPruw=

internal/cache/handlers.go

@@ -12,6 +12,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"sort"
+	"strconv"
 	"sync"
 
 	"github.com/elazarl/goproxy"
@@ -161,6 +162,34 @@ const (
 	keyValue  = "key"
 )
 
+// bodyless reports whether an HTTP response must not carry a message body, per
+// RFC 7230 3.3.3: any response to a HEAD request, all 1xx (informational)
+// responses, 204 No Content, 205 Reset Content, and 304 Not Modified. The
+// client will not read a body for these regardless of framing headers, so if we
+// attach a body (or a bogus chunked terminator) the extra bytes are left in the
+// connection and desync the next request on the now keep-alive MITM tunnel.
+//
+// 101 Switching Protocols is deliberately excluded: its "body" is the upgraded
+// connection stream (e.g. WebSocket), so it must be passed through untouched.
+func bodyless(status int, method string) bool {
+	if status == http.StatusSwitchingProtocols {
+		return false
+	}
+	if method == http.MethodHead {
+		return true
+	}
+	switch {
+	case status >= 100 && status < 200:
+		return true
+	case status == http.StatusNoContent,
+		status == http.StatusResetContent,
+		status == http.StatusNotModified:
+		return true
+	default:
+		return false
+	}
+}
+
 // OnRequest checks to see if the response is cached, if so responds with the cached data.
 func (d *DB) OnRequest(r *http.Request, proxyCtx *goproxy.ProxyCtx) (*http.Request, *http.Response) {
 	if d == nil {
@@ -185,19 +214,51 @@ func (d *DB) OnRequest(r *http.Request, proxyCtx *goproxy.ProxyCtx) (*http.Reque
 	key := key(r)
 	proxyctx.SetValue(proxyCtx, keyValue, key)
 	if entry, ok := d.cacheDB[key]; ok {
+		resp := &http.Response{}
+		resp.Request = r
+		resp.Header = entry.ResponseHeaders
+		resp.StatusCode = entry.Status
+
+		if bodyless(entry.Status, r.Method) {
+			// Serve bodyless responses (1xx/204/205/304/HEAD) with no body and
+			// no transfer-encoding so goproxy frames them correctly. Attaching a
+			// body here would make goproxy stamp a chunked terminator the
+			// client never reads, desyncing the keep-alive MITM tunnel.
+			resp.Body = http.NoBody
+			resp.ContentLength = 0
+			// A HEAD response legitimately advertises the Content-Length the
+			// equivalent GET would return. Restore it from the cached headers so
+			// a hit reports the same length as the original miss. This is safe
+			// only for HEAD: Go skips the ContentLength/body mismatch check for
+			// responses to HEAD, whereas a non-HEAD status (e.g. GET 304) with a
+			// non-zero ContentLength and an empty body would fail resp.Write.
+			if r.Method == http.MethodHead {
+				if n, err := strconv.ParseInt(resp.Header.Get("Content-Length"), 10, 64); err == nil {
+					resp.ContentLength = n
+				}
+			}
+			proxyctx.SetValue(proxyCtx, wasCached, true)
+			d.cached++
+			return r, resp
+		}
+
 		f, err := os.Open(entry.FilePath)
 		if err != nil {
+			// The cache entry exists but its file is gone/unreadable. Fall
+			// through to the upstream request without marking it as cached so
+			// logs and cache statistics stay accurate.
 			logrus.Errorln("failed to open cache file:", err)
 			return r, nil
 		}
-		proxyctx.SetValue(proxyCtx, wasCached, true)
-		d.cached++
-		resp := &http.Response{}
-		resp.Request = r
 		resp.TransferEncoding = r.TransferEncoding
-		resp.Header = entry.ResponseHeaders
-		resp.StatusCode = entry.Status
 		resp.Body = f
+		// Set ContentLength from the cache file so goproxy can length-delimit
+		// the response instead of guessing, avoiding framing ambiguity.
+		if fi, statErr := f.Stat(); statErr == nil {
+			resp.ContentLength = fi.Size()
+		}
+		proxyctx.SetValue(proxyCtx, wasCached, true)
+		d.cached++
 		return r, resp
 	}
 	return r, nil
@@ -214,6 +275,11 @@ func (d *DB) OnResponse(resp *http.Response, proxyCtx *goproxy.ProxyCtx) *http.R
 		logrus.Warnln("Received nil response")
 		return resp
 	}
+	if resp.StatusCode == http.StatusSwitchingProtocols {
+		// 101: the body is the upgraded connection stream (e.g. WebSocket).
+		// Never cache or reframe it.
+		return resp
+	}
 	k, ok := proxyctx.GetValue(proxyCtx, keyValue)
 	if !ok {
 		// can't calculate key as response body is empty
@@ -229,12 +295,34 @@ func (d *DB) OnResponse(resp *http.Response, proxyCtx *goproxy.ProxyCtx) *http.R
 		return resp
 	}
 
+	if bodyless(resp.StatusCode, resp.Request.Method) {
+		// Bodyless responses (1xx/204/205/304/HEAD) carry no body. An earlier
+		// response handler may have replaced resp.Body with a wrapper (e.g.
+		// PythonIndexHandler swaps in a replay reader even for http.NoBody), so
+		// we cannot assume it is still http.NoBody here. Close any wrapper and
+		// restore http.NoBody so goproxy sees an unmodified empty body and frames
+		// the response without a chunked terminator. Leaving a non-NoBody body in
+		// place would make goproxy stamp a "0\r\n\r\n" the client never reads,
+		// desyncing the keep-alive MITM tunnel. ContentLength is left untouched so
+		// a HEAD still advertises the length its GET would return.
+		if resp.Body != nil && resp.Body != http.NoBody {
+			_ = resp.Body.Close()
+			resp.Body = http.NoBody
+		}
+		d.cacheDB[key] = &Entry{
+			Status:          resp.StatusCode,
+			ResponseHeaders: resp.Header,
+		}
+		return resp
+	}
+
 	fileName := fmt.Sprintf("%06d-%v", d.nextNumber(), sanitize(resp.Request.Host))
 	f, err := os.Create(filepath.Clean(filepath.Join(d.cacheDir, fileName)))
 	if err != nil {
 		logrus.Warnln("Failed to write to cache:", err.Error())
 		return resp
 	}
+
 	resp.Body = TeeReadCloser(resp.Body, f, func() {
 		d.Lock()
 		defer d.Unlock()