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.
2026-03-01 12:07:42 +01:00 · 2026-03-01 03:40:43 +08:00
6 changed files with 83 additions and 0 deletions
--- 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)
--- 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)
--- 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 {
--- 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("<html><body>original</body></html>"),
+	}
+	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), "<html><body>original</body></html>")
+}
--- 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
--- 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 != "" {