Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
Mirrored from https://github.com/Slazee/caddy.git
Synced Nov 13, 05:30 UTC
reverseproxy: Don't buffer chunked requests (fix #5366) (#5367)
* reverseproxy: Don't buffer chunked requests (fix #5366) Mostly reverts 845bc4d50b437995d574819850206e4b3db4040d (#5289) Adds warning for unsafe config. Deprecates unsafe properties in favor of simpler, safer designed ones. * Update modules/caddyhttp/reverseproxy/caddyfile.go Co-authored-by: Y.Horie <u5.horie@gmail.com> * Update modules/caddyhttp/reverseproxy/reverseproxy.go Co-authored-by: Y.Horie <u5.horie@gmail.com> * Update modules/caddyhttp/reverseproxy/reverseproxy.go Co-authored-by: Y.Horie <u5.horie@gmail.com> * Remove unused code --------- Co-authored-by: Y.Horie <u5.horie@gmail.com>
Matt Holt committed
Feb 11, 2023 at 17:25 UTC
4b119a475fa345e7d6eb44c71cdf7ab645bd381e
2 files changed
+80
-41
modules/caddyhttp/reverseproxy/caddyfile.go
+28
-7
@@ -521,19 +521,39 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
521
h.FlushInterval = caddy.Duration(dur)
522
}
523
524
- case "buffer_requests":
524
+ case "request_buffers", "response_buffers":
525
+ subdir := d.Val()
526
+ if !d.NextArg() {
527
+ return d.ArgErr()
528
+ }
529
+ size, err := humanize.ParseBytes(d.Val())
530
+ if err != nil {
531
+ return d.Errf("invalid byte size '%s': %v", d.Val(), err)
532
+ }
533
if d.NextArg() {
534
return d.ArgErr()
535
}
528
- h.BufferRequests = true
536
+ if subdir == "request_buffers" {
537
+ h.RequestBuffers = int64(size)
538
+ } else if subdir == "response_buffers" {
539
+ h.ResponseBuffers = int64(size)
540
530
- case "buffer_responses":
541
+ }
542
+
543
+ // TODO: These three properties are deprecated; remove them sometime after v2.6.4
544
+ case "buffer_requests": // TODO: deprecated
545
if d.NextArg() {
546
return d.ArgErr()
547
}
534
- h.BufferResponses = true
535
-
536
- case "max_buffer_size":
548
+ caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: buffer_requests: use request_buffers instead (with a maximum buffer size)")
549
+ h.DeprecatedBufferRequests = true
550
+ case "buffer_responses": // TODO: deprecated
551
+ if d.NextArg() {
552
+ return d.ArgErr()
553
+ }
554
+ caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: buffer_responses: use response_buffers instead (with a maximum buffer size)")
555
+ h.DeprecatedBufferResponses = true
556
+ case "max_buffer_size": // TODO: deprecated
557
if !d.NextArg() {
558
return d.ArgErr()
559
}
@@ -544,7 +564,8 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
564
if d.NextArg() {
565
return d.ArgErr()
566
}
547
- h.MaxBufferSize = int64(size)
567
+ caddy.Log().Named("config.adapter.caddyfile").Warn("DEPRECATED: max_buffer_size: use request_buffers and/or response_buffers instead (with maximum buffer sizes)")
568
+ h.DeprecatedMaxBufferSize = int64(size)
569
570
case "trusted_proxies":
571
for d.NextArg() {
modules/caddyhttp/reverseproxy/reverseproxy.go
+52
-34
@@ -136,21 +136,27 @@ type Handler struct {
136
// are also set implicitly.
137
Headers *headers.Handler `json:"headers,omitempty"`
138
139
- // If true, the entire request body will be read and buffered
140
- // in memory before being proxied to the backend. This should
141
- // be avoided if at all possible for performance reasons, but
142
- // could be useful if the backend is intolerant of read latency.
143
- BufferRequests bool `json:"buffer_requests,omitempty"`
144
-
145
- // If true, the entire response body will be read and buffered
146
- // in memory before being proxied to the client. This should
147
- // be avoided if at all possible for performance reasons, but
148
- // could be useful if the backend has tighter memory constraints.
149
- BufferResponses bool `json:"buffer_responses,omitempty"`
139
+ // DEPRECATED: Do not use; will be removed. See request_buffers instead.
140
+ DeprecatedBufferRequests bool `json:"buffer_requests,omitempty"`
141
+
142
+ // DEPRECATED: Do not use; will be removed. See response_buffers instead.
143
+ DeprecatedBufferResponses bool `json:"buffer_responses,omitempty"`
144
+
145
+ // DEPRECATED: Do not use; will be removed. See request_buffers and response_buffers instead.
146
+ DeprecatedMaxBufferSize int64 `json:"max_buffer_size,omitempty"`
147
+
148
+ // If nonzero, the entire request body up to this size will be read
149
+ // and buffered in memory before being proxied to the backend. This
150
+ // should be avoided if at all possible for performance reasons, but
151
+ // could be useful if the backend is intolerant of read latency or
152
+ // chunked encodings.
153
+ RequestBuffers int64 `json:"request_buffers,omitempty"`
154
151
- // If body buffering is enabled, the maximum size of the buffers
152
- // used for the requests and responses (in bytes).
153
- MaxBufferSize int64 `json:"max_buffer_size,omitempty"`
155
+ // If nonzero, the entire response body up to this size will be read
156
+ // and buffered in memory before being proxied to the client. This
157
+ // should be avoided if at all possible for performance reasons, but
158
+ // could be useful if the backend has tighter memory constraints.
159
+ ResponseBuffers int64 `json:"response_buffers,omitempty"`
160
161
// If configured, rewrites the copy of the upstream request.
162
// Allows changing the request method and URI (path and query).
@@ -221,11 +227,28 @@ func (h *Handler) Provision(ctx caddy.Context) error {
227
h.connections = make(map[io.ReadWriteCloser]openConnection)
228
h.connectionsMu = new(sync.Mutex)
229
230
+ // TODO: remove deprecated fields sometime after v2.6.4
231
+ if h.DeprecatedBufferRequests {
232
+ h.logger.Warn("DEPRECATED: buffer_requests: this property will be removed soon; use request_buffers instead (and set a maximum buffer size)")
233
+ }
234
+ if h.DeprecatedBufferResponses {
235
+ h.logger.Warn("DEPRECATED: buffer_responses: this property will be removed soon; use response_buffers instead (and set a maximum buffer size)")
236
+ }
237
+ if h.DeprecatedMaxBufferSize != 0 {
238
+ h.logger.Warn("DEPRECATED: max_buffer_size: this property will be removed soon; use request_buffers and/or response_buffers instead (and set maximum buffer sizes)")
239
+ }
240
+
241
+ // warn about unsafe buffering config
242
+ if h.RequestBuffers == -1 || h.ResponseBuffers == -1 {
243
+ h.logger.Warn("UNLIMITED BUFFERING: buffering is enabled without any cap on buffer size, which can result in OOM crashes")
244
+ }
245
+
246
// verify SRV compatibility - TODO: LookupSRV deprecated; will be removed
247
for i, v := range h.Upstreams {
248
if v.LookupSRV == "" {
249
continue
250
}
251
+ h.logger.Warn("DEPRECATED: lookup_srv: will be removed in a near-future version of Caddy; use the http.reverse_proxy.upstreams.srv module instead")
252
if h.HealthChecks != nil && h.HealthChecks.Active != nil {
253
return fmt.Errorf(`upstream: lookup_srv is incompatible with active health checks: %d: {"dial": %q, "lookup_srv": %q}`, i, v.Dial, v.LookupSRV)
254
}
@@ -622,9 +645,8 @@ func (h Handler) prepareRequest(req *http.Request, repl *caddy.Replacer) (*http.
645
// attacks, so it is strongly recommended to only use this
646
// feature if absolutely required, if read timeouts are
647
// set, and if body size is limited
625
- if (h.BufferRequests || isChunkedRequest(req)) && req.Body != nil {
626
- req.Body, req.ContentLength = h.bufferedBody(req.Body)
627
- req.Header.Set("Content-Length", strconv.FormatInt(req.ContentLength, 10))
648
+ if h.RequestBuffers != 0 && req.Body != nil {
649
+ req.Body, _ = h.bufferedBody(req.Body, h.RequestBuffers)
650
}
651
652
if req.ContentLength == 0 {
@@ -852,8 +874,8 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe
874
}
875
876
// if enabled, buffer the response body
855
- if h.BufferResponses {
856
- res.Body, _ = h.bufferedBody(res.Body)
877
+ if h.ResponseBuffers != 0 {
878
+ res.Body, _ = h.bufferedBody(res.Body, h.ResponseBuffers)
879
}
880
881
// see if any response handler is configured for this response from the backend
@@ -1122,15 +1144,20 @@ func (h Handler) provisionUpstream(upstream *Upstream) {
1144
}
1145
}
1146
1125
-// bufferedBody reads originalBody into a buffer, then returns a reader for the buffer.
1126
-// Always close the return value when done with it, just like if it was the original body!
1127
-func (h Handler) bufferedBody(originalBody io.ReadCloser) (io.ReadCloser, int64) {
1147
+// bufferedBody reads originalBody into a buffer with maximum size of limit (-1 for unlimited),
1148
+// then returns a reader for the buffer along with how many bytes were buffered. Always close
1149
+// the return value when done with it, just like if it was the original body! If limit is 0
1150
+// (which it shouldn't be), this function returns its input; i.e. is a no-op, for safety.
1151
+func (h Handler) bufferedBody(originalBody io.ReadCloser, limit int64) (io.ReadCloser, int64) {
1152
+ if limit == 0 {
1153
+ return originalBody, 0
1154
+ }
1155
var written int64
1156
buf := bufPool.Get().(*bytes.Buffer)
1157
buf.Reset()
1131
- if h.MaxBufferSize > 0 {
1132
- n, err := io.CopyN(buf, originalBody, h.MaxBufferSize)
1133
- if err != nil || n == h.MaxBufferSize {
1158
+ if limit > 0 {
1159
+ n, err := io.CopyN(buf, originalBody, limit)
1160
+ if err != nil || n == limit {
1161
return bodyReadCloser{
1162
Reader: io.MultiReader(buf, originalBody),
1163
buf: buf,
@@ -1147,15 +1174,6 @@ func (h Handler) bufferedBody(originalBody io.ReadCloser) (io.ReadCloser, int64)
1174
}, written
1175
}
1176
1150
-func isChunkedRequest(req *http.Request) bool {
1151
- for _, transferEncoding := range req.TransferEncoding {
1152
- if transferEncoding == "chunked" {
1153
- return true
1154
- }
1155
- }
1156
- return false
1157
-}
1158
-
1177
// cloneRequest makes a semi-deep clone of origReq.
1178
//
1179
// Most of this code is borrowed from the Go stdlib reverse proxy,