From fc6ba7efad0e5dbe34eab3a4d893090e0a460004 Mon Sep 17 00:00:00 2001 From: Anton Chernikov Date: Tue, 12 Mar 2019 19:10:19 +0300 Subject: [PATCH 1/2] [HttpClient] logger integration --- .../Component/HttpClient/CurlHttpClient.php | 11 +++++++++-- src/Symfony/Component/HttpClient/HttpClient.php | 16 +++++++++++++--- .../Component/HttpClient/NativeHttpClient.php | 8 +++++++- src/Symfony/Component/HttpClient/composer.json | 3 ++- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/HttpClient/CurlHttpClient.php b/src/Symfony/Component/HttpClient/CurlHttpClient.php index 39a5fde78f38f..e863febe2c782 100644 --- a/src/Symfony/Component/HttpClient/CurlHttpClient.php +++ b/src/Symfony/Component/HttpClient/CurlHttpClient.php @@ -11,6 +11,8 @@ namespace Symfony\Component\HttpClient; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Symfony\Component\HttpClient\Exception\TransportException; use Symfony\Component\HttpClient\Response\CurlResponse; use Symfony\Component\HttpClient\Response\ResponseStream; @@ -34,6 +36,7 @@ final class CurlHttpClient implements HttpClientInterface private $defaultOptions = self::OPTIONS_DEFAULTS; private $multi; + private $logger; /** * @param array $defaultOptions Default requests' options @@ -41,8 +44,10 @@ final class CurlHttpClient implements HttpClientInterface * * @see HttpClientInterface::OPTIONS_DEFAULTS for available options */ - public function __construct(array $defaultOptions = [], int $maxHostConnections = 6) + public function __construct(array $defaultOptions = [], LoggerInterface $logger = null, int $maxHostConnections = 6) { + $this->logger = $logger ?? new NullLogger(); + if ($defaultOptions) { [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, self::OPTIONS_DEFAULTS); } @@ -86,6 +91,7 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections */ public function request(string $method, string $url, array $options = []): ResponseInterface { + $this->logger->notice('Making a request', ['url' => $url, 'method' => $method, 'client' => static::class]); [$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions); $scheme = $url['scheme']; $authority = $url['authority']; @@ -103,6 +109,7 @@ public function request(string $method, string $url, array $options = []): Respo ]; if ('GET' === $method && !$options['body'] && $expectedHeaders === $pushedHeaders) { + $this->logger->debug('Creating pushed response'); // Reinitialize the pushed response with request's options $pushedResponse->__construct($this->multi, $url, $options); @@ -156,7 +163,7 @@ public function request(string $method, string $url, array $options = []): Respo // DNS cache removals require curl 7.42 or higher // On lower versions, we have to create a new multi handle curl_multi_close($this->multi->handle); - $this->multi->handle = (new self())->multi->handle; + $this->multi->handle = (new self([], $this->logger))->multi->handle; } foreach ($options['resolve'] as $host => $ip) { diff --git a/src/Symfony/Component/HttpClient/HttpClient.php b/src/Symfony/Component/HttpClient/HttpClient.php index 6d680d9b49837..3e44ca0d67913 100644 --- a/src/Symfony/Component/HttpClient/HttpClient.php +++ b/src/Symfony/Component/HttpClient/HttpClient.php @@ -11,6 +11,8 @@ namespace Symfony\Component\HttpClient; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Symfony\Contracts\HttpClient\HttpClientInterface; /** @@ -28,12 +30,20 @@ final class HttpClient * * @see HttpClientInterface::OPTIONS_DEFAULTS for available options */ - public static function create(array $defaultOptions = [], int $maxHostConnections = 6): HttpClientInterface + public static function create(array $defaultOptions = [], LoggerInterface $logger = null, int $maxHostConnections = 6): HttpClientInterface { + if (null === $logger) { + $logger = new NullLogger(); + } + if (\extension_loaded('curl')) { - return new CurlHttpClient($defaultOptions, $maxHostConnections); + $logger->debug('Curl extension is enabled. Creating client.', ['client' => CurlHttpClient::class]); + + return new CurlHttpClient($defaultOptions, $logger, $maxHostConnections); } - return new NativeHttpClient($defaultOptions, $maxHostConnections); + $logger->debug('Curl extension is disabled. Creating client.', ['client' => NativeHttpClient::class]); + + return new NativeHttpClient($defaultOptions, $logger, $maxHostConnections); } } diff --git a/src/Symfony/Component/HttpClient/NativeHttpClient.php b/src/Symfony/Component/HttpClient/NativeHttpClient.php index d120dbc83b7b7..ab8ba31a09be2 100644 --- a/src/Symfony/Component/HttpClient/NativeHttpClient.php +++ b/src/Symfony/Component/HttpClient/NativeHttpClient.php @@ -11,6 +11,8 @@ namespace Symfony\Component\HttpClient; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Symfony\Component\HttpClient\Exception\TransportException; use Symfony\Component\HttpClient\Response\NativeResponse; use Symfony\Component\HttpClient\Response\ResponseStream; @@ -34,6 +36,7 @@ final class NativeHttpClient implements HttpClientInterface private $defaultOptions = self::OPTIONS_DEFAULTS; private $multi; + private $logger; /** * @param array $defaultOptions Default requests' options @@ -41,8 +44,10 @@ final class NativeHttpClient implements HttpClientInterface * * @see HttpClientInterface::OPTIONS_DEFAULTS for available options */ - public function __construct(array $defaultOptions = [], int $maxHostConnections = 6) + public function __construct(array $defaultOptions = [], LoggerInterface $logger = null, int $maxHostConnections = 6) { + $this->logger = $logger ?? new NullLogger(); + if ($defaultOptions) { [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, self::OPTIONS_DEFAULTS); } @@ -68,6 +73,7 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections */ public function request(string $method, string $url, array $options = []): ResponseInterface { + $this->logger->notice('Making a request', ['url' => $url, 'method' => $method, 'client' => static::class]); [$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions); if ($options['bindto'] && file_exists($options['bindto'])) { diff --git a/src/Symfony/Component/HttpClient/composer.json b/src/Symfony/Component/HttpClient/composer.json index 803b424d3d339..b310f269c31b9 100644 --- a/src/Symfony/Component/HttpClient/composer.json +++ b/src/Symfony/Component/HttpClient/composer.json @@ -21,7 +21,8 @@ "require": { "php": "^7.1.3", "symfony/contracts": "^1.1", - "symfony/polyfill-php73": "^1.11" + "symfony/polyfill-php73": "^1.11", + "psr/log": "~1.0" }, "require-dev": { "nyholm/psr7": "^1.0", From 26d15c8bbe253e97bb029390a3c41ae975b271c2 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 4 Apr 2019 16:02:57 +0200 Subject: [PATCH 2/2] [HttpClient] log requests, responses and pushes when they happen --- .../Resources/config/http_client.xml | 4 ++ .../Component/HttpClient/CurlHttpClient.php | 56 +++++++++++++------ .../Component/HttpClient/HttpClient.php | 17 ++---- .../Component/HttpClient/NativeHttpClient.php | 18 +++--- .../HttpClient/Response/CurlResponse.php | 30 +++++++--- .../HttpClient/Response/NativeResponse.php | 14 ++++- .../HttpClient/Response/ResponseTrait.php | 4 ++ .../HttpClient/Tests/CurlHttpClientTest.php | 49 ++++++++++++++++ .../Component/HttpClient/composer.json | 4 +- .../Component/HttpKernel/HttpClientKernel.php | 10 +--- 10 files changed, 144 insertions(+), 62 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/http_client.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/http_client.xml index c21d115828fa5..bf7b7b9f64055 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/http_client.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/http_client.xml @@ -6,9 +6,13 @@ + + + + diff --git a/src/Symfony/Component/HttpClient/CurlHttpClient.php b/src/Symfony/Component/HttpClient/CurlHttpClient.php index e863febe2c782..f7cb71577f2a7 100644 --- a/src/Symfony/Component/HttpClient/CurlHttpClient.php +++ b/src/Symfony/Component/HttpClient/CurlHttpClient.php @@ -11,8 +11,9 @@ namespace Symfony\Component\HttpClient; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; use Symfony\Component\HttpClient\Exception\TransportException; use Symfony\Component\HttpClient\Response\CurlResponse; use Symfony\Component\HttpClient\Response\ResponseStream; @@ -30,24 +31,23 @@ * * @experimental in 4.3 */ -final class CurlHttpClient implements HttpClientInterface +final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface { use HttpClientTrait; + use LoggerAwareTrait; private $defaultOptions = self::OPTIONS_DEFAULTS; private $multi; - private $logger; /** * @param array $defaultOptions Default requests' options * @param int $maxHostConnections The maximum number of connections to a single host + * @param int $maxPendingPushes The maximum number of pushed responses to accept in the queue * * @see HttpClientInterface::OPTIONS_DEFAULTS for available options */ - public function __construct(array $defaultOptions = [], LoggerInterface $logger = null, int $maxHostConnections = 6) + public function __construct(array $defaultOptions = [], int $maxHostConnections = 6, int $maxPendingPushes = 50) { - $this->logger = $logger ?? new NullLogger(); - if ($defaultOptions) { [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, self::OPTIONS_DEFAULTS); } @@ -70,7 +70,7 @@ public function __construct(array $defaultOptions = [], LoggerInterface $logger ]; // Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/bug.php?id=77535 - if (\PHP_VERSION_ID < 70217 || (\PHP_VERSION_ID >= 70300 && \PHP_VERSION_ID < 70304)) { + if (0 >= $maxPendingPushes || \PHP_VERSION_ID < 70217 || (\PHP_VERSION_ID >= 70300 && \PHP_VERSION_ID < 70304)) { return; } @@ -79,8 +79,10 @@ public function __construct(array $defaultOptions = [], LoggerInterface $logger return; } - curl_multi_setopt($mh, CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi) { - return self::handlePush($parent, $pushed, $requestHeaders, $multi); + $logger = &$this->logger; + + curl_multi_setopt($mh, CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes, &$logger) { + return self::handlePush($parent, $pushed, $requestHeaders, $multi, $maxPendingPushes, $logger); }); } @@ -91,7 +93,6 @@ public function __construct(array $defaultOptions = [], LoggerInterface $logger */ public function request(string $method, string $url, array $options = []): ResponseInterface { - $this->logger->notice('Making a request', ['url' => $url, 'method' => $method, 'client' => static::class]); [$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions); $scheme = $url['scheme']; $authority = $url['authority']; @@ -109,14 +110,19 @@ public function request(string $method, string $url, array $options = []): Respo ]; if ('GET' === $method && !$options['body'] && $expectedHeaders === $pushedHeaders) { - $this->logger->debug('Creating pushed response'); + $this->logger && $this->logger->info(sprintf('Connecting request to pushed response: %s %s', $method, $url)); + // Reinitialize the pushed response with request's options - $pushedResponse->__construct($this->multi, $url, $options); + $pushedResponse->__construct($this->multi, $url, $options, $this->logger); return $pushedResponse; } + + $this->logger && $this->logger->info(sprintf('Rejecting pushed response for "%s": authorization headers don\'t match the request', $url)); } + $this->logger && $this->logger->info(sprintf('Request: %s %s', $method, $url)); + $curlopts = [ CURLOPT_URL => $url, CURLOPT_USERAGENT => 'Symfony HttpClient/Curl', @@ -163,7 +169,7 @@ public function request(string $method, string $url, array $options = []): Respo // DNS cache removals require curl 7.42 or higher // On lower versions, we have to create a new multi handle curl_multi_close($this->multi->handle); - $this->multi->handle = (new self([], $this->logger))->multi->handle; + $this->multi->handle = (new self())->multi->handle; } foreach ($options['resolve'] as $host => $ip) { @@ -262,7 +268,7 @@ public function request(string $method, string $url, array $options = []): Respo } } - return new CurlResponse($this->multi, $ch, $options, $method, self::createRedirectResolver($options, $host)); + return new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host)); } /** @@ -289,9 +295,10 @@ public function __destruct() } } - private static function handlePush($parent, $pushed, array $requestHeaders, \stdClass $multi): int + private static function handlePush($parent, $pushed, array $requestHeaders, \stdClass $multi, int $maxPendingPushes, ?LoggerInterface $logger): int { $headers = []; + $origin = curl_getinfo($parent, CURLINFO_EFFECTIVE_URL); foreach ($requestHeaders as $h) { if (false !== $i = strpos($h, ':', 1)) { @@ -299,20 +306,33 @@ private static function handlePush($parent, $pushed, array $requestHeaders, \std } } - if ('GET' !== $headers[':method'] || isset($headers['range'])) { + if (!isset($headers[':method']) || !isset($headers[':scheme']) || !isset($headers[':authority']) || !isset($headers[':path']) || 'GET' !== $headers[':method'] || isset($headers['range'])) { + $logger && $logger->info(sprintf('Rejecting pushed response from "%s": pushed headers are invalid', $origin)); + return CURL_PUSH_DENY; } $url = $headers[':scheme'].'://'.$headers[':authority']; + if ($maxPendingPushes <= \count($multi->pushedResponses)) { + $logger && $logger->info(sprintf('Rejecting pushed response from "%s" for "%s": the queue is full', $origin, $url)); + + return CURL_PUSH_DENY; + } + // curl before 7.65 doesn't validate the pushed ":authority" header, // but this is a MUST in the HTTP/2 RFC; let's restrict pushes to the original host, // ignoring domains mentioned as alt-name in the certificate for now (same as curl). - if (0 !== strpos(curl_getinfo($parent, CURLINFO_EFFECTIVE_URL), $url.'/')) { + if (0 !== strpos($origin, $url.'/')) { + $logger && $logger->info(sprintf('Rejecting pushed response from "%s": server is not authoritative for "%s"', $origin, $url)); + return CURL_PUSH_DENY; } - $multi->pushedResponses[$url.$headers[':path']] = [ + $url .= $headers[':path']; + $logger && $logger->info(sprintf('Queueing pushed response: %s', $url)); + + $multi->pushedResponses[$url] = [ new CurlResponse($multi, $pushed), [ $headers['authorization'] ?? null, diff --git a/src/Symfony/Component/HttpClient/HttpClient.php b/src/Symfony/Component/HttpClient/HttpClient.php index 3e44ca0d67913..25d3f6f87425c 100644 --- a/src/Symfony/Component/HttpClient/HttpClient.php +++ b/src/Symfony/Component/HttpClient/HttpClient.php @@ -11,8 +11,6 @@ namespace Symfony\Component\HttpClient; -use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; use Symfony\Contracts\HttpClient\HttpClientInterface; /** @@ -27,23 +25,16 @@ final class HttpClient /** * @param array $defaultOptions Default requests' options * @param int $maxHostConnections The maximum number of connections to a single host + * @param int $maxPendingPushes The maximum number of pushed responses to accept in the queue * * @see HttpClientInterface::OPTIONS_DEFAULTS for available options */ - public static function create(array $defaultOptions = [], LoggerInterface $logger = null, int $maxHostConnections = 6): HttpClientInterface + public static function create(array $defaultOptions = [], int $maxHostConnections = 6, int $maxPendingPushes = 50): HttpClientInterface { - if (null === $logger) { - $logger = new NullLogger(); - } - if (\extension_loaded('curl')) { - $logger->debug('Curl extension is enabled. Creating client.', ['client' => CurlHttpClient::class]); - - return new CurlHttpClient($defaultOptions, $logger, $maxHostConnections); + return new CurlHttpClient($defaultOptions, $maxHostConnections, $maxPendingPushes); } - $logger->debug('Curl extension is disabled. Creating client.', ['client' => NativeHttpClient::class]); - - return new NativeHttpClient($defaultOptions, $logger, $maxHostConnections); + return new NativeHttpClient($defaultOptions, $maxHostConnections); } } diff --git a/src/Symfony/Component/HttpClient/NativeHttpClient.php b/src/Symfony/Component/HttpClient/NativeHttpClient.php index ab8ba31a09be2..0549fccc49c54 100644 --- a/src/Symfony/Component/HttpClient/NativeHttpClient.php +++ b/src/Symfony/Component/HttpClient/NativeHttpClient.php @@ -11,8 +11,8 @@ namespace Symfony\Component\HttpClient; -use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; use Symfony\Component\HttpClient\Exception\TransportException; use Symfony\Component\HttpClient\Response\NativeResponse; use Symfony\Component\HttpClient\Response\ResponseStream; @@ -30,13 +30,13 @@ * * @experimental in 4.3 */ -final class NativeHttpClient implements HttpClientInterface +final class NativeHttpClient implements HttpClientInterface, LoggerAwareInterface { use HttpClientTrait; + use LoggerAwareTrait; private $defaultOptions = self::OPTIONS_DEFAULTS; private $multi; - private $logger; /** * @param array $defaultOptions Default requests' options @@ -44,10 +44,8 @@ final class NativeHttpClient implements HttpClientInterface * * @see HttpClientInterface::OPTIONS_DEFAULTS for available options */ - public function __construct(array $defaultOptions = [], LoggerInterface $logger = null, int $maxHostConnections = 6) + public function __construct(array $defaultOptions = [], int $maxHostConnections = 6) { - $this->logger = $logger ?? new NullLogger(); - if ($defaultOptions) { [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, self::OPTIONS_DEFAULTS); } @@ -73,7 +71,6 @@ public function __construct(array $defaultOptions = [], LoggerInterface $logger */ public function request(string $method, string $url, array $options = []): ResponseInterface { - $this->logger->notice('Making a request', ['url' => $url, 'method' => $method, 'client' => static::class]); [$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions); if ($options['bindto'] && file_exists($options['bindto'])) { @@ -211,7 +208,10 @@ public function request(string $method, string $url, array $options = []): Respo $context = stream_context_create($context, ['notification' => $notification]); self::configureHeadersAndProxy($context, $host, $options['request_headers'], $proxy, $noProxy); - return new NativeResponse($this->multi, $context, implode('', $url), $options, $gzipEnabled, $info, $resolveRedirect, $onProgress); + $url = implode('', $url); + $this->logger && $this->logger->info(sprintf('Request: %s %s', $method, $url)); + + return new NativeResponse($this->multi, $context, $url, $options, $gzipEnabled, $info, $resolveRedirect, $onProgress, $this->logger); } /** diff --git a/src/Symfony/Component/HttpClient/Response/CurlResponse.php b/src/Symfony/Component/HttpClient/Response/CurlResponse.php index 8c52ba501bdb8..39e256785d341 100644 --- a/src/Symfony/Component/HttpClient/Response/CurlResponse.php +++ b/src/Symfony/Component/HttpClient/Response/CurlResponse.php @@ -11,6 +11,7 @@ namespace Symfony\Component\HttpClient\Response; +use Psr\Log\LoggerInterface; use Symfony\Component\HttpClient\Chunk\FirstChunk; use Symfony\Component\HttpClient\Exception\TransportException; use Symfony\Contracts\HttpClient\ResponseInterface; @@ -29,7 +30,7 @@ final class CurlResponse implements ResponseInterface /** * @internal */ - public function __construct(\stdClass $multi, $ch, array $options = null, string $method = 'GET', callable $resolveRedirect = null) + public function __construct(\stdClass $multi, $ch, array $options = null, LoggerInterface $logger = null, string $method = 'GET', callable $resolveRedirect = null) { $this->multi = $multi; @@ -41,6 +42,7 @@ public function __construct(\stdClass $multi, $ch, array $options = null, string } $this->id = $id = (int) $ch; + $this->logger = $logger; $this->timeout = $options['timeout'] ?? null; $this->info['http_method'] = $method; $this->info['user_data'] = $options['user_data'] ?? null; @@ -64,8 +66,8 @@ public function __construct(\stdClass $multi, $ch, array $options = null, string $content = ($options['buffer'] ?? true) ? $content : null; } - curl_setopt($ch, CURLOPT_HEADERFUNCTION, static function ($ch, string $data) use (&$info, &$headers, $options, $multi, $id, &$location, $resolveRedirect): int { - return self::parseHeaderLine($ch, $data, $info, $headers, $options, $multi, $id, $location, $resolveRedirect); + curl_setopt($ch, CURLOPT_HEADERFUNCTION, static function ($ch, string $data) use (&$info, &$headers, $options, $multi, $id, &$location, $resolveRedirect, $logger): int { + return self::parseHeaderLine($ch, $data, $info, $headers, $options, $multi, $id, $location, $resolveRedirect, $logger); }); if (null === $options) { @@ -103,7 +105,9 @@ public function __construct(\stdClass $multi, $ch, array $options = null, string throw new TransportException($response->info['error']); } - if (\in_array(curl_getinfo($ch = $response->handle, CURLINFO_PRIVATE), ['headers', 'destruct'], true)) { + $waitFor = curl_getinfo($ch = $response->handle, CURLINFO_PRIVATE); + + if (\in_array($waitFor, ['headers', 'destruct'], true)) { try { if (\defined('CURLOPT_STREAM_WEIGHT')) { curl_setopt($ch, CURLOPT_STREAM_WEIGHT, 32); @@ -115,6 +119,8 @@ public function __construct(\stdClass $multi, $ch, array $options = null, string $response->close(); throw $e; } + } elseif ('content' === $waitFor && ($response->multi->handlesActivity[$response->id][0] ?? null) instanceof FirstChunk) { + self::stream([$response])->current(); } curl_setopt($ch, CURLOPT_HEADERFUNCTION, null); @@ -156,8 +162,8 @@ public function getInfo(string $type = null) public function __destruct() { try { - if (null === $this->timeout || isset($this->info['url'])) { - return; // pushed response + if (null === $this->timeout) { + return; // Unused pushed response } if ('content' === $waitFor = curl_getinfo($this->handle, CURLINFO_PRIVATE)) { @@ -171,7 +177,13 @@ public function __destruct() $this->close(); // Clear local caches when the only remaining handles are about pushed responses - if (\count($this->multi->openHandles) === \count($this->multi->pushedResponses)) { + if (!$this->multi->openHandles) { + if ($this->logger) { + foreach ($this->multi->pushedResponses as $url => $response) { + $this->logger->info(sprintf('Unused pushed response: %s', $url)); + } + } + $this->multi->pushedResponses = []; // Schedule DNS cache eviction for the next request $this->multi->dnsCache[2] = $this->multi->dnsCache[2] ?: $this->multi->dnsCache[1]; @@ -249,7 +261,7 @@ protected static function select(\stdClass $multi, float $timeout): int /** * Parses header lines as curl yields them to us. */ - private static function parseHeaderLine($ch, string $data, array &$info, array &$headers, ?array $options, \stdClass $multi, int $id, ?string &$location, ?callable $resolveRedirect): int + private static function parseHeaderLine($ch, string $data, array &$info, array &$headers, ?array $options, \stdClass $multi, int $id, ?string &$location, ?callable $resolveRedirect, ?LoggerInterface $logger): int { if (!\in_array($waitFor = @curl_getinfo($ch, CURLINFO_PRIVATE), ['headers', 'destruct'], true)) { return \strlen($data); // Ignore HTTP trailers @@ -306,6 +318,8 @@ private static function parseHeaderLine($ch, string $data, array &$info, array & } curl_setopt($ch, CURLOPT_PRIVATE, 'content'); + } elseif (null !== $info['redirect_url'] && $logger) { + $logger->info(sprintf('Redirecting: %s %s', $info['http_code'], $info['redirect_url'])); } return \strlen($data); diff --git a/src/Symfony/Component/HttpClient/Response/NativeResponse.php b/src/Symfony/Component/HttpClient/Response/NativeResponse.php index 2ab498a731ea7..6c3330a1638e4 100644 --- a/src/Symfony/Component/HttpClient/Response/NativeResponse.php +++ b/src/Symfony/Component/HttpClient/Response/NativeResponse.php @@ -11,6 +11,7 @@ namespace Symfony\Component\HttpClient\Response; +use Psr\Log\LoggerInterface; use Symfony\Component\HttpClient\Chunk\FirstChunk; use Symfony\Component\HttpClient\Exception\TransportException; use Symfony\Contracts\HttpClient\ResponseInterface; @@ -35,12 +36,13 @@ final class NativeResponse implements ResponseInterface /** * @internal */ - public function __construct(\stdClass $multi, $context, string $url, $options, bool $gzipEnabled, array &$info, callable $resolveRedirect, ?callable $onProgress) + public function __construct(\stdClass $multi, $context, string $url, $options, bool $gzipEnabled, array &$info, callable $resolveRedirect, ?callable $onProgress, ?LoggerInterface $logger) { $this->multi = $multi; $this->id = (int) $context; $this->context = $context; $this->url = $url; + $this->logger = $logger; $this->timeout = $options['timeout']; $this->info = &$info; $this->resolveRedirect = $resolveRedirect; @@ -107,13 +109,19 @@ private function open(): void $this->info['start_time'] = microtime(true); $url = $this->url; - do { + while (true) { // Send request and follow redirects when needed $this->info['fopen_time'] = microtime(true); $this->handle = $h = fopen($url, 'r', false, $this->context); self::addResponseHeaders($http_response_header, $this->info, $this->headers); $url = ($this->resolveRedirect)($this->multi, $this->headers['location'][0] ?? null, $this->context); - } while (null !== $url); + + if (null === $url) { + break; + } + + $this->logger && $this->logger->info(sprintf('Redirecting: %s %s', $this->info['http_code'], $url ?? $this->url)); + } } catch (\Throwable $e) { $this->close(); $this->multi->handlesActivity[$this->id][] = null; diff --git a/src/Symfony/Component/HttpClient/Response/ResponseTrait.php b/src/Symfony/Component/HttpClient/Response/ResponseTrait.php index b42aa6f188e85..734359f08d375 100644 --- a/src/Symfony/Component/HttpClient/Response/ResponseTrait.php +++ b/src/Symfony/Component/HttpClient/Response/ResponseTrait.php @@ -30,6 +30,7 @@ */ trait ResponseTrait { + private $logger; private $headers = []; /** @@ -299,6 +300,9 @@ public static function stream(iterable $responses, float $timeout = null): \Gene } elseif ($chunk instanceof ErrorChunk) { unset($responses[$j]); $isTimeout = true; + } elseif ($chunk instanceof FirstChunk && $response->logger) { + $info = $response->getInfo(); + $response->logger->info(sprintf('Response: %s %s', $info['http_code'], $info['url'])); } yield $response => $chunk; diff --git a/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php index 476dc729762f1..b5e82732bc7cd 100644 --- a/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php +++ b/src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\HttpClient\Tests; +use Psr\Log\AbstractLogger; use Symfony\Component\HttpClient\CurlHttpClient; use Symfony\Contracts\HttpClient\HttpClientInterface; use Symfony\Contracts\HttpClient\Test\HttpClientTestCase; @@ -24,4 +25,52 @@ protected function getHttpClient(string $testCase): HttpClientInterface { return new CurlHttpClient(); } + + /** + * @requires PHP 7.2.17 + * @requires extension curl 7.61 + */ + public function testHttp2Push() + { + if (\PHP_VERSION_ID >= 70300 && \PHP_VERSION_ID < 70304) { + $this->markTestSkipped('PHP 7.3.0 to 7.3.3 don\'t support HTTP/2 PUSH'); + } + + if (!\defined('CURLMOPT_PUSHFUNCTION') || !(CURL_VERSION_HTTP2 & curl_version()['features'])) { + $this->markTestSkipped('curl is not compiled with support for HTTP/2 PUSH'); + } + + $logger = new class() extends AbstractLogger { + public $logs = []; + + public function log($level, $message, array $context = []) + { + $this->logs[] = $message; + } + }; + + $client = new CurlHttpClient(); + $client->setLogger($logger); + + $index = $client->request('GET', 'https://http2-push.io'); + $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->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', + ]; + $this->assertSame($expected, $logger->logs); + } } diff --git a/src/Symfony/Component/HttpClient/composer.json b/src/Symfony/Component/HttpClient/composer.json index b310f269c31b9..0469bb41ddebd 100644 --- a/src/Symfony/Component/HttpClient/composer.json +++ b/src/Symfony/Component/HttpClient/composer.json @@ -20,9 +20,9 @@ }, "require": { "php": "^7.1.3", + "psr/log": "^1.0", "symfony/contracts": "^1.1", - "symfony/polyfill-php73": "^1.11", - "psr/log": "~1.0" + "symfony/polyfill-php73": "^1.11" }, "require-dev": { "nyholm/psr7": "^1.0", diff --git a/src/Symfony/Component/HttpKernel/HttpClientKernel.php b/src/Symfony/Component/HttpKernel/HttpClientKernel.php index 2c04e670cc05f..f60a84eabe257 100644 --- a/src/Symfony/Component/HttpKernel/HttpClientKernel.php +++ b/src/Symfony/Component/HttpKernel/HttpClientKernel.php @@ -11,8 +11,6 @@ namespace Symfony\Component\HttpKernel; -use Psr\Log\LoggerInterface; -use Psr\Log\NullLogger; use Symfony\Component\HttpClient\HttpClient; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -31,22 +29,18 @@ final class HttpClientKernel implements HttpKernelInterface { private $client; - private $logger; - public function __construct(HttpClientInterface $client = null, LoggerInterface $logger = null) + public function __construct(HttpClientInterface $client = null) { if (!class_exists(HttpClient::class)) { throw new \LogicException(sprintf('You cannot use "%s" as the HttpClient component is not installed. Try running "composer require symfony/http-client".', __CLASS__)); } $this->client = $client ?? HttpClient::create(); - $this->logger = $logger ?? new NullLogger(); } public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQUEST, $catch = true) { - $this->logger->debug(sprintf('Request: %s %s', $request->getMethod(), $request->getUri())); - $headers = $this->getHeaders($request); $body = ''; if (null !== $part = $this->getBody($request)) { @@ -59,8 +53,6 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ 'max_redirects' => 0, ] + $request->attributes->get('http_client_options', [])); - $this->logger->debug(sprintf('Response: %s %s', $response->getStatusCode(), $request->getUri())); - $response = new Response($response->getContent(!$catch), $response->getStatusCode(), $response->getHeaders(!$catch)); $response->headers = new class($response->headers->all()) extends ResponseHeaderBag {