From 59238adb5b6ec8831392011ec8abc189d8a67d1a Mon Sep 17 00:00:00 2001 From: yusing Date: Sun, 1 Mar 2026 03:40:43 +0800 Subject: [PATCH] fix(middleware): skip body rewriters when buffering fails Prevent response modifiers that require body rewriting from running when the body rewrite gate blocks buffering (for example, chunked transfer encoding). Add an explicit `requiresBodyRewrite` capability and implement it for HTML/theme/error-page modifiers, including bypass delegation. Also add a regression test to ensure the original response body remains readable and is not closed prematurely when rewrite is blocked. This commit fixeds the "http: read on closed response body" with empty page error happens when body-rewriting middleware (like themed) runs on responses where body rewrite is blocked (e.g. chunked), then the gate restores an already-closed original body. --- internal/net/gphttp/middleware/bypass.go | 4 ++ .../gphttp/middleware/custom_error_page.go | 4 ++ .../net/gphttp/middleware/middleware_chain.go | 12 ++++ .../net/gphttp/middleware/middleware_test.go | 55 +++++++++++++++++++ internal/net/gphttp/middleware/modify_html.go | 4 ++ internal/net/gphttp/middleware/themed.go | 4 ++ 6 files changed, 83 insertions(+) diff --git a/internal/net/gphttp/middleware/bypass.go b/internal/net/gphttp/middleware/bypass.go index c598dff2..df3cb61f 100644 --- a/internal/net/gphttp/middleware/bypass.go +++ b/internal/net/gphttp/middleware/bypass.go @@ -122,6 +122,10 @@ func (c *checkBypass) modifyResponse(resp *http.Response) error { return c.modRes.modifyResponse(resp) } +func (c *checkBypass) requiresBodyRewrite() bool { + return requiresBodyRewrite(c.modRes) +} + func (m *Middleware) withCheckBypass() any { if len(m.Bypass) > 0 { modReq, _ := m.impl.(RequestModifier) diff --git a/internal/net/gphttp/middleware/custom_error_page.go b/internal/net/gphttp/middleware/custom_error_page.go index 6c4e009e..379d28db 100644 --- a/internal/net/gphttp/middleware/custom_error_page.go +++ b/internal/net/gphttp/middleware/custom_error_page.go @@ -20,6 +20,10 @@ var CustomErrorPage = NewMiddleware[customErrorPage]() const StaticFilePathPrefix = "/$gperrorpage/" +func (customErrorPage) requiresBodyRewrite() bool { + return true +} + // before implements RequestModifier. func (customErrorPage) before(w http.ResponseWriter, r *http.Request) (proceed bool) { return !ServeStaticErrorPageFile(w, r) diff --git a/internal/net/gphttp/middleware/middleware_chain.go b/internal/net/gphttp/middleware/middleware_chain.go index 6d192201..b9c1781c 100644 --- a/internal/net/gphttp/middleware/middleware_chain.go +++ b/internal/net/gphttp/middleware/middleware_chain.go @@ -13,6 +13,10 @@ type middlewareChain struct { modResps []ResponseModifier } +type bodyRewriteRequired interface { + requiresBodyRewrite() bool +} + // TODO: check conflict or duplicates. func NewMiddlewareChain(name string, chain []*Middleware) *Middleware { chainMid := &middlewareChain{} @@ -59,6 +63,9 @@ func modifyResponseWithBodyRewriteGate(mr ResponseModifier, resp *http.Response) originalBody := resp.Body originalContentLength := resp.ContentLength allowBodyRewrite := canBufferAndModifyResponseBody(responseHeaderForBodyRewriteGate(resp)) + if !allowBodyRewrite && requiresBodyRewrite(mr) { + return nil + } if err := mr.modifyResponse(resp); err != nil { return err @@ -87,6 +94,11 @@ func modifyResponseWithBodyRewriteGate(mr ResponseModifier, resp *http.Response) return nil } +func requiresBodyRewrite(mr ResponseModifier) bool { + required, ok := mr.(bodyRewriteRequired) + return ok && required.requiresBodyRewrite() +} + func responseHeaderForBodyRewriteGate(resp *http.Response) http.Header { h := resp.Header.Clone() if len(resp.TransferEncoding) > 0 && len(h.Values("Transfer-Encoding")) == 0 { diff --git a/internal/net/gphttp/middleware/middleware_test.go b/internal/net/gphttp/middleware/middleware_test.go index 0b212561..340e5872 100644 --- a/internal/net/gphttp/middleware/middleware_test.go +++ b/internal/net/gphttp/middleware/middleware_test.go @@ -1,6 +1,7 @@ package middleware import ( + "errors" "io" "net/http" "net/http/httptest" @@ -30,6 +31,29 @@ type testResponseRewrite struct { Body string `json:"body"` } +type closeSensitiveBody struct { + data []byte + offset int + closed bool +} + +func (b *closeSensitiveBody) Read(p []byte) (int, error) { + if b.closed { + return 0, errors.New("http: read on closed response body") + } + if b.offset >= len(b.data) { + return 0, io.EOF + } + n := copy(p, b.data[b.offset:]) + b.offset += n + return n, nil +} + +func (b *closeSensitiveBody) Close() error { + b.closed = true + return nil +} + func (t testResponseRewrite) modifyResponse(resp *http.Response) error { resp.StatusCode = t.StatusCode resp.Header.Set(t.HeaderKey, t.HeaderVal) @@ -226,3 +250,34 @@ func TestMiddlewareResponseRewriteGateServeHTTP(t *testing.T) { }) } } + +func TestMiddlewareResponseRewriteGateSkipsBodyRewriterWhenRewriteBlocked(t *testing.T) { + originalBody := &closeSensitiveBody{ + data: []byte("original"), + } + req := httptest.NewRequest(http.MethodGet, "http://example.com", nil) + resp := &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"text/html; charset=utf-8"}, + "Transfer-Encoding": []string{"chunked"}, + }, + Body: originalBody, + ContentLength: -1, + TransferEncoding: []string{"chunked"}, + Request: req, + } + + themedMid, err := Themed.New(OptionsRaw{ + "theme": DarkTheme, + }) + expect.NoError(t, err) + + respMod, ok := themedMid.impl.(ResponseModifier) + expect.True(t, ok) + expect.NoError(t, modifyResponseWithBodyRewriteGate(respMod, resp)) + + data, err := io.ReadAll(resp.Body) + expect.NoError(t, err) + expect.Equal(t, string(data), "original") +} diff --git a/internal/net/gphttp/middleware/modify_html.go b/internal/net/gphttp/middleware/modify_html.go index 3557e884..ede67a54 100644 --- a/internal/net/gphttp/middleware/modify_html.go +++ b/internal/net/gphttp/middleware/modify_html.go @@ -22,6 +22,10 @@ type modifyHTML struct { var ModifyHTML = NewMiddleware[modifyHTML]() +func (*modifyHTML) requiresBodyRewrite() bool { + return true +} + func (m *modifyHTML) before(_ http.ResponseWriter, req *http.Request) bool { req.Header.Set("Accept-Encoding", "identity") return true diff --git a/internal/net/gphttp/middleware/themed.go b/internal/net/gphttp/middleware/themed.go index 8e0ca4bf..17ef4c6c 100644 --- a/internal/net/gphttp/middleware/themed.go +++ b/internal/net/gphttp/middleware/themed.go @@ -54,6 +54,10 @@ func (m *themed) modifyResponse(resp *http.Response) error { return m.m.modifyResponse(resp) } +func (*themed) requiresBodyRewrite() bool { + return true +} + func (m *themed) finalize() error { m.m.Target = "body" if m.FontURL != "" && m.FontFamily != "" {