Skip to content

Commit b6d3e36

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '5.3' into 5.4
* 5.3: [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 4d46e5b + 22e63bc commit b6d3e36

9 files changed

+112
-114
lines changed

‎CurlHttpClient.php

Lines changed: 17 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 $defaultOptions = self::OPTIONS_DEFAULTS + [
4140
'auth_ntlm' => null, // array|string - an array containing the username as first value, and optionally the
@@ -45,15 +44,18 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface,
4544
],
4645
];
4746

47+
/**
48+
* @var LoggerInterface|null
49+
*/
50+
private $logger;
51+
4852
/**
4953
* An internal object to share state between the client and its responses.
5054
*
5155
* @var CurlClientState
5256
*/
5357
private $multi;
5458

55-
private static $curlVersion;
56-
5759
/**
5860
* @param array $defaultOptions Default request's options
5961
* @param int $maxHostConnections The maximum number of connections to a single host
@@ -73,33 +75,12 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
7375
[, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions);
7476
}
7577

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

100-
curl_multi_setopt($this->multi->handle, \CURLMOPT_PUSHFUNCTION, function ($parent, $pushed, array $requestHeaders) use ($maxPendingPushes) {
101-
return $this->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
102-
});
81+
public function setLogger(LoggerInterface $logger): void
82+
{
83+
$this->logger = $this->multi->logger = $logger;
10384
}
10485

10586
/**
@@ -145,7 +126,7 @@ public function request(string $method, string $url, array $options = []): Respo
145126
$curlopts[\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_1_0;
146127
} elseif (1.1 === (float) $options['http_version']) {
147128
$curlopts[\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_1_1;
148-
} elseif (\defined('CURL_VERSION_HTTP2') && (\CURL_VERSION_HTTP2 & self::$curlVersion['features']) && ('https:' === $scheme || 2.0 === (float) $options['http_version'])) {
129+
} elseif (\defined('CURL_VERSION_HTTP2') && (\CURL_VERSION_HTTP2 & CurlClientState::$curlVersion['features']) && ('https:' === $scheme || 2.0 === (float) $options['http_version'])) {
149130
$curlopts[\CURLOPT_HTTP_VERSION] = \CURL_HTTP_VERSION_2_0;
150131
}
151132

@@ -188,11 +169,10 @@ public function request(string $method, string $url, array $options = []): Respo
188169
$this->multi->dnsCache->evictions = [];
189170
$port = parse_url($authority, \PHP_URL_PORT) ?: ('http:' === $scheme ? 80 : 443);
190171

191-
if ($resolve && 0x072A00 > self::$curlVersion['version_number']) {
172+
if ($resolve && 0x072A00 > CurlClientState::$curlVersion['version_number']) {
192173
// DNS cache removals require curl 7.42 or higher
193174
// On lower versions, we have to create a new multi handle
194-
curl_multi_close($this->multi->handle);
195-
$this->multi->handle = (new self())->multi->handle;
175+
$this->multi->reset();
196176
}
197177

198178
foreach ($options['resolve'] as $host => $ip) {
@@ -317,7 +297,7 @@ public function request(string $method, string $url, array $options = []): Respo
317297
}
318298
}
319299

320-
return $pushedResponse ?? new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host), self::$curlVersion['version_number']);
300+
return $pushedResponse ?? new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host), CurlClientState::$curlVersion['version_number']);
321301
}
322302

323303
/**
@@ -333,75 +313,18 @@ public function stream($responses, float $timeout = null): ResponseStreamInterfa
333313

334314
if (\is_resource($this->multi->handle) || $this->multi->handle instanceof \CurlMultiHandle) {
335315
$active = 0;
336-
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active));
316+
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)) {
317+
}
337318
}
338319

339320
return new ResponseStream(CurlResponse::stream($responses, $timeout));
340321
}
341322

342323
public function reset()
343324
{
344-
$this->multi->logger = $this->logger;
345325
$this->multi->reset();
346326
}
347327

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

‎Internal/CurlClientState.php

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

1414
use Psr\Log\LoggerInterface;
15+
use Symfony\Component\HttpClient\Response\CurlResponse;
1516

1617
/**
1718
* Internal representation of the cURL client's state.
@@ -34,10 +35,44 @@ final class CurlClientState extends ClientState
3435
/** @var LoggerInterface|null */
3536
public $logger;
3637

37-
public function __construct()
38+
public static $curlVersion;
39+
40+
private $maxHostConnections;
41+
private $maxPendingPushes;
42+
43+
public function __construct(int $maxHostConnections, int $maxPendingPushes)
3844
{
45+
self::$curlVersion = self::$curlVersion ?? curl_version();
46+
3947
$this->handle = curl_multi_init();
4048
$this->dnsCache = new DnsCache();
49+
$this->maxHostConnections = $maxHostConnections;
50+
$this->maxPendingPushes = $maxPendingPushes;
51+
52+
// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
53+
if (\defined('CURLPIPE_MULTIPLEX')) {
54+
curl_multi_setopt($this->handle, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
55+
}
56+
if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) {
57+
$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;
58+
}
59+
if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) {
60+
curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
61+
}
62+
63+
// Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/77535
64+
if (0 >= $maxPendingPushes || \PHP_VERSION_ID < 70217 || (\PHP_VERSION_ID >= 70300 && \PHP_VERSION_ID < 70304)) {
65+
return;
66+
}
67+
68+
// HTTP/2 push crashes before curl 7.61
69+
if (!\defined('CURLMOPT_PUSHFUNCTION') || 0x073D00 > self::$curlVersion['version_number'] || !(\CURL_VERSION_HTTP2 & self::$curlVersion['features'])) {
70+
return;
71+
}
72+
73+
curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, function ($parent, $pushed, array $requestHeaders) use ($maxPendingPushes) {
74+
return $this->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
75+
});
4176
}
4277

4378
public function reset()
@@ -57,32 +92,63 @@ public function reset()
5792
curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, null);
5893
}
5994

60-
$active = 0;
61-
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->handle, $active));
95+
$this->__construct($this->maxHostConnections, $this->maxPendingPushes);
6296
}
97+
}
98+
99+
public function __wakeup()
100+
{
101+
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
102+
}
63103

104+
public function __destruct()
105+
{
64106
foreach ($this->openHandles as [$ch]) {
65107
if (\is_resource($ch) || $ch instanceof \CurlHandle) {
66108
curl_setopt($ch, \CURLOPT_VERBOSE, false);
67109
}
68110
}
69-
70-
curl_multi_close($this->handle);
71-
$this->handle = curl_multi_init();
72111
}
73112

74-
public function __sleep(): array
113+
private function handlePush($parent, $pushed, array $requestHeaders, int $maxPendingPushes): int
75114
{
76-
throw new \BadMethodCallException('Cannot serialize '.__CLASS__);
77-
}
115+
$headers = [];
116+
$origin = curl_getinfo($parent, \CURLINFO_EFFECTIVE_URL);
78117

79-
public function __wakeup()
80-
{
81-
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
82-
}
118+
foreach ($requestHeaders as $h) {
119+
if (false !== $i = strpos($h, ':', 1)) {
120+
$headers[substr($h, 0, $i)][] = substr($h, 1 + $i);
121+
}
122+
}
83123

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

‎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
@@ -142,15 +142,15 @@ public function __wakeup()
142142
*/
143143
abstract protected function close(): void;
144144

145-
private static function initialize(self $response, float $timeout = null): void
145+
private static function initialize(self $response): void
146146
{
147147
if (null !== $response->getInfo('error')) {
148148
throw new TransportException($response->getInfo('error'));
149149
}
150150

151151
try {
152-
if (($response->initializer)($response, $timeout)) {
153-
foreach (self::stream([$response], $timeout) as $chunk) {
152+
if (($response->initializer)($response, -0.0)) {
153+
foreach (self::stream([$response], -0.0) as $chunk) {
154154
if ($chunk->isFirst()) {
155155
break;
156156
}

‎Response/CurlResponse.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
170170
};
171171

172172
// Schedule the request in a non-blocking way
173+
$multi->lastTimeout = null;
173174
$multi->openHandles[$id] = [$ch, $options];
174175
curl_multi_add_handle($multi->handle, $ch);
175176

‎Response/NativeResponse.php

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

196196
$host = parse_url($this->info['redirect_url'] ?? $this->url, \PHP_URL_HOST);
197+
$this->multi->lastTimeout = null;
197198
$this->multi->openHandles[$this->id] = [&$this->pauseExpiry, $h, $this->buffer, $this->onProgress, &$this->remaining, &$this->info, $host];
198199
$this->multi->hosts[$host] = 1 + ($this->multi->hosts[$host] ?? 0);
199200
}

‎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)