Skip to content

Commit

Permalink
Merge branch '6.4' into 7.1
Browse files Browse the repository at this point in the history 
* 6.4:
  [HttpClient] More consistency cleanups
  [HttpClient] Remove unrelevant test
  [DependencyInjection] Fix PhpDoc type
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Nov 25, 2024
2 parents 128f3a2 + 7e5c9fd commit 140bbcd
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 25 deletions.
10 changes: 4 additions & 6 deletions CurlHttpClient.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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])) {
Expand All @@ -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']);
Expand Down
11 changes: 6 additions & 5 deletions NativeHttpClient.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Expand All @@ -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);
Expand Down Expand Up @@ -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'])) {
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 4 additions & 6 deletions Response/AmpResponse.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,16 +341,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()));
}
Expand Down
8 changes: 0 additions & 8 deletions Tests/NoPrivateNetworkHttpClientTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]));
Expand Down

0 comments on commit 140bbcd

Please sign in to comment.