From ccc4d2eb9fdcf88b09abff5e31e1f320cd9389c3 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 5 Sep 2025 01:06:13 +0200 Subject: [PATCH] feat: add setup check for request buffering on FPM 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]: https://github.com/php/php-src/issues/9441 ref: https://github.com/nextcloud/server/issues/7995 Signed-off-by: Ferdinand Thiessen --- apps/settings/appinfo/routes.php | 3 + .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + apps/settings/lib/AppInfo/Application.php | 2 + .../lib/Controller/CheckSetupController.php | 22 ++++- .../lib/SetupChecks/RequestBuffering.php | 96 +++++++++++++++++++ 6 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 apps/settings/lib/SetupChecks/RequestBuffering.php diff --git a/apps/settings/appinfo/routes.php b/apps/settings/appinfo/routes.php index c3f1333e6baf9..c29d50ac18f47 100644 --- a/apps/settings/appinfo/routes.php +++ b/apps/settings/appinfo/routes.php @@ -44,10 +44,13 @@ ['name' => 'Users#usersListByGroup', 'url' => '/settings/users/{group}', 'verb' => 'GET', 'requirements' => ['group' => '.+'] , 'root' => ''], ['name' => 'Users#setPreference', 'url' => '/settings/users/preferences/{key}', 'verb' => 'POST' , 'root' => ''], ['name' => 'LogSettings#download', 'url' => '/settings/admin/log/download', 'verb' => 'GET' , 'root' => ''], + ['name' => 'CheckSetup#setupCheckManager', 'url' => '/settings/setupcheck', 'verb' => 'GET' , 'root' => ''], ['name' => 'CheckSetup#check', 'url' => '/settings/ajax/checksetup', 'verb' => 'GET' , 'root' => ''], + ['name' => 'CheckSetup#checkContentLengthHeader', 'url' => '/settings/setupcheck/test-content-length', 'verb' => 'PUT' , 'root' => ''], ['name' => 'CheckSetup#getFailedIntegrityCheckFiles', 'url' => '/settings/integrity/failed', 'verb' => 'GET' , 'root' => ''], ['name' => 'CheckSetup#rescanFailedIntegrityCheck', 'url' => '/settings/integrity/rescan', 'verb' => 'GET' , 'root' => ''], + ['name' => 'PersonalSettings#index', 'url' => '/settings/user/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'personal-info'] , 'root' => ''], ['name' => 'AdminSettings#index', 'url' => '/settings/admin/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'server'] , 'root' => ''], ['name' => 'AdminSettings#form', 'url' => '/settings/admin/{section}', 'verb' => 'GET' , 'root' => ''], diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index bb63026da77a3..bda380a366869 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -128,6 +128,7 @@ 'OCA\\Settings\\SetupChecks\\PushService' => $baseDir . '/../lib/SetupChecks/PushService.php', 'OCA\\Settings\\SetupChecks\\RandomnessSecure' => $baseDir . '/../lib/SetupChecks/RandomnessSecure.php', 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php', + 'OCA\\Settings\\SetupChecks\\RequestBuffering' => $baseDir . '/../lib/SetupChecks/RequestBuffering.php', 'OCA\\Settings\\SetupChecks\\SchedulingTableSize' => $baseDir . '/../lib/SetupChecks/SchedulingTableSize.php', 'OCA\\Settings\\SetupChecks\\SecurityHeaders' => $baseDir . '/../lib/SetupChecks/SecurityHeaders.php', 'OCA\\Settings\\SetupChecks\\SupportedDatabase' => $baseDir . '/../lib/SetupChecks/SupportedDatabase.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index cca48b409ad15..8b5621cd86b92 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -143,6 +143,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\SetupChecks\\PushService' => __DIR__ . '/..' . '/../lib/SetupChecks/PushService.php', 'OCA\\Settings\\SetupChecks\\RandomnessSecure' => __DIR__ . '/..' . '/../lib/SetupChecks/RandomnessSecure.php', 'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php', + 'OCA\\Settings\\SetupChecks\\RequestBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/RequestBuffering.php', 'OCA\\Settings\\SetupChecks\\SchedulingTableSize' => __DIR__ . '/..' . '/../lib/SetupChecks/SchedulingTableSize.php', 'OCA\\Settings\\SetupChecks\\SecurityHeaders' => __DIR__ . '/..' . '/../lib/SetupChecks/SecurityHeaders.php', 'OCA\\Settings\\SetupChecks\\SupportedDatabase' => __DIR__ . '/..' . '/../lib/SetupChecks/SupportedDatabase.php', diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php index 6e59e56fe8218..23acfd46124e8 100644 --- a/apps/settings/lib/AppInfo/Application.php +++ b/apps/settings/lib/AppInfo/Application.php @@ -66,6 +66,7 @@ use OCA\Settings\SetupChecks\PushService; use OCA\Settings\SetupChecks\RandomnessSecure; use OCA\Settings\SetupChecks\ReadOnlyConfig; +use OCA\Settings\SetupChecks\RequestBuffering; use OCA\Settings\SetupChecks\SchedulingTableSize; use OCA\Settings\SetupChecks\SecurityHeaders; use OCA\Settings\SetupChecks\SupportedDatabase; @@ -203,6 +204,7 @@ public function register(IRegistrationContext $context): void { $context->registerSetupCheck(PhpOutputBuffering::class); $context->registerSetupCheck(RandomnessSecure::class); $context->registerSetupCheck(ReadOnlyConfig::class); + $context->registerSetupCheck(RequestBuffering::class); $context->registerSetupCheck(SecurityHeaders::class); $context->registerSetupCheck(SchedulingTableSize::class); $context->registerSetupCheck(SupportedDatabase::class); diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 82a46bea4fb58..9b2a508f9dd93 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -11,10 +11,12 @@ use OC\IntegrityCheck\Checker; use OCA\Settings\Settings\Admin\Overview; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\AnonRateLimit; use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; @@ -53,6 +55,14 @@ public function setupCheckManager(): DataResponse { return new DataResponse($this->setupCheckManager->runAll()); } + /** + * @return DataResponse + */ + #[AuthorizedAdminSetting(settings: Overview::class)] + public function check() { + return new DataResponse($this->setupCheckManager->runAll()); + } + /** * @return RedirectResponse */ @@ -125,10 +135,14 @@ public function getFailedIntegrityCheckFiles(): DataDisplayResponse { } /** - * @return DataResponse + * Used for setup checks to verify the server has request caching. + * @NoSubAdminRequired */ - #[AuthorizedAdminSetting(settings: Overview::class)] - public function check() { - return new DataResponse($this->setupCheckManager->runAll()); + #[PublicPage()] + #[NoCSRFRequired()] + #[AnonRateLimit(limit: 10, period: 600)] + #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] + public function checkContentLengthHeader(): DataResponse { + return new DataResponse(($this->request->getHeader('Content-Length'))); } } diff --git a/apps/settings/lib/SetupChecks/RequestBuffering.php b/apps/settings/lib/SetupChecks/RequestBuffering.php new file mode 100644 index 0000000000000..e158b384fd425 --- /dev/null +++ b/apps/settings/lib/SetupChecks/RequestBuffering.php @@ -0,0 +1,96 @@ +l10n->t('Request buffering'); + } + + public function run(): SetupResult { + $usingFPM = function_exists('fastcgi_finish_request'); + if (!$usingFPM && !\OC::$CLI) { + return SetupResult::success( + $this->l10n->t('Not using PHP-FPM.') + ); + } + + $works = null; + $stream = Psr7\Utils::streamFor(str_repeat('x', 1337)); + $options = [ + 'body' => $stream, + 'headers' => [ + 'Transfer-Encoding' => 'chunked', + ], + ]; + $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')) { + continue; + } + + $body = $response->getBody(); + $body = is_resource($body) ? stream_get_contents($body) : $body; + $works = $works || $body === '"1337"'; + } + + if ($works === null) { + return SetupResult::info( + $this->l10n->t('Could not check that your web server has configured request buffering. Please check manually.'), + ); + } elseif ($works === true) { + return SetupResult::success( + $this->l10n->t('Your web server seems to have request buffering configured correctly.'), + ); + } else { + if ($usingFPM) { + return SetupResult::error( + $this->l10n->t('Your web server is not configured for request buffering, this will cause broken uploads with some clients.') + . ' ' + . $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 + return SetupResult::warning( + $this->l10n->t('Your web server is not configured for request buffering, if you are running PHP-FPM this will cause broken uploads with some clients.') + . ' ' + . $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.'), + ); + } + } + } +}