Upgrade goproxy to v1.9.0

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

@@ -214,6 +214,46 @@ func (d *DB) OnResponse(resp *http.Response, proxyCtx *goproxy.ProxyCtx) *http.R
 		logrus.Warnln("Received nil response")
 		return resp
 	}
+	if resp.StatusCode == http.StatusSwitchingProtocols {
+		return resp
+	}
+	if responseMustNotHaveBody(resp) {
+		invalidContentLength := resp.StatusCode == http.StatusResetContent &&
+			(resp.ContentLength != 0 || resp.Header.Get("Content-Length") != "")
+		if resp.Body != nil && resp.Body != http.NoBody ||
+			len(resp.TransferEncoding) > 0 || resp.Header.Get("Transfer-Encoding") != "" ||
+			invalidContentLength {
+			method := ""
+			if resp.Request != nil {
+				method = resp.Request.Method
+			}
+			logrus.Warnf("Response has no body (method: %s, status: %d)", method, resp.StatusCode)
+		}
+		if resp.Body != nil && resp.Body != http.NoBody {
+			_ = resp.Body.Close()
+		}
+		resp.Body = http.NoBody
+		resp.TransferEncoding = nil
+		resp.Header.Del("Transfer-Encoding")
+		if resp.StatusCode == http.StatusResetContent {
+			resp.ContentLength = 0
+			resp.Header.Del("Content-Length")
+		}
+		return resp
+	}
+	if resp.Body == nil {
+		method := ""
+		if resp.Request != nil {
+			method = resp.Request.Method
+		}
+		logrus.Errorf("Response unexpectedly has nil body (method: %s, status: %d)", method, resp.StatusCode)
+		resp.Body = http.NoBody
+		resp.ContentLength = 0
+		resp.TransferEncoding = nil
+		resp.Header.Del("Content-Length")
+		resp.Header.Del("Transfer-Encoding")
+		return resp
+	}
 	k, ok := proxyctx.GetValue(proxyCtx, keyValue)
 	if !ok {
 		// can't calculate key as response body is empty
@@ -259,6 +299,14 @@ func (d *DB) OnResponse(resp *http.Response, proxyCtx *goproxy.ProxyCtx) *http.R
 	return resp
 }
 
+func responseMustNotHaveBody(resp *http.Response) bool {
+	return resp.StatusCode >= 100 && resp.StatusCode < 200 ||
+		resp.StatusCode == http.StatusNoContent ||
+		resp.StatusCode == http.StatusResetContent ||
+		resp.StatusCode == http.StatusNotModified ||
+		resp.Request != nil && resp.Request.Method == http.MethodHead
+}
+
 var sanitizeRegex = regexp.MustCompile(`\W`)
 
 func sanitize(host string) string {

internal/cache/handlers_test.go

@@ -16,8 +16,11 @@ import (
 	"time"
 
 	"github.com/elazarl/goproxy"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/dependabot/proxy/internal/proxyctx"
 )
 
 // None of these tests should make network calls
@@ -111,6 +114,173 @@ func TestCache(t *testing.T) {
 	})
 }
 
+func TestCache_BodyForbiddenResponses(t *testing.T) {
+	tests := []struct {
+		name               string
+		method             string
+		statusCode         int
+		clearContentLength bool
+	}{
+		{name: "informational", method: http.MethodGet, statusCode: http.StatusEarlyHints},
+		{name: "HEAD", method: http.MethodHead, statusCode: http.StatusOK},
+		{name: "no content", method: http.MethodGet, statusCode: http.StatusNoContent},
+		{name: "reset content", method: http.MethodGet, statusCode: http.StatusResetContent, clearContentLength: true},
+		{name: "not modified", method: http.MethodGet, statusCode: http.StatusNotModified},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var logOutput bytes.Buffer
+			originalOutput := logrus.StandardLogger().Out
+			logrus.SetOutput(&logOutput)
+			defer logrus.SetOutput(originalOutput)
+
+			cacher, err := New(true, t.TempDir())
+			require.NoError(t, err)
+
+			req := httptest.NewRequestWithContext(t.Context(), test.method, URL, nil)
+			proxyCtx := &goproxy.ProxyCtx{Req: req}
+			proxyctx.SetValue(proxyCtx, keyValue, Key{Method: req.Method, URL: req.URL.String()})
+
+			originalBody := &BufferWithClose{}
+			resp := &http.Response{
+				Request:          req,
+				StatusCode:       test.statusCode,
+				Header:           http.Header{"Content-Length": []string{"10"}, "Transfer-Encoding": []string{"chunked"}},
+				Body:             originalBody,
+				ContentLength:    10,
+				TransferEncoding: []string{"chunked"},
+			}
+
+			result := cacher.OnResponse(resp, proxyCtx)
+
+			assert.Same(t, resp, result)
+			assert.Equal(t, http.NoBody, result.Body)
+			assert.True(t, originalBody.WasCloseCalled)
+			assert.Empty(t, result.TransferEncoding)
+			assert.Empty(t, result.Header.Values("Transfer-Encoding"))
+			if test.clearContentLength {
+				assert.Zero(t, result.ContentLength)
+				assert.Empty(t, result.Header.Values("Content-Length"))
+			} else {
+				assert.Equal(t, int64(10), result.ContentLength)
+				assert.Equal(t, "10", result.Header.Get("Content-Length"))
+			}
+			assert.Empty(t, cacher.cacheDB)
+			assert.Zero(t, cacher.callCursor)
+			assert.Contains(t, logOutput.String(), "Response has no body (method: "+test.method+", status: "+strconv.Itoa(test.statusCode)+")")
+		})
+	}
+}
+
+func TestCache_RoutineBodyForbiddenResponseDoesNotWarn(t *testing.T) {
+	var logOutput bytes.Buffer
+	originalOutput := logrus.StandardLogger().Out
+	logrus.SetOutput(&logOutput)
+	defer logrus.SetOutput(originalOutput)
+
+	cacher, err := New(true, t.TempDir())
+	require.NoError(t, err)
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, URL, nil)
+	resp := &http.Response{
+		Request:    req,
+		StatusCode: http.StatusNotModified,
+		Header:     make(http.Header),
+		Body:       http.NoBody,
+	}
+
+	result := cacher.OnResponse(resp, &goproxy.ProxyCtx{Req: req})
+
+	assert.Same(t, resp, result)
+	assert.Equal(t, http.NoBody, result.Body)
+	assert.NotContains(t, logOutput.String(), "Response has no body")
+}
+
+func TestCache_SwitchingProtocolsPreservesUpgradedStream(t *testing.T) {
+	cacher, err := New(true, t.TempDir())
+	require.NoError(t, err)
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, URL, nil)
+	proxyCtx := &goproxy.ProxyCtx{Req: req}
+	proxyctx.SetValue(proxyCtx, keyValue, Key{Method: req.Method, URL: req.URL.String()})
+
+	upgradedStream := &BufferWithClose{}
+	resp := &http.Response{
+		Request:    req,
+		StatusCode: http.StatusSwitchingProtocols,
+		Header:     http.Header{"Upgrade": []string{"websocket"}},
+		Body:       upgradedStream,
+	}
+
+	result := cacher.OnResponse(resp, proxyCtx)
+
+	assert.Same(t, resp, result)
+	assert.Same(t, upgradedStream, result.Body)
+	assert.False(t, upgradedStream.WasCloseCalled)
+	assert.Empty(t, cacher.cacheDB)
+	assert.Zero(t, cacher.callCursor)
+}
+
+func TestCache_UnexpectedNilBodyIsNotCached(t *testing.T) {
+	var logOutput bytes.Buffer
+	originalOutput := logrus.StandardLogger().Out
+	logrus.SetOutput(&logOutput)
+	defer logrus.SetOutput(originalOutput)
+
+	cacheDir := t.TempDir()
+	cacher, err := New(true, cacheDir)
+	require.NoError(t, err)
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, URL, nil)
+	proxyCtx := &goproxy.ProxyCtx{Req: req}
+	proxyctx.SetValue(proxyCtx, keyValue, Key{Method: req.Method, URL: req.URL.String()})
+	resp := &http.Response{
+		Request:          req,
+		StatusCode:       http.StatusOK,
+		Header:           http.Header{"Content-Length": []string{"10"}, "Transfer-Encoding": []string{"chunked"}},
+		ContentLength:    10,
+		TransferEncoding: []string{"chunked"},
+	}
+
+	result := cacher.OnResponse(resp, proxyCtx)
+
+	assert.Same(t, resp, result)
+	assert.Equal(t, http.NoBody, result.Body)
+	assert.Zero(t, result.ContentLength)
+	assert.Empty(t, result.TransferEncoding)
+	assert.Empty(t, result.Header.Values("Content-Length"))
+	assert.Empty(t, result.Header.Values("Transfer-Encoding"))
+	assert.Empty(t, cacher.cacheDB)
+	assert.Zero(t, cacher.callCursor)
+	entries, err := os.ReadDir(cacheDir)
+	require.NoError(t, err)
+	assert.Empty(t, entries)
+	assert.Contains(t, logOutput.String(), "Response unexpectedly has nil body (method: GET, status: 200)")
+}
+
+func TestCache_ZeroByteBodyIsCached(t *testing.T) {
+	cacher, err := New(true, t.TempDir())
+	require.NoError(t, err)
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, URL, nil)
+	proxyCtx := &goproxy.ProxyCtx{Req: req}
+	proxyctx.SetValue(proxyCtx, keyValue, Key{Method: req.Method, URL: req.URL.String()})
+	resp := &http.Response{
+		Request:    req,
+		StatusCode: http.StatusOK,
+		Body:       io.NopCloser(strings.NewReader("")),
+	}
+
+	result := cacher.OnResponse(resp, proxyCtx)
+	_, err = io.ReadAll(result.Body)
+	require.NoError(t, err)
+	require.NoError(t, result.Body.Close())
+
+	assert.Len(t, cacher.cacheDB, 1)
+	assert.Equal(t, 1, cacher.callCursor)
+}
+
 func Test_sanitize(t *testing.T) {
 	var tests = []struct {
 		Input, Expected string

proxy.go

@@ -27,6 +27,10 @@ type Proxy struct {
 }
 
 func newProxy(envSettings config.ProxyEnvSettings, cfg *config.Config, blockedIps []net.IP) *Proxy {
+	return newProxyWithCacheDir(envSettings, cfg, blockedIps, "/cache")
+}
+
+func newProxyWithCacheDir(envSettings config.ProxyEnvSettings, cfg *config.Config, blockedIps []net.IP, cacheDir string) *Proxy {
 	var err error
 
 	if err := setCA([]byte(cfg.CA.Cert), []byte(cfg.CA.Key)); err != nil {
@@ -62,7 +66,7 @@ func newProxy(envSettings config.ProxyEnvSettings, cfg *config.Config, blockedIp
 	proxy.OnResponse().DoFunc(logger.logResponse)
 
 	enableCache := os.Getenv("PROXY_CACHE") == "true"
-	cacher, err := cache.New(enableCache, "/cache")
+	cacher, err := cache.New(enableCache, cacheDir)
 	if err != nil {
 		log.Fatal(err)
 	}