diff --git a/build/stubs/sapi.php b/build/stubs/sapi.php new file mode 100644 index 0000000000000..cf5ede4f0398b --- /dev/null +++ b/build/stubs/sapi.php @@ -0,0 +1,16 @@ +setOutput($output); } } + + if ($response->getFlushEarly() && $container->get(IConfig::class)->getSystemValueBool('flush_response_early', true)) { + self::finishRequest($container, $io); + } + } + + /** + * Hand the finished response to the client, so that the remaining work of + * this process does not keep it waiting. + */ + private static function finishRequest(DIContainer $container, IOutput $io): void { + // The remaining work must not be cut off when the client goes away + ignore_user_abort(true); + + // Otherwise the next request of the same client blocks on the session + // lock for as long as this process keeps working + try { + $container->get(ISession::class)->close(); + } catch (ContainerExceptionInterface) { + } + + // The client may send a follow-up request that then reads data from + // before the transaction + try { + if ($container->get(IDBConnection::class)->inTransaction()) { + $container->get(LoggerInterface::class)->warning( + 'A response was flushed to the client while a database transaction was still open. The client may not be able to observe the pending writes yet.', + ['app' => 'core'], + ); + } + } catch (ContainerExceptionInterface) { + } + + $io->finishRequest(); } } diff --git a/lib/private/AppFramework/Http/Output.php b/lib/private/AppFramework/Http/Output.php index 83b6ee6e45089..038d7d1d360be 100644 --- a/lib/private/AppFramework/Http/Output.php +++ b/lib/private/AppFramework/Http/Output.php @@ -88,4 +88,29 @@ public function setCookie($name, $value, $expire, $path, $domain, $secure, $http 'samesite' => $sameSite ]); } + + #[\Override] + public function finishRequest(): bool { + // php-fpm, and the SAPIs aliasing it (recent LiteSpeed, FrankenPHP) + if (function_exists('fastcgi_finish_request')) { + fastcgi_finish_request(); + return true; + } + + if (function_exists('litespeed_finish_request')) { + litespeed_finish_request(); + return true; + } + + // mod_php, cgi, cli, … cannot give the connection back to the web server, + // so only push out what we have. ob_flush() instead of ob_end_flush() keeps + // the buffer around for error handling, and is silenced because output + // handlers are allowed to be non-flushable. + if (ob_get_level() > 0) { + @ob_flush(); + } + flush(); + + return false; + } } diff --git a/lib/public/AppFramework/Http/IOutput.php b/lib/public/AppFramework/Http/IOutput.php index bd20251a8ba2e..753617b87df96 100644 --- a/lib/public/AppFramework/Http/IOutput.php +++ b/lib/public/AppFramework/Http/IOutput.php @@ -57,4 +57,14 @@ public function setHttpResponseCode($code); * @since 8.1.0 */ public function setCookie($name, $value, $expire, $path, $domain, $secure, $httpOnly, $sameSite = 'Lax'); + + /** + * Send the response produced so far to the client, so the remaining work of + * this process does not delay it. Nothing may be written to the output after. + * + * @return bool true if the connection was closed, false if the response was + * only flushed because the SAPI does not support closing it + * @since 35.0.0 + */ + public function finishRequest(): bool; } diff --git a/lib/public/AppFramework/Http/Response.php b/lib/public/AppFramework/Http/Response.php index cccd6a318a4c7..623879d5abba8 100644 --- a/lib/public/AppFramework/Http/Response.php +++ b/lib/public/AppFramework/Http/Response.php @@ -64,6 +64,7 @@ class Response { /** @var bool */ private $throttled = false; + private bool $flushEarly = false; /** @var array */ private $throttleMetadata = []; @@ -397,4 +398,37 @@ public function getThrottleMetadata() { public function isThrottled() { return $this->throttled; } + + /** + * Request that the response is sent to the client as soon as it is complete, + * instead of when the PHP process is done. Enable this only when the + * controller leaves work behind that the client does not have to wait for. + * + * Things to be aware of before enabling this: + * + * - The session is closed before the response goes out, so anything running + * afterwards can no longer write to it. + * - The status code and the body are final at that point. Errors happening + * afterwards can only be logged, never reported to the client. + * - Nothing may be written to the output afterwards, the Content-Length has + * already been announced. + * - The worker of the web server stays occupied until the process really + * ends, so this improves latency but not throughput. + * - Administrators can turn this off globally with the `flush_response_early` + * system config option + * + * @since 35.0.0 + */ + public function setFlushEarly(bool $flushEarly): void { + $this->flushEarly = $flushEarly; + } + + /** + * Whether the response should be sent to the client as soon as it is complete + * + * @since 35.0.0 + */ + public function getFlushEarly(): bool { + return $this->flushEarly; + } } diff --git a/psalm.xml b/psalm.xml index 6e04adfaade86..7b1d967ddbb7c 100644 --- a/psalm.xml +++ b/psalm.xml @@ -129,6 +129,7 @@ + diff --git a/tests/lib/AppFramework/AppTest.php b/tests/lib/AppFramework/AppTest.php index 334eeefdab6c2..ba60ca6dc3012 100644 --- a/tests/lib/AppFramework/AppTest.php +++ b/tests/lib/AppFramework/AppTest.php @@ -14,6 +14,11 @@ use OCP\AppFramework\Controller; use OCP\AppFramework\Http\IOutput; use OCP\AppFramework\Http\Response; +use OCP\AppFramework\Http\StreamResponse; +use OCP\IConfig; +use OCP\IDBConnection; +use OCP\ISession; +use Psr\Log\LoggerInterface; function rrmdir($directory) { $files = array_diff(scandir($directory), ['.','..']); @@ -86,6 +91,92 @@ public function testControllerNameAndMethodAreBeingPassed(): void { $this->container); } + public function testRequestIsNotFinishedEarlyByDefault(): void { + $this->dispatcher->expects($this->once()) + ->method('dispatch') + ->willReturn(['HTTP/2.0 200 OK', [], [], $this->output, new Response()]); + + $this->io->expects($this->never()) + ->method('finishRequest'); + + App::main($this->controllerName, $this->controllerMethod, $this->container, []); + } + + public function testRequestIsFinishedEarlyWhenTheResponseAsksForIt(): void { + $response = new Response(); + $response->setFlushEarly(true); + + $this->dispatcher->expects($this->once()) + ->method('dispatch') + ->willReturn(['HTTP/2.0 200 OK', [], [], $this->output, $response]); + + $session = $this->createMock(ISession::class); + $session->expects($this->once()) + ->method('close'); + $this->container[ISession::class] = $session; + + $connection = $this->createMock(IDBConnection::class); + $connection->method('inTransaction') + ->willReturn(false); + $this->container[IDBConnection::class] = $connection; + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->never()) + ->method('warning'); + $this->container[LoggerInterface::class] = $logger; + + $this->io->expects($this->once()) + ->method('finishRequest'); + + App::main($this->controllerName, $this->controllerMethod, $this->container, []); + } + + public function testRequestIsNotFinishedEarlyWhenDisabledByTheAdmin(): void { + $response = new Response(); + $response->setFlushEarly(true); + + $this->dispatcher->expects($this->once()) + ->method('dispatch') + ->willReturn(['HTTP/2.0 200 OK', [], [], $this->output, $response]); + + $config = $this->createMock(IConfig::class); + $config->method('getSystemValueBool') + ->with('flush_response_early', true) + ->willReturn(false); + $this->container[IConfig::class] = $config; + + $this->io->expects($this->never()) + ->method('finishRequest'); + + App::main($this->controllerName, $this->controllerMethod, $this->container, []); + } + + public function testOpenTransactionIsLoggedWhenFinishingEarly(): void { + $response = new Response(); + $response->setFlushEarly(true); + + $this->dispatcher->expects($this->once()) + ->method('dispatch') + ->willReturn(['HTTP/2.0 200 OK', [], [], $this->output, $response]); + + $this->container[ISession::class] = $this->createMock(ISession::class); + + $connection = $this->createMock(IDBConnection::class); + $connection->method('inTransaction') + ->willReturn(true); + $this->container[IDBConnection::class] = $connection; + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once()) + ->method('warning'); + $this->container[LoggerInterface::class] = $logger; + + $this->io->expects($this->once()) + ->method('finishRequest'); + + App::main($this->controllerName, $this->controllerMethod, $this->container, []); + } + public function testBuildAppNamespace(): void { $ns = App::buildAppNamespace('someapp'); $this->assertEquals('OCA\Someapp', $ns); @@ -144,7 +235,9 @@ public function testNoOutput(string $statusCode): void { } public function testCallbackIsCalled(): void { - $mock = $this->getMockBuilder('OCP\AppFramework\Http\ICallbackResponse') + // The dispatcher always hands back a Response, not a bare ICallbackResponse + $mock = $this->getMockBuilder(StreamResponse::class) + ->disableOriginalConstructor() ->getMock(); $return = ['HTTP/2.0 200 OK', [], [], $this->output, $mock]; diff --git a/tests/lib/AppFramework/Http/OutputTest.php b/tests/lib/AppFramework/Http/OutputTest.php index 2ba93833dd135..635b3004013ab 100644 --- a/tests/lib/AppFramework/Http/OutputTest.php +++ b/tests/lib/AppFramework/Http/OutputTest.php @@ -27,4 +27,53 @@ public function testSetReadfileStream(): void { $output = new Output(''); $output->setReadfile(fopen(__FILE__, 'r')); } + + /** The buffer has to survive the flush so error handling can still use it */ + public function testFinishRequestKeepsTheOutputBuffer(): void { + $this->skipIfConnectionCanBeClosed(); + $output = new Output(''); + + ob_start(); + $levelBefore = ob_get_level(); + $closed = $output->finishRequest(); + $levelAfter = ob_get_level(); + ob_end_clean(); + + $this->assertFalse($closed); + $this->assertSame($levelBefore, $levelAfter); + } + + /** Draining a handler that refuses to be flushed in a loop would never end */ + public function testFinishRequestWithNonFlushableBuffer(): void { + $this->skipIfConnectionCanBeClosed(); + $output = new Output(''); + + ob_start( + static fn (string $buffer): string => $buffer, + 0, + PHP_OUTPUT_HANDLER_CLEANABLE | PHP_OUTPUT_HANDLER_REMOVABLE, + ); + $levelBefore = ob_get_level(); + $closed = $output->finishRequest(); + $levelAfter = ob_get_level(); + ob_end_clean(); + + $this->assertFalse($closed); + $this->assertSame($levelBefore, $levelAfter); + } + + public function testFinishRequestWithoutAnyOutputBuffer(): void { + $this->skipIfConnectionCanBeClosed(); + $output = new Output(''); + + $level = ob_get_level(); + $this->assertFalse($output->finishRequest()); + $this->assertSame($level, ob_get_level()); + } + + private function skipIfConnectionCanBeClosed(): void { + if (function_exists('fastcgi_finish_request') || function_exists('litespeed_finish_request')) { + $this->markTestSkipped('This SAPI closes the connection, which would tear down the output buffer of the test runner'); + } + } } diff --git a/tests/lib/AppFramework/Http/ResponseTest.php b/tests/lib/AppFramework/Http/ResponseTest.php index 4af5cb2443fcb..b473deb0ac161 100644 --- a/tests/lib/AppFramework/Http/ResponseTest.php +++ b/tests/lib/AppFramework/Http/ResponseTest.php @@ -259,4 +259,16 @@ public function testGetThrottleMetadata(): void { $this->childResponse->throttle(['foo' => 'bar']); $this->assertSame(['foo' => 'bar'], $this->childResponse->getThrottleMetadata()); } + + public function testFlushEarlyIsOptIn(): void { + $this->assertFalse($this->childResponse->getFlushEarly()); + } + + public function testSetFlushEarly(): void { + $this->childResponse->setFlushEarly(true); + $this->assertTrue($this->childResponse->getFlushEarly()); + + $this->childResponse->setFlushEarly(false); + $this->assertFalse($this->childResponse->getFlushEarly()); + } }