From a4178f1369a8ca9291cf4c6eb3a46032081a16f3 Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Tue, 30 Jul 2019 10:14:29 +0200 Subject: [PATCH] [HttpClient] add "max_duration" option --- .../DependencyInjection/Configuration.php | 6 ++++++ .../Resources/config/schema/symfony-1.0.xsd | 1 + .../php/http_client_full_default_options.php | 1 + .../xml/http_client_full_default_options.xml | 1 + .../yml/http_client_full_default_options.yml | 1 + .../FrameworkExtensionTest.php | 1 + src/Symfony/Component/HttpClient/CHANGELOG.md | 1 + .../Component/HttpClient/CurlHttpClient.php | 4 ++++ .../Component/HttpClient/HttpClientTrait.php | 1 + .../Component/HttpClient/NativeHttpClient.php | 18 +++++++++++++++- .../HttpClient/Tests/MockHttpClientTest.php | 13 ++++++++++++ .../Component/HttpClient/composer.json | 2 +- .../HttpClient/HttpClientInterface.php | 2 ++ .../HttpClient/Test/Fixtures/web/index.php | 10 +++++++++ .../HttpClient/Test/HttpClientTestCase.php | 21 +++++++++++++++++++ 15 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index bb0dd7f1eb5c..527458352a09 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -1367,6 +1367,9 @@ private function addHttpClientSection(ArrayNodeDefinition $rootNode) ->floatNode('timeout') ->info('The idle timeout, defaults to the "default_socket_timeout" ini parameter.') ->end() + ->floatNode('max_duration') + ->info('The maximum execution time for the request+response as a whole.') + ->end() ->scalarNode('bindto') ->info('A network interface name, IP address, a host name or a UNIX socket to bind to.') ->end() @@ -1503,6 +1506,9 @@ private function addHttpClientSection(ArrayNodeDefinition $rootNode) ->floatNode('timeout') ->info('Defaults to "default_socket_timeout" ini parameter.') ->end() + ->floatNode('max_duration') + ->info('The maximum execution time for the request+response as a whole.') + ->end() ->scalarNode('bindto') ->info('A network interface name, IP address, a host name or a UNIX socket to bind to.') ->end() diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index b6cd142fb4a9..f1dae61035fc 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -495,6 +495,7 @@ + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_full_default_options.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_full_default_options.php index 04a227c24cb1..cf83e31af2f0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_full_default_options.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_full_default_options.php @@ -9,6 +9,7 @@ 'resolve' => ['localhost' => '127.0.0.1'], 'proxy' => 'proxy.org', 'timeout' => 3.5, + 'max_duration' => 10.1, 'bindto' => '127.0.0.1', 'verify_peer' => true, 'verify_host' => true, diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_full_default_options.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_full_default_options.xml index 2ea78874d217..aaee41943381 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_full_default_options.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_full_default_options.xml @@ -11,6 +11,7 @@ proxy="proxy.org" bindto="127.0.0.1" timeout="3.5" + max-duration="10.1" verify-peer="true" max-redirects="2" http-version="2.0" diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_full_default_options.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_full_default_options.yml index 5993be1778fe..ba3aa46259b4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_full_default_options.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_full_default_options.yml @@ -8,6 +8,7 @@ framework: resolve: {'localhost': '127.0.0.1'} proxy: proxy.org timeout: 3.5 + max_duration: 10.1 bindto: 127.0.0.1 verify_peer: true verify_host: true diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 92ab0ab0576a..11055b011737 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -1547,6 +1547,7 @@ public function testHttpClientFullDefaultOptions() $this->assertSame(['localhost' => '127.0.0.1'], $defaultOptions['resolve']); $this->assertSame('proxy.org', $defaultOptions['proxy']); $this->assertSame(3.5, $defaultOptions['timeout']); + $this->assertSame(10.1, $defaultOptions['max_duration']); $this->assertSame('127.0.0.1', $defaultOptions['bindto']); $this->assertTrue($defaultOptions['verify_peer']); $this->assertTrue($defaultOptions['verify_host']); diff --git a/src/Symfony/Component/HttpClient/CHANGELOG.md b/src/Symfony/Component/HttpClient/CHANGELOG.md index c9cd7a60f012..b61ceddf7515 100644 --- a/src/Symfony/Component/HttpClient/CHANGELOG.md +++ b/src/Symfony/Component/HttpClient/CHANGELOG.md @@ -9,6 +9,7 @@ CHANGELOG * added support for NTLM authentication * added `$response->toStream()` to cast responses to regular PHP streams * made `Psr18Client` implement relevant PSR-17 factories and have streaming responses + * added `max_duration` option 4.3.0 ----- diff --git a/src/Symfony/Component/HttpClient/CurlHttpClient.php b/src/Symfony/Component/HttpClient/CurlHttpClient.php index 92e58a2c6bd8..34857fc01510 100644 --- a/src/Symfony/Component/HttpClient/CurlHttpClient.php +++ b/src/Symfony/Component/HttpClient/CurlHttpClient.php @@ -284,6 +284,10 @@ public function request(string $method, string $url, array $options = []): Respo $curlopts[file_exists($options['bindto']) ? CURLOPT_UNIX_SOCKET_PATH : CURLOPT_INTERFACE] = $options['bindto']; } + if (0 < $options['max_duration']) { + $curlopts[CURLOPT_TIMEOUT_MS] = 1000 * $options['max_duration']; + } + $ch = curl_init(); foreach ($curlopts as $opt => $value) { diff --git a/src/Symfony/Component/HttpClient/HttpClientTrait.php b/src/Symfony/Component/HttpClient/HttpClientTrait.php index e732f4d34a6b..c5c1cdb25f05 100644 --- a/src/Symfony/Component/HttpClient/HttpClientTrait.php +++ b/src/Symfony/Component/HttpClient/HttpClientTrait.php @@ -113,6 +113,7 @@ private static function prepareRequest(?string $method, ?string $url, array $opt // Finalize normalization of options $options['http_version'] = (string) ($options['http_version'] ?? '') ?: null; $options['timeout'] = (float) ($options['timeout'] ?? ini_get('default_socket_timeout')); + $options['max_duration'] = isset($options['max_duration']) ? (float) $options['max_duration'] : 0; return [$url, $options]; } diff --git a/src/Symfony/Component/HttpClient/NativeHttpClient.php b/src/Symfony/Component/HttpClient/NativeHttpClient.php index b7f0f47df9b9..f39d72c4501a 100644 --- a/src/Symfony/Component/HttpClient/NativeHttpClient.php +++ b/src/Symfony/Component/HttpClient/NativeHttpClient.php @@ -113,7 +113,12 @@ public function request(string $method, string $url, array $options = []): Respo if ($onProgress = $options['on_progress']) { // Memoize the last progress to ease calling the callback periodically when no network transfer happens $lastProgress = [0, 0]; - $onProgress = static function (...$progress) use ($onProgress, &$lastProgress, &$info) { + $maxDuration = 0 < $options['max_duration'] ? $options['max_duration'] : INF; + $onProgress = static function (...$progress) use ($onProgress, &$lastProgress, &$info, $maxDuration) { + if ($info['total_time'] >= $maxDuration) { + throw new TransportException(sprintf('Max duration was reached for "%s".', implode('', $info['url']))); + } + $progressInfo = $info; $progressInfo['url'] = implode('', $info['url']); unset($progressInfo['size_body']); @@ -127,6 +132,13 @@ public function request(string $method, string $url, array $options = []): Respo $onProgress($lastProgress[0], $lastProgress[1], $progressInfo); }; + } elseif (0 < $options['max_duration']) { + $maxDuration = $options['max_duration']; + $onProgress = static function () use (&$info, $maxDuration): void { + if ($info['total_time'] >= $maxDuration) { + throw new TransportException(sprintf('Max duration was reached for "%s".', implode('', $info['url']))); + } + }; } // Always register a notification callback to compute live stats about the response @@ -166,6 +178,10 @@ public function request(string $method, string $url, array $options = []): Respo $options['headers'][] = 'User-Agent: Symfony HttpClient/Native'; } + if (0 < $options['max_duration']) { + $options['timeout'] = min($options['max_duration'], $options['timeout']); + } + $context = [ 'http' => [ 'protocol_version' => $options['http_version'] ?: '1.1', diff --git a/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php index 943fb089a2b8..91fc21873ada 100644 --- a/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php +++ b/src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php @@ -123,6 +123,19 @@ protected function getHttpClient(string $testCase): HttpClientInterface $body = ['<1>', '', '<2>']; $responses[] = new MockResponse($body, ['response_headers' => $headers]); break; + + case 'testMaxDuration': + $mock = $this->getMockBuilder(ResponseInterface::class)->getMock(); + $mock->expects($this->any()) + ->method('getContent') + ->willReturnCallback(static function (): void { + usleep(100000); + + throw new TransportException('Max duration was reached.'); + }); + + $responses[] = $mock; + break; } return new MockHttpClient($responses); diff --git a/src/Symfony/Component/HttpClient/composer.json b/src/Symfony/Component/HttpClient/composer.json index 4351f97ff96a..9ecab88de642 100644 --- a/src/Symfony/Component/HttpClient/composer.json +++ b/src/Symfony/Component/HttpClient/composer.json @@ -22,7 +22,7 @@ "require": { "php": "^7.1.3", "psr/log": "^1.0", - "symfony/http-client-contracts": "^1.1.4", + "symfony/http-client-contracts": "^1.1.6", "symfony/polyfill-php73": "^1.11" }, "require-dev": { diff --git a/src/Symfony/Contracts/HttpClient/HttpClientInterface.php b/src/Symfony/Contracts/HttpClient/HttpClientInterface.php index c974ba97e30b..b9ae4c658be9 100644 --- a/src/Symfony/Contracts/HttpClient/HttpClientInterface.php +++ b/src/Symfony/Contracts/HttpClient/HttpClientInterface.php @@ -53,6 +53,8 @@ interface HttpClientInterface 'proxy' => null, // string - by default, the proxy-related env vars handled by curl SHOULD be honored 'no_proxy' => null, // string - a comma separated list of hosts that do not require a proxy to be reached 'timeout' => null, // float - the idle timeout - defaults to ini_get('default_socket_timeout') + 'max_duration' => 0, // float - the maximum execution time for the request+response as a whole; + // a value lower than or equal to 0 means it is unlimited 'bindto' => '0', // string - the interface or the local socket to bind to 'verify_peer' => true, // see https://php.net/context.ssl for the following options 'verify_host' => true, diff --git a/src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php b/src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php index bc613868f2bf..9bb06ae9efbd 100644 --- a/src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php +++ b/src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php @@ -132,6 +132,16 @@ header('Content-Encoding: gzip'); echo str_repeat('-', 1000); exit; + + case '/max-duration': + ignore_user_abort(false); + while (true) { + echo '<1>'; + @ob_flush(); + flush(); + usleep(500); + } + exit; } header('Content-Type: application/json', true); diff --git a/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php b/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php index 075f73a2c1a4..92db86d6b937 100644 --- a/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php +++ b/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php @@ -778,4 +778,25 @@ public function testGzipBroken() $this->expectException(TransportExceptionInterface::class); $response->getContent(); } + + public function testMaxDuration() + { + $client = $this->getHttpClient(__FUNCTION__); + $response = $client->request('GET', 'http://localhost:8057/max-duration', [ + 'max_duration' => 0.1, + ]); + + $start = microtime(true); + + try { + $response->getContent(); + } catch (TransportExceptionInterface $e) { + $this->addToAssertionCount(1); + } + + $duration = microtime(true) - $start; + + $this->assertGreaterThanOrEqual(0.1, $duration); + $this->assertLessThan(0.2, $duration); + } }