From b71a90f58d487576df982a5f2ef9307aaea7c039 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Thu, 2 Jan 2025 10:07:28 +0800 Subject: [PATCH 1/4] buffer requests for fastcgi by default --- modules/caddyhttp/reverseproxy/reverseproxy.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index f9485c570d2..6526afb1d51 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -22,6 +22,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy/fastcgi" "io" "net" "net/http" @@ -243,6 +244,11 @@ func (h *Handler) Provision(ctx caddy.Context) error { return fmt.Errorf("loading transport: %v", err) } h.Transport = mod.(http.RoundTripper) + // enable request buffering for fastcgi if not configured + // TODO: better default buffering for fastcgi requests without content length + if _, ok := h.Transport.(*fastcgi.Transport); ok && h.RequestBuffers == 0 { + h.RequestBuffers = 4096 + } } if h.LoadBalancing != nil && h.LoadBalancing.SelectionPolicyRaw != nil { mod, err := ctx.LoadModule(h.LoadBalancing, "SelectionPolicyRaw") From e0aa52821ec1c8d7078090a4b48a81b5d07aae95 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Thu, 2 Jan 2025 10:24:24 +0800 Subject: [PATCH 2/4] fix import cycle --- modules/caddyhttp/reverseproxy/reverseproxy.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 6526afb1d51..f9bc0c7e8cd 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -22,7 +22,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy/fastcgi" "io" "net" "net/http" @@ -246,7 +245,7 @@ func (h *Handler) Provision(ctx caddy.Context) error { h.Transport = mod.(http.RoundTripper) // enable request buffering for fastcgi if not configured // TODO: better default buffering for fastcgi requests without content length - if _, ok := h.Transport.(*fastcgi.Transport); ok && h.RequestBuffers == 0 { + if module, ok := h.Transport.(caddy.Module); ok && module.CaddyModule().ID.Name() == "fastcgi" && h.RequestBuffers == 0 { h.RequestBuffers = 4096 } } From 7b193d5b0fb555af284afeb68612923fe01ed743 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Thu, 2 Jan 2025 16:27:02 +0800 Subject: [PATCH 3/4] fix the return value of bufferedBody --- modules/caddyhttp/reverseproxy/reverseproxy.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index f9bc0c7e8cd..f4c3095324c 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -1221,13 +1221,14 @@ func (h Handler) bufferedBody(originalBody io.ReadCloser, limit int64) (io.ReadC buf := bufPool.Get().(*bytes.Buffer) buf.Reset() if limit > 0 { - n, err := io.CopyN(buf, originalBody, limit) - if (err != nil && err != io.EOF) || n == limit { + var err error + written, err = io.CopyN(buf, originalBody, limit) + if (err != nil && err != io.EOF) || written == limit { return bodyReadCloser{ Reader: io.MultiReader(buf, originalBody), buf: buf, body: originalBody, - }, n + }, written } } else { written, _ = io.Copy(buf, originalBody) From 3ba88fe13d43615830020ff2b09f8779f1267935 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Thu, 2 Jan 2025 16:45:53 +0800 Subject: [PATCH 4/4] more comments about fastcgi buffering --- modules/caddyhttp/reverseproxy/reverseproxy.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index f4c3095324c..230bec9510c 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -244,7 +244,15 @@ func (h *Handler) Provision(ctx caddy.Context) error { } h.Transport = mod.(http.RoundTripper) // enable request buffering for fastcgi if not configured - // TODO: better default buffering for fastcgi requests without content length + // This is because most fastcgi servers are php-fpm that require the content length to be set to read the body, golang + // std has fastcgi implementation that doesn't need this value to process the body, but we can safely assume that's + // not used. + // http3 requests have a negative content length for GET and HEAD requests, if that header is not sent. + // see: https://github.com/caddyserver/caddy/issues/6678#issuecomment-2472224182 + // Though it appears even if CONTENT_LENGTH is invalid, php-fpm can handle just fine if the body is empty (no Stdin records sent). + // php-fpm will hang if there is any data in the body though, https://github.com/caddyserver/caddy/issues/5420#issuecomment-2415943516 + + // TODO: better default buffering for fastcgi requests without content length, in theory a value of 1 should be enough, make it bigger anyway if module, ok := h.Transport.(caddy.Module); ok && module.CaddyModule().ID.Name() == "fastcgi" && h.RequestBuffers == 0 { h.RequestBuffers = 4096 }