Skip to content

Commit 3d12585

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.4' into 7.3
* 6.4: [SecurityBundle] Fix tests with Symfony 7.4 [HttpClient] Fix sharing CurlClientState between clones of CurlHttpClient instances Revert "[HttpClient] Lazily initialize CurlClientState" [Yaml] Fix regression handling blank lines in unquoted scalars Fix the creation of a redis connection with only ext-relay [DependencyInjection] Reset resolved state when setting a parameter Fix Request getPathInfo docblock
2 parents 6bb245e + a67de20 commit 3d12585

File tree

3 files changed

+106
-89
lines changed

3 files changed

+106
-89
lines changed

‎CurlHttpClient.php‎

Lines changed: 19 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface,
5151

5252
private ?LoggerInterface $logger = null;
5353

54-
private int $maxHostConnections;
55-
private int $maxPendingPushes;
56-
5754
/**
5855
* An internal object to share state between the client and its responses.
5956
*/
@@ -72,31 +69,25 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
7269
throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\CurlHttpClient" as the "curl" extension is not installed.');
7370
}
7471

75-
$this->maxHostConnections = $maxHostConnections;
76-
$this->maxPendingPushes = $maxPendingPushes;
77-
7872
$this->defaultOptions['buffer'] ??= self::shouldBuffer(...);
7973

8074
if ($defaultOptions) {
8175
[, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions);
8276
}
77+
78+
$this->multi = new CurlClientState($maxHostConnections, $maxPendingPushes);
8379
}
8480

8581
public function setLogger(LoggerInterface $logger): void
8682
{
87-
$this->logger = $logger;
88-
if (isset($this->multi)) {
89-
$this->multi->logger = $logger;
90-
}
83+
$this->logger = $this->multi->logger = $logger;
9184
}
9285

9386
/**
9487
* @see HttpClientInterface::OPTIONS_DEFAULTS for available options
9588
*/
9689
public function request(string $method, string $url, array $options = []): ResponseInterface
9790
{
98-
$multi = $this->ensureState();
99-
10091
[$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions);
10192
$scheme = $url['scheme'];
10293
$authority = $url['authority'];
@@ -176,24 +167,24 @@ public function request(string $method, string $url, array $options = []): Respo
176167
}
177168

178169
// curl's resolve feature varies by host:port but ours varies by host only, let's handle this with our own DNS map
179-
if (isset($multi->dnsCache->hostnames[$host])) {
180-
$options['resolve'] += [$host => $multi->dnsCache->hostnames[$host]];
170+
if (isset($this->multi->dnsCache->hostnames[$host])) {
171+
$options['resolve'] += [$host => $this->multi->dnsCache->hostnames[$host]];
181172
}
182173

183-
if ($options['resolve'] || $multi->dnsCache->evictions) {
174+
if ($options['resolve'] || $this->multi->dnsCache->evictions) {
184175
// First reset any old DNS cache entries then add the new ones
185-
$resolve = $multi->dnsCache->evictions;
186-
$multi->dnsCache->evictions = [];
176+
$resolve = $this->multi->dnsCache->evictions;
177+
$this->multi->dnsCache->evictions = [];
187178

188179
if ($resolve && 0x072A00 > CurlClientState::$curlVersion['version_number']) {
189180
// DNS cache removals require curl 7.42 or higher
190-
$multi->reset();
181+
$this->multi->reset();
191182
}
192183

193184
foreach ($options['resolve'] as $resolveHost => $ip) {
194185
$resolve[] = null === $ip ? "-$resolveHost:$port" : "$resolveHost:$port:$ip";
195-
$multi->dnsCache->hostnames[$resolveHost] = $ip;
196-
$multi->dnsCache->removals["-$resolveHost:$port"] = "-$resolveHost:$port";
186+
$this->multi->dnsCache->hostnames[$resolveHost] = $ip;
187+
$this->multi->dnsCache->removals["-$resolveHost:$port"] = "-$resolveHost:$port";
197188
}
198189

199190
$curlopts[\CURLOPT_RESOLVE] = $resolve;
@@ -299,16 +290,16 @@ public function request(string $method, string $url, array $options = []): Respo
299290
$curlopts += $options['extra']['curl'];
300291
}
301292

302-
if ($pushedResponse = $multi->pushedResponses[$url] ?? null) {
303-
unset($multi->pushedResponses[$url]);
293+
if ($pushedResponse = $this->multi->pushedResponses[$url] ?? null) {
294+
unset($this->multi->pushedResponses[$url]);
304295

305296
if (self::acceptPushForRequest($method, $options, $pushedResponse)) {
306297
$this->logger?->debug(\sprintf('Accepting pushed response: "%s %s"', $method, $url));
307298

308299
// Reinitialize the pushed response with request's options
309300
$ch = $pushedResponse->handle;
310301
$pushedResponse = $pushedResponse->response;
311-
$pushedResponse->__construct($multi, $url, $options, $this->logger);
302+
$pushedResponse->__construct($this->multi, $url, $options, $this->logger);
312303
} else {
313304
$this->logger?->debug(\sprintf('Rejecting pushed response: "%s"', $url));
314305
$pushedResponse = null;
@@ -318,7 +309,7 @@ public function request(string $method, string $url, array $options = []): Respo
318309
if (!$pushedResponse) {
319310
$ch = curl_init();
320311
$this->logger?->info(\sprintf('Request: "%s %s"', $method, $url));
321-
$curlopts += [\CURLOPT_SHARE => $multi->share];
312+
$curlopts += [\CURLOPT_SHARE => $this->multi->share];
322313
}
323314

324315
foreach ($curlopts as $opt => $value) {
@@ -331,7 +322,7 @@ public function request(string $method, string $url, array $options = []): Respo
331322
}
332323
}
333324

334-
return $pushedResponse ?? new CurlResponse($multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $authority), CurlClientState::$curlVersion['version_number'], $url);
325+
return $pushedResponse ?? new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $authority), CurlClientState::$curlVersion['version_number'], $url);
335326
}
336327

337328
public function stream(ResponseInterface|iterable $responses, ?float $timeout = null): ResponseStreamInterface
@@ -340,11 +331,9 @@ public function stream(ResponseInterface|iterable $responses, ?float $timeout =
340331
$responses = [$responses];
341332
}
342333

343-
$multi = $this->ensureState();
344-
345-
if ($multi->handle instanceof \CurlMultiHandle) {
334+
if ($this->multi->handle instanceof \CurlMultiHandle) {
346335
$active = 0;
347-
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($multi->handle, $active)) {
336+
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)) {
348337
}
349338
}
350339

@@ -353,9 +342,7 @@ public function stream(ResponseInterface|iterable $responses, ?float $timeout =
353342

354343
public function reset(): void
355344
{
356-
if (isset($this->multi)) {
357-
$this->multi->reset();
358-
}
345+
$this->multi->reset();
359346
}
360347

361348
/**
@@ -454,16 +441,6 @@ private static function createRedirectResolver(array $options, string $authority
454441
};
455442
}
456443

457-
private function ensureState(): CurlClientState
458-
{
459-
if (!isset($this->multi)) {
460-
$this->multi = new CurlClientState($this->maxHostConnections, $this->maxPendingPushes);
461-
$this->multi->logger = $this->logger;
462-
}
463-
464-
return $this->multi;
465-
}
466-
467444
private function findConstantName(int $opt): ?string
468445
{
469446
$constants = array_filter(get_defined_constants(), static fn ($v, $k) => $v === $opt && 'C' === $k[0] && (str_starts_with($k, 'CURLOPT_') || str_starts_with($k, 'CURLINFO_')), \ARRAY_FILTER_USE_BOTH);

‎Internal/CurlClientState.php‎

Lines changed: 55 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -37,45 +37,15 @@ final class CurlClientState extends ClientState
3737

3838
public static array $curlVersion;
3939

40-
public function __construct(int $maxHostConnections, int $maxPendingPushes)
41-
{
40+
public function __construct(
41+
private int $maxHostConnections,
42+
private int $maxPendingPushes,
43+
) {
4244
self::$curlVersion ??= curl_version();
43-
44-
$this->handle = curl_multi_init();
4545
$this->dnsCache = new DnsCache();
46-
$this->reset();
4746

48-
// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
49-
if (\defined('CURLPIPE_MULTIPLEX')) {
50-
curl_multi_setopt($this->handle, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
51-
}
52-
if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS') && 0 < $maxHostConnections) {
53-
$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, $maxHostConnections) ? min(50 * $maxHostConnections, 4294967295) : $maxHostConnections;
54-
}
55-
if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) {
56-
curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
57-
}
58-
59-
// Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/77535
60-
if (0 >= $maxPendingPushes) {
61-
return;
62-
}
63-
64-
// HTTP/2 push crashes before curl 7.61
65-
if (!\defined('CURLMOPT_PUSHFUNCTION') || 0x073D00 > self::$curlVersion['version_number'] || !(\CURL_VERSION_HTTP2 & self::$curlVersion['features'])) {
66-
return;
67-
}
68-
69-
// Clone to prevent a circular reference
70-
$multi = clone $this;
71-
$multi->handle = null;
72-
$multi->share = null;
73-
$multi->pushedResponses = &$this->pushedResponses;
74-
$multi->logger = &$this->logger;
75-
$multi->handlesActivity = &$this->handlesActivity;
76-
$multi->openHandles = &$this->openHandles;
77-
78-
curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, static fn ($parent, $pushed, array $requestHeaders) => $multi->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes));
47+
// handle and share are initialized lazily in __get()
48+
unset($this->handle, $this->share);
7949
}
8050

8151
public function reset(): void
@@ -89,17 +59,59 @@ public function reset(): void
8959
$this->dnsCache->evictions = $this->dnsCache->evictions ?: $this->dnsCache->removals;
9060
$this->dnsCache->removals = $this->dnsCache->hostnames = [];
9161

92-
$this->share = curl_share_init();
62+
unset($this->share);
63+
}
64+
65+
public function __get(string $name): mixed
66+
{
67+
if ('share' === $name) {
68+
$this->share = curl_share_init();
69+
70+
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_DNS);
71+
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_SSL_SESSION);
9372

94-
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_DNS);
95-
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_SSL_SESSION);
73+
if (\defined('CURL_LOCK_DATA_CONNECT')) {
74+
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_CONNECT);
75+
}
9676

97-
if (\defined('CURL_LOCK_DATA_CONNECT')) {
98-
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_CONNECT);
77+
return $this->share;
9978
}
79+
80+
if ('handle' === $name) {
81+
$this->handle = curl_multi_init();
82+
83+
// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
84+
if (\defined('CURLPIPE_MULTIPLEX')) {
85+
curl_multi_setopt($this->handle, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
86+
}
87+
if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS') && 0 < $this->maxHostConnections) {
88+
$this->maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, $this->maxHostConnections) ? min(50 * $this->maxHostConnections, 4294967295) : $this->maxHostConnections;
89+
}
90+
if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $this->maxHostConnections) {
91+
curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $this->maxHostConnections);
92+
}
93+
94+
// Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/77535
95+
if (0 < $this->maxPendingPushes && (\defined('CURLMOPT_PUSHFUNCTION') && 0x073D00 <= self::$curlVersion['version_number'] && (\CURL_VERSION_HTTP2 & self::$curlVersion['features']))) {
96+
// Clone to prevent a circular reference
97+
$multi = clone $this;
98+
$multi->handle = null;
99+
$multi->share = null;
100+
$multi->pushedResponses = &$this->pushedResponses;
101+
$multi->logger = &$this->logger;
102+
$multi->handlesActivity = &$this->handlesActivity;
103+
$multi->openHandles = &$this->openHandles;
104+
105+
curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, $multi->handlePush(...));
106+
}
107+
108+
return $this->handle;
109+
}
110+
111+
throw new \LogicException(\sprintf('Unknown property "%s" on "%s".', $name, self::class));
100112
}
101113

102-
private function handlePush($parent, $pushed, array $requestHeaders, int $maxPendingPushes): int
114+
private function handlePush($parent, $pushed, array $requestHeaders): int
103115
{
104116
$headers = [];
105117
$origin = curl_getinfo($parent, \CURLINFO_EFFECTIVE_URL);
@@ -127,7 +139,7 @@ private function handlePush($parent, $pushed, array $requestHeaders, int $maxPen
127139
return \CURL_PUSH_DENY;
128140
}
129141

130-
if ($maxPendingPushes <= \count($this->pushedResponses)) {
142+
if ($this->maxPendingPushes <= \count($this->pushedResponses)) {
131143
$fifoUrl = key($this->pushedResponses);
132144
unset($this->pushedResponses[$fifoUrl]);
133145
$this->logger?->debug(\sprintf('Evicting oldest pushed response: "%s"', $fifoUrl));

‎Tests/CurlHttpClientTest.php‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
use Symfony\Component\HttpClient\CurlHttpClient;
1515
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
16-
use Symfony\Contracts\HttpClient\HttpClientInterface;
1716

1817
/**
1918
* @requires extension curl
@@ -22,7 +21,7 @@
2221
*/
2322
class CurlHttpClientTest extends HttpClientTestCase
2423
{
25-
protected function getHttpClient(string $testCase): HttpClientInterface
24+
protected function getHttpClient(string $testCase): CurlHttpClient
2625
{
2726
if (!str_contains($testCase, 'Push')) {
2827
return new CurlHttpClient(['verify_peer' => false, 'verify_host' => false]);
@@ -48,13 +47,42 @@ public function testHandleIsReinitOnReset()
4847
{
4948
$httpClient = $this->getHttpClient(__FUNCTION__);
5049

51-
$r = new \ReflectionMethod($httpClient, 'ensureState');
52-
$clientState = $r->invoke($httpClient);
50+
$r = new \ReflectionProperty($httpClient, 'multi');
51+
$clientState = $r->getValue($httpClient);
5352
$initialShareId = $clientState->share;
5453
$httpClient->reset();
5554
self::assertNotSame($initialShareId, $clientState->share);
5655
}
5756

57+
public function testCurlClientStateIsSharedBetweenClones()
58+
{
59+
$client = $this->getHttpClient(__FUNCTION__);
60+
$cloneA = $client->withOptions(['headers' => ['Foo: bar']]);
61+
$cloneB = $client->withOptions(['headers' => ['Foo: baz']]);
62+
63+
$r = new \ReflectionProperty($client, 'multi');
64+
$state = $r->getValue($client);
65+
66+
self::assertSame($state, $r->getValue($cloneA));
67+
self::assertSame($state, $r->getValue($cloneB));
68+
}
69+
70+
public function testCurlClientStateInitializesHandlesLazily()
71+
{
72+
$client = $this->getHttpClient(__FUNCTION__);
73+
74+
$r = new \ReflectionProperty($client, 'multi');
75+
$state = $r->getValue($client);
76+
77+
self::assertFalse(isset($state->handle));
78+
self::assertFalse(isset($state->share));
79+
80+
$client->request('GET', 'http://127.0.0.1:8057/json')->getStatusCode();
81+
82+
self::assertInstanceOf(\CurlMultiHandle::class, $state->handle);
83+
self::assertInstanceOf(\CurlShareHandle::class, $state->share);
84+
}
85+
5886
public function testProcessAfterReset()
5987
{
6088
$client = $this->getHttpClient(__FUNCTION__);

0 commit comments

Comments
 (0)