From 8bfb89cd6209063e85687192737fcde5dc3d1911 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Mon, 27 Jul 2026 06:31:40 -0400 Subject: [PATCH] fix `is_external` for anchor, mailto and tel links in navs `URL::isExternal()` appended a trailing slash to the whole URL, fragment included, so `http://site.test#anchor` became `http://site.test#anchor/` and no longer matched the site URL. It now strips the query string and fragment before comparing, and treats protocol-relative URLs as external. `URL::makeAbsolute()` prepended the site URL to anything without an `http`/`https` scheme, turning `mailto:` and `tel:` links into internal ones. Those are now left alone. Co-Authored-By: Claude Opus 5 --- src/Facades/Endpoint/URL.php | 23 ++++++++++++++++++++--- tests/Facades/UrlTest.php | 22 ++++++++++++++++++++++ tests/Tags/StructureTagTest.php | 26 ++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/Facades/Endpoint/URL.php b/src/Facades/Endpoint/URL.php index 4eb7182c91c..8bc6e8f0b75 100644 --- a/src/Facades/Endpoint/URL.php +++ b/src/Facades/Endpoint/URL.php @@ -206,12 +206,23 @@ public function makeAbsolute(?string $url): string return self::tidy($url); } + // Protocol-relative URLs and other schemes (mailto:, tel:, etc) already + // point somewhere else, so prepending the site URL would mangle them. + if (Str::startsWith($url, '//') || self::hasScheme($url)) { + return $url; + } + $url = Str::ensureLeft($url, '/'); $url = Str::ensureLeft($url, self::getRequestRootUrl()); return self::tidy($url); } + private function hasScheme(?string $url): bool + { + return (bool) preg_match('/^[a-z][a-z0-9+.\-]*:/i', (string) $url); + } + /** * Get the current URL. */ @@ -262,15 +273,21 @@ public function isExternal(?string $url): bool return false; } - $url = Str::ensureRight($url, '/'); + $cacheKey = $url; + + if (Str::startsWith($url, '//')) { + return self::$externalSiteUrlsCache[$cacheKey] = true; + } if (Str::startsWith($url, ['/', '?', '#'])) { - return self::$externalSiteUrlsCache[$url] = false; + return self::$externalSiteUrlsCache[$cacheKey] = false; } + $url = Str::ensureRight(Str::before(Str::before($url, '#'), '?'), '/'); + $isExternal = ! Str::startsWith($url, Str::ensureRight(Site::current()->absoluteUrl(), '/')); - return self::$externalSiteUrlsCache[$url] = $isExternal; + return self::$externalSiteUrlsCache[$cacheKey] = $isExternal; } /** diff --git a/tests/Facades/UrlTest.php b/tests/Facades/UrlTest.php index 8986f593271..5ec9e778100 100644 --- a/tests/Facades/UrlTest.php +++ b/tests/Facades/UrlTest.php @@ -239,10 +239,20 @@ public function it_determines_external_url() $this->assertTrue(URL::isExternal('http://external-site.com/some-slug')); $this->assertTrue(URL::isExternal('http://external-site.com/some-slug?foo')); $this->assertTrue(URL::isExternal('http://external-site.com/some-slug#anchor')); + $this->assertTrue(URL::isExternal('//external-site.com')); + $this->assertTrue(URL::isExternal('mailto:foo@external-site.com')); + $this->assertTrue(URL::isExternal('tel:+441234567890')); $this->assertFalse(URL::isExternal('http://this-site.com')); $this->assertFalse(URL::isExternal('http://this-site.com/')); $this->assertFalse(URL::isExternal('http://this-site.com/some-slug')); + $this->assertFalse(URL::isExternal('http://this-site.com#anchor')); + $this->assertFalse(URL::isExternal('http://this-site.com/#anchor')); + $this->assertFalse(URL::isExternal('http://this-site.com?query=1')); + $this->assertFalse(URL::isExternal('http://this-site.com/some-slug#anchor')); $this->assertFalse(URL::isExternal('/foo')); + $this->assertFalse(URL::isExternal('/#anchor')); + $this->assertFalse(URL::isExternal('/foo#anchor')); + $this->assertFalse(URL::isExternal('?query=1')); $this->assertFalse(URL::isExternal('#anchor')); $this->assertFalse(URL::isExternal('')); $this->assertFalse(URL::isExternal(null)); @@ -260,7 +270,9 @@ public function it_determines_external_url_when_using_relative_in_config() $this->assertFalse(URL::isExternal('http://absolute-url-resolved-from-request.com')); $this->assertFalse(URL::isExternal('http://absolute-url-resolved-from-request.com/')); $this->assertFalse(URL::isExternal('http://absolute-url-resolved-from-request.com/some-slug')); + $this->assertFalse(URL::isExternal('http://absolute-url-resolved-from-request.com#anchor')); $this->assertFalse(URL::isExternal('/foo')); + $this->assertFalse(URL::isExternal('/#anchor')); $this->assertFalse(URL::isExternal('#anchor')); $this->assertFalse(URL::isExternal('')); $this->assertFalse(URL::isExternal(null)); @@ -591,6 +603,16 @@ public static function absoluteProvider() ]; } + #[Test] + public function it_leaves_urls_pointing_elsewhere_alone_when_making_them_absolute() + { + $this->setSiteValue('en', 'url', 'http://this-site.com/'); + + $this->assertSame('mailto:foo@external-site.com', URL::makeAbsolute('mailto:foo@external-site.com')); + $this->assertSame('tel:+441234567890', URL::makeAbsolute('tel:+441234567890')); + $this->assertSame('//external-site.com', URL::makeAbsolute('//external-site.com')); + } + #[Test] public function making_urls_absolute_ignores_front_controller_in_request_root() { diff --git a/tests/Tags/StructureTagTest.php b/tests/Tags/StructureTagTest.php index 1c23d02ef9d..8b9f1cb1c0d 100644 --- a/tests/Tags/StructureTagTest.php +++ b/tests/Tags/StructureTagTest.php @@ -455,6 +455,32 @@ public function it_sets_is_current_and_is_parent_for_a_nav_when_home_is_an_entry $this->assertEquals('[home][1=parent][1-1=parent][1-1-1=current][1-1-1-1][2][3]', $result); } + #[Test] + public function it_sets_is_external_for_a_nav() + { + $this->setSiteValue('en', 'url', 'http://localhost/'); + + $this->makeNav([ + ['id' => '1', 'title' => 'Entry', 'url' => '/about'], + ['id' => '2', 'title' => 'Homepage anchor', 'url' => '/#anchor'], + ['id' => '3', 'title' => 'Bare anchor', 'url' => '#anchor'], + ['id' => '4', 'title' => 'Page anchor', 'url' => '/about#anchor'], + ['id' => '5', 'title' => 'Query string', 'url' => '/about?query=1'], + ['id' => '6', 'title' => 'Own domain', 'url' => 'http://localhost/about'], + ['id' => '7', 'title' => 'External', 'url' => 'https://statamic.com'], + ['id' => '8', 'title' => 'Protocol relative', 'url' => '//statamic.com'], + ['id' => '9', 'title' => 'Email', 'url' => 'mailto:foo@statamic.com'], + ['id' => '10', 'title' => 'Phone', 'url' => 'tel:+441234567890'], + ]); + + $template = '{{ nav:test }}[{{ id }}{{ if is_external }}=external{{ /if }}]{{ /nav:test }}'; + + $this->assertEquals( + '[1][2][3][4][5][6][7=external][8=external][9=external][10=external]', + (string) Antlers::parse($template, [], true) + ); + } + #[Test] public function it_sets_is_parent_based_on_the_url_too() {