From 5b3495f271690774827828b637d2dd95963783da Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Fri, 31 Dec 2021 15:34:09 +0100 Subject: [PATCH 1/6] [HttpClient] Turn negative timeout to a very long timeout --- HttpClientTrait.php | 5 ++++- Tests/HttpClientTestCase.php | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/HttpClientTrait.php b/HttpClientTrait.php index 07bd717..c763cbe 100644 --- a/HttpClientTrait.php +++ b/HttpClientTrait.php @@ -147,7 +147,10 @@ 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')); + if (0 > $options['timeout'] = (float) ($options['timeout'] ?? ini_get('default_socket_timeout'))) { + $options['timeout'] = 172800.0; // 2 days + } + $options['max_duration'] = isset($options['max_duration']) ? (float) $options['max_duration'] : 0; return [$url, $options]; diff --git a/Tests/HttpClientTestCase.php b/Tests/HttpClientTestCase.php index 36e76ee..0fc6dc4 100644 --- a/Tests/HttpClientTestCase.php +++ b/Tests/HttpClientTestCase.php @@ -179,4 +179,13 @@ public function testDebugInfoOnDestruct() $this->assertNotEmpty($traceInfo['debug']); } + + public function testNegativeTimeout() + { + $client = $this->getHttpClient(__FUNCTION__); + + $this->assertSame(200, $client->request('GET', 'http://localhost:8057', [ + 'timeout' => -1, + ])->getStatusCode()); + } } From 09a4fef4263869d0ac5cdbf77a1e9d0280942719 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 2 Jan 2022 10:41:36 +0100 Subject: [PATCH 2/6] Bump license year --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index 2358414..74cdc2d 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2018-2021 Fabien Potencier +Copyright (c) 2018-2022 Fabien Potencier Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal From c4e28fc333914313b9d5eb1be9709c36880a158d Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sun, 2 Jan 2022 01:51:31 +0000 Subject: [PATCH 3/6] [HttpClient] Remove deprecated usage of `GuzzleHttp\Promise\queue` --- HttplugClient.php | 3 ++- Internal/HttplugWaitLoop.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/HttplugClient.php b/HttplugClient.php index d6f53be..86c72e4 100644 --- a/HttplugClient.php +++ b/HttplugClient.php @@ -13,6 +13,7 @@ use GuzzleHttp\Promise\Promise as GuzzlePromise; use GuzzleHttp\Promise\RejectedPromise; +use GuzzleHttp\Promise\Utils; use Http\Client\Exception\NetworkException; use Http\Client\Exception\RequestException; use Http\Client\HttpAsyncClient; @@ -69,7 +70,7 @@ public function __construct(HttpClientInterface $client = null, ResponseFactoryI $this->client = $client ?? HttpClient::create(); $this->responseFactory = $responseFactory; $this->streamFactory = $streamFactory ?? ($responseFactory instanceof StreamFactoryInterface ? $responseFactory : null); - $this->promisePool = \function_exists('GuzzleHttp\Promise\queue') ? new \SplObjectStorage() : null; + $this->promisePool = class_exists(Utils::class) ? new \SplObjectStorage() : null; if (null === $this->responseFactory || null === $this->streamFactory) { if (!class_exists(Psr17Factory::class) && !class_exists(Psr17FactoryDiscovery::class)) { diff --git a/Internal/HttplugWaitLoop.php b/Internal/HttplugWaitLoop.php index 3f287fe..f6bdd5e 100644 --- a/Internal/HttplugWaitLoop.php +++ b/Internal/HttplugWaitLoop.php @@ -47,7 +47,7 @@ public function wait(?ResponseInterface $pendingResponse, float $maxDuration = n return 0; } - $guzzleQueue = \GuzzleHttp\Promise\queue(); + $guzzleQueue = \GuzzleHttp\Promise\Utils::queue(); if (0.0 === $remainingDuration = $maxDuration) { $idleTimeout = 0.0; From f1b0537960801479970ddfce147d8bd38b7c66df Mon Sep 17 00:00:00 2001 From: plozmun Date: Wed, 12 Jan 2022 21:49:45 +0100 Subject: [PATCH 4/6] [HttpClient] Remove deprecated usage of GuzzleHttp\Promise\promise_for --- Response/HttplugPromise.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Response/HttplugPromise.php b/Response/HttplugPromise.php index 2231464..2efacca 100644 --- a/Response/HttplugPromise.php +++ b/Response/HttplugPromise.php @@ -11,7 +11,7 @@ namespace Symfony\Component\HttpClient\Response; -use function GuzzleHttp\Promise\promise_for; +use GuzzleHttp\Promise\Create; use GuzzleHttp\Promise\PromiseInterface as GuzzlePromiseInterface; use Http\Promise\Promise as HttplugPromiseInterface; use Psr\Http\Message\ResponseInterface as Psr7ResponseInterface; @@ -74,7 +74,7 @@ private function wrapThenCallback(?callable $callback): ?callable } return static function ($value) use ($callback) { - return promise_for($callback($value)); + return Create::promiseFor($callback($value)); }; } } From 848a70d4f5321c006c1d9f6865aae93df4f49f50 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 13 Jan 2022 16:11:29 +0100 Subject: [PATCH 5/6] [HttpClient] fix resetting DNS/etc when calling CurlHttpClient::reset() --- CurlHttpClient.php | 6 ++-- Internal/CurlClientState.php | 49 +++++++++++--------------- Response/CurlResponse.php | 66 +++++++++++++++--------------------- Tests/CurlHttpClientTest.php | 4 +-- 4 files changed, 52 insertions(+), 73 deletions(-) diff --git a/CurlHttpClient.php b/CurlHttpClient.php index f30c343..86e4d68 100644 --- a/CurlHttpClient.php +++ b/CurlHttpClient.php @@ -168,7 +168,6 @@ public function request(string $method, string $url, array $options = []): Respo if ($resolve && 0x072A00 > CurlClientState::$curlVersion['version_number']) { // DNS cache removals require curl 7.42 or higher - // On lower versions, we have to create a new multi handle $this->multi->reset(); } @@ -280,6 +279,7 @@ public function request(string $method, string $url, array $options = []): Respo if (!$pushedResponse) { $ch = curl_init(); $this->logger && $this->logger->info(sprintf('Request: "%s %s"', $method, $url)); + $curlopts += [\CURLOPT_SHARE => $this->multi->share]; } foreach ($curlopts as $opt => $value) { @@ -306,9 +306,9 @@ public function stream($responses, float $timeout = null): ResponseStreamInterfa throw new \TypeError(sprintf('"%s()" expects parameter 1 to be an iterable of CurlResponse objects, "%s" given.', __METHOD__, \is_object($responses) ? \get_class($responses) : \gettype($responses))); } - if (\is_resource($mh = $this->multi->handles[0] ?? null) || $mh instanceof \CurlMultiHandle) { + if (\is_resource($this->multi->handle) || $this->multi->handle instanceof \CurlMultiHandle) { $active = 0; - while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($mh, $active)) { + while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)) { } } diff --git a/Internal/CurlClientState.php b/Internal/CurlClientState.php index c078233..b7211b1 100644 --- a/Internal/CurlClientState.php +++ b/Internal/CurlClientState.php @@ -23,8 +23,10 @@ */ final class CurlClientState extends ClientState { - /** @var array<\CurlMultiHandle|resource> */ - public $handles = []; + /** @var \CurlMultiHandle|resource */ + public $handle; + /** @var \CurlShareHandle|resource */ + public $share; /** @var PushedResponse[] */ public $pushedResponses = []; /** @var DnsCache */ @@ -34,27 +36,23 @@ final class CurlClientState extends ClientState public static $curlVersion; - private $maxHostConnections; - private $maxPendingPushes; - public function __construct(int $maxHostConnections, int $maxPendingPushes) { self::$curlVersion = self::$curlVersion ?? curl_version(); - array_unshift($this->handles, $mh = curl_multi_init()); + $this->handle = curl_multi_init(); $this->dnsCache = new DnsCache(); - $this->maxHostConnections = $maxHostConnections; - $this->maxPendingPushes = $maxPendingPushes; + $this->reset(); // Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order if (\defined('CURLPIPE_MULTIPLEX')) { - curl_multi_setopt($mh, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX); + curl_multi_setopt($this->handle, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX); } if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) { - $maxHostConnections = curl_multi_setopt($mh, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections; + $maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections; } if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) { - curl_multi_setopt($mh, \CURLMOPT_MAXCONNECTS, $maxHostConnections); + curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections); } // Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/77535 @@ -67,17 +65,8 @@ public function __construct(int $maxHostConnections, int $maxPendingPushes) return; } - // Clone to prevent a circular reference - $multi = clone $this; - $multi->handles = [$mh]; - $multi->pushedResponses = &$this->pushedResponses; - $multi->logger = &$this->logger; - $multi->handlesActivity = &$this->handlesActivity; - $multi->openHandles = &$this->openHandles; - $multi->lastTimeout = &$this->lastTimeout; - - curl_multi_setopt($mh, \CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes) { - return $multi->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes); + curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, function ($parent, $pushed, array $requestHeaders) use ($maxPendingPushes) { + return $this->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes); }); } @@ -85,10 +74,7 @@ public function reset() { foreach ($this->pushedResponses as $url => $response) { $this->logger && $this->logger->debug(sprintf('Unused pushed response: "%s"', $url)); - - foreach ($this->handles as $mh) { - curl_multi_remove_handle($mh, $response->handle); - } + curl_multi_remove_handle($this->handle, $response->handle); curl_close($response->handle); } @@ -96,11 +82,14 @@ public function reset() $this->dnsCache->evictions = $this->dnsCache->evictions ?: $this->dnsCache->removals; $this->dnsCache->removals = $this->dnsCache->hostnames = []; - if (\defined('CURLMOPT_PUSHFUNCTION')) { - curl_multi_setopt($this->handles[0], \CURLMOPT_PUSHFUNCTION, null); - } + $this->share = curl_share_init(); + + curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_DNS); + curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_SSL_SESSION); - $this->__construct($this->maxHostConnections, $this->maxPendingPushes); + if (\defined('CURL_LOCK_DATA_CONNECT')) { + curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_CONNECT); + } } private function handlePush($parent, $pushed, array $requestHeaders, int $maxPendingPushes): int diff --git a/Response/CurlResponse.php b/Response/CurlResponse.php index 2d0d76e..cbd70e9 100644 --- a/Response/CurlResponse.php +++ b/Response/CurlResponse.php @@ -150,7 +150,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null, // Schedule the request in a non-blocking way $multi->lastTimeout = null; $multi->openHandles[$id] = [$ch, $options]; - curl_multi_add_handle($multi->handles[0], $ch); + curl_multi_add_handle($multi->handle, $ch); $this->canary = new Canary(static function () use ($ch, $multi, $id) { unset($multi->openHandles[$id], $multi->handlesActivity[$id]); @@ -160,9 +160,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null, return; } - foreach ($multi->handles as $mh) { - curl_multi_remove_handle($mh, $ch); - } + curl_multi_remove_handle($multi->handle, $ch); curl_setopt_array($ch, [ \CURLOPT_NOPROGRESS => true, \CURLOPT_PROGRESSFUNCTION => null, @@ -244,7 +242,7 @@ public function __destruct() */ private static function schedule(self $response, array &$runningResponses): void { - if (isset($runningResponses[$i = (int) $response->multi->handles[0]])) { + if (isset($runningResponses[$i = (int) $response->multi->handle])) { $runningResponses[$i][1][$response->id] = $response; } else { $runningResponses[$i] = [$response->multi, [$response->id => $response]]; @@ -276,47 +274,39 @@ private static function perform(ClientState $multi, array &$responses = null): v try { self::$performing = true; + $active = 0; + while (\CURLM_CALL_MULTI_PERFORM === ($err = curl_multi_exec($multi->handle, $active))) { + } - foreach ($multi->handles as $i => $mh) { - $active = 0; - while (\CURLM_CALL_MULTI_PERFORM === ($err = curl_multi_exec($mh, $active))) { - } + if (\CURLM_OK !== $err) { + throw new TransportException(curl_multi_strerror($err)); + } - if (\CURLM_OK !== $err) { - throw new TransportException(curl_multi_strerror($err)); + while ($info = curl_multi_info_read($multi->handle)) { + if (\CURLMSG_DONE !== $info['msg']) { + continue; } + $result = $info['result']; + $id = (int) $ch = $info['handle']; + $waitFor = @curl_getinfo($ch, \CURLINFO_PRIVATE) ?: '_0'; - while ($info = curl_multi_info_read($mh)) { - if (\CURLMSG_DONE !== $info['msg']) { - continue; - } - $result = $info['result']; - $id = (int) $ch = $info['handle']; - $waitFor = @curl_getinfo($ch, \CURLINFO_PRIVATE) ?: '_0'; - - if (\in_array($result, [\CURLE_SEND_ERROR, \CURLE_RECV_ERROR, /*CURLE_HTTP2*/ 16, /*CURLE_HTTP2_STREAM*/ 92], true) && $waitFor[1] && 'C' !== $waitFor[0]) { - curl_multi_remove_handle($mh, $ch); - $waitFor[1] = (string) ((int) $waitFor[1] - 1); // decrement the retry counter - curl_setopt($ch, \CURLOPT_PRIVATE, $waitFor); - curl_setopt($ch, \CURLOPT_FORBID_REUSE, true); - - if (0 === curl_multi_add_handle($mh, $ch)) { - continue; - } - } + if (\in_array($result, [\CURLE_SEND_ERROR, \CURLE_RECV_ERROR, /*CURLE_HTTP2*/ 16, /*CURLE_HTTP2_STREAM*/ 92], true) && $waitFor[1] && 'C' !== $waitFor[0]) { + curl_multi_remove_handle($multi->handle, $ch); + $waitFor[1] = (string) ((int) $waitFor[1] - 1); // decrement the retry counter + curl_setopt($ch, \CURLOPT_PRIVATE, $waitFor); + curl_setopt($ch, \CURLOPT_FORBID_REUSE, true); - if (\CURLE_RECV_ERROR === $result && 'H' === $waitFor[0] && 400 <= ($responses[(int) $ch]->info['http_code'] ?? 0)) { - $multi->handlesActivity[$id][] = new FirstChunk(); + if (0 === curl_multi_add_handle($multi->handle, $ch)) { + continue; } - - $multi->handlesActivity[$id][] = null; - $multi->handlesActivity[$id][] = \in_array($result, [\CURLE_OK, \CURLE_TOO_MANY_REDIRECTS], true) || '_0' === $waitFor || curl_getinfo($ch, \CURLINFO_SIZE_DOWNLOAD) === curl_getinfo($ch, \CURLINFO_CONTENT_LENGTH_DOWNLOAD) ? null : new TransportException(sprintf('%s for "%s".', curl_strerror($result), curl_getinfo($ch, \CURLINFO_EFFECTIVE_URL))); } - if (!$active && 0 < $i) { - curl_multi_close($mh); - unset($multi->handles[$i]); + if (\CURLE_RECV_ERROR === $result && 'H' === $waitFor[0] && 400 <= ($responses[(int) $ch]->info['http_code'] ?? 0)) { + $multi->handlesActivity[$id][] = new FirstChunk(); } + + $multi->handlesActivity[$id][] = null; + $multi->handlesActivity[$id][] = \in_array($result, [\CURLE_OK, \CURLE_TOO_MANY_REDIRECTS], true) || '_0' === $waitFor || curl_getinfo($ch, \CURLINFO_SIZE_DOWNLOAD) === curl_getinfo($ch, \CURLINFO_CONTENT_LENGTH_DOWNLOAD) ? null : new TransportException(sprintf('%s for "%s".', curl_strerror($result), curl_getinfo($ch, \CURLINFO_EFFECTIVE_URL))); } } finally { self::$performing = false; @@ -335,7 +325,7 @@ private static function select(ClientState $multi, float $timeout): int $timeout = min($timeout, 0.01); } - return curl_multi_select($multi->handles[array_key_last($multi->handles)], $timeout); + return curl_multi_select($multi->handle, $timeout); } /** diff --git a/Tests/CurlHttpClientTest.php b/Tests/CurlHttpClientTest.php index c8bb52c..e932470 100644 --- a/Tests/CurlHttpClientTest.php +++ b/Tests/CurlHttpClientTest.php @@ -143,9 +143,9 @@ public function testHandleIsReinitOnReset() $r = new \ReflectionProperty($httpClient, 'multi'); $r->setAccessible(true); $clientState = $r->getValue($httpClient); - $initialHandleId = (int) $clientState->handles[0]; + $initialShareId = $clientState->share; $httpClient->reset(); - self::assertNotSame($initialHandleId, (int) $clientState->handles[0]); + self::assertNotSame($initialShareId, $clientState->share); } public function testProcessAfterReset() From 8129ccd6233338e1d495b7734c003053766cb262 Mon Sep 17 00:00:00 2001 From: Adrien Wilmet Date: Wed, 19 Jan 2022 10:31:25 +0100 Subject: [PATCH 6/6] [HttpClient] Fix Failed to open stream: Too many open files --- Internal/CurlClientState.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Internal/CurlClientState.php b/Internal/CurlClientState.php index b7211b1..5821f67 100644 --- a/Internal/CurlClientState.php +++ b/Internal/CurlClientState.php @@ -23,9 +23,9 @@ */ final class CurlClientState extends ClientState { - /** @var \CurlMultiHandle|resource */ + /** @var \CurlMultiHandle|resource|null */ public $handle; - /** @var \CurlShareHandle|resource */ + /** @var \CurlShareHandle|resource|null */ public $share; /** @var PushedResponse[] */ public $pushedResponses = []; @@ -65,8 +65,17 @@ public function __construct(int $maxHostConnections, int $maxPendingPushes) return; } - curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, function ($parent, $pushed, array $requestHeaders) use ($maxPendingPushes) { - return $this->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes); + // Clone to prevent a circular reference + $multi = clone $this; + $multi->handle = null; + $multi->share = null; + $multi->pushedResponses = &$this->pushedResponses; + $multi->logger = &$this->logger; + $multi->handlesActivity = &$this->handlesActivity; + $multi->openHandles = &$this->openHandles; + + curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes) { + return $multi->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes); }); }