From fea2edfc7333a7b24449a509724abf69da8e5fc0 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Tue, 3 Nov 2026 06:26:31 +1100 Subject: [PATCH] fix: close the connection when OPA refuses an upload When OPA denied a request with a body, net/http was meant to notice the large unread body and reply with Connection: close. otelhttp replaces r.Body, which hides the unread length, so net/http instead read up to 256KiB of the refused body before writing the 403. Setting Connection: close up front makes it reply immediately and close the connection, so clients and proxies don't reuse a socket that still has the unsent upload queued on it. Amp-Thread-ID: https://ampcode.com/threads/T-01a19db1-1772-7511-b6d4-83c4f42d74e8 Co-authored-by: Amp --- internal/opa/opa.go | 6 ++++++ internal/opa/opa_test.go | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/internal/opa/opa.go b/internal/opa/opa.go index 60786da4..1e797255 100644 --- a/internal/opa/opa.go +++ b/internal/opa/opa.go @@ -67,6 +67,12 @@ func Middleware(ctx context.Context, cfg Config, next http.Handler) (http.Handle } if !allowed { logger.Warn("OPA denied request", "method", r.Method, "path", r.URL.Path, "remote_addr", r.RemoteAddr) + // The unread body is still on the wire, so the connection can't be reused. Saying so + // up front also stops net/http reading up to 256KiB of the body before replying, which + // it does because otelhttp replaces r.Body and hides the unread length from it. + if r.ContentLength != 0 { + w.Header().Set("Connection", "close") + } http.Error(w, "forbidden", http.StatusForbidden) return } diff --git a/internal/opa/opa_test.go b/internal/opa/opa_test.go index 065b2e49..8ade95d2 100644 --- a/internal/opa/opa_test.go +++ b/internal/opa/opa_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/alecthomas/assert/v2" @@ -365,6 +366,27 @@ test_delete_allowed if authz.allow with input as {"method": "DELETE"} } } +func TestMiddlewareDeniedUploadClosesConnection(t *testing.T) { + policy := `package cachew.authz +default allow := false +` + next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}) + handler, err := opa.Middleware(t.Context(), opa.Config{Policy: policy}, next) + assert.NoError(t, err) + + upload := httptest.NewRequest(http.MethodPost, "/api/v1/object/ns/key", strings.NewReader("data")) + upload = upload.WithContext(logging.ContextWithLogger(upload.Context(), slog.Default())) + w := httptest.NewRecorder() + handler.ServeHTTP(w, upload) + assert.Equal(t, http.StatusForbidden, w.Code) + assert.Equal(t, "close", w.Header().Get("Connection")) + + w = httptest.NewRecorder() + handler.ServeHTTP(w, newRequest(http.MethodGet, "/api/v1/object/ns/key")) + assert.Equal(t, http.StatusForbidden, w.Code) + assert.Equal(t, "", w.Header().Get("Connection")) +} + func TestMiddlewareEmptyPolicyDeniesAll(t *testing.T) { policy := `package cachew.authz `