From d485a40c3b9e4bd76d27d76ab894fb9a18c47763 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Mon, 25 Nov 2024 16:05:01 +0100 Subject: [PATCH 1/2] [HttpClient] Remove unrelevant test --- Tests/NoPrivateNetworkHttpClientTest.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Tests/NoPrivateNetworkHttpClientTest.php b/Tests/NoPrivateNetworkHttpClientTest.php index 0eba5d6..8d72bc7 100644 --- a/Tests/NoPrivateNetworkHttpClientTest.php +++ b/Tests/NoPrivateNetworkHttpClientTest.php @@ -142,14 +142,6 @@ public function testNonCallableOnProgressCallback() $client->request('GET', $url, ['on_progress' => $customCallback]); } - public function testConstructor() - { - $this->expectException(\TypeError::class); - $this->expectExceptionMessage('Argument 2 passed to "Symfony\Component\HttpClient\NoPrivateNetworkHttpClient::__construct()" must be of the type array, string or null. "int" given.'); - - new NoPrivateNetworkHttpClient(new MockHttpClient(), 3); - } - private function getMockHttpClient(string $ipAddr, string $content) { return new MockHttpClient(new MockResponse($content, ['primary_ip' => $ipAddr])); From 7e5c9fdca8062dfef93b457c36d1d922f21595c6 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 25 Nov 2024 16:30:26 +0100 Subject: [PATCH 2/2] [HttpClient] More consistency cleanups --- CurlHttpClient.php | 10 ++++------ NativeHttpClient.php | 11 ++++++----- Response/AmpResponse.php | 10 ++++------ 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/CurlHttpClient.php b/CurlHttpClient.php index 7ae7591..7d99620 100644 --- a/CurlHttpClient.php +++ b/CurlHttpClient.php @@ -323,7 +323,7 @@ public function request(string $method, string $url, array $options = []): Respo } } - return $pushedResponse ?? new CurlResponse($multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host, $port), CurlClientState::$curlVersion['version_number'], $url); + return $pushedResponse ?? new CurlResponse($multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $authority), CurlClientState::$curlVersion['version_number'], $url); } public function stream(ResponseInterface|iterable $responses, ?float $timeout = null): ResponseStreamInterface @@ -404,12 +404,11 @@ private static function readRequestBody(int $length, \Closure $body, string &$bu * * Work around CVE-2018-1000007: Authorization and Cookie headers should not follow redirects - fixed in Curl 7.64 */ - private static function createRedirectResolver(array $options, string $host, int $port): \Closure + private static function createRedirectResolver(array $options, string $authority): \Closure { $redirectHeaders = []; if (0 < $options['max_redirects']) { - $redirectHeaders['host'] = $host; - $redirectHeaders['port'] = $port; + $redirectHeaders['authority'] = $authority; $redirectHeaders['with_auth'] = $redirectHeaders['no_auth'] = array_filter($options['headers'], static fn ($h) => 0 !== stripos($h, 'Host:')); if (isset($options['normalized_headers']['authorization'][0]) || isset($options['normalized_headers']['cookie'][0])) { @@ -433,8 +432,7 @@ private static function createRedirectResolver(array $options, string $host, int } if ($redirectHeaders && isset($location['authority'])) { - $port = parse_url($location['authority'], \PHP_URL_PORT) ?: ('http:' === $location['scheme'] ? 80 : 443); - $requestHeaders = parse_url($location['authority'], \PHP_URL_HOST) === $redirectHeaders['host'] && $redirectHeaders['port'] === $port ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth']; + $requestHeaders = $location['authority'] === $redirectHeaders['authority'] ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth']; curl_setopt($ch, \CURLOPT_HTTPHEADER, $requestHeaders); } elseif ($noContent && $redirectHeaders) { curl_setopt($ch, \CURLOPT_HTTPHEADER, $redirectHeaders['with_auth']); diff --git a/NativeHttpClient.php b/NativeHttpClient.php index a14aa54..7e742c4 100644 --- a/NativeHttpClient.php +++ b/NativeHttpClient.php @@ -262,6 +262,7 @@ public function request(string $method, string $url, array $options = []): Respo $context = stream_context_create($context, ['notification' => $notification]); $resolver = static function ($multi) use ($context, $options, $url, &$info, $onProgress) { + $authority = $url['authority']; [$host, $port] = self::parseHostPort($url, $info); if (!isset($options['normalized_headers']['host'])) { @@ -275,7 +276,7 @@ public function request(string $method, string $url, array $options = []): Respo $url['authority'] = substr_replace($url['authority'], $ip, -\strlen($host) - \strlen($port), \strlen($host)); } - return [self::createRedirectResolver($options, $host, $port, $proxy, $info, $onProgress), implode('', $url)]; + return [self::createRedirectResolver($options, $authority, $proxy, $info, $onProgress), implode('', $url)]; }; return new NativeResponse($this->multi, $context, implode('', $url), $options, $info, $resolver, $onProgress, $this->logger); @@ -373,11 +374,11 @@ private static function dnsResolve(string $host, NativeClientState $multi, array /** * Handles redirects - the native logic is too buggy to be used. */ - private static function createRedirectResolver(array $options, string $host, string $port, ?array $proxy, array &$info, ?\Closure $onProgress): \Closure + private static function createRedirectResolver(array $options, string $authority, ?array $proxy, array &$info, ?\Closure $onProgress): \Closure { $redirectHeaders = []; if (0 < $maxRedirects = $options['max_redirects']) { - $redirectHeaders = ['host' => $host, 'port' => $port]; + $redirectHeaders = ['authority' => $authority]; $redirectHeaders['with_auth'] = $redirectHeaders['no_auth'] = array_filter($options['headers'], static fn ($h) => 0 !== stripos($h, 'Host:')); if (isset($options['normalized_headers']['authorization']) || isset($options['normalized_headers']['cookie'])) { @@ -435,8 +436,8 @@ private static function createRedirectResolver(array $options, string $host, str [$host, $port] = self::parseHostPort($url, $info); if ($locationHasHost) { - // Authorization and Cookie headers MUST NOT follow except for the initial host name - $requestHeaders = $redirectHeaders['host'] === $host && $redirectHeaders['port'] === $port ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth']; + // Authorization and Cookie headers MUST NOT follow except for the initial authority name + $requestHeaders = $redirectHeaders['authority'] === $url['authority'] ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth']; $requestHeaders[] = 'Host: '.$host.$port; $dnsResolve = !self::configureHeadersAndProxy($context, $host, $requestHeaders, $proxy, 'https:' === $url['scheme']); } else { diff --git a/Response/AmpResponse.php b/Response/AmpResponse.php index c658515..340a417 100644 --- a/Response/AmpResponse.php +++ b/Response/AmpResponse.php @@ -339,16 +339,14 @@ private static function followRedirects(Request $originRequest, AmpClientState $ $request->setTlsHandshakeTimeout($originRequest->getTlsHandshakeTimeout()); $request->setTransferTimeout($originRequest->getTransferTimeout()); - if (\in_array($status, [301, 302, 303], true)) { + if (303 === $status || \in_array($status, [301, 302], true) && 'POST' === $response->getRequest()->getMethod()) { + // Do like curl and browsers: turn POST to GET on 301, 302 and 303 $originRequest->removeHeader('transfer-encoding'); $originRequest->removeHeader('content-length'); $originRequest->removeHeader('content-type'); - // Do like curl and browsers: turn POST to GET on 301, 302 and 303 - if ('POST' === $response->getRequest()->getMethod() || 303 === $status) { - $info['http_method'] = 'HEAD' === $response->getRequest()->getMethod() ? 'HEAD' : 'GET'; - $request->setMethod($info['http_method']); - } + $info['http_method'] = 'HEAD' === $response->getRequest()->getMethod() ? 'HEAD' : 'GET'; + $request->setMethod($info['http_method']); } else { $request->setBody(AmpBody::rewind($response->getRequest()->getBody())); }