Skip to content

[HttpClient] Fix option "resolve" with IPv6 addresses #58915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/Symfony/Component/HttpClient/HttpClientTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,14 @@
if ($resolve = $options['resolve'] ?? false) {
$options['resolve'] = [];
foreach ($resolve as $k => $v) {
$options['resolve'][substr(self::parseUrl('http://'.$k)['authority'], 2)] = (string) $v;
if ('' === $v = (string) $v) {
throw new InvalidArgumentException(sprintf('Option "resolve" for host "%s" cannot be empty.', $k));
}
if ('[' === $v[0] && ']' === substr($v, -1) && str_contains($v, ':')) {
$v = substr($v, 1, -1);
}

$options['resolve'][substr(self::parseUrl('http://'.$k)['authority'], 2)] = $v;
}
}

Expand All @@ -220,7 +227,14 @@

if ($resolve = $defaultOptions['resolve'] ?? false) {
foreach ($resolve as $k => $v) {
$options['resolve'] += [substr(self::parseUrl('http://'.$k)['authority'], 2) => (string) $v];
if ('' === $v = (string) $v) {
throw new InvalidArgumentException(sprintf('Option "resolve" for host "%s" cannot be empty.', $k));
}
if ('[' === $v[0] && ']' === substr($v, -1) && str_contains($v, ':')) {
$v = substr($v, 1, -1);
}

$options['resolve'] += [substr(self::parseUrl('http://'.$k)['authority'], 2) => $v];

Check failure on line 237 in src/Symfony/Component/HttpClient/HttpClientTrait.php

View workflow job for this annotation

GitHub Actions / Psalm

InvalidArrayOffset

src/Symfony/Component/HttpClient/HttpClientTrait.php:237:40: InvalidArrayOffset: Cannot create offset of type false|string, expecting array-key (see https://psalm.dev/115)

Check failure on line 237 in src/Symfony/Component/HttpClient/HttpClientTrait.php

View workflow job for this annotation

GitHub Actions / Psalm

InvalidArrayOffset

src/Symfony/Component/HttpClient/HttpClientTrait.php:237:40: InvalidArrayOffset: Cannot create offset of type false|string, expecting array-key (see https://psalm.dev/115)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpClient/Internal/AmpListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ public function startTlsNegotiation(Request $request): Promise
public function startSendingRequest(Request $request, Stream $stream): Promise
{
$host = $stream->getRemoteAddress()->getHost();
$this->info['primary_ip'] = $host;

if (false !== strpos($host, ':')) {
$host = '['.$host.']';
}

$this->info['primary_ip'] = $host;
$this->info['primary_port'] = $stream->getRemoteAddress()->getPort();
$this->info['pretransfer_time'] = microtime(true) - $this->info['start_time'];
$this->info['debug'] .= sprintf("* Connected to %s (%s) port %d\n", $request->getUri()->getHost(), $host, $this->info['primary_port']);
Expand Down
20 changes: 16 additions & 4 deletions src/Symfony/Component/HttpClient/Internal/AmpResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,31 @@ public function __construct(array &$dnsMap)

public function resolve(string $name, ?int $typeRestriction = null): Promise
{
if (!isset($this->dnsMap[$name]) || !\in_array($typeRestriction, [Record::A, null], true)) {
$recordType = Record::A;
$ip = $this->dnsMap[$name] ?? null;

if (null !== $ip && str_contains($ip, ':')) {
$recordType = Record::AAAA;
}
if (null === $ip || $recordType !== ($typeRestriction ?? $recordType)) {
return Dns\resolver()->resolve($name, $typeRestriction);
}

return new Success([new Record($this->dnsMap[$name], Record::A, null)]);
return new Success([new Record($ip, $recordType, null)]);
}

public function query(string $name, int $type): Promise
{
if (!isset($this->dnsMap[$name]) || Record::A !== $type) {
$recordType = Record::A;
$ip = $this->dnsMap[$name] ?? null;

if (null !== $ip && str_contains($ip, ':')) {
$recordType = Record::AAAA;
}
if (null === $ip || $recordType !== $type) {
return Dns\resolver()->query($name, $type);
}

return new Success([new Record($this->dnsMap[$name], Record::A, null)]);
return new Success([new Record($ip, $recordType, null)]);
}
}
19 changes: 13 additions & 6 deletions src/Symfony/Component/HttpClient/NativeHttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ public function request(string $method, string $url, array $options = []): Respo
if (str_starts_with($options['bindto'], 'host!')) {
$options['bindto'] = substr($options['bindto'], 5);
}
if ((\PHP_VERSION_ID < 80223 || 80300 <= \PHP_VERSION_ID && 80311 < \PHP_VERSION_ID) && '\\' === \DIRECTORY_SEPARATOR && '[' === $options['bindto'][0]) {
$options['bindto'] = preg_replace('{^\[[^\]]++\]}', '[$0]', $options['bindto']);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the failure on Windows. I verified that on Windows, before PHP 8.2.23, bindto needs double brackets for binding to IPv6 addresses ([[::1]]:0).

I looked at the changelog of PHP 8.2.23 and the only PR that looks related is
php/php-src#15068

I also compiled PHP 8.2.22 on my Ubuntu and the issue doesn't show up, while it does on Windows with the same version.

}
}

$hasContentLength = isset($options['normalized_headers']['content-length']);
Expand Down Expand Up @@ -330,31 +333,35 @@ private static function parseHostPort(array $url, array &$info): array
*/
private static function dnsResolve($host, NativeClientState $multi, array &$info, ?\Closure $onProgress): string
{
if (null === $ip = $multi->dnsCache[$host] ?? null) {
$flag = '' !== $host && '[' === $host[0] && ']' === $host[-1] && str_contains($host, ':') ? \FILTER_FLAG_IPV6 : \FILTER_FLAG_IPV4;
$ip = \FILTER_FLAG_IPV6 === $flag ? substr($host, 1, -1) : $host;

if (filter_var($ip, \FILTER_VALIDATE_IP, $flag)) {
// The host is already an IP address
} elseif (null === $ip = $multi->dnsCache[$host] ?? null) {
$info['debug'] .= "* Hostname was NOT found in DNS cache\n";
$now = microtime(true);

if ('[' === $host[0] && ']' === $host[-1] && filter_var(substr($host, 1, -1), \FILTER_VALIDATE_IP, \FILTER_FLAG_IPV6)) {
$ip = [$host];
} elseif (!$ip = gethostbynamel($host)) {
if (!$ip = gethostbynamel($host)) {
throw new TransportException(sprintf('Could not resolve host "%s".', $host));
}

$info['namelookup_time'] = microtime(true) - ($info['start_time'] ?: $now);
$multi->dnsCache[$host] = $ip = $ip[0];
$info['debug'] .= "* Added {$host}:0:{$ip} to DNS cache\n";
} else {
$info['debug'] .= "* Hostname was found in DNS cache\n";
$host = str_contains($ip, ':') ? "[$ip]" : $ip;
}

$info['namelookup_time'] = microtime(true) - ($info['start_time'] ?: $now);
$info['primary_ip'] = $ip;

if ($onProgress) {
// Notify DNS resolution
$onProgress();
}

return $ip;
return $host;
}

/**
Expand Down
19 changes: 17 additions & 2 deletions src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,18 @@ public function testIdnResolve()
$this->assertSame(200, $response->getStatusCode());
}

public function testIPv6Resolve()
{
TestHttpServer::start(-8087, '[::1]');

$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://symfony.com:8087/', [
'resolve' => ['symfony.com' => '::1'],
]);

$this->assertSame(200, $response->getStatusCode());
}

public function testNotATimeout()
{
$client = $this->getHttpClient(__FUNCTION__);
Expand Down Expand Up @@ -1168,7 +1180,7 @@ public function testBindToPort()

public function testBindToPortV6()
{
TestHttpServer::start(8087, '[::1]');
TestHttpServer::start(-8087);

$client = $this->getHttpClient(__FUNCTION__);
$response = $client->request('GET', 'http://[::1]:8087', ['bindto' => '[::1]:9876']);
Expand All @@ -1177,6 +1189,9 @@ public function testBindToPortV6()
$vars = $response->toArray();

self::assertSame('::1', $vars['REMOTE_ADDR']);
self::assertSame('9876', $vars['REMOTE_PORT']);

if ('\\' !== \DIRECTORY_SEPARATOR) {
self::assertSame('9876', $vars['REMOTE_PORT']);
}
}
}
9 changes: 8 additions & 1 deletion src/Symfony/Contracts/HttpClient/Test/TestHttpServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,15 @@ class TestHttpServer
/**
* @return Process
*/
public static function start(int $port = 8057, $ip = '127.0.0.1')
public static function start(int $port = 8057)
{
if (0 > $port) {
$port = -$port;
$ip = '[::1]';
} else {
$ip = '127.0.0.1';
}

if (isset(self::$process[$port])) {
self::$process[$port]->stop();
} else {
Expand Down
Loading