8000 bug #50004 [HttpClient] fix proxied redirects in curl client (matthi4s) · symfony/symfony@547e876 · GitHub
[go: up one dir, main page]
Skip to content

Commit 547e876

Browse files
fabpotfabpot
committed
bug #50004 [HttpClient] fix proxied redirects in curl client (matthi4s)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [HttpClient] fix proxied redirects in curl client | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - When using the `CurlHttpClient` with a proxy defined using the options array, the proxy isn't used after the first request on redirects. The `CURLOPT_PROXY` option is overwritten in the redirect resolver, but the `$options` variable isn't present in that case which causes it to be overwritten with environment variables or nothing, which disables the proxy. I don't know why the proxy option gets rewritten with every redirect, since the same curl handle is used, it should not be necessary to set it again. It also wasn't set before the original commit which introduced this bug (9e5305e). I've removed that part, but I can also change it to pass on the reference to the `$options` variable, which also fixes the issue. In that case, I would suggest moving the validation of the environment variable to a separate function to avoid duplicating that part. I've also added a test case that fails for the curl client without the fix but succeeds for the other clients and the curl client with both possible fixes. Commits ------- 9854226 [HttpClient] fix proxied redirects in curl client
2 parents 8c1af42 + 9854226 commit 547e876

File tree

4 files changed

+34
-20
lines changed

4 files changed

+34
-20
lines changed

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

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,7 @@ public function request(string $method, string $url, array $options = []): Respo
9595
$scheme = $url['scheme'];
9696
$authority = $url['authority'];
9797
$host = parse_url($authority, \PHP_URL_HOST);
98-
$proxy = $options['proxy']
99-
?? ('https:' === $url['scheme'] ? $_SERVER['https_proxy'] ?? $_SERVER['HTTPS_PROXY'] ?? null : null)
100-
// Ignore HTTP_PROXY except on the CLI to work around httpoxy set of vulnerabilities
101-
?? $_SERVER['http_proxy'] ?? (\in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) ? $_SERVER['HTTP_PROXY'] ?? null : null) ?? $_SERVER['all_proxy'] ?? $_SERVER['ALL_PROXY'] ?? null;
98+
$proxy = self::getProxyUrl($options['proxy'], $url);
10299
$url = implode('', $url);
103100

104101
if (!isset($options['normalized_headers']['user-agent'])) {
@@ -411,7 +408,7 @@ private static function createRedirectResolver(array $options, string $host): \C
411408
}
412409
}
413410

414-
return static function ($ch, string $location, bool $noContent) use (&$redirectHeaders) {
411+
return static function ($ch, string $location, bool $noContent) use (&$redirectHeaders, $options) {
415412
try {
416413
$location = self::parseUrl($location);
417414
} catch (InvalidArgumentException $e) {
@@ -436,11 +433,7 @@ private static function createRedirectResolver(array $options, string $host): \C
436433
$url = self::parseUrl(curl_getinfo($ch, \CURLINFO_EFFECTIVE_URL));
437434
$url = self::resolveUrl($location, $url);
438435

439-
curl_setopt($ch, \CURLOPT_PROXY, $options['proxy']
440-
?? ('https:' === $url['scheme'] ? $_SERVER['https_proxy'] ?? $_SERVER['HTTPS_PROXY'] ?? null : null)
441-
// Ignore HTTP_PROXY except on the CLI to work around httpoxy set of vulnerabilities
442-
?? $_SERVER['http_proxy'] ?? (\in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) ? $_SERVER['HTTP_PROXY'] ?? null : null) ?? $_SERVER['all_proxy'] ?? $_SERVER['ALL_PROXY'] ?? null
443-
);
436+
curl_setopt($ch, \CURLOPT_PROXY, self::getProxyUrl($options['proxy'], $url));
444437

445438
return implode('', $url);
446439
};

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -655,16 +655,7 @@ private static function mergeQueryString(?string $queryString, array $queryArray
655655
*/
656656
private static function getProxy(?string $proxy, array $url, ?string $noProxy): ?array
657657
{
658-
if (null === $proxy) {
659-
// Ignore HTTP_PROXY except on the CLI to work around httpoxy set of vulnerabilities
660-
$proxy = $_SERVER['http_proxy'] ?? (\in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) ? $_SERVER['HTTP_PROXY'] ?? null : null) ?? $_SERVER['all_proxy'] ?? $_SERVER['ALL_PROXY'] ?? null;
661-
662-
if ('https:' === $url['scheme']) {
663-
$proxy = $_SERVER['https_proxy'] ?? $_SERVER['HTTPS_PROXY'] ?? $proxy;
664-
}
665-
}
666-
667-
if (null === $proxy) {
658+
if (null === $proxy = self::getProxyUrl($proxy, $url)) {
668659
return null;
669660
}
670661

@@ -692,6 +683,22 @@ private static function getProxy(?string $proxy, array $url, ?string $noProxy):
692683
];
693684
}
694685

686+
private static function getProxyUrl(?string $proxy, array $url): ?string
687+
{
688+
if (null !== $proxy) {
689+
return $proxy;
690+
}
691+
692+
// Ignore HTTP_PROXY except on the CLI to work around httpoxy set of vulnerabilities
693+
$proxy = $_SERVER['http_proxy'] ?? (\in_array(\PHP_SAPI, ['cli', 'phpdbg'], true) ? $_SERVER['HTTP_PROXY'] ?? null : null) ?? $_SERVER['all_proxy'] ?? $_SERVER['ALL_PROXY'] ?? null;
694+
695+
if ('https:' === $url['scheme']) {
696+
$proxy = $_SERVER['https_proxy'] ?? $_SERVER['HTTPS_PROXY'] ?? $proxy;
697+
}
698+
699+
return $proxy;
700+
}
701+
695702
private static function shouldBuffer(array $headers): bool
696703
{
697704
if (null === $contentType = $headers['content-type'][0] ?? null) {

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@
8686
header('Location: //?foo=bar', true, 301);
8787
break;
8888

89+
case '/301/proxy':
90+
case 'http://localhost:8057/301/proxy':
91+
case 'http://127.0.0.1:8057/301/proxy':
92+
header('Location: http://localhost:8057/', true, 301);
93+
break;
94+
8995
case '/302':
9096
if (!isset($vars['HTTP_AUTHORIZATION'])) {
9197
header('Location: http://localhost:8057/', true, 302);

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,14 @@ public function testProxy()
969969
} finally {
970970
unset($_SERVER['http_proxy']);
971971
}
972+
973+
$response = $client->request('GET', 'http://localhost:8057/301/proxy', [
974+
'proxy' => 'http://localhost:8057',
975+
]);
976+
977+
$body = $response->toArray();
978+
$this->assertSame('localhost:8057', $body['HTTP_HOST']);
979+
$this->assertMatchesRegularExpression('#^http://(localhost|127\.0\.0\.1):8057/$#', $body['REQUEST_URI']);
972980
}
973981

974982
public function testNoProxy()

0 commit comments

Comments
 (0)
0