Merge branch '5.4' into 6.4

* 5.4: [HttpClient] Resolve hostnames in NoPrivateNetworkHttpClient [security-http] Check owner of persisted remember-me cookie
symfony · Nov 13, 2024 · cb4073c · cb4073c
2 parents 05d88cb + 3b643b8
commit cb4073c
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 15 deletions.
diff --git a/HttpOptions.php b/HttpOptions.php
@@ -156,6 +156,8 @@ public function buffer(bool $buffer): static
     }
 
     /**
+     * @param callable(int, int, array, \Closure|null=):void $callback
+     *
      * @return $this
      */
     public function setOnProgress(callable $callback): static

diff --git a/NativeHttpClient.php b/NativeHttpClient.php
@@ -137,7 +137,15 @@ public function request(string $method, string $url, array $options = []): Respo
 
         if ($onProgress = $options['on_progress']) {
             $maxDuration = 0 < $options['max_duration'] ? $options['max_duration'] : \INF;
-            $onProgress = static function (...$progress) use ($onProgress, &$info, $maxDuration) {
+            $multi = $this->multi;
+            $resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
+                if (null !== $ip) {
+                    $multi->dnsCache[$host] = $ip;
+                }
+
+                return $multi->dnsCache[$host] ?? null;
+            };
+            $onProgress = static function (...$progress) use ($onProgress, &$info, $maxDuration, $resolve) {
                 if ($info['total_time'] >= $maxDuration) {
                     throw new TransportException(sprintf('Max duration was reached for "%s".', implode('', $info['url'])));
                 }
@@ -156,7 +164,7 @@ public function request(string $method, string $url, array $options = []): Respo
                     $lastProgress = $progress ?: $lastProgress;
                 }
 
-                $onProgress($lastProgress[0], $lastProgress[1], $progressInfo);
+                $onProgress($lastProgress[0], $lastProgress[1], $progressInfo, $resolve);
             };
         } elseif (0 < $options['max_duration']) {
             $maxDuration = $options['max_duration'];

diff --git a/NoPrivateNetworkHttpClient.php b/NoPrivateNetworkHttpClient.php
@@ -56,14 +56,27 @@ public function request(string $method, string $url, array $options = []): Respo
 
         $subnets = $this->subnets;
 
-        $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets): void {
+        $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info, ?\Closure $resolve = null) use ($onProgress, $subnets): void {
             static $lastUrl = '';
             static $lastPrimaryIp = '';
 
             if ($info['url'] !== $lastUrl) {
                 $host = trim(parse_url($info['url'], PHP_URL_HOST) ?: '', '[]');
+                $resolve ??= static fn () => null;
+
+                if (($ip = $host)
+                    && !filter_var($ip, \FILTER_VALIDATE_IP, \FILTER_FLAG_IPV6)
+                    && !filter_var($ip, \FILTER_VALIDATE_IP, \FILTER_FLAG_IPV4)
+                    && !$ip = $resolve($host)
+                ) {
+                    if ($ip = @(dns_get_record($host, \DNS_A)[0]['ip'] ?? null)) {
+                        $resolve($host, $ip);
+                    } elseif ($ip = @(dns_get_record($host, \DNS_AAAA)[0]['ipv6'] ?? null)) {
+                        $resolve($host, '['.$ip.']');
+                    }
+                }
 
-                if ($host && IpUtils::checkIp($host, $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
+                if ($ip && IpUtils::checkIp($ip, $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
                     throw new TransportException(sprintf('Host "%s" is blocked for "%s".', $host, $info['url']));
                 }
 

diff --git a/Response/AmpResponse.php b/Response/AmpResponse.php
@@ -88,10 +88,17 @@ public function __construct(AmpClientState $multi, Request $request, array $opti
         $info['max_duration'] = $options['max_duration'];
         $info['debug'] = '';
 
+        $resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
+            if (null !== $ip) {
+                $multi->dnsCache[$host] = $ip;
+            }
+
+            return $multi->dnsCache[$host] ?? null;
+        };
         $onProgress = $options['on_progress'] ?? static function () {};
-        $onProgress = $this->onProgress = static function () use (&$info, $onProgress) {
+        $onProgress = $this->onProgress = static function () use (&$info, $onProgress, $resolve) {
             $info['total_time'] = microtime(true) - $info['start_time'];
-            $onProgress((int) $info['size_download'], ((int) (1 + $info['download_content_length']) ?: 1) - 1, (array) $info);
+            $onProgress((int) $info['size_download'], ((int) (1 + $info['download_content_length']) ?: 1) - 1, (array) $info, $resolve);
         };
 
         $pauseDeferred = new Deferred();

diff --git a/Response/AsyncContext.php b/Response/AsyncContext.php
@@ -161,8 +161,8 @@ public function replaceRequest(string $method, string $url, array $options = [])
         $this->info['previous_info'][] = $info = $this->response->getInfo();
         if (null !== $onProgress = $options['on_progress'] ?? null) {
             $thisInfo = &$this->info;
-            $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use (&$thisInfo, $onProgress) {
-                $onProgress($dlNow, $dlSize, $thisInfo + $info);
+            $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info, ?\Closure $resolve = null) use (&$thisInfo, $onProgress) {
+                $onProgress($dlNow, $dlSize, $thisInfo + $info, $resolve);
             };
         }
         if (0 < ($info['max_duration'] ?? 0) && 0 < ($info['total_time'] ?? 0)) {

diff --git a/Response/AsyncResponse.php b/Response/AsyncResponse.php
@@ -52,8 +52,8 @@ public function __construct(HttpClientInterface $client, string $method, string
 
         if (null !== $onProgress = $options['on_progress'] ?? null) {
             $thisInfo = &$this->info;
-            $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use (&$thisInfo, $onProgress) {
-                $onProgress($dlNow, $dlSize, $thisInfo + $info);
+            $options['on_progress'] = static function (int $dlNow, int $dlSize, array $info, ?\Closure $resolve = null) use (&$thisInfo, $onProgress) {
+                $onProgress($dlNow, $dlSize, $thisInfo + $info, $resolve);
             };
         }
         $this->response = $client->request($method, $url, ['buffer' => false] + $options);

diff --git a/Response/CurlResponse.php b/Response/CurlResponse.php
@@ -118,13 +118,20 @@ public function __construct(CurlClientState $multi, \CurlHandle|string $ch, ?arr
         curl_pause($ch, \CURLPAUSE_CONT);
 
         if ($onProgress = $options['on_progress']) {
+            $resolve = static function (string $host, ?string $ip = null) use ($multi): ?string {
+                if (null !== $ip) {
+                    $multi->dnsCache->hostnames[$host] = $ip;
+                }
+
+                return $multi->dnsCache->hostnames[$host] ?? null;
+            };
             $url = isset($info['url']) ? ['url' => $info['url']] : [];
             curl_setopt($ch, \CURLOPT_NOPROGRESS, false);
-            curl_setopt($ch, \CURLOPT_PROGRESSFUNCTION, static function ($ch, $dlSize, $dlNow) use ($onProgress, &$info, $url, $multi, $debugBuffer) {
+            curl_setopt($ch, \CURLOPT_PROGRESSFUNCTION, static function ($ch, $dlSize, $dlNow) use ($onProgress, &$info, $url, $multi, $debugBuffer, $resolve) {
                 try {
                     rewind($debugBuffer);
                     $debug = ['debug' => stream_get_contents($debugBuffer)];
-                    $onProgress($dlNow, $dlSize, $url + curl_getinfo($ch) + $info + $debug);
+                    $onProgress($dlNow, $dlSize, $url + curl_getinfo($ch) + $info + $debug, $resolve);
                 } catch (\Throwable $e) {
                     $multi->handlesActivity[(int) $ch][] = null;
                     $multi->handlesActivity[(int) $ch][] = $e;

diff --git a/Tests/HttpClientTestCase.php b/Tests/HttpClientTestCase.php
@@ -15,6 +15,7 @@
 use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
 use Symfony\Component\HttpClient\Exception\TransportException;
 use Symfony\Component\HttpClient\Internal\ClientState;
+use Symfony\Component\HttpClient\NoPrivateNetworkHttpClient;
 use Symfony\Component\HttpClient\Response\StreamWrapper;
 use Symfony\Component\Process\Exception\ProcessFailedException;
 use Symfony\Component\Process\Process;
@@ -462,6 +463,28 @@ public function testMisspelledScheme()
         $httpClient->request('GET', 'http:/localhost:8057/');
     }
 
+    public function testNoPrivateNetwork()
+    {
+        $client = $this->getHttpClient(__FUNCTION__);
+        $client = new NoPrivateNetworkHttpClient($client);
+
+        $this->expectException(TransportException::class);
+        $this->expectExceptionMessage('Host "localhost" is blocked');
+
+        $client->request('GET', 'http://localhost:8888');
+    }
+
+    public function testNoPrivateNetworkWithResolve()
+    {
+        $client = $this->getHttpClient(__FUNCTION__);
+        $client = new NoPrivateNetworkHttpClient($client);
+
+        $this->expectException(TransportException::class);
+        $this->expectExceptionMessage('Host "symfony.com" is blocked');
+
+        $client->request('GET', 'http://symfony.com', ['resolve' => ['symfony.com' => '127.0.0.1']]);
+    }
+
     /**
      * @dataProvider getRedirectWithAuthTests
      */

diff --git a/Tests/MockHttpClientTest.php b/Tests/MockHttpClientTest.php
@@ -331,14 +331,17 @@ protected function getHttpClient(string $testCase): HttpClientInterface
 
         switch ($testCase) {
             default:
-                return new MockHttpClient(function (string $method, string $url, array $options) use ($client) {
+                return new MockHttpClient(function (string $method, string $url, array $options) use ($client, $testCase) {
                     try {
                         // force the request to be completed so that we don't test side effects of the transport
                         $response = $client->request($method, $url, ['buffer' => false] + $options);
                         $content = $response->getContent(false);
 
                         return new MockResponse($content, $response->getInfo());
                     } catch (\Throwable $e) {
+                        if (str_starts_with($testCase, 'testNoPrivateNetwork')) {
+                            throw $e;
+                        }
                         $this->fail($e->getMessage());
                     }
                 });

diff --git a/TraceableHttpClient.php b/TraceableHttpClient.php
@@ -55,11 +55,11 @@ public function request(string $method, string $url, array $options = []): Respo
             $content = false;
         }
 
-        $options['on_progress'] = function (int $dlNow, int $dlSize, array $info) use (&$traceInfo, $onProgress) {
+        $options['on_progress'] = function (int $dlNow, int $dlSize, array $info, ?\Closure $resolve = null) use (&$traceInfo, $onProgress) {
             $traceInfo = $info;
 
             if (null !== $onProgress) {
-                $onProgress($dlNow, $dlSize, $info);
+                $onProgress($dlNow, $dlSize, $info, $resolve);
             }
         };