Skip to content

Commit d90dd8d

Browse files
fabpotfabpot
committed
bug #31834 [HttpClient] Don't throw InvalidArgumentException on bad Location header (nicolas-grekas)
This PR was merged into the 4.3 branch. Discussion ---------- [HttpClient] Don't throw InvalidArgumentException on bad Location header | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #31776 | License | MIT | Doc PR | - Instead, just stop following redirections and throw a `RedirectionExceptionInterface` as usual when throwing is not disabled. Commits ------- 4acca42 [HttpClient] Don't throw InvalidArgumentException on bad Location header
2 parents 036d7b6 + 4acca42 commit d90dd8d

File tree

5 files changed

+50
-11
lines changed

5 files changed

+50
-11
lines changed

‎src/Symfony/Component/HttpClient/CurlHttpClient.php

+9-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Psr\Log\LoggerAwareInterface;
1515
use Psr\Log\LoggerAwareTrait;
1616
use Psr\Log\LoggerInterface;
17+
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
1718
use Symfony\Component\HttpClient\Exception\TransportException;
1819
use Symfony\Component\HttpClient\Internal\CurlClientState;
1920
use Symfony\Component\HttpClient\Internal\PushedResponse;
@@ -392,14 +393,20 @@ private static function createRedirectResolver(array $options, string $host): \C
392393
}
393394

394395
return static function ($ch, string $location) use ($redirectHeaders) {
395-
if ($redirectHeaders && $host = parse_url($location, PHP_URL_HOST)) {
396+
try {
397+
$location = self::parseUrl($location);
398+
} catch (InvalidArgumentException $e) {
399+
return null;
400+
}
401+
402+
if ($redirectHeaders && $host = parse_url('http:'.$location['authority'], PHP_URL_HOST)) {
396403
$requestHeaders = $redirectHeaders['host'] === $host ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
397404
curl_setopt($ch, CURLOPT_HTTPHEADER, $requestHeaders);
398405
}
399406

400407
$url = self::parseUrl(curl_getinfo($ch, CURLINFO_EFFECTIVE_URL));
401408

402-
return implode('', self::resolveUrl(self::parseUrl($location), $url));
409+
return implode('', self::resolveUrl($location, $url));
403410
};
404411
}
405412
}

‎src/Symfony/Component/HttpClient/NativeHttpClient.php

+10-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Psr\Log\LoggerAwareInterface;
1515
use Psr\Log\LoggerAwareTrait;
16+
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
1617
use Symfony\Component\HttpClient\Exception\TransportException;
1718
use Symfony\Component\HttpClient\Internal\NativeClientState;
1819
use Symfony\Component\HttpClient\Response\NativeResponse;
@@ -352,7 +353,15 @@ private static function createRedirectResolver(array $options, string $host, ?ar
352353
return null;
353354
}
354355

355-
$url = self::resolveUrl(self::parseUrl($location), $info['url']);
356+
try {
357+
$url = self::parseUrl($location);
358+
} catch (InvalidArgumentException $e) {
359+
$info['redirect_url'] = null;
360+
361+
return null;
362+
}
363+
364+
$url = self::resolveUrl($url, $info['url']);
356365
$info['redirect_url'] = implode('', $url);
357366

358367
if ($info['redirect_count'] >= $maxRedirects) {

‎src/Symfony/Component/HttpClient/Response/CurlResponse.php

+13-8
Original file line numberDiff line numberDiff line change
@@ -310,14 +310,19 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &
310310
$info['redirect_url'] = null;
311311

312312
if (300 <= $statusCode && $statusCode < 400 && null !== $location) {
313-
$info['redirect_url'] = $resolveRedirect($ch, $location);
314-
$url = parse_url($location ?? ':');
315-
316-
if (isset($url['host']) && null !== $ip = $multi->dnsCache->hostnames[$url['host'] = strtolower($url['host'])] ?? null) {
317-
// Populate DNS cache for redirects if needed
318-
$port = $url['port'] ?? ('http' === ($url['scheme'] ?? parse_url(curl_getinfo($ch, CURLINFO_EFFECTIVE_URL), PHP_URL_SCHEME)) ? 80 : 443);
319-
curl_setopt($ch, CURLOPT_RESOLVE, ["{$url['host']}:$port:$ip"]);
320-
$multi->dnsCache->removals["-{$url['host']}:$port"] = "-{$url['host']}:$port";
313+
if (null === $info['redirect_url'] = $resolveRedirect($ch, $location)) {
314+
$options['max_redirects'] = curl_getinfo($ch, CURLINFO_REDIRECT_COUNT);
315+
curl_setopt($ch, CURLOPT_FOLLOWLOCATION, false);
316+
curl_setopt($ch, CURLOPT_MAXREDIRS, $options['max_redirects']);
317+
} else {
318+
$url = parse_url($location ?? ':');
319+
320+
if (isset($url['host']) && null !== $ip = $multi->dnsCache->hostnames[$url['host'] = strtolower($url['host'])] ?? null) {
321+
// Populate DNS cache for redirects if needed
322+
$port = $url['port'] ?? ('http' === ($url['scheme'] ?? parse_url(curl_getinfo($ch, CURLINFO_EFFECTIVE_URL), PHP_URL_SCHEME)) ? 80 : 443);
323+
curl_setopt($ch, CURLOPT_RESOLVE, ["{$url['host']}:$port:$ip"]);
324+
$multi->dnsCache->removals["-{$url['host']}:$port"] = "-{$url['host']}:$port";
325+
}
321326
}
322327
}
323328

‎src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php

+4
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@
5555
header('Location: http://foo.example.', true, 301);
5656
break;
5757

58+
case '/301/invalid':
59+
header('Location: //?foo=bar', true, 301);
60+
break;
61+
5862
case '/302':
5963
if (!isset($vars['HTTP_AUTHORIZATION'])) {
6064
header('Location: http://localhost:8057/', true, 302);

‎src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php

+14
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,20 @@ public function testRedirects()
259259
$this->assertSame($expected, $filteredHeaders);
260260
}
261261

262+
public function testInvalidRedirect()
263+
{
264+
$client = $this->getHttpClient(__FUNCTION__);
265+
$response = $client->request('GET', 'http://localhost:8057/301/invalid');
266+
267+
$this->assertSame(301, $response->getStatusCode());
268+
$this->assertSame(['//?foo=bar'], $response->getHeaders(false)['location']);
269+
$this->assertSame(0, $response->getInfo('redirect_count'));
270+
$this->assertNull($response->getInfo('redirect_url'));
271+
272+
$this->expectException(RedirectionExceptionInterface::class);
273+
$response->getHeaders();
274+
}
275+
262276
public function testRelativeRedirects()
263277
{
264278
$client = $this->getHttpClient(__FUNCTION__);

0 commit comments

Comments
 (0)