From 019bce7230082fff265bf7d6a437d58fda394346 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 3 Sep 2019 14:17:24 +0200 Subject: [PATCH] [HttpClient] improve handling of HTTP/2 PUSH --- .../Component/HttpClient/CurlHttpClient.php | 67 +++++++++++-------- .../HttpClient/Internal/PushedResponse.php | 12 ++-- .../HttpClient/Response/CurlResponse.php | 5 +- .../HttpClient/Tests/CurlHttpClientTest.php | 21 +++--- 4 files changed, 55 insertions(+), 50 deletions(-) diff --git a/src/Symfony/Component/HttpClient/CurlHttpClient.php b/src/Symfony/Component/HttpClient/CurlHttpClient.php index 93c57ca9f2..0173c4fd62 100644 --- a/src/Symfony/Component/HttpClient/CurlHttpClient.php +++ b/src/Symfony/Component/HttpClient/CurlHttpClient.php @@ -55,7 +55,7 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface * * @see HttpClientInterface::OPTIONS_DEFAULTS for available options */ - public function __construct(array $defaultOptions = [], int $maxHostConnections = 6, int $maxPendingPushes = 50) + public function __construct(array $defaultOptions = [], int $maxHostConnections = 6, int $maxPendingPushes = 0) { if (!\extension_loaded('curl')) { throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\CurlHttpClient" as the "curl" extension is not installed.'); @@ -105,20 +105,16 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface $host = parse_url($authority, PHP_URL_HOST); $url = implode('', $url); + if (!isset($options['normalized_headers']['user-agent'])) { + $options['normalized_headers']['user-agent'][] = 'Symfony HttpClient/Curl'; + $options['headers'][] = 'User-Agent: Symfony HttpClient/Curl'; + } + 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 = ['authorization', 'cookie', 'x-requested-with', 'range']; - foreach ($expectedHeaders as $k => $v) { - $expectedHeaders[$k] = null; - foreach ($options['normalized_headers'][$v] ?? [] as $h) { - $expectedHeaders[$k][] = substr($h, 2 + \strlen($v)); - } - } - - if ('GET' === $method && $expectedHeaders === $pushedResponse->headers && !$options['body']) { - $this->logger && $this->logger->debug(sprintf('Connecting request to pushed response: "%s %s"', $method, $url)); + if (self::acceptPushForRequest($method, $options, $pushedResponse)) { + $this->logger && $this->logger->debug(sprintf('Accepting pushed response: "%s %s"', $method, $url)); // Reinitialize the pushed response with request's options $pushedResponse->response->__construct($this->multi, $url, $options, $this->logger); @@ -126,14 +122,13 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface return $pushedResponse->response; } - $this->logger && $this->logger->debug(sprintf('Rejecting pushed response for "%s": authorization headers don\'t match the request', $url)); + $this->logger && $this->logger->debug(sprintf('Rejecting pushed response: "%s".', $url)); } $this->logger && $this->logger->info(sprintf('Request: "%s %s"', $method, $url)); $curlopts = [ CURLOPT_URL => $url, - CURLOPT_USERAGENT => 'Symfony HttpClient/Curl', CURLOPT_TCP_NODELAY => true, CURLOPT_PROTOCOLS => CURLPROTO_HTTP | CURLPROTO_HTTPS, CURLOPT_REDIR_PROTOCOLS => CURLPROTO_HTTP | CURLPROTO_HTTPS, @@ -306,7 +301,7 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface $active = 0; while (CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)); - foreach ($this->multi->openHandles as $ch) { + foreach ($this->multi->openHandles as [$ch]) { curl_setopt($ch, CURLOPT_VERBOSE, false); } } @@ -318,17 +313,17 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface foreach ($requestHeaders as $h) { if (false !== $i = strpos($h, ':', 1)) { - $headers[substr($h, 0, $i)] = substr($h, 1 + $i); + $headers[substr($h, 0, $i)][] = substr($h, 1 + $i); } } - if (!isset($headers[':method']) || !isset($headers[':scheme']) || !isset($headers[':authority']) || !isset($headers[':path']) || 'GET' !== $headers[':method'] || isset($headers['range'])) { + if (!isset($headers[':method']) || !isset($headers[':scheme']) || !isset($headers[':authority']) || !isset($headers[':path'])) { $logger && $logger->debug(sprintf('Rejecting pushed response from "%s": pushed headers are invalid', $origin)); return CURL_PUSH_DENY; } - $url = $headers[':scheme'].'://'.$headers[':authority']; + $url = $headers[':scheme'][0].'://'.$headers[':authority'][0]; if ($maxPendingPushes <= \count($multi->pushedResponses)) { $logger && $logger->debug(sprintf('Rejecting pushed response from "%s" for "%s": the queue is full', $origin, $url)); @@ -345,22 +340,38 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface return CURL_PUSH_DENY; } - $url .= $headers[':path']; + $url .= $headers[':path'][0]; $logger && $logger->debug(sprintf('Queueing pushed response: "%s"', $url)); - $multi->pushedResponses[$url] = new PushedResponse( - new CurlResponse($multi, $pushed), - [ - $headers['authorization'] ?? null, - $headers['cookie'] ?? null, - $headers['x-requested-with'] ?? null, - null, - ] - ); + $multi->pushedResponses[$url] = new PushedResponse(new CurlResponse($multi, $pushed), $headers, $multi->openHandles[(int) $parent][1] ?? []); return CURL_PUSH_OK; } + /** + * Accepts pushed responses only if their headers related to authentication match the request. + */ + private static function acceptPushForRequest(string $method, array $options, PushedResponse $pushedResponse): bool + { + if ($options['body'] || $method !== $pushedResponse->requestHeaders[':method'][0]) { + return false; + } + + foreach (['proxy', 'no_proxy', 'bindto'] as $k) { + if ($options[$k] !== $pushedResponse->parentOptions[$k]) { + return false; + } + } + + foreach (['authorization', 'cookie', 'range', 'proxy-authorization'] as $k) { + if (($pushedResponse->requestHeaders[$k] ?? null) !== ($options['normalized_headers'][$k] ?? null)) { + return false; + } + } + + return true; + } + /** * Wraps the request's body callback to allow it to return strings longer than curl requested. */ diff --git a/src/Symfony/Component/HttpClient/Internal/PushedResponse.php b/src/Symfony/Component/HttpClient/Internal/PushedResponse.php index 632f0c41d0..6f8e8fda3a 100644 --- a/src/Symfony/Component/HttpClient/Internal/PushedResponse.php +++ b/src/Symfony/Component/HttpClient/Internal/PushedResponse.php @@ -14,7 +14,7 @@ namespace Symfony\Component\HttpClient\Internal; use Symfony\Component\HttpClient\Response\CurlResponse; /** - * A pushed response with headers. + * A pushed response with its request headers. * * @author Alexander M. Turek * @@ -22,15 +22,17 @@ use Symfony\Component\HttpClient\Response\CurlResponse; */ final class PushedResponse { - /** @var CurlResponse */ public $response; /** @var string[] */ - public $headers; + public $requestHeaders; - public function __construct(CurlResponse $response, array $headers) + public $parentOptions = []; + + public function __construct(CurlResponse $response, array $requestHeaders, array $parentOptions) { $this->response = $response; - $this->headers = $headers; + $this->requestHeaders = $requestHeaders; + $this->parentOptions = $parentOptions; } } diff --git a/src/Symfony/Component/HttpClient/Response/CurlResponse.php b/src/Symfony/Component/HttpClient/Response/CurlResponse.php index a064361763..b9f245a34a 100644 --- a/src/Symfony/Component/HttpClient/Response/CurlResponse.php +++ b/src/Symfony/Component/HttpClient/Response/CurlResponse.php @@ -120,9 +120,6 @@ final class CurlResponse implements ResponseInterface if (\in_array($waitFor, ['headers', 'destruct'], true)) { try { - if (\defined('CURLOPT_STREAM_WEIGHT')) { - curl_setopt($ch, CURLOPT_STREAM_WEIGHT, 32); - } self::stream([$response])->current(); } catch (\Throwable $e) { // Persist timeouts thrown during initialization @@ -140,7 +137,7 @@ final class CurlResponse implements ResponseInterface }; // Schedule the request in a non-blocking way - $multi->openHandles[$id] = $ch; + $multi->openHandles[$id] = [$ch, $options]; curl_multi_add_handle($multi->handle, $ch); self::perform($multi); } diff --git a/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php index 2c27bb7b3d..604a37f37a 100644 --- a/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php +++ b/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php @@ -47,27 +47,22 @@ class CurlHttpClientTest extends HttpClientTestCase } }; - $client = new CurlHttpClient(); + $client = new CurlHttpClient([], 6, 2); $client->setLogger($logger); - $index = $client->request('GET', 'https://http2-push.io'); + $index = $client->request('GET', 'https://http2.akamai.com/'); $index->getContent(); - $css = $client->request('GET', 'https://http2-push.io/css/style.css'); - $js = $client->request('GET', 'https://http2-push.io/js/http2-push.js'); + $css = $client->request('GET', 'https://http2.akamai.com/resources/push.css'); $css->getHeaders(); - $js->getHeaders(); $expected = [ - 'Request: "GET https://http2-push.io/"', - 'Queueing pushed response: "https://http2-push.io/css/style.css"', - 'Queueing pushed response: "https://http2-push.io/js/http2-push.js"', - 'Response: "200 https://http2-push.io/"', - 'Connecting request to pushed response: "GET https://http2-push.io/css/style.css"', - 'Connecting request to pushed response: "GET https://http2-push.io/js/http2-push.js"', - 'Response: "200 https://http2-push.io/css/style.css"', - 'Response: "200 https://http2-push.io/js/http2-push.js"', + 'Request: "GET https://http2.akamai.com/"', + 'Queueing pushed response: "https://http2.akamai.com/resources/push.css"', + 'Response: "200 https://http2.akamai.com/"', + 'Accepting pushed response: "GET https://http2.akamai.com/resources/push.css"', + 'Response: "200 https://http2.akamai.com/resources/push.css"', ]; $this->assertSame($expected, $logger->logs); }