8000 bug #45015 [HttpClient] fix resetting DNS/etc when calling CurlHttpCl… · symfony/symfony@5f6d897 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5f6d897

Browse files
nicolas-grekasnicolas-grekas
committed
bug #45015 [HttpClient] fix resetting DNS/etc when calling CurlHttpClient::reset() (nicolas-grekas)
This PR was merged into the 4.4 branch. Discussion ---------- [HttpClient] fix resetting DNS/etc when calling CurlHttpClient::reset() | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #44900 | License | MIT | Doc PR | - This PR reverts #44625 and uses a new curl-share handle instead to reset any state. Commits ------- d426646 [HttpClient] fix resetting DNS/etc when calling CurlHttpClient::reset()
2 parents 9f5238d + d426646 commit 5f6d897

File tree

4 files changed

+52
-73
lines changed

4 files changed

+52
-73
lines changed

‎src/Symfony/Component/HttpClient/CurlHttpClient.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,6 @@ public function request(string $method, string $url, array $options = []): Respo
168168

169169
if ($resolve && 0x072A00 > CurlClientState::$curlVersion['version_number']) {
170170
// DNS cache removals require curl 7.42 or higher
171-
// On lower versions, we have to create a new multi handle
172171
$this->multi->reset();
173172
}
174173

@@ -280,6 +279,7 @@ public function request(string $method, string $url, array $options = []): Respo
280279
if (!$pushedResponse) {
281280
$ch = curl_init();
282281
$this->logger && $this->logger->info(sprintf('Request: "%s %s"', $method, $url));
282+
$curlopts += [\CURLOPT_SHARE => $this->multi->share];
283283
}
284284

285285
foreach ($curlopts as $opt => $value) {
@@ -306,9 +306,9 @@ public function stream($responses, float $timeout = null): ResponseStreamInterfa
306306
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)));
307307
}
308308

309-
if (\is_resource($mh = $this->multi->handles[0] ?? null) || $mh instanceof \CurlMultiHandle) {
309+
if (\is_resource($this->multi->handle) || $this->multi->handle instanceof \CurlMultiHandle) {
310310
$active = 0;
311-
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($mh, $active)) {
311+
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)) {
312312
}
313313
}
314314

‎src/Symfony/Component/HttpClient/Internal/CurlClientState.php

Lines changed: 19 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
*/
2424
final class CurlClientState extends ClientState
2525
{
26-
/** @var array<\CurlMultiHandle|resource> */
27-
public $handles = [];
26+
/** @var \CurlMultiHandle|resource */
27+
public $handle;
28+
/** @var \CurlShareHandle|resource */
29+
public $share;
2830
/** @var PushedResponse[] */
2931
public $pushedResponses = [];
3032
/** @var DnsCache */
@@ -34,27 +36,23 @@ final class CurlClientState extends ClientState
3436

3537
public static $curlVersion;
3638

37-
private $maxHostConnections;
38-
private $maxPendingPushes;
39-
4039
public function __construct(int $maxHostConnections, int $maxPendingPushes)
4140
{
4241
self::$curlVersion = self::$curlVersion ?? curl_version();
4342

44-
array_unshift($this->handles, $mh = curl_multi_init());
43+
$this->handle = curl_multi_init();
4544
$this->dnsCache = new DnsCache();
46-
$this->maxHostConnections = $maxHostConnections;
47-
$this->maxPendingPushes = $maxPendingPushes;
45+
$this->reset();
4846

4947
// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
5048
if (\defined('CURLPIPE_MULTIPLEX')) {
51-
curl_multi_setopt($mh, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
49+
curl_multi_setopt($this->handle, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
5250
}
5351
if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) {
54-
$maxHostConnections = curl_multi_setopt($mh, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;
52+
$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;
5553
}
5654
if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) {
57-
curl_multi_setopt($mh, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
55+
curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
5856
}
5957

6058
// Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/77535
@@ -67,40 +65,31 @@ public function __construct(int $maxHostConnections, int $maxPendingPushes)
6765
return;
6866
}
6967

70-
// Clone to prevent a circular reference
71-
$multi = clone $this;
72-
$multi->handles = [$mh];
73-
$multi->pushedResponses = &$this->pushedResponses;
74-
$multi->logger = &$this->logger;
75-
$multi->handlesActivity = &$this->handlesActivity;
76-
$multi->openHandles = &$this->openHandles;
77-
$multi->lastTimeout = &$this->lastTimeout;
78-
79-
curl_multi_setopt($mh, \CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes) {
80-
return $multi->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
68+
curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, function ($parent, $pushed, array $requestHeaders) use ($maxPendingPushes) {
69+
return $this->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
8170
});
8271
}
8372

8473
public function reset()
8574
{
8675
foreach ($this->pushedResponses as $url => $response) {
8776
$this->logger && $this->logger->debug(sprintf('Unused pushed response: "%s"', $url));
88-
89-
foreach ($this->handles as $mh) {
90-
curl_multi_remove_handle($mh, $response->handle);
91-
}
77+
curl_multi_remove_handle($this->handle, $response->handle);
9278
curl_close($response->handle);
9379
}
9480

9581
$this->pushedResponses = [];
9682
$this->dnsCache->evictions = $this->dnsCache->evictions ?: $this->dnsCache->removals;
9783
$this->dnsCache->removals = $this->dnsCache->hostnames = [];
9884

99-
if (\defined('CURLMOPT_PUSHFUNCTION')) {
100-
curl_multi_setopt($this->handles[0], \CURLMOPT_PUSHFUNCTION, null);
101-
}
85+
$this->share = curl_share_init();
86+
87+
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_DNS);
88+
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_SSL_SESSION);
10289

103-
$this->__construct($this->maxHostConnections, $this->maxPendingPushes);
90+
if (\defined('CURL_LOCK_DATA_CONNECT')) {
91+
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_CONNECT);
92+
}
10493
}
10594

10695
private function handlePush($parent, $pushed, array $requestHeaders, int $maxPendingPushes): int

‎src/Symfony/Component/HttpClient/Response/CurlResponse.php

Lines changed: 28 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
150150
// Schedule the request in a non-blocking way
151151
$multi->lastTimeout = null;
152152
$multi->openHandles[$id] = [$ch, $options];
153-
curl_multi_add_handle($multi->handles[0], $ch);
153+
curl_multi_add_handle($multi->handle, $ch);
154154

155155
$this->canary = new Canary(static function () use ($ch, $multi, $id) {
156156
unset($multi->openHandles[$id], $multi->handlesActivity[$id]);
@@ -160,9 +160,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
160160
return;
161161
}
162162

163-
foreach ($multi->handles as $mh) {
164-
curl_multi_remove_handle($mh, $ch);
165-
}
163+
curl_multi_remove_handle($multi->handle, $ch);
166164
curl_setopt_array($ch, [
167165
\CURLOPT_NOPROGRESS => true,
168166
\CURLOPT_PROGRESSFUNCTION => null,
@@ -244,7 +242,7 @@ public function __destruct()
244242
*/
245243
private static function schedule(self $response, array &$runningResponses): void
246244
{
247-
if (isset($runningResponses[$i = (int) $response->multi->handles[0]])) {
245+
if (isset($runningResponses[$i = (int) $response->multi->handle])) {
248246
$runningResponses[$i][1][$response->id] = $response;
249247
} else {
250248
$runningResponses[$i] = [$response->multi, [$response->id => $response]];
@@ -276,47 +274,39 @@ private static function perform(ClientState $multi, array &$responses = null): v
276274

277275
try {
278276
self::$performing = true;
277+
$active = 0;
278+
while (\CURLM_CALL_MULTI_PERFORM === ($err = curl_multi_exec($multi->handle, $active))) {
279+
}
279280

280-
foreach ($multi->handles as $i => $mh) {
281-
$active = 0;
282-
while (\CURLM_CALL_MULTI_PERFORM === ($err = curl_multi_exec($mh, $active))) {
283-
}
281+
if (\CURLM_OK !== $err) {
282+
throw new TransportException(curl_multi_strerror($err));
283+
}
284284

285-
if (\CURLM_OK !== $err) {
286-
throw new TransportException(curl_multi_strerror($err));
285+
while ($info = curl_multi_info_read($multi->handle)) {
286+
if (\CURLMSG_DONE !== $info['msg']) {
287+
continue;
287288
}
289+
$result = $info['result'];
290+
$id = (int) $ch = $info['handle'];
291+
$waitFor = @curl_getinfo($ch, \CURLINFO_PRIVATE) ?: '_0';
288292

289-
while ($info = curl_multi_info_read($mh)) {
290-
if (\CURLMSG_DONE !== $info['msg']) {
291-
continue;
292-
}
293-
$result = $info['result'];
294-
$id = (int) $ch = $info['handle'];
295-
$waitFor = @curl_getinfo($ch, \CURLINFO_PRIVATE) ?: '_0';
296-
297-
if (\in_array($result, [\CURLE_SEND_ERROR, \CURLE_RECV_ERROR, /*CURLE_HTTP2*/ 16, /*CURLE_HTTP2_STREAM*/ 92], true) && $waitFor[1] && 'C' !== $waitFor[0]) {
298-
curl_multi_remove_handle($mh, $ch);
299-
$waitFor[1] = (string) ((int) $waitFor[1] - 1); // decrement the retry counter
300-
curl_setopt($ch, \CURLOPT_PRIVATE, $waitFor);
301-
curl_setopt($ch, \CURLOPT_FORBID_REUSE, true);
302-
303-
if (0 === curl_multi_add_handle($mh, $ch)) {
304-
continue;
305-
}
306-
}
293+
if (\in_array($result, [\CURLE_SEND_ERROR, \CURLE_RECV_ERROR, /*CURLE_HTTP2*/ 16, /*CURLE_HTTP2_STREAM*/ 92], true) && $waitFor[1] && 'C' !== $waitFor[0]) {
294+
curl_multi_remove_handle($multi->handle, $ch);
295+
$waitFor[1] = (string) ((int) $waitFor[1] - 1); // decrement the retry counter
296+
curl_setopt($ch, \CURLOPT_PRIVATE, $waitFor);
297+
curl_setopt($ch, \CURLOPT_FORBID_REUSE, true);
307298

308-
if (\CURLE_RECV_ERROR === $result && 'H' === $waitFor[0] && 400 <= ($responses[(int) $ch]->info['http_code'] ?? 0)) {
309-
$multi->handlesActivity[$id][] = new FirstChunk();
299+
if (0 === curl_multi_add_handle($multi->handle, $ch)) {
300+
continue;
310301
}
311-
312-
$multi->handlesActivity[$id][] = null;
313-
$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)));
314302
}
315303

316-
if (!$active && 0 < $i) {
317-
curl_multi_close($mh);
318-
unset($multi->handles[$i]);
304+
if (\CURLE_RECV_ERROR === $result && 'H' === $waitFor[0] && 400 <= ($responses[(int) $ch]->info['http_code'] ?? 0)) {
305+
$multi->handlesActivity[$id][] = new FirstChunk();
319306
}
307+
308+
$multi->handlesActivity[$id][] = null;
309+
$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)));
320310
}
321311
} finally {
322312
self::$performing = false;
@@ -335,7 +325,7 @@ private static function select(ClientState $multi, float $timeout): int
335325
$timeout = min($timeout, 0.01);
336326
}
337327

338-
return curl_multi_select($multi->handles[array_key_last($multi->handles)], $timeout);
328+
return curl_multi_select($multi->handle, $timeout);
339329
}
340330

341331
/**

‎src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ public function testHandleIsReinitOnReset()
143143
$r = new \ReflectionProperty($httpClient, 'multi');
144144
$r->setAccessible(true);
145145
$clientState = $r->getValue($httpClient);
146-
$initialHandleId = (int) $clientState->handles[0];
146+
$initialShareId = $clientState->share;
147147
$httpClient->reset();
148-
self::assertNotSame($initialHandleId, (int) $clientState->handles[0]);
148+
self::assertNotSame($initialShareId, $clientState->share);
149149
}
150150

151151
public function testProcessAfterReset()

0 commit comments

Comments
 (0)
0