Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
Mirrored from https://github.com/Slazee/caddy.git
Synced Nov 11, 03:30 UTC
reverseproxy: Don't drop read errors with unlimited buffers (#8207)
With request_buffers or response_buffers set to unlimited, the error from reading the body was discarded and whatever had been read was treated as the complete body. A request cut short by request_body max_size was forwarded truncated with a 200, and a truncated chunked upstream response was sent to the client as complete. Handle a read error the same way the limited case already does.
Jay Hemnani committed
Oct 13, 2026 at 19:55 UTC
fc3d83400b95c4df8d5d2928d2ed22ff42192729
3 files changed
+93
-11
caddytest/integration/reverseproxy_test.go
+50
@@ -1103,3 +1103,53 @@ func TestReverseProxyResponseHandling(t *testing.T) {
1103
}
1104
}
1105
}
1106
+
1107
+// TestReverseProxyUnlimitedRequestBuffersRespectMaxSize verifies that a request
1108
+// body cut short by request_body max_size is rejected with 413 instead of being
1109
+// forwarded to the upstream as if it were complete.
1110
+func TestReverseProxyUnlimitedRequestBuffersRespectMaxSize(t *testing.T) {
1111
+ var forwarded atomic.Int32
1112
+ srv := &http.Server{
1113
+ Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1114
+ if _, err := io.ReadAll(r.Body); err == nil {
1115
+ forwarded.Add(1)
1116
+ }
1117
+ }),
1118
+ }
1119
+ ln, err := net.Listen("tcp", "127.0.0.1:0")
1120
+ if err != nil {
1121
+ t.Fatalf("failed to listen: %v", err)
1122
+ }
1123
+ go srv.Serve(ln)
1124
+ t.Cleanup(func() { srv.Close(); ln.Close() })
1125
+
1126
+ tester := caddytest.NewTester(t)
1127
+ tester.InitServer(fmt.Sprintf(`
1128
+ {
1129
+ skip_install_trust
1130
+ admin localhost:2999
1131
+ http_port 9080
1132
+ https_port 9443
1133
+ grace_period 1ns
1134
+ }
1135
+ http://localhost:9080 {
1136
+ request_body {
1137
+ max_size 10
1138
+ }
1139
+ reverse_proxy %s {
1140
+ request_buffers unlimited
1141
+ }
1142
+ }
1143
+ `, ln.Addr().String()), "caddyfile")
1144
+
1145
+ req, err := http.NewRequest(http.MethodPost, "http://localhost:9080/", strings.NewReader("abcdefghijklm"))
1146
+ if err != nil {
1147
+ t.Fatalf("creating request: %v", err)
1148
+ }
1149
+ res := tester.AssertResponseCode(req, http.StatusRequestEntityTooLarge)
1150
+ res.Body.Close()
1151
+
1152
+ if n := forwarded.Load(); n != 0 {
1153
+ t.Errorf("upstream received %d complete request bodies, want 0", n)
1154
+ }
1155
+}
modules/caddyhttp/reverseproxy/buffering_test.go
+32
@@ -1,9 +1,11 @@
1
package reverseproxy
2
3
import (
4
+ "errors"
5
"io"
6
"strings"
7
"testing"
8
+ "testing/iotest"
9
)
10
11
type zeroReader struct{}
@@ -121,3 +123,33 @@ func TestBufferingBoundary(t *testing.T) {
123
}
124
}
125
}
126
+
127
+// TestBufferingKeepsReadError checks that a body which fails partway through
128
+// (a request_body max_size limit, a truncated upstream response) is not
129
+// passed on as if it were complete.
130
+func TestBufferingKeepsReadError(t *testing.T) {
131
+ errRead := errors.New("read failed")
132
+
133
+ for _, limit := range []int64{-1, 5, 100} {
134
+ var h Handler
135
+ body := io.NopCloser(io.MultiReader(strings.NewReader("0123456789"), iotest.ErrReader(errRead)))
136
+ res, _ := h.bufferedBody(body, limit)
137
+
138
+ got, err := io.ReadAll(res)
139
+ if !errors.Is(err, errRead) {
140
+ t.Errorf("limit %d: reading buffered body: got error %v, want %v", limit, err, errRead)
141
+ }
142
+ if string(got) != "0123456789" {
143
+ t.Errorf("limit %d: got %q, want %q", limit, got, "0123456789")
144
+ }
145
+
146
+ brc, ok := res.(bodyReadCloser)
147
+ if !ok {
148
+ t.Fatalf("limit %d: expected a bodyReadCloser", limit)
149
+ }
150
+ if brc.body == nil {
151
+ t.Errorf("limit %d: body reported as fully buffered", limit)
152
+ }
153
+ res.Close()
154
+ }
155
+}
modules/caddyhttp/reverseproxy/reverseproxy.go
+11
-11
@@ -1657,27 +1657,27 @@ func (h Handler) provisionUpstream(upstream *Upstream, dynamic bool) {
1657
// (which it shouldn't be), this function returns its input; i.e. is a no-op, for safety.
1658
// Otherwise, it returns bodyReadCloser, the original body will be closed and body will be nil
1659
// if it's explicitly configured to buffer all or EOF is reached when reading.
1660
-// TODO: the error during reading is discarded if the limit is negative, should the error be propagated
1661
-// to upstream/downstream?
1660
+// If reading fails, the body is returned as partially buffered, so the read error
1661
+// is not hidden from the caller.
1662
func (h Handler) bufferedBody(originalBody io.ReadCloser, limit int64) (io.ReadCloser, int64) {
1663
if limit == 0 {
1664
return originalBody, 0
1665
}
1666
var written int64
1667
+ var err error
1668
buf := bufPool.Get().(*bytes.Buffer)
1669
buf.Reset()
1670
if limit > 0 {
1670
- var err error
1671
written, err = io.CopyN(buf, originalBody, limit)
1672
- if (err != nil && err != io.EOF) || written == limit {
1673
- return bodyReadCloser{
1674
- Reader: io.MultiReader(buf, originalBody),
1675
- buf: buf,
1676
- body: originalBody,
1677
- }, written
1678
- }
1672
} else {
1680
- written, _ = io.Copy(buf, originalBody)
1673
+ written, err = io.Copy(buf, originalBody)
1674
+ }
1675
+ if (err != nil && err != io.EOF) || written == limit {
1676
+ return bodyReadCloser{
1677
+ Reader: io.MultiReader(buf, originalBody),
1678
+ buf: buf,
1679
+ body: originalBody,
1680
+ }, written
1681
}
1682
originalBody.Close() // no point in keeping it open
1683
return bodyReadCloser{