-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat: add setup check for request buffering on FPM #54912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Context: Using `Transfer-Encoding: chunked` of HTTP 1.1 PHP-FPM has a bug[1] where the request body is not passed to PHP if the `Content-Length` header is missing, while FastCGI in general allows this (I could reproduce that FastCGI passed the request stream from NGinx) PHP-FPM does not forward this to the PHP application. This means when using PHP-FPM we get an empty request body and thus every `PUT` will be an empty file. I tested that `mod_php` is not affected, while it also has no `Content-Length` header, it correctly passed the stream and thus also works without buffering the request. Only PHP-FPM needs buffering of the request so that a `Content-Length` header can be generated. To enable this on Apache set: `SetEnvIfNoCase Transfer-Encoding "chunked" proxy-sendcl=1` On NGinx: `fastcgi_request_buffering on;`. [1]: php/php-src#9441 ref: #7995 Signed-off-by: Ferdinand Thiessen <[email protected]>
|
/backport to stable32 |
|
/backport to stable31 |
| $url = $this->urlGenerator->linkToRoute('settings.CheckSetup.checkContentLengthHeader'); | ||
| foreach ($this->runRequest('PUT', $url, ['ignoreSSL' => true, 'options' => $options]) as $response) { | ||
| $contentType = $response->getHeader('Content-Type'); | ||
| if (!str_contains(strtolower($contentType), 'application/json')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’d like a comment for this one, what does it mean if content type is something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then its not a Nextcloud response - some safe guard for reverse proxys so the following check on the response can be done.
| . $this->l10n->t('Due to a limitation of PHP-FPM chunked requests will not be passed to Nextcloud if the server does not buffer such requests.'), | ||
| ); | ||
| } else { | ||
| // Not using FPM but we are on CLI so we do not know if FPM is used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The endpoint could return $usingFPM while we’re at it, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
Summary
Context: Using
Transfer-Encoding: chunkedof HTTP 1.1PHP-FPM has a bug1 where the request body is not passed to PHP if the
Content-Lengthheader is missing, while FastCGI in general allows this (I could reproduce that FastCGI passed the request stream from NGinx) PHP-FPM does not forward this to the PHP application.This means when using PHP-FPM we get an empty request body and thus every
PUTwill be an empty file.I tested that
mod_phpis not affected, while it also has noContent-Lengthheader, it correctly passed the stream and thus also works without buffering the request.Only PHP-FPM needs buffering of the request so that a
Content-Lengthheader can be generated.To enable this on Apache set:
SetEnvIfNoCase Transfer-Encoding "chunked" proxy-sendcl=1On NGinx:
fastcgi_request_buffering on;.ref: #7995
Checklist