diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c5a2f8..bb6b48b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ ChangeLog ========= +6.0.1 (2023-06-26) +------------------ + +* #207 fix: handle client disconnect properly with ignore_user_abort true (@kesselb) + 6.0.0 (2022-08-31) ------------------ @@ -9,6 +14,12 @@ ChangeLog * #192 Set min PHP to 7.4 and add type declarations (@phil-davis) * #194 Adjust type declarations a little bit (@phil-davis) +5.1.7 (2023-06-26) +------------------ + +* #98 and #176 Add more tests (@peter279k) +* #207 fix: handle client disconnect properly with ignore_user_abort true (@kesselb) + 5.1.6 (2022-07-15) ------------------ diff --git a/lib/Client.php b/lib/Client.php index da192ad..3f6ed5c 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -301,8 +301,6 @@ public function setThrowExceptions(bool $throwExceptions): void * Adds a CURL setting. * * These settings will be included in every HTTP request. - * - * @param mixed $value */ public function addCurlSetting(int $name, $value): void { @@ -340,7 +338,7 @@ protected function doRequest(RequestInterface $request): ResponseInterface * * @var resource|null */ - private $curlHandle = null; + private $curlHandle; /** * Handler for curl_multi requests. @@ -349,7 +347,7 @@ protected function doRequest(RequestInterface $request): ResponseInterface * * @var resource|null */ - private $curlMultiHandle = null; + private $curlMultiHandle; /** * Has a list of curl handles, as well as their associated success and diff --git a/lib/Message.php b/lib/Message.php index 01dc98f..a999999 100644 --- a/lib/Message.php +++ b/lib/Message.php @@ -22,7 +22,7 @@ abstract class Message implements MessageInterface * * @var resource|string|callable|null */ - protected $body = null; + protected $body; /** * Contains the list of HTTP headers. diff --git a/lib/Response.php b/lib/Response.php index 934d724..19ac87b 100644 --- a/lib/Response.php +++ b/lib/Response.php @@ -101,7 +101,7 @@ class Response extends Message implements ResponseInterface * @param array|null $headers * @param resource|string|callable|null $body */ - public function __construct($status = 500, ?array $headers = null, $body = null) + public function __construct($status = 500, array $headers = null, $body = null) { if (null !== $status) { $this->setStatus($status); diff --git a/lib/Sapi.php b/lib/Sapi.php index c9e56d4..b37c023 100644 --- a/lib/Sapi.php +++ b/lib/Sapi.php @@ -4,8 +4,6 @@ namespace Sabre\HTTP; -use InvalidArgumentException; - /** * PHP SAPI. * @@ -115,6 +113,12 @@ public static function sendResponse(ResponseInterface $response): void if ($copied <= 0) { break; } + // Abort on client disconnect. + // With ignore_user_abort(true), the script is not aborted on client disconnect. + // To avoid reading the entire stream and dismissing the data afterward, check between the chunks if the client is still there. + if (1 === ignore_user_abort() && 1 === connection_aborted()) { + break; + } $left -= $copied; } } else { @@ -222,11 +226,11 @@ public static function createFromServerArray(array $serverArray): Request } if (null === $url) { - throw new InvalidArgumentException('The _SERVER array must have a REQUEST_URI key'); + throw new \InvalidArgumentException('The _SERVER array must have a REQUEST_URI key'); } if (null === $method) { - throw new InvalidArgumentException('The _SERVER array must have a REQUEST_METHOD key'); + throw new \InvalidArgumentException('The _SERVER array must have a REQUEST_METHOD key'); } $r = new Request($method, $url, $headers); $r->setHttpVersion($httpVersion); diff --git a/lib/Version.php b/lib/Version.php index e07a43c..3b0822e 100644 --- a/lib/Version.php +++ b/lib/Version.php @@ -16,5 +16,5 @@ class Version /** * Full version number. */ - public const VERSION = '6.0.0'; + public const VERSION = '6.0.1'; } diff --git a/lib/functions.php b/lib/functions.php index f1e0b09..b9c62dd 100644 --- a/lib/functions.php +++ b/lib/functions.php @@ -28,7 +28,7 @@ * See: * http://tools.ietf.org/html/rfc7231#section-7.1.1.1 * - * @return bool|DateTime + * @return bool|\DateTime */ function parseDate(string $dateString) { @@ -64,7 +64,7 @@ function parseDate(string $dateString) } try { - return new DateTime($dateString, new \DateTimeZone('UTC')); + return new \DateTime($dateString, new \DateTimeZone('UTC')); } catch (\Exception $e) { return false; } @@ -73,7 +73,7 @@ function parseDate(string $dateString) /** * Transforms a DateTime object to a valid HTTP/1.1 Date header value. */ -function toDate(DateTime $dateTime): string +function toDate(\DateTime $dateTime): string { // We need to clone it, as we don't want to affect the existing // DateTime. @@ -168,9 +168,9 @@ function negotiateContentType(?string $acceptHeaderValue, array $availableOption // Does this entry win? if ( - ($proposal['quality'] > $lastQuality) || - ($proposal['quality'] === $lastQuality && $specificity > $lastSpecificity) || - ($proposal['quality'] === $lastQuality && $specificity === $lastSpecificity && $optionIndex < $lastOptionIndex) + ($proposal['quality'] > $lastQuality) + || ($proposal['quality'] === $lastQuality && $specificity > $lastSpecificity) + || ($proposal['quality'] === $lastQuality && $specificity === $lastSpecificity && $optionIndex < $lastOptionIndex) ) { $lastQuality = $proposal['quality']; $lastSpecificity = $specificity; diff --git a/phpstan.neon b/phpstan.neon index d3b8e15..5802f6f 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -12,7 +12,7 @@ parameters: path: lib/Client.php - message: "#^Left side of || is always false.$#" - count: 1 + count: 2 path: lib/Client.php - message: "#^Strict comparison using === between null and array will always evaluate to false.$#" diff --git a/tests/HTTP/Auth/DigestTest.php b/tests/HTTP/Auth/DigestTest.php index 536e724..dbc9416 100644 --- a/tests/HTTP/Auth/DigestTest.php +++ b/tests/HTTP/Auth/DigestTest.php @@ -42,7 +42,7 @@ public function testDigest(): void $nc.':'. $cnonce.':'. 'auth:'. - md5('GET'.':'.'/') + md5('GET:/') ); $this->request->setMethod('GET'); @@ -71,7 +71,7 @@ public function testInvalidDigest(): void $nc.':'. $cnonce.':'. 'auth:'. - md5('GET'.':'.'/') + md5('GET:/') ); $this->request->setMethod('GET'); @@ -107,7 +107,7 @@ public function testDigestAuthInt(): void $nc.':'. $cnonce.':'. 'auth-int:'. - md5('POST'.':'.'/'.':'.md5('body')) + md5('POST:/:'.md5('body')) ); $this->request->setMethod('POST'); @@ -135,7 +135,7 @@ public function testDigestAuthBoth(): void $nc.':'. $cnonce.':'. 'auth-int:'. - md5('POST'.':'.'/'.':'.md5('body')) + md5('POST:/:'.md5('body')) ); $this->request->setMethod('POST'); diff --git a/tests/HTTP/SapiTest.php b/tests/HTTP/SapiTest.php index 9571cd1..39d97ed 100644 --- a/tests/HTTP/SapiTest.php +++ b/tests/HTTP/SapiTest.php @@ -144,6 +144,7 @@ public function testSend(): void /** * @runInSeparateProcess + * * @depends testSend */ public function testSendLimitedByContentLengthString(): void @@ -179,6 +180,7 @@ public function testRecognizeHttp2(): void /** * @runInSeparateProcess + * * @depends testSend */ public function testSendLimitedByContentLengthStream(): void @@ -203,7 +205,9 @@ public function testSendLimitedByContentLengthStream(): void /** * @runInSeparateProcess + * * @depends testSend + * * @dataProvider sendContentRangeStreamData */ public function testSendContentRangeStream( @@ -277,6 +281,7 @@ public function sendContentRangeStreamData(): array /** * @runInSeparateProcess + * * @depends testSend */ public function testSendWorksWithCallbackAsBody(): void @@ -295,4 +300,35 @@ public function testSendWorksWithCallbackAsBody(): void $this->assertEquals('foo', $result); } + + public function testSendConnectionAborted(): void + { + $baseUrl = getenv('BASEURL'); + if (!$baseUrl) { + $this->markTestSkipped('Set an environment value BASEURL to continue'); + } + + $url = rtrim($baseUrl, '/').'/connection_aborted.php'; + $chunk_size = 4 * 1024 * 1024; + $fetch_size = 6 * 1024 * 1024; + + $stream = fopen($url, 'r'); + $size = 0; + + while ($size <= $fetch_size) { + $temp = fread($stream, 8192); + if (false === $temp) { + break; + } + $size += strlen($temp); + } + + fclose($stream); + + sleep(5); + + $bytes_read = file_get_contents(sys_get_temp_dir().'/dummy_stream_read_counter'); + $this->assertEquals($chunk_size * 2, $bytes_read); + $this->assertGreaterThanOrEqual($fetch_size, $bytes_read); + } } diff --git a/tests/www/connection_aborted.php b/tests/www/connection_aborted.php new file mode 100644 index 0000000..e9f62ab --- /dev/null +++ b/tests/www/connection_aborted.php @@ -0,0 +1,69 @@ +position = 0; + + return true; + } + + public function stream_read(int $count): string + { + $this->position += $count; + + return random_bytes($count); + } + + public function stream_tell(): int + { + return $this->position; + } + + public function stream_eof(): bool + { + return $this->position > 25 * 1024 * 1024; + } + + public function stream_close(): void + { + file_put_contents(sys_get_temp_dir().'/dummy_stream_read_counter', $this->position); + } +} + +/* + * The DummyStream wrapper has two functions: + * - Provide dummy data. + * - Count how many bytes have been read. + */ +stream_wrapper_register('dummy', DummyStream::class); + +/* + * Overwrite default connection handling. + * The default behaviour is however for your script to be aborted when the remote client disconnects. + * + * Nextcloud/ownCloud set ignore_user_abort(true) on purpose to work around + * some edge cases where the default behavior would end a script too early. + * + * https://github.com/owncloud/core/issues/22370 + * https://github.com/owncloud/core/pull/26775 + */ +ignore_user_abort(true); + +$body = fopen('dummy://hello', 'r'); + +$response = new HTTP\Response(); +$response->setStatus(200); +$response->addHeader('Content-Length', 25 * 1024 * 1024); +$response->setBody($body); + +HTTP\Sapi::sendResponse($response);