Skip to content

Commit 140bbcd

Browse files
Merge branch '6.4' into 7.1
* 6.4: [HttpClient] More consistency cleanups [HttpClient] Remove unrelevant test [DependencyInjection] Fix PhpDoc type
2 parents 128f3a2 + 7e5c9fd commit 140bbcd

File tree

4 files changed

+14
-25
lines changed

4 files changed

+14
-25
lines changed

CurlHttpClient.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ public function request(string $method, string $url, array $options = []): Respo
323323
}
324324
}
325325

326-
return $pushedResponse ?? new CurlResponse($multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host, $port), CurlClientState::$curlVersion['version_number'], $url);
326+
return $pushedResponse ?? new CurlResponse($multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $authority), CurlClientState::$curlVersion['version_number'], $url);
327327
}
328328

329329
public function stream(ResponseInterface|iterable $responses, ?float $timeout = null): ResponseStreamInterface
@@ -404,12 +404,11 @@ private static function readRequestBody(int $length, \Closure $body, string &$bu
404404
*
405405
* Work around CVE-2018-1000007: Authorization and Cookie headers should not follow redirects - fixed in Curl 7.64
406406
*/
407-
private static function createRedirectResolver(array $options, string $host, int $port): \Closure
407+
private static function createRedirectResolver(array $options, string $authority): \Closure
408408
{
409409
$redirectHeaders = [];
410410
if (0 < $options['max_redirects']) {
411-
$redirectHeaders['host'] = $host;
412-
$redirectHeaders['port'] = $port;
411+
$redirectHeaders['authority'] = $authority;
413412
$redirectHeaders['with_auth'] = $redirectHeaders['no_auth'] = array_filter($options['headers'], static fn ($h) => 0 !== stripos($h, 'Host:'));
414413

415414
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
433432
}
434433

435434
if ($redirectHeaders && isset($location['authority'])) {
436-
$port = parse_url($location['authority'], \PHP_URL_PORT) ?: ('http:' === $location['scheme'] ? 80 : 443);
437-
$requestHeaders = parse_url($location['authority'], \PHP_URL_HOST) === $redirectHeaders['host'] && $redirectHeaders['port'] === $port ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
435+
$requestHeaders = $location['authority'] === $redirectHeaders['authority'] ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
438436
curl_setopt($ch, \CURLOPT_HTTPHEADER, $requestHeaders);
439437
} elseif ($noContent && $redirectHeaders) {
440438
curl_setopt($ch, \CURLOPT_HTTPHEADER, $redirectHeaders['with_auth']);

NativeHttpClient.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ public function request(string $method, string $url, array $options = []): Respo
262262
$context = stream_context_create($context, ['notification' => $notification]);
263263

264264
$resolver = static function ($multi) use ($context, $options, $url, &$info, $onProgress) {
265+
$authority = $url['authority'];
265266
[$host, $port] = self::parseHostPort($url, $info);
266267

267268
if (!isset($options['normalized_headers']['host'])) {
@@ -275,7 +276,7 @@ public function request(string $method, string $url, array $options = []): Respo
275276
$url['authority'] = substr_replace($url['authority'], $ip, -\strlen($host) - \strlen($port), \strlen($host));
276277
}
277278

278-
return [self::createRedirectResolver($options, $host, $port, $proxy, $info, $onProgress), implode('', $url)];
279+
return [self::createRedirectResolver($options, $authority, $proxy, $info, $onProgress), implode('', $url)];
279280
};
280281

281282
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
373374
/**
374375
* Handles redirects - the native logic is too buggy to be used.
375376
*/
376-
private static function createRedirectResolver(array $options, string $host, string $port, ?array $proxy, array &$info, ?\Closure $onProgress): \Closure
377+
private static function createRedirectResolver(array $options, string $authority, ?array $proxy, array &$info, ?\Closure $onProgress): \Closure
377378
{
378379
$redirectHeaders = [];
379380
if (0 < $maxRedirects = $options['max_redirects']) {
380-
$redirectHeaders = ['host' => $host, 'port' => $port];
381+
$redirectHeaders = ['authority' => $authority];
381382
$redirectHeaders['with_auth'] = $redirectHeaders['no_auth'] = array_filter($options['headers'], static fn ($h) => 0 !== stripos($h, 'Host:'));
382383

383384
if (isset($options['normalized_headers']['authorization']) || isset($options['normalized_headers']['cookie'])) {
@@ -435,8 +436,8 @@ private static function createRedirectResolver(array $options, string $host, str
435436
[$host, $port] = self::parseHostPort($url, $info);
436437

437438
if ($locationHasHost) {
438-
// Authorization and Cookie headers MUST NOT follow except for the initial host name
439-
$requestHeaders = $redirectHeaders['host'] === $host && $redirectHeaders['port'] === $port ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
439+
// Authorization and Cookie headers MUST NOT follow except for the initial authority name
440+
$requestHeaders = $redirectHeaders['authority'] === $url['authority'] ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
440441
$requestHeaders[] = 'Host: '.$host.$port;
441442
$dnsResolve = !self::configureHeadersAndProxy($context, $host, $requestHeaders, $proxy, 'https:' === $url['scheme']);
442443
} else {

Response/AmpResponse.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,16 +341,14 @@ private static function followRedirects(Request $originRequest, AmpClientState $
341341
$request->setTlsHandshakeTimeout($originRequest->getTlsHandshakeTimeout());
342342
$request->setTransferTimeout($originRequest->getTransferTimeout());
343343

344-
if (\in_array($status, [301, 302, 303], true)) {
344+
if (303 === $status || \in_array($status, [301, 302], true) && 'POST' === $response->getRequest()->getMethod()) {
345+
// Do like curl and browsers: turn POST to GET on 301, 302 and 303
345346
$originRequest->removeHeader('transfer-encoding');
346347
$originRequest->removeHeader('content-length');
347348
$originRequest->removeHeader('content-type');
348349

349-
// Do like curl and browsers: turn POST to GET on 301, 302 and 303
350-
if ('POST' === $response->getRequest()->getMethod() || 303 === $status) {
351-
$info['http_method'] = 'HEAD' === $response->getRequest()->getMethod() ? 'HEAD' : 'GET';
352-
$request->setMethod($info['http_method']);
353-
}
350+
$info['http_method'] = 'HEAD' === $response->getRequest()->getMethod() ? 'HEAD' : 'GET';
351+
$request->setMethod($info['http_method']);
354352
} else {
355353
$request->setBody(AmpBody::rewind($response->getRequest()->getBody()));
356354
}

Tests/NoPrivateNetworkHttpClientTest.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,6 @@ public function testNonCallableOnProgressCallback()
142142
$client->request('GET', $url, ['on_progress' => $customCallback]);
143143
}
144144

145-
public function testConstructor()
146-
{
147-
$this->expectException(\TypeError::class);
148-
$this->expectExceptionMessage('Argument 2 passed to "Symfony\Component\HttpClient\NoPrivateNetworkHttpClient::__construct()" must be of the type array, string or null. "int" given.');
149-
150-
new NoPrivateNetworkHttpClient(new MockHttpClient(), 3);
151-
}
152-
153145
private function getMockHttpClient(string $ipAddr, string $content)
154146
{
155147
return new MockHttpClient(new MockResponse($content, ['primary_ip' => $ipAddr]));

0 commit comments

Comments
 (0)