From 10b7d10d27fb288f47d98750f8c463320f46bf24 Mon Sep 17 00:00:00 2001 From: Aaron Parecki Date: Sat, 3 Oct 2026 00:17:25 +0000 Subject: [PATCH] Pin every resolved address in safe mode, not just the last curl keeps only the last CURLOPT_RESOLVE entry for a given host and port, so passing one entry per address meant a host with both IPv6 and IPv4 addresses was only ever tried on the last one. A server reachable on just the other address (e.g. listening only on ::1) failed to connect. Pass all the addresses in a single entry instead, which curl has supported since 7.59. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/p3k/HTTP.php | 8 +++++--- src/p3k/HTTP/Pinnable.php | 4 ++-- tests/CurlPinningTest.php | 14 ++++++++++++++ tests/SafeModeTest.php | 2 +- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/p3k/HTTP.php b/src/p3k/HTTP.php index 42bd29f..9c17b91 100644 --- a/src/p3k/HTTP.php +++ b/src/p3k/HTTP.php @@ -103,9 +103,11 @@ class HTTP { $pinnable = $this->_transport instanceof HTTP\Pinnable; if($pinnable) { - $this->_transport->pin_addresses(array_map(function($address) use($check) { - return $check['host'] . ':' . $check['port'] . ':' . (strpos($address, ':') !== false ? '[' . $address . ']' : $address); - }, $check['addresses'])); + // One entry listing every address: curl keeps only the last entry for + // a given host and port, which would drop all but one address + $this->_transport->pin_addresses([$check['host'] . ':' . $check['port'] . ':' . implode(',', array_map(function($address) { + return strpos($address, ':') !== false ? '[' . $address . ']' : $address; + }, $check['addresses']))]); } try { $response = $this->_build_response($this->_send($method, $url, $body, $headers)); diff --git a/src/p3k/HTTP/Pinnable.php b/src/p3k/HTTP/Pinnable.php index 3c5b423..cbbb249 100644 --- a/src/p3k/HTTP/Pinnable.php +++ b/src/p3k/HTTP/Pinnable.php @@ -9,8 +9,8 @@ namespace p3k\HTTP; interface Pinnable { /** - * @param array|null $resolve "host:port:address" entries, as for - * CURLOPT_RESOLVE; null lifts the restriction. + * @param array|null $resolve "host:port:address[,address...]" entries, as + * for CURLOPT_RESOLVE; null lifts the restriction. */ public function pin_addresses($resolve); diff --git a/tests/CurlPinningTest.php b/tests/CurlPinningTest.php index 46e0000..57c8ab0 100644 --- a/tests/CurlPinningTest.php +++ b/tests/CurlPinningTest.php @@ -44,6 +44,20 @@ class CurlPinningTest extends TestCase { $this->assertSame('host=pinned.example:' . self::$port, $response['body']); } + // curl keeps only the last CURLOPT_RESOLVE entry for a host and port, so + // every resolved address has to go in a single entry. Otherwise a host with + // both IPv6 and IPv4 addresses is only tried on the last one. + public function testTriesEveryPinnedAddress() { + $http = new HTTP('test'); + $http->set_safe_mode(true, ['127.0.0.0/8'], function($host) { + // Nothing listens on 127.0.0.3, so this only succeeds if curl can fall back to 127.0.0.1 + return $host === 'pinned.example' ? ['127.0.0.1', '127.0.0.3'] : []; + }); + $response = $http->get('http://pinned.example:' . self::$port . '/'); + $this->assertSame(200, $response['code']); + $this->assertSame('host=pinned.example:' . self::$port, $response['body']); + } + public function testRedirectsAreCheckedHopByHop() { $port = self::$port; $response = $this->http()->get("http://pinned.example:$port/?to=" . rawurlencode("http://127.0.0.1:$port/")); diff --git a/tests/SafeModeTest.php b/tests/SafeModeTest.php index 22dec96..80e5896 100644 --- a/tests/SafeModeTest.php +++ b/tests/SafeModeTest.php @@ -33,7 +33,7 @@ class SafeModeTest extends TestCase { $http = $this->http(['https://b.example/' => [200, '', 'ok']], $transport); $response = $http->get('https://b.example/'); $this->assertSame(200, $response['code']); - $this->assertSame(['b.example:443:93.184.216.35', 'b.example:443:[2606:2800:220:1::]'], $transport->requests[0]['pinned']); + $this->assertSame(['b.example:443:93.184.216.35,[2606:2800:220:1::]'], $transport->requests[0]['pinned']); $this->assertSame(0, $transport->max_redirects); }