Skip to content

Commit

Permalink
bug #30967 [HttpClient] Document the state object that is passed arou…
Browse files Browse the repository at this point in the history
…nd 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.
  • Loading branch information
fabpot committed Apr 12, 2019
2 parents 936355e + 20f4eb3 commit 9edd84b
Show file tree
Hide file tree
Showing 11 changed files with 240 additions and 64 deletions.
55 changes: 27 additions & 28 deletions src/Symfony/Component/HttpClient/CurlHttpClient.php
Expand Up @@ -15,6 +15,8 @@
use Psr\Log\LoggerAwareTrait;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\Internal\CurlClientState;
use Symfony\Component\HttpClient\Internal\PushedResponse;
use Symfony\Component\HttpClient\Response\CurlResponse;
use Symfony\Component\HttpClient\Response\ResponseStream;
use Symfony\Contracts\HttpClient\HttpClientInterface;
Expand All @@ -37,6 +39,12 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
use LoggerAwareTrait;

private $defaultOptions = self::OPTIONS_DEFAULTS;

/**
* An internal object to share state between the client and its responses.
*
* @var CurlClientState
*/
private $multi;

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

$mh = curl_multi_init();
$this->multi = $multi = new CurlClientState();

// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
if (\defined('CURLPIPE_MULTIPLEX')) {
curl_multi_setopt($mh, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
curl_multi_setopt($this->multi->handle, CURLMOPT_PIPELINING, CURLPIPE_MULTIPLEX);
}
curl_multi_setopt($mh, CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : PHP_INT_MAX);

// Use an internal stdClass object to share state between the client and its responses
$this->multi = $multi = (object) [
'openHandles' => [],
'handlesActivity' => [],
'handle' => $mh,
'pushedResponses' => [],
'dnsCache' => [[], [], []],
];
curl_multi_setopt($this->multi->handle, CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : PHP_INT_MAX);

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

$logger = &$this->logger;

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

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

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

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

return $pushedResponse;
return $pushedResponse->response;
}

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

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

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

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

foreach ($options['resolve'] as $host => $ip) {
$resolve[] = null === $ip ? "-$host:$port" : "$host:$port:$ip";
$this->multi->dnsCache[0][$host] = $ip;
$this->multi->dnsCache[1]["-$host:$port"] = "-$host:$port";
$this->multi->dnsCache->hostnames[$host] = $ip;
$this->multi->dnsCache->removals["-$host:$port"] = "-$host:$port";
}

$curlopts[CURLOPT_RESOLVE] = $resolve;
Expand Down Expand Up @@ -299,7 +298,7 @@ public function __destruct()
}
}

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

$multi->pushedResponses[$url] = [
$multi->pushedResponses[$url] = new PushedResponse(
new CurlResponse($multi, $pushed),
[
$headers['authorization'] ?? null,
$headers['cookie'] ?? null,
$headers['x-requested-with'] ?? null,
null,
],
];
]
);

return CURL_PUSH_OK;
}
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/HttpClient/Internal/ClientState.php
@@ -0,0 +1,25 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpClient\Internal;

/**
* Internal representation of the client state.
*
* @author Alexander M. Turek <me@derrabus.de>
*
* @internal
*/
class ClientState
{
public $handlesActivity = [];
public $openHandles = [];
}
35 changes: 35 additions & 0 deletions src/Symfony/Component/HttpClient/Internal/CurlClientState.php
@@ -0,0 +1,35 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpClient\Internal;

/**
* Internal representation of the cURL client's state.
*
* @author Alexander M. Turek <me@derrabus.de>
*
* @internal
*/
final class CurlClientState extends ClientState
{
/** @var resource */
public $handle;
/** @var PushedResponse[] */
public $pushedResponses = [];
/** @var DnsCache */
public $dnsCache;

public function __construct()
{
$this->handle = curl_multi_init();
$this->dnsCache = new DnsCache();
}
}
39 changes: 39 additions & 0 deletions src/Symfony/Component/HttpClient/Internal/DnsCache.php
@@ -0,0 +1,39 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpClient\Internal;

/**
* Cache for resolved DNS queries.
*
* @author Alexander M. Turek <me@derrabus.de>
*
* @internal
*/
final class DnsCache
{
/**
* Resolved hostnames (hostname => IP address).
*
* @var string[]
*/
public $hostnames = [];

/**
* @var string[]
*/
public $removals = [];

/**
* @var string[]
*/
public $evictions = [];
}
44 changes: 44 additions & 0 deletions src/Symfony/Component/HttpClient/Internal/NativeClientState.php
@@ -0,0 +1,44 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpClient\Internal;

use Symfony\Component\HttpClient\Response\NativeResponse;

/**
* Internal representation of the native client's state.
*
* @author Alexander M. Turek <me@derrabus.de>
*
* @internal
*/
final class NativeClientState extends ClientState
{
/** @var int */
public $id;
/** @var NativeResponse[] */
public $pendingResponses = [];
/** @var int */
public $maxHostConnections = PHP_INT_MAX;
/** @var int */
public $responseCount = 0;
/** @var string[] */
public $dnsCache = [];
/** @var resource[] */
public $handles = [];
/** @var bool */
public $sleep = false;

public function __construct()
{
$this->id = random_int(PHP_INT_MIN, PHP_INT_MAX);
}
}
36 changes: 36 additions & 0 deletions src/Symfony/Component/HttpClient/Internal/PushedResponse.php
@@ -0,0 +1,36 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpClient\Internal;

use Symfony\Component\HttpClient\Response\CurlResponse;

/**
* A pushed response with headers.
*
* @author Alexander M. Turek <me@derrabus.de>
*
* @internal
*/
final class PushedResponse
{
/** @var CurlResponse */
public $response;

/** @var string[] */
public $headers;

public function __construct(CurlResponse $response, array $headers)
{
$this->response = $response;
$this->headers = $headers;
}
}
21 changes: 7 additions & 14 deletions src/Symfony/Component/HttpClient/NativeHttpClient.php
Expand Up @@ -14,6 +14,7 @@
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\Internal\NativeClientState;
use Symfony\Component\HttpClient\Response\NativeResponse;
use Symfony\Component\HttpClient\Response\ResponseStream;
use Symfony\Contracts\HttpClient\HttpClientInterface;
Expand All @@ -36,6 +37,8 @@ final class NativeHttpClient implements HttpClientInterface, LoggerAwareInterfac
use LoggerAwareTrait;

private $defaultOptions = self::OPTIONS_DEFAULTS;

/** @var NativeClientState */
private $multi;

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

// Use an internal stdClass object to share state between the client and its responses
$this->multi = (object) [
'openHandles' => [],
'handlesActivity' => [],
'pendingResponses' => [],
'maxHostConnections' => 0 < $maxHostConnections ? $maxHostConnections : PHP_INT_MAX,
'responseCount' => 0,
'dnsCache' => [],
'handles' => [],
'sleep' => false,
'id' => random_int(PHP_INT_MIN, PHP_INT_MAX),
];
$this->multi = new NativeClientState();
$this->multi->maxHostConnections = 0 < $maxHostConnections ? $maxHostConnections : PHP_INT_MAX;
}

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

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

Expand Down

0 comments on commit 9edd84b

Please sign in to comment.