Skip to content

Commit 9edd84b

Browse files
fabpotfabpot
committed
bug symfony#30967 [HttpClient] Document the state object that is passed around by the HttpClient (derrabus)
This PR was merged into the 4.3-dev branch. Discussion ---------- [HttpClient] Document the state object that is passed around by the HttpClient | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A In an attempt to make the code of the new HttpClient component more understandable, I've introduced internal classes that document the `$multi` object that is being passed around between *Client and *Response classes. My goal is to make the code more accessible to potential contributors and static code analyzers. Commits ------- 20f4eb3 Document the state object that is passed around by the HttpClient.
2 parents 936355e + 20f4eb3 commit 9edd84b

File tree

11 files changed

+240
-64
lines changed

11 files changed

+240
-64
lines changed

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

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
use Psr\Log\LoggerAwareTrait;
1616
use Psr\Log\LoggerInterface;
1717
use Symfony\Component\HttpClient\Exception\TransportException;
18+
use Symfony\Component\HttpClient\Internal\CurlClientState;
19+
use Symfony\Component\HttpClient\Internal\PushedResponse;
1820
use Symfony\Component\HttpClient\Response\CurlResponse;
1921
use Symfony\Component\HttpClient\Response\ResponseStream;
2022
use Symfony\Contracts\HttpClient\HttpClientInterface;
@@ -37,6 +39,12 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
3739
use LoggerAwareTrait;
3840

3941
private $defaultOptions = self::OPTIONS_DEFAULTS;
42+
43+
/**
44+
* An internal object to share state between the client and its responses.
45+
*
46+
* @var CurlClientState
47+
*/
4048
private $multi;
4149

4250
/**
@@ -56,22 +64,13 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
5664
[, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, self::OPTIONS_DEFAULTS);
5765
}
5866

59-
$mh = curl_multi_init();
67+
$this->multi = $multi = new CurlClientState();
6068

6169
// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
6270
if (\defined('CURLPIPE_MULTIPLEX')) {
63-
curl_multi_setopt($mh, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
71+
curl_multi_setopt($this->multi->handle, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
6472
}
65-
curl_multi_setopt($mh, CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : PHP_INT_MAX);
66-
67-
// Use an internal stdClass object to share state between the client and its responses
68-
$this->multi = $multi = (object) [
69-
'openHandles' => [],
70-
'handlesActivity' => [],
71-
'handle' => $mh,
72-
'pushedResponses' => [],
73-
'dnsCache' => [[], [], []],
74-
];
73+
curl_multi_setopt($this->multi->handle, CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : PHP_INT_MAX);
7574

7675
// Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/bug.php?id=77535
7776
if (0 >= $maxPendingPushes || \PHP_VERSION_ID < 70217 || (\PHP_VERSION_ID >= 70300 && \PHP_VERSION_ID < 70304)) {
@@ -85,7 +84,7 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
8584

8685
$logger = &$this->logger;
8786

88-
curl_multi_setopt($mh, CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes, &$logger) {
87+
curl_multi_setopt($this->multi->handle, CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes, &$logger) {
8988
return self::handlePush($parent, $pushed, $requestHeaders, $multi, $maxPendingPushes, $logger);
9089
});
9190
}
@@ -103,7 +102,7 @@ public function request(string $method, string $url, array $options = []): Respo
103102
$host = parse_url($authority, PHP_URL_HOST);
104103
$url = implode('', $url);
105104

106-
if ([$pushedResponse, $pushedHeaders] = $this->multi->pushedResponses[$url] ?? null) {
105+
if ($pushedResponse = $this->multi->pushedResponses[$url] ?? null) {
107106
unset($this->multi->pushedResponses[$url]);
108107
// Accept pushed responses only if their headers related to authentication match the request
109108
$expectedHeaders = [
@@ -113,13 +112,13 @@ public function request(string $method, string $url, array $options = []): Respo
113112
$options['headers']['range'] ?? null,
114113
];
115114

116-
if ('GET' === $method && $expectedHeaders === $pushedHeaders && !$options['body']) {
115+
if ('GET' === $method && $expectedHeaders === $pushedResponse->headers && !$options['body']) {
117116
$this->logger && $this->logger->debug(sprintf('Connecting request to pushed response: "%s %s"', $method, $url));
118117

119118
// Reinitialize the pushed response with request's options
120-
$pushedResponse->__construct($this->multi, $url, $options, $this->logger);
119+
$pushedResponse->response->__construct($this->multi, $url, $options, $this->logger);
121120

122-
return $pushedResponse;
121+
return $pushedResponse->response;
123122
}
124123

125124
$this->logger && $this->logger->debug(sprintf('Rejecting pushed response for "%s": authorization headers don\'t match the request', $url));
@@ -159,14 +158,14 @@ public function request(string $method, string $url, array $options = []): Respo
159158
}
160159

161160
// curl's resolve feature varies by host:port but ours varies by host only, let's handle this with our own DNS map
162-
if (isset($this->multi->dnsCache[0][$host])) {
163-
$options['resolve'] += [$host => $this->multi->dnsCache[0][$host]];
161+
if (isset($this->multi->dnsCache->hostnames[$host])) {
162+
$options['resolve'] += [$host => $this->multi->dnsCache->hostnames[$host]];
164163
}
165164

166-
if ($options['resolve'] || $this->multi->dnsCache[2]) {
165+
if ($options['resolve'] || $this->multi->dnsCache->evictions) {
167166
// First reset any old DNS cache entries then add the new ones
168-
$resolve = $this->multi->dnsCache[2];
169-
$this->multi->dnsCache[2] = [];
167+
$resolve = $this->multi->dnsCache->evictions;
168+
$this->multi->dnsCache->evictions = [];
170169
$port = parse_url($authority, PHP_URL_PORT) ?: ('http:' === $scheme ? 80 : 443);
171170

172171
if ($resolve && 0x072a00 > curl_version()['version_number']) {
@@ -178,8 +177,8 @@ public function request(string $method, string $url, array $options = []): Respo
178177

179178
foreach ($options['resolve'] as $host => $ip) {
180179
$resolve[] = null === $ip ? "-$host:$port" : "$host:$port:$ip";
181-
$this->multi->dnsCache[0][$host] = $ip;
182-
$this->multi->dnsCache[1]["-$host:$port"] = "-$host:$port";
180+
$this->multi->dnsCache->hostnames[$host] = $ip;
181+
$this->multi->dnsCache->removals["-$host:$port"] = "-$host:$port";
183182
}
184183

185184
$curlopts[CURLOPT_RESOLVE] = $resolve;
@@ -299,7 +298,7 @@ public function __destruct()
299298
}
300299
}
301300

302-
private static function handlePush($parent, $pushed, array $requestHeaders, \stdClass $multi, int $maxPendingPushes, ?LoggerInterface $logger): int
301+
private static function handlePush($parent, $pushed, array $requestHeaders, CurlClientState $multi, int $maxPendingPushes, ?LoggerInterface $logger): int
303302
{
304303
$headers = [];
305304
$origin = curl_getinfo($parent, CURLINFO_EFFECTIVE_URL);
@@ -336,15 +335,15 @@ private static function handlePush($parent, $pushed, array $requestHeaders, \std
336335
$url .= $headers[':path'];
337336
$logger && $logger->debug(sprintf('Queueing pushed response: "%s"', $url));
338337

339-
$multi->pushedResponses[$url] = [
338+
$multi->pushedResponses[$url] = new PushedResponse(
340339
new CurlResponse($multi, $pushed),
341340
[
342341
$headers['authorization'] ?? null,
343342
$headers['cookie'] ?? null,
344343
$headers['x-requested-with'] ?? null,
345344
null,
346-
],
347-
];
345+
]
346+
);
348347

349348
return CURL_PUSH_OK;
350349
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpClient\Internal;
13+
14+
/**
15+
* Internal representation of the client state.
16+
*
17+
* @author Alexander M. Turek <me@derrabus.de>
18+
*
19+
* @internal
20+
*/
21+
class ClientState
22+
{
23+
public $handlesActivity = [];
24+
public $openHandles = [];
25+
}

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpClient\Internal;
13+
14+
/**
15+
* Internal representation of the cURL client's state.
16+
*
17+
* @author Alexander M. Turek <me@derrabus.de>
18+
*
19+
* @internal
20+
*/
21+
final class CurlClientState extends ClientState
22+
{
23+
/** @var resource */
24+
public $handle;
25+
/** @var PushedResponse[] */
26+
public $pushedResponses = [];
27+
/** @var DnsCache */
28+
public $dnsCache;
29+
30+
public function __construct()
31+
{
32+
$this->handle = curl_multi_init();
33+
$this->dnsCache = new DnsCache();
34+
}
35+
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpClient\Internal;
13+
14+
/**
15+
* Cache for resolved DNS queries.
16+
*
17+
* @author Alexander M. Turek <me@derrabus.de>
18+
*
19+
* @internal
20+
*/
21+
final class DnsCache
22+
{
23+
/**
24+
* Resolved hostnames (hostname => IP address).
25+
*
26+
* @var string[]
27+
*/
28+
public $hostnames = [];
29+
30+
/**
31+
* @var string[]
32+
*/
33+
public $removals = [];
34+
35+
/**
36+
* @var string[]
37+
*/
38+
public $evictions = [];
39+
}

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpClient\Internal;
13+
14+
use Symfony\Component\HttpClient\Response\NativeResponse;
15+
16+
/**
17+
* Internal representation of the native client's state.
18+
*
19+
* @author Alexander M. Turek <me@derrabus.de>
20+
*
21+
* @internal
22+
*/
23+
final class NativeClientState extends ClientState
24+
{
25+
/** @var int */
26+
public $id;
27+
/** @var NativeResponse[] */
28+
public $pendingResponses = [];
29+
/** @var int */
30+
public $maxHostConnections = PHP_INT_MAX;
31+
/** @var int */
32+
public $responseCount = 0;
33+
/** @var string[] */
34+
public $dnsCache = [];
35+
/** @var resource[] */
36+
public $handles = [];
37+
/** @var bool */
38+
public $sleep = false;
39+
40+
public function __construct()
41+
{
42+
$this->id = random_int(PHP_INT_MIN, PHP_INT_MAX);
43+
}
44+
}

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpClient\Internal;
13+
14+
use Symfony\Component\HttpClient\Response\CurlResponse;
15+
16+
/**
17+
* A pushed response with headers.
18+
*
19+
* @author Alexander M. Turek <me@derrabus.de>
20+
*
21+
* @internal
22+
*/
23+
final class PushedResponse
24+
{
25+
/** @var CurlResponse */
26+
public $response;
27+
28+
/** @var string[] */
29+
public $headers;
30+
31+
public function __construct(CurlResponse $response, array $headers)
32+
{
33+
$this->response = $response;
34+
$this->headers = $headers;
35+
}
36+
}

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

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Psr\Log\LoggerAwareInterface;
1515
use Psr\Log\LoggerAwareTrait;
1616
use Symfony\Component\HttpClient\Exception\TransportException;
17+
use Symfony\Component\HttpClient\Internal\NativeClientState;
1718
use Symfony\Component\HttpClient\Response\NativeResponse;
1819
use Symfony\Component\HttpClient\Response\ResponseStream;
1920
use Symfony\Contracts\HttpClient\HttpClientInterface;
@@ -36,6 +37,8 @@ final class NativeHttpClient implements HttpClientInterface, LoggerAwareInterfac
3637
use LoggerAwareTrait;
3738

3839
private $defaultOptions = self::OPTIONS_DEFAULTS;
40+
41+
/** @var NativeClientState */
3942
private $multi;
4043

4144
/**
@@ -50,18 +53,8 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
5053
[, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, self::OPTIONS_DEFAULTS);
5154
}
5255

53-
// Use an internal stdClass object to share state between the client and its responses
54-
$this->multi = (object) [
55-
'openHandles' => [],
56-
'handlesActivity' => [],
57-
'pendingResponses' => [],
58-
'maxHostConnections' => 0 < $maxHostConnections ? $maxHostConnections : PHP_INT_MAX,
59-
'responseCount' => 0,
60-
'dnsCache' => [],
61-
'handles' => [],
62-
'sleep' => false,
63-
'id' => random_int(PHP_INT_MIN, PHP_INT_MAX),
64-
];
56+
$this->multi = new NativeClientState();
57+
$this->multi->maxHostConnections = 0 < $maxHostConnections ? $maxHostConnections : PHP_INT_MAX;
6558
}
6659

6760
/**
@@ -291,7 +284,7 @@ private static function getProxy(?string $proxy, array $url): ?array
291284
/**
292285
* Resolves the IP of the host using the local DNS cache if possible.
293286
*/
294-
private static function dnsResolve(array $url, \stdClass $multi, array &$info, ?\Closure $onProgress): array
287+
private static function dnsResolve(array $url, NativeClientState $multi, array &$info, ?\Closure $onProgress): array
295288
{
296289
if ($port = parse_url($url['authority'], PHP_URL_PORT) ?: '') {
297290
$info['primary_port'] = $port;
@@ -342,7 +335,7 @@ private static function createRedirectResolver(array $options, string $host, ?ar
342335
}
343336
}
344337

345-
return static function (\stdClass $multi, ?string $location, $context) use ($redirectHeaders, $proxy, $noProxy, &$info, $maxRedirects, $onProgress): ?string {
338+
return static function (NativeClientState $multi, ?string $location, $context) use ($redirectHeaders, $proxy, $noProxy, &$info, $maxRedirects, $onProgress): ?string {
346339
if (null === $location || $info['http_code'] < 300 || 400 <= $info['http_code']) {
347340
$info['redirect_url'] = null;
348341

0 commit comments

Comments
 (0)