Skip to content

Commit e9af470

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '5.4' into 6.0
* 5.4: [HttpClient] Fix closing curl-multi handle too early on destruct [PropertyInfo] fix precedence of __get() vs properties [Form] Improve Persian (Farsi) Translation For Forms [Uid] Add ulid keyword in composer.json fix: lowest version of psr container supported [HttpClient] Don't reset timeout counter when initializing requests
2 parents 046bc56 + b6d3e36 commit e9af470

9 files changed

+107
-115
lines changed

‎CurlHttpClient.php

Lines changed: 14 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
namespace Symfony\Component\HttpClient;
1313

1414
use Psr\Log\LoggerAwareInterface;
15-
use Psr\Log\LoggerAwareTrait;
15+
use Psr\Log\LoggerInterface;
1616
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
1717
use Symfony\Component\HttpClient\Exception\TransportException;
1818
use Symfony\Component\HttpClient\Internal\CurlClientState;
@@ -35,7 +35,6 @@
3535
final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface, ResetInterface
3636
{
3737
use HttpClientTrait;
38-
use LoggerAwareTrait;
3938

4039
private array $defaultOptions = self::OPTIONS_DEFAULTS + [
4140
'auth_ntlm' => null, // array|string - an array containing the username as first value, and optionally the
@@ -45,13 +44,13 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface,
4544
],
4645
];
4746

47+
private ?LoggerInterface $logger = null;
48+
4849
/**
4950
* An internal object to share state between the client and its responses.
5051
*/
5152
private $multi;
5253

53-
private static array $curlVersion;
54-
5554
/**
5655
* @param array $defaultOptions Default request's options
5756
* @param int $maxHostConnections The maximum number of connections to a single host
@@ -71,33 +70,12 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
7170
[, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions);
7271
}
7372

74-
$this->multi = new CurlClientState();
75-
self::$curlVersion = self::$curlVersion ?? curl_version();
76-
77-
// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
78-
if (\defined('CURLPIPE_MULTIPLEX')) {
79-
curl_multi_setopt($this->multi->handle, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
80-
}
81-
if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) {
82-
$maxHostConnections = curl_multi_setopt($this->multi->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;
83-
}
84-
if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) {
85-
curl_multi_setopt($this->multi->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
86-
}
87-
88-
// Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/77535
89-
if (0 >= $maxPendingPushes) {
90-
return;
91-
}
92-
93-
// HTTP/2 push crashes before curl 7.61
94-
if (!\defined('CURLMOPT_PUSHFUNCTION') || 0x073D00 > self::$curlVersion['version_number'] || !(\CURL_VERSION_HTTP2 & self::$curlVersion['features'])) {
95-
return;
96-
}
73+
$this->multi = new CurlClientState($maxHostConnections, $maxPendingPushes);
74+
}
9775

98-
curl_multi_setopt($this->multi->handle, \CURLMOPT_PUSHFUNCTION, function ($parent, $pushed, array $requestHeaders) use ($maxPendingPushes) {
99-
return $this->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
100-
});
76+
public function setLogger(LoggerInterface $logger): void
77+
{
78+
$this->logger = $this->multi->logger = $logger;
10179
}
10280

10381
/**
@@ -143,7 +121,7 @@ public function request(string $method, string $url, array $options = []): Respo
143121
$curlopts[\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_1_0;
144122
} elseif (1.1 === (float) $options['http_version']) {
145123
$curlopts[\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_1_1;
146-
} elseif (\defined('CURL_VERSION_HTTP2') && (\CURL_VERSION_HTTP2 & self::$curlVersion['features']) && ('https:' === $scheme || 2.0 === (float) $options['http_version'])) {
124+
} elseif (\defined('CURL_VERSION_HTTP2') && (\CURL_VERSION_HTTP2 & CurlClientState::$curlVersion['features']) && ('https:' === $scheme || 2.0 === (float) $options['http_version'])) {
147125
$curlopts[\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_2_0;
148126
}
149127

@@ -186,11 +164,10 @@ public function request(string $method, string $url, array $options = []): Respo
186164
$this->multi->dnsCache->evictions = [];
187165
$port = parse_url($authority, \PHP_URL_PORT) ?: ('http:' === $scheme ? 80 : 443);
188166

189-
if ($resolve && 0x072A00 > self::$curlVersion['version_number']) {
167+
if ($resolve && 0x072A00 > CurlClientState::$curlVersion['version_number']) {
190168
// DNS cache removals require curl 7.42 or higher
191169
// On lower versions, we have to create a new multi handle
192-
curl_multi_close($this->multi->handle);
193-
$this->multi->handle = (new self())->multi->handle;
170+
$this->multi->reset();
194171
}
195172

196173
foreach ($options['resolve'] as $host => $ip) {
@@ -315,7 +292,7 @@ public function request(string $method, string $url, array $options = []): Respo
315292
}
316293
}
317294

318-
return $pushedResponse ?? new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host), self::$curlVersion['version_number']);
295+
return $pushedResponse ?? new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host), CurlClientState::$curlVersion['version_number']);
319296
}
320297

321298
/**
@@ -329,75 +306,18 @@ public function stream(ResponseInterface|iterable $responses, float $timeout = n
329306

330307
if ($this->multi->handle instanceof \CurlMultiHandle) {
331308
$active = 0;
332-
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active));
309+
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)) {
310+
}
333311
}
334312

335313
return new ResponseStream(CurlResponse::stream($responses, $timeout));
336314
}
337315

338316
public function reset()
339317
{
340-
$this->multi->logger = $this->logger;
341318
$this->multi->reset();
342319
}
343320

344-
public function __sleep(): array
345-
{
346-
throw new \BadMethodCallException('Cannot serialize '.__CLASS__);
347-
}
348-
349-
public function __wakeup()
350-
{
351-
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
352-
}
353-
354-
public function __destruct()
355-
{
356-
$this->multi->logger = $this->logger;
357-
}
358-
359-
private function handlePush($parent, $pushed, array $requestHeaders, int $maxPendingPushes): int
360-
{
361-
$headers = [];
362-
$origin = curl_getinfo($parent, \CURLINFO_EFFECTIVE_URL);
363-
364-
foreach ($requestHeaders as $h) {
365-
if (false !== $i = strpos($h, ':', 1)) {
366-
$headers[substr($h, 0, $i)][] = substr($h, 1 + $i);
367-
}
368-
}
369-
370-
if (!isset($headers[':method']) || !isset($headers[':scheme']) || !isset($headers[':authority']) || !isset($headers[':path'])) {
371-
$this->logger && $this->logger->debug(sprintf('Rejecting pushed response from "%s": pushed headers are invalid', $origin));
372-
373-
return \CURL_PUSH_DENY;
374-
}
375-
376-
$url = $headers[':scheme'][0].'://'.$headers[':authority'][0];
377-
378-
// curl before 7.65 doesn't validate the pushed ":authority" header,
379-
// but this is a MUST in the HTTP/2 RFC; let's restrict pushes to the original host,
380-
// ignoring domains mentioned as alt-name in the certificate for now (same as curl).
381-
if (!str_starts_with($origin, $url.'/')) {
382-
$this->logger && $this->logger->debug(sprintf('Rejecting pushed response from "%s": server is not authoritative for "%s"', $origin, $url));
383-
384-
return \CURL_PUSH_DENY;
385-
}
386-
387-
if ($maxPendingPushes <= \count($this->multi->pushedResponses)) {
388-
$fifoUrl = key($this->multi->pushedResponses);
389-
unset($this->multi->pushedResponses[$fifoUrl]);
390-
$this->logger && $this->logger->debug(sprintf('Evicting oldest pushed response: "%s"', $fifoUrl));
391-
}
392-
393-
$url .= $headers[':path'][0];
394-
$this->logger && $this->logger->debug(sprintf('Queueing pushed response: "%s"', $url));
395-
396-
$this->multi->pushedResponses[$url] = new PushedResponse(new CurlResponse($this->multi, $pushed), $headers, $this->multi->openHandles[(int) $parent][1] ?? [], $pushed);
397-
398-
return \CURL_PUSH_OK;
399-
}
400-
401321
/**
402322
* Accepts pushed responses only if their headers related to authentication match the request.
403323
*/

‎Internal/CurlClientState.php

Lines changed: 80 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\HttpClient\Internal;
1313

14+
use Psr\Log\LoggerInterface;
15+
use Symfony\Component\HttpClient\Response\CurlResponse;
1416

1517
/**
1618
* Internal representation of the cURL client's state.
@@ -28,12 +30,42 @@ final class CurlClientState extends ClientState
2830
/** @var float[] */
2931
public array $pauseExpiries = [];
3032
public int $execCounter = \PHP_INT_MIN;
31-
public $logger = null;
33+
public ?LoggerInterface $logger = null;
3234

33-
public function __construct()
35+
public static array $curlVersion;
36+
37+
private int $maxHostConnections;
38+
private int $maxPendingPushes;
39+
40+
public function __construct(int $maxHostConnections, int $maxPendingPushes)
3441
{
42+
self::$curlVersion = self::$curlVersion ?? curl_version();
43+
3544
$this->handle = curl_multi_init();
3645
$this->dnsCache = new DnsCache();
46+
$this->maxHostConnections = $maxHostConnections;
47+
$this->maxPendingPushes = $maxPendingPushes;
48+
49+
// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
50+
if (\defined('CURLPIPE_MULTIPLEX')) {
51+
curl_multi_setopt($this->handle, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
52+
}
53+
if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) {
54+
$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;
55+
}
56+
if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) {
57+
curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
58+
}
59+
60+
// Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/77535
61+
if (0 >= $maxPendingPushes) {
62+
return;
63+
}
64+
65+
// HTTP/2 push crashes before curl 7.61
66+
if (!\defined('CURLMOPT_PUSHFUNCTION') || 0x073D00 > self::$curlVersion['version_number'] || !(\CURL_VERSION_HTTP2 & self::$curlVersion['features'])) {
67+
return;
68+
}
3769
}
3870

3971
public function reset()
@@ -53,32 +85,63 @@ public function reset()
5385
curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, null);
5486
}
5587

56-
$active = 0;
57-
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->handle, $active));
88+
$this->__construct($this->maxHostConnections, $this->maxPendingPushes);
5889
}
90+
}
91+
92+
public function __wakeup()
93+
{
94+
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
95+
}
5996

97+
public function __destruct()
98+
{
6099
foreach ($this->openHandles as [$ch]) {
61100
if ($ch instanceof \CurlHandle) {
62101
curl_setopt($ch, \CURLOPT_VERBOSE, false);
63102
}
64103
}
65-
66-
curl_multi_close($this->handle);
67-
$this->handle = curl_multi_init();
68104
}
69105

70-
public function __sleep(): array
106+
private function handlePush($parent, $pushed, array $requestHeaders, int $maxPendingPushes): int
71107
{
72-
throw new \BadMethodCallException('Cannot serialize '.__CLASS__);
73-
}
108+
$headers = [];
109+
$origin = curl_getinfo($parent, \CURLINFO_EFFECTIVE_URL);
74110

75-
public function __wakeup()
76-
{
77-
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
78-
}
111+
foreach ($requestHeaders as $h) {
112+
if (false !== $i = strpos($h, ':', 1)) {
113+
$headers[substr($h, 0, $i)][] = substr($h, 1 + $i);
114+
}
115+
}
79116

80-
public function __destruct()
81-
{
82-
$this->reset();
117+
if (!isset($headers[':method']) || !isset($headers[':scheme']) || !isset($headers[':authority']) || !isset($headers[':path'])) {
118+
$this->logger && $this->logger->debug(sprintf('Rejecting pushed response from "%s": pushed headers are invalid', $origin));
119+
120+
return \CURL_PUSH_DENY;
121+
}
122+
123+
$url = $headers[':scheme'][0].'://'.$headers[':authority'][0];
124+
125+
// curl before 7.65 doesn't validate the pushed ":authority" header,
126+
// but this is a MUST in the HTTP/2 RFC; let's restrict pushes to the original host,
127+
// ignoring domains mentioned as alt-name in the certificate for now (same as curl).
128+
if (!str_starts_with($origin, $url.'/')) {
129+
$this->logger && $this->logger->debug(sprintf('Rejecting pushed response from "%s": server is not authoritative for "%s"', $origin, $url));
130+
131+
return \CURL_PUSH_DENY;
132+
}
133+
134+
if ($maxPendingPushes <= \count($this->pushedResponses)) {
135+
$fifoUrl = key($this->pushedResponses);
136+
unset($this->pushedResponses[$fifoUrl]);
137+
$this->logger && $this->logger->debug(sprintf('Evicting oldest pushed response: "%s"', $fifoUrl));
138+
}
139+
140+
$url .= $headers[':path'][0];
141+
$this->logger && $this->logger->debug(sprintf('Queueing pushed response: "%s"', $url));
142+
143+
$this->pushedResponses[$url] = new PushedResponse(new CurlResponse($this, $pushed), $headers, $this->openHandles[(int) $parent][1] ?? [], $pushed);
144+
145+
return \CURL_PUSH_OK;
83146
}
84147
}

‎Response/AmpResponse.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ public function __construct(AmpClientState $multi, Request $request, array $opti
125125
}
126126
};
127127

128+
$multi->lastTimeout = null;
128129
$multi->openHandles[$id] = $id;
129130
++$multi->responseCount;
130131

‎Response/CommonResponseTrait.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,15 @@ public function __wakeup()
138138
*/
139139
abstract protected function close(): void;
140140

141-
private static function initialize(self $response, float $timeout = null): void
141+
private static function initialize(self $response): void
142142
{
143143
if (null !== $response->getInfo('error')) {
144144
throw new TransportException($response->getInfo('error'));
145145
}
146146

147147
try {
148-
if (($response->initializer)($response, $timeout)) {
149-
foreach (self::stream([$response], $timeout) as $chunk) {
148+
if (($response->initializer)($response, -0.0)) {
149+
foreach (self::stream([$response], -0.0) as $chunk) {
150150
if ($chunk->isFirst()) {
151151
break;
152152
}

‎Response/CurlResponse.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ public function __construct(CurlClientState $multi, \CurlHandle|string $ch, arra
172172
};
173173

174174
// Schedule the request in a non-blocking way
175+
$multi->lastTimeout = null;
175176
$multi->openHandles[$id] = [$ch, $options];
176177
curl_multi_add_handle($multi->handle, $ch);
177178

‎Response/NativeResponse.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ private function open(): void
202202
}
203203

204204
$host = parse_url($this->info['redirect_url'] ?? $this->url, \PHP_URL_HOST);
205+
$this->multi->lastTimeout = null;
205206
$this->multi->openHandles[$this->id] = [&$this->pauseExpiry, $h, $this->buffer, $this->onProgress, &$this->remaining, &$this->info, $host];
206207
$this->multi->hosts[$host] = 1 + ($this->multi->hosts[$host] ?? 0);
207208
}

‎Response/TransportResponseTrait.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ private function doDestruct()
138138
$this->shouldBuffer = true;
139139

140140
if ($this->initializer && null === $this->info['error']) {
141-
self::initialize($this, -0.0);
141+
self::initialize($this);
142142
$this->checkStatusCode();
143143
}
144144
}

‎Tests/MockHttpClientTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ protected function getHttpClient(string $testCase): HttpClientInterface
278278
$this->markTestSkipped('Real transport required');
279279
break;
280280

281+
case 'testTimeoutOnInitialize':
281282
case 'testTimeoutOnDestruct':
282283
$this->markTestSkipped('Real transport required');
283284
break;

‎Tests/NativeHttpClientTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ public function testInformationalResponseStream()
2626
$this->markTestSkipped('NativeHttpClient doesn\'t support informational status codes.');
2727
}
2828

29+
public function testTimeoutOnInitialize()
30+
{
31+
$this->markTestSkipped('NativeHttpClient doesn\'t support opening concurrent requests.');
32+
}
33+
2934
public function testTimeoutOnDestruct()
3035
{
3136
$this->markTestSkipped('NativeHttpClient doesn\'t support opening concurrent requests.');

0 commit comments

Comments
 (0)