From 944febcde4b535398b481cbd6ca1e4ff456548d4 Mon Sep 17 00:00:00 2001 From: Ceridwen Date: Fri, 20 Feb 2026 09:57:06 +0000 Subject: [PATCH 1/4] Check status codes without response body behaviour. --- e2e_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/e2e_test.go b/e2e_test.go index 770974c4..6794540a 100644 --- a/e2e_test.go +++ b/e2e_test.go @@ -877,3 +877,28 @@ func TestE2EMaxConnectionAge_ConnectionAgeSet(t *testing.T) { require.NotEmpty(t, connectionStart) }) } + +func TestE2EStatusCodesWithoutAResponseBody(t *testing.T) { + flavours(t, func(t *testing.T, flav e2eFlavour) { + ctx, cancel := flav.Context() + defer cancel() + + svc := Service(func(req Request) Response { + response := req.Response([]byte("hello")) + response.Header.Set("content-type", "text/plain") + response.StatusCode = http.StatusNotModified // Not Modified + return response + }) + svc = svc.Filter(ErrorFilter) + s := flav.Serve(svc) + defer s.Stop(context.Background()) + + req := NewRequest(ctx, "GET", flav.URL(s), nil) + rsp := req.Send().Response() + require.NoError(t, rsp.Error) + assert.Equal(t, http.StatusNotModified, rsp.StatusCode) + rspBody, err := rsp.BodyBytes(true) + require.NoError(t, err) + assert.Empty(t, rspBody) + }) +} From 5c82ef97160750e8656bcaae561348bc2f581389 Mon Sep 17 00:00:00 2001 From: Ceridwen Date: Fri, 20 Feb 2026 10:08:54 +0000 Subject: [PATCH 2/4] Use an early return here to make the logic clearer. --- http.go | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/http.go b/http.go index 7c5ee886..6e3e3f6d 100644 --- a/http.go +++ b/http.go @@ -127,24 +127,26 @@ func HttpHandler(svc Service) http.Handler { rwHeader[k] = v } rw.WriteHeader(rsp.StatusCode) - if rsp.Body != nil && bodyAllowedForStatus(rsp.StatusCode) { - defer rsp.Body.Close() - buf := *httpChunkBufPool.Get().(*[]byte) - defer httpChunkBufPool.Put(&buf) - if isStreamingRsp(rsp) { - // Streaming responses use copyChunked(), which takes care of flushing transparently - if _, err := copyChunked(rw, rsp.Body, buf); err != nil { - slog.Log(slog.Eventf(copyErrSeverity(err), req, "Couldn't send streaming response body", err)) - - // Prevent the client from accidentally consuming a truncated stream by aborting the response. - // The official way of interrupting an HTTP reply mid-stream is panic(http.ErrAbortHandler), which - // works for both HTTP/1.1 and HTTP.2. https://github.com/golang/go/issues/17790 - panic(http.ErrAbortHandler) - } - } else { - if _, err := io.CopyBuffer(rw, rsp.Body, buf); err != nil { - slog.Log(slog.Eventf(copyErrSeverity(err), req, "Couldn't send response body", err)) - } + if rsp.Body == nil || !bodyAllowedForStatus(rsp.StatusCode) { + return + } + + defer rsp.Body.Close() + buf := *httpChunkBufPool.Get().(*[]byte) + defer httpChunkBufPool.Put(&buf) + if isStreamingRsp(rsp) { + // Streaming responses use copyChunked(), which takes care of flushing transparently + if _, err := copyChunked(rw, rsp.Body, buf); err != nil { + slog.Log(slog.Eventf(copyErrSeverity(err), req, "Couldn't send streaming response body", err)) + + // Prevent the client from accidentally consuming a truncated stream by aborting the response. + // The official way of interrupting an HTTP reply mid-stream is panic(http.ErrAbortHandler), which + // works for both HTTP/1.1 and HTTP.2. https://github.com/golang/go/issues/17790 + panic(http.ErrAbortHandler) + } + } else { + if _, err := io.CopyBuffer(rw, rsp.Body, buf); err != nil { + slog.Log(slog.Eventf(copyErrSeverity(err), req, "Couldn't send response body", err)) } } }) From d2de69e2f808680b5335411534baf0d7aba3faba Mon Sep 17 00:00:00 2001 From: Ceridwen Date: Fri, 20 Feb 2026 10:12:13 +0000 Subject: [PATCH 3/4] Ensure we always close the response body in the event of not sending the body. --- e2e_test.go | 22 ++++++++++++++++++++++ http.go | 7 ++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/e2e_test.go b/e2e_test.go index 6794540a..95a0c259 100644 --- a/e2e_test.go +++ b/e2e_test.go @@ -878,15 +878,36 @@ func TestE2EMaxConnectionAge_ConnectionAgeSet(t *testing.T) { }) } +type closeTrackingReadCloser struct { + inner io.ReadCloser + isClosed *bool +} + +func (c *closeTrackingReadCloser) Read(p []byte) (n int, err error) { + return c.inner.Read(p) +} + +func (c *closeTrackingReadCloser) Close() error { + *c.isClosed = true + return c.inner.Close() +} + func TestE2EStatusCodesWithoutAResponseBody(t *testing.T) { flavours(t, func(t *testing.T, flav e2eFlavour) { ctx, cancel := flav.Context() defer cancel() + responseBodyWasClosed := false + svc := Service(func(req Request) Response { response := req.Response([]byte("hello")) response.Header.Set("content-type", "text/plain") response.StatusCode = http.StatusNotModified // Not Modified + oldBody := response.Body + response.Body = &closeTrackingReadCloser{ + inner: oldBody, + isClosed: &responseBodyWasClosed, + } return response }) svc = svc.Filter(ErrorFilter) @@ -900,5 +921,6 @@ func TestE2EStatusCodesWithoutAResponseBody(t *testing.T) { rspBody, err := rsp.BodyBytes(true) require.NoError(t, err) assert.Empty(t, rspBody) + assert.True(t, responseBodyWasClosed) }) } diff --git a/http.go b/http.go index 6e3e3f6d..634d0f19 100644 --- a/http.go +++ b/http.go @@ -127,11 +127,16 @@ func HttpHandler(svc Service) http.Handler { rwHeader[k] = v } rw.WriteHeader(rsp.StatusCode) - if rsp.Body == nil || !bodyAllowedForStatus(rsp.StatusCode) { + if rsp.Body == nil { return } defer rsp.Body.Close() + + if !bodyAllowedForStatus(rsp.StatusCode) { + return + } + buf := *httpChunkBufPool.Get().(*[]byte) defer httpChunkBufPool.Put(&buf) if isStreamingRsp(rsp) { From 4d7439a9463e3ecc8d63bd8441fdfc2f50b1d6cd Mon Sep 17 00:00:00 2001 From: Ceridwen Date: Fri, 20 Feb 2026 11:05:38 +0000 Subject: [PATCH 4/4] Ensure we always close the response body unless nil or hijacked. --- http.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/http.go b/http.go index 634d0f19..d217fd7d 100644 --- a/http.go +++ b/http.go @@ -117,23 +117,22 @@ func HttpHandler(svc Service) http.Handler { } rsp := svc(req) - // If the connection was hijacked, we should not attempt to write anything out if rsp.hijacked { return } + // If the connection has been hijacked, the hijacker is responsible for any + // resource cleanup (as per http.Hijacker) + if rsp.Body != nil { + defer rsp.Body.Close() + } + rwHeader := rw.Header() for k, v := range rsp.Header { rwHeader[k] = v } rw.WriteHeader(rsp.StatusCode) - if rsp.Body == nil { - return - } - - defer rsp.Body.Close() - - if !bodyAllowedForStatus(rsp.StatusCode) { + if rsp.Body == nil || !bodyAllowedForStatus(rsp.StatusCode) { return }