reverseproxy: buffer requests for fastcgi by default (#6759)

* buffer requests for fastcgi by default * fix import cycle * fix the return value of bufferedBody * more comments about fastcgi buffering --------- Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

WeidiDeng committed Jan 3, 2025 at 02:18 UTC 1bd567d7ad41d5509e2aa60cf36e749f195ad83c

1 file changed +17 -3

modules/caddyhttp/reverseproxy/reverseproxy.go

+17 -3

@@ -243,6 +243,19 @@ func (h *Handler) Provision(ctx caddy.Context) error {

243 return fmt.Errorf("loading transport: %v", err)

244 }

245 h.Transport = mod.(http.RoundTripper)

246 + // enable request buffering for fastcgi if not configured

247 + // This is because most fastcgi servers are php-fpm that require the content length to be set to read the body, golang

248 + // std has fastcgi implementation that doesn't need this value to process the body, but we can safely assume that's

249 + // not used.

250 + // http3 requests have a negative content length for GET and HEAD requests, if that header is not sent.

251 + // see: https://github.com/caddyserver/caddy/issues/6678#issuecomment-2472224182

252 + // Though it appears even if CONTENT_LENGTH is invalid, php-fpm can handle just fine if the body is empty (no Stdin records sent).

253 + // php-fpm will hang if there is any data in the body though, https://github.com/caddyserver/caddy/issues/5420#issuecomment-2415943516

254 +

255 + // TODO: better default buffering for fastcgi requests without content length, in theory a value of 1 should be enough, make it bigger anyway

256 + if module, ok := h.Transport.(caddy.Module); ok && module.CaddyModule().ID.Name() == "fastcgi" && h.RequestBuffers == 0 {

257 + h.RequestBuffers = 4096

258 + }

259 }

260 if h.LoadBalancing != nil && h.LoadBalancing.SelectionPolicyRaw != nil {

261 mod, err := ctx.LoadModule(h.LoadBalancing, "SelectionPolicyRaw")

@@ -1216,13 +1229,14 @@ func (h Handler) bufferedBody(originalBody io.ReadCloser, limit int64) (io.ReadC

1229 buf := bufPool.Get().(*bytes.Buffer)

1230 buf.Reset()

1231 if limit > 0 {

1219 - n, err := io.CopyN(buf, originalBody, limit)

1220 - if (err != nil && err != io.EOF) || n == limit {

1232 + var err error

1233 + written, err = io.CopyN(buf, originalBody, limit)

1234 + if (err != nil && err != io.EOF) || written == limit {

1235 return bodyReadCloser{

1236 Reader: io.MultiReader(buf, originalBody),

1237 buf: buf,

1238 body: originalBody,

1225 - }, n

1239 + }, written

1240 }

1241 } else {

1242 written, _ = io.Copy(buf, originalBody)