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, 04:45 UTC
fastcgi: explain the 411 a body without a length gets (#7956)
A request whose body has no length of its own — Transfer-Encoding: chunked, or HTTP/2 and HTTP/3 requests sent without the header — cannot be forwarded over FastCGI until it has been buffered in full, because CGI/1.1 requires CONTENT_LENGTH and php-fpm hangs when it is absent or wrong and the body is not empty. When request_buffers is too small to hold the whole body, the length stays unknown and the request is refused with 411. The refusal said none of that. RoundTrip passes r.ContentLength to Post, which is -1 for such a request, so the operator got a bare "411 Length Required" with either no error at all or strconv's "invalid syntax" on a value they never wrote. On #7386 that sent two people looking for the fault in their backend. The 411 now carries the reason and the remedy, and the unknown-length case is told apart from a genuinely malformed value. ParseUint becomes ParseInt plus an explicit negative check, which is strictly tighter: values above MaxInt64 used to be accepted and are unreachable anyway, since CONTENT_LENGTH is always FormatInt of an int64 by the time Do sees it. No status code changes. Every request that was refused before is still refused; raising request_buffers is still what makes a large chunked body work. Tests: the reachable path through Post, the unusable CONTENT_LENGTH values Do itself guards against, an integration test driving a chunked body through a fastcgi reverse_proxy over a unix socket at each side of the buffer boundary (including request_buffers -1, which succeeds at any size and is the remedy), and a boundary test recording that a body exactly the size of the buffer counts as partial. That last one is deliberately not changed here. Telling "exactly the limit" apart from "more to come" costs a read that a paused stream may never answer, and bufferedBody also backs response_buffers: measured against a body that delivers exactly the limit and stops, the current code hands the buffered prefix on immediately, while peeking one byte first delivers nothing at all. Co-authored-by: Aditya <205600203+Rohilalala@users.noreply.github.com> Co-authored-by: Zen Dodd <mail@steadytao.com>
Rohilalala committed
Sep 23, 2026 at 21:01 UTC
c798d4c23831dc5aff4198519defb78c8dffad9a
4 files changed
+339
-7
caddytest/integration/fastcgi_chunked_test.go
new
+142
@@ -0,0 +1,142 @@
1
+package integration
2
+
3
+import (
4
+ "fmt"
5
+ "io"
6
+ "net"
7
+ "net/http"
8
+ "net/http/fcgi"
9
+ "os"
10
+ "runtime"
11
+ "strings"
12
+ "testing"
13
+
14
+ "github.com/caddyserver/caddy/v2/caddytest"
15
+)
16
+
17
+// TestFastCGIChunkedRequestBody drives the case from caddyserver/caddy#7386: a
18
+// client sending a body with Transfer-Encoding: chunked through a fastcgi
19
+// reverse_proxy over a unix socket.
20
+//
21
+// FastCGI cannot express a body of unknown length — CGI/1.1 requires
22
+// CONTENT_LENGTH, and php-fpm hangs when it is absent or wrong and the body is
23
+// non-empty — so such a body is only forwardable once it has been buffered in
24
+// full. request_buffers is therefore what decides the outcome, and the default
25
+// of 4096 for this transport is far below a git push or a file upload.
26
+func TestFastCGIChunkedRequestBody(t *testing.T) {
27
+ if runtime.GOOS == "windows" {
28
+ t.SkipNow()
29
+ }
30
+
31
+ socketName := tempSocketName(t)
32
+
33
+ // Report how many body bytes actually arrived, so a truncated body shows up
34
+ // as a failure instead of passing quietly.
35
+ ln, err := net.Listen("unix", socketName)
36
+ if err != nil {
37
+ t.Fatalf("failed to listen on the socket: %s", err)
38
+ }
39
+ go fcgi.Serve(ln, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { //nolint:errcheck
40
+ n, _ := io.Copy(io.Discard, r.Body)
41
+ fmt.Fprintf(w, "received %d", n)
42
+ }))
43
+ t.Cleanup(func() { ln.Close() })
44
+ runtime.Gosched()
45
+
46
+ const requestBuffers = 4096
47
+
48
+ tester := caddytest.NewTester(t)
49
+ tester.InitServer(fmt.Sprintf(`
50
+ {
51
+ "admin": {"listen": "localhost:2999"},
52
+ "apps": {
53
+ "pki": {"certificate_authorities": {"local": {"install_trust": false}}},
54
+ "http": {
55
+ "grace_period": 1,
56
+ "servers": {
57
+ "srv0": {
58
+ "listen": [":18080"],
59
+ "routes": [
60
+ {
61
+ "match": [{"path": ["/limited"]}],
62
+ "handle": [{
63
+ "handler": "reverse_proxy",
64
+ "request_buffers": %d,
65
+ "transport": {"protocol": "fastcgi"},
66
+ "upstreams": [{"dial": "unix/%s"}]
67
+ }]
68
+ },
69
+ {
70
+ "match": [{"path": ["/unlimited"]}],
71
+ "handle": [{
72
+ "handler": "reverse_proxy",
73
+ "request_buffers": -1,
74
+ "transport": {"protocol": "fastcgi"},
75
+ "upstreams": [{"dial": "unix/%s"}]
76
+ }]
77
+ }
78
+ ]
79
+ }
80
+ }
81
+ }
82
+ }
83
+ }
84
+ `, requestBuffers, socketName, socketName), "json")
85
+
86
+ tests := []struct {
87
+ name string
88
+ path string
89
+ size int
90
+ wantStatus int
91
+ }{
92
+ {name: "small body fits the buffer", path: "/limited", size: 16, wantStatus: http.StatusOK},
93
+ {name: "one below the buffer", path: "/limited", size: requestBuffers - 1, wantStatus: http.StatusOK},
94
+ // The body is buffered up to the limit and no further, so its length
95
+ // stays unknown and FastCGI has nothing to put in CONTENT_LENGTH.
96
+ {name: "exactly the buffer", path: "/limited", size: requestBuffers, wantStatus: http.StatusLengthRequired},
97
+ {name: "above the buffer", path: "/limited", size: requestBuffers * 4, wantStatus: http.StatusLengthRequired},
98
+ // Unlimited buffering is the remedy: the whole body is read, so its
99
+ // length is known and the request goes through at any size.
100
+ {name: "above the buffer, buffering unlimited", path: "/unlimited", size: requestBuffers * 4, wantStatus: http.StatusOK},
101
+ }
102
+
103
+ for _, tc := range tests {
104
+ t.Run(tc.name, func(t *testing.T) {
105
+ body := strings.Repeat("x", tc.size)
106
+ // A strings.Reader would let net/http size the body; wrapping it in
107
+ // a bare io.Reader is what makes the request chunked.
108
+ req, err := http.NewRequest(http.MethodPost, "http://localhost:18080"+tc.path, struct{ io.Reader }{strings.NewReader(body)})
109
+ if err != nil {
110
+ t.Fatalf("building request: %s", err)
111
+ }
112
+
113
+ resp := tester.AssertResponseCode(req, tc.wantStatus)
114
+ defer resp.Body.Close()
115
+ if tc.wantStatus != http.StatusOK {
116
+ return
117
+ }
118
+ got, err := io.ReadAll(resp.Body)
119
+ if err != nil {
120
+ t.Fatalf("reading response: %s", err)
121
+ }
122
+ // Every byte has to reach the upstream, not just a buffer's worth.
123
+ if want := fmt.Sprintf("received %d", tc.size); string(got) != want {
124
+ t.Errorf("upstream got %q, want %q", got, want)
125
+ }
126
+ })
127
+ }
128
+}
129
+
130
+// tempSocketName borrows a name inside a valid path to bind a unix socket on.
131
+func tempSocketName(t *testing.T) string {
132
+ t.Helper()
133
+ f, err := os.CreateTemp("", "*.sock")
134
+ if err != nil {
135
+ t.Fatalf("failed to create TempFile: %s", err)
136
+ }
137
+ name := f.Name()
138
+ f.Close()
139
+ os.Remove(name)
140
+ t.Cleanup(func() { os.Remove(name) })
141
+ return name
142
+}
modules/caddyhttp/reverseproxy/buffering_test.go
+39
@@ -2,6 +2,7 @@ package reverseproxy
2
3
import (
4
"io"
5
+ "strings"
6
"testing"
7
)
8
@@ -82,3 +83,41 @@ func TestBuffering(t *testing.T) {
83
})
84
}
85
}
86
+
87
+// TestBufferingBoundary documents where a buffered body stops being fully
88
+// buffered, which is what decides whether the caller can learn its length and
89
+// set Content-Length. A body the size of the limit counts as partial: io.CopyN
90
+// filling the buffer cannot be told apart from a body with more to come, and
91
+// finding out would mean a blocking read on a stream that may never deliver.
92
+func TestBufferingBoundary(t *testing.T) {
93
+ const body = "0123456789"
94
+
95
+ for limit := int64(1); limit <= int64(len(body))+2; limit++ {
96
+ var h Handler
97
+ res, read := h.bufferedBody(io.NopCloser(strings.NewReader(body)), limit)
98
+
99
+ // Whatever the limit, the upstream must still receive every byte.
100
+ got, err := io.ReadAll(res)
101
+ if err != nil {
102
+ t.Fatalf("limit %d: reading buffered body: %v", limit, err)
103
+ }
104
+ if string(got) != body {
105
+ t.Errorf("limit %d: body changed: got %q, want %q", limit, got, body)
106
+ }
107
+ if err := res.Close(); err != nil {
108
+ t.Errorf("limit %d: closing buffered body: %v", limit, err)
109
+ }
110
+
111
+ brc, ok := res.(bodyReadCloser)
112
+ if !ok {
113
+ t.Fatalf("limit %d: expected a bodyReadCloser", limit)
114
+ }
115
+ wantFull := limit > int64(len(body))
116
+ if gotFull := brc.body == nil; gotFull != wantFull {
117
+ t.Errorf("limit %d: fully buffered = %v, want %v", limit, gotFull, wantFull)
118
+ }
119
+ if wantFull && read != int64(len(body)) {
120
+ t.Errorf("limit %d: read %d bytes, want %d", limit, read, len(body))
121
+ }
122
+ }
123
+}
modules/caddyhttp/reverseproxy/fastcgi/client.go
+24
-7
@@ -26,6 +26,8 @@ package fastcgi
26
import (
27
"bufio"
28
"bytes"
29
+ "errors"
30
+ "fmt"
31
"io"
32
"mime/multipart"
33
"net"
@@ -125,6 +127,15 @@ const (
127
// not synchronized because we don't care what the contents are
128
var pad [maxPad]byte
129
130
+// FastCGI has no way to express a body of unknown length: CONTENT_LENGTH is
131
+// required by CGI/1.1, and php-fpm hangs when it is absent or wrong and the
132
+// body is not empty. A request that arrives without a length therefore has to
133
+// be buffered in full before it can be forwarded.
134
+var (
135
+ errNoContentLength = errors.New("request body has no known length; increase request_buffers so the body can be buffered in full to determine it")
136
+ errBadContentLength = errors.New("request body length is not a valid CONTENT_LENGTH")
137
+)
138
+
139
// client implements a FastCGI client, which is a standard for
140
// interfacing external applications with Web servers.
141
type client struct {
@@ -138,13 +149,19 @@ type client struct {
149
// Do makes the request and returns an io.Reader that translates the data read
150
// from the FastCGI responder out of FastCGI packets before returning it.
151
func (c *client) Do(p map[string]string, req io.Reader) (r io.Reader, err error) {
141
- // check for CONTENT_LENGTH, since the lack of it or wrong value will cause the backend to hang
142
- if clStr, ok := p["CONTENT_LENGTH"]; !ok {
143
- return nil, caddyhttp.Error(http.StatusLengthRequired, nil)
144
- } else if _, err := strconv.ParseUint(clStr, 10, 64); err != nil {
145
- // stdlib won't return a negative Content-Length, but we check just in case,
146
- // the most likely cause is from a missing content length, which is -1
147
- return nil, caddyhttp.Error(http.StatusLengthRequired, err)
152
+ // check for CONTENT_LENGTH, since the lack of it or wrong value will cause the backend to hang.
153
+ // A request with no length of its own (Transfer-Encoding: chunked, or an HTTP/2 or HTTP/3
154
+ // request sent without the header) only acquires one by being buffered in full, so say so:
155
+ // the bare status alone has sent people looking for the fault in their backend.
156
+ if clStr, ok := p["CONTENT_LENGTH"]; !ok || clStr == "" {
157
+ return nil, caddyhttp.Error(http.StatusLengthRequired, errNoContentLength)
158
+ } else if l, err := strconv.ParseInt(clStr, 10, 64); err != nil {
159
+ return nil, caddyhttp.Error(http.StatusLengthRequired, fmt.Errorf("%w: %w", errBadContentLength, err))
160
+ } else if l < 0 {
161
+ // net/http reports -1 for a body whose length it does not know, which is
162
+ // what a chunked request looks like by the time it reaches here. This is
163
+ // the case operators actually hit, so it gets the actionable message.
164
+ return nil, caddyhttp.Error(http.StatusLengthRequired, errNoContentLength)
165
}
166
167
writer := &streamWriter{c: c}
modules/caddyhttp/reverseproxy/fastcgi/contentlength_test.go
new
+134
@@ -0,0 +1,134 @@
1
+// Copyright 2015 Matthew Holt and The Caddy Authors
2
+//
3
+// Licensed under the Apache License, Version 2.0 (the "License");
4
+// you may not use this file except in compliance with the License.
5
+// You may obtain a copy of the License at
6
+//
7
+// http://www.apache.org/licenses/LICENSE-2.0
8
+//
9
+// Unless required by applicable law or agreed to in writing, software
10
+// distributed under the License is distributed on an "AS IS" BASIS,
11
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12
+// See the License for the specific language governing permissions and
13
+// limitations under the License.
14
+
15
+package fastcgi
16
+
17
+import (
18
+ "errors"
19
+ "net/http"
20
+ "strings"
21
+ "testing"
22
+
23
+ "github.com/caddyserver/caddy/v2/modules/caddyhttp"
24
+)
25
+
26
+// TestDoRejectsUnusableContentLength pins what the client does with a
27
+// CONTENT_LENGTH it cannot use, and which of those cases carries which message.
28
+//
29
+// Only "-1" is reachable through Transport.RoundTrip, which is the shape a
30
+// chunked request takes: Get/Head/Options/Post all set CONTENT_LENGTH before
31
+// Do sees it, from the r.ContentLength that net/http reports as -1 when it has
32
+// no length. The absent and empty cases guard Do's own contract for any future
33
+// caller; an absent key was the one that used to return a 411 with no error at
34
+// all, while an empty value carried strconv's syntax error, true but useless
35
+// to an operator. TestPostRejectsUnknownLengthWithRemedy covers the reachable
36
+// path end to end.
37
+func TestDoRejectsUnusableContentLength(t *testing.T) {
38
+ t.Parallel()
39
+
40
+ tests := []struct {
41
+ name string
42
+ env map[string]string
43
+ wantErr error
44
+ }{
45
+ {
46
+ name: "absent",
47
+ env: map[string]string{},
48
+ wantErr: errNoContentLength,
49
+ },
50
+ {
51
+ name: "empty",
52
+ env: map[string]string{"CONTENT_LENGTH": ""},
53
+ wantErr: errNoContentLength,
54
+ },
55
+ {
56
+ // The reported case: what a chunked request becomes by the time it
57
+ // reaches here, and the only one production can produce.
58
+ name: "negative, as a chunked request arrives",
59
+ env: map[string]string{"CONTENT_LENGTH": "-1"},
60
+ wantErr: errNoContentLength,
61
+ },
62
+ {
63
+ name: "not a number",
64
+ env: map[string]string{"CONTENT_LENGTH": "banana"},
65
+ wantErr: errBadContentLength,
66
+ },
67
+ }
68
+
69
+ for _, tc := range tests {
70
+ t.Run(tc.name, func(t *testing.T) {
71
+ t.Parallel()
72
+
73
+ c := &client{}
74
+ _, err := c.Do(tc.env, strings.NewReader("body"))
75
+ if err == nil {
76
+ t.Fatal("expected an error")
77
+ }
78
+
79
+ var handlerErr caddyhttp.HandlerError
80
+ if !errors.As(err, &handlerErr) {
81
+ t.Fatalf("error %v is not a caddyhttp.HandlerError", err)
82
+ }
83
+ if handlerErr.StatusCode != http.StatusLengthRequired {
84
+ t.Errorf("status = %d, want %d", handlerErr.StatusCode, http.StatusLengthRequired)
85
+ }
86
+ // The status alone is what sent people hunting through their
87
+ // backend logs, so the cause has to travel with it.
88
+ if handlerErr.Err == nil {
89
+ t.Fatal("the 411 carries no error to explain it")
90
+ }
91
+ if !errors.Is(err, tc.wantErr) {
92
+ t.Errorf("error %v does not wrap %v", err, tc.wantErr)
93
+ }
94
+ })
95
+ }
96
+}
97
+
98
+// TestNoContentLengthErrorNamesTheRemedy keeps the message actionable: the
99
+// operator has to learn that buffering is what supplies the missing length.
100
+func TestNoContentLengthErrorNamesTheRemedy(t *testing.T) {
101
+ t.Parallel()
102
+
103
+ if !strings.Contains(errNoContentLength.Error(), "request_buffers") {
104
+ t.Errorf("error %q does not name request_buffers as the remedy", errNoContentLength)
105
+ }
106
+}
107
+
108
+// TestPostRejectsUnknownLengthWithRemedy goes through the entry point RoundTrip
109
+// actually uses. Transport.RoundTrip passes r.ContentLength straight to Post,
110
+// and net/http reports -1 for a chunked body, so this is the exact call the
111
+// reporter of #7386 made. Asserting it only against Do would let the message
112
+// rot on a branch production never reaches.
113
+func TestPostRejectsUnknownLengthWithRemedy(t *testing.T) {
114
+ t.Parallel()
115
+
116
+ c := &client{}
117
+ env := map[string]string{}
118
+
119
+ _, err := c.Post(env, http.MethodPost, "application/octet-stream", strings.NewReader("body"), -1)
120
+ if err == nil {
121
+ t.Fatal("expected an error")
122
+ }
123
+ if !errors.Is(err, errNoContentLength) {
124
+ t.Errorf("error %v does not wrap errNoContentLength", err)
125
+ }
126
+ if !strings.Contains(err.Error(), "request_buffers") {
127
+ t.Errorf("the error operators see, %q, does not name the remedy", err)
128
+ }
129
+
130
+ var handlerErr caddyhttp.HandlerError
131
+ if !errors.As(err, &handlerErr) || handlerErr.StatusCode != http.StatusLengthRequired {
132
+ t.Errorf("error %v is not a 411 HandlerError", err)
133
+ }
134
+}