[6.x] Fix infinite recursion on self-referencing redirects

lang/en/validation.php

@@ -205,6 +205,7 @@
     'parent_exceeds_max_depth' => 'This would exceed the maximum depth.',
     'reserved' => 'This is a reserved word.',
     'reserved_field_handle' => 'Field with a handle of :handle is a reserved word.',
+    'link_cannot_reference_itself' => 'An entry cannot link to itself.',
     'unique_entry_value' => 'This value has already been taken.',
     'unique_form_handle' => 'This value has already been taken.',
     'unique_term_value' => 'This value has already been taken.',

src/Fieldtypes/Link.php

@@ -77,6 +77,20 @@ protected function configFieldItems(): array
         ];
     }
 
+    public function rules(): array
+    {
+        return [
+            ...parent::rules(),
+            function ($attribute, $value, $fail) {
+                $parent = $this->field->parent();
+
+                if ($this->field->handle() === 'redirect' && $parent instanceof Entry && $value === 'entry::'.$parent->id()) {
+                    $fail(__('statamic::validation.link_cannot_reference_itself'));
+                }
+            },
+        ];
+    }
+
     public function augment($value)
     {
         $localize = ! $this->canSelectAcrossSites();

src/Http/Responses/DataResponse.php

@@ -82,7 +82,8 @@ protected function getRedirect()
         // they might have set one of the magic values like @child or entry::some-id.
         $redirect = ResolveRedirect::resolve($redirect, $this->data);
 
-        if ($redirect === 404) {
+        // Missing target (404) or a redirect loop (null).
+        if ($redirect === 404 || $redirect === null) {
             throw new NotFoundHttpException;
         }
 

src/Routing/ResolveRedirect.php

@@ -11,6 +11,13 @@
 
 class ResolveRedirect
 {
+    // Redirects followed in a row before we assume a loop (self-redirect, A -> B -> A).
+    private const MAX_DEPTH = 10;
+
+    private static int $depth = 0;
+
+    private static bool $cycleDetected = false;
+
     public function __invoke($redirect, $parent = null, $localize = false)
     {
         return $this->resolve($redirect, $parent, $localize);
@@ -22,6 +29,29 @@ public function resolve($redirect, $parent = null, $localize = false)
             return null;
         }
 
+        // Resolving an entry's url() resolves its own redirect, so a loop never ends.
+        // Flag it so every level unwinds to null and entries fall back to their own URL.
+        if (self::$depth >= self::MAX_DEPTH) {
+            self::$cycleDetected = true;
+
+            return null;
+        }
+
+        self::$depth++;
+
+        try {
+            $result = $this->resolveItem($redirect, $parent, $localize);
+
+            return self::$cycleDetected ? null : $result;
+        } finally {
+            if (--self::$depth === 0) {
+                self::$cycleDetected = false;
+            }
+        }
+    }
+
+    private function resolveItem($redirect, $parent, $localize)
+    {
         if (! $item = $this->item($redirect, $parent, $localize)) {
             return 404;
         }

tests/Fieldtypes/LinkTest.php

@@ -3,6 +3,7 @@
 namespace Tests\Fieldtypes;
 
 use Facades\Statamic\Routing\ResolveRedirect;
+use Illuminate\Support\Facades\Validator;
 use Mockery;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
@@ -46,6 +47,39 @@ public function it_augments_string_to_string()
         $this->assertEquals(['url' => '/foo'], $augmented->toArray());
     }
 
+    #[Test]
+    #[DataProvider('selfReferenceProvider')]
+    public function it_doesnt_allow_an_entry_to_link_to_itself($value, $passes)
+    {
+        $field = new Field('redirect', ['type' => 'link']);
+        $field->setParent((new Entry)->id('abc'));
+
+        $validator = Validator::make(['redirect' => $value], $field->rules());
+
+        $this->assertSame($passes, $validator->passes());
+    }
+
+    public static function selfReferenceProvider()
+    {
+        return [
+            'itself' => ['entry::abc', false],
+            'another entry' => ['entry::def', true],
+            'url' => ['/abc', true],
+            'child' => ['@child', true],
+        ];
+    }
+
+    #[Test]
+    public function it_allows_an_entry_to_link_to_itself_in_a_non_redirect_field()
+    {
+        $field = new Field('cta', ['type' => 'link']);
+        $field->setParent((new Entry)->id('abc'));
+
+        $validator = Validator::make(['cta' => 'entry::abc'], $field->rules());
+
+        $this->assertTrue($validator->passes());
+    }
+
     #[Test]
     public function it_augments_reference_to_object()
     {

tests/FrontendTest.php

@@ -235,6 +235,15 @@ public function drafts_are_not_visible()
         $this->get('/about')->assertStatus(404);
     }
 
+    #[Test]
+    public function an_entry_redirecting_to_itself_is_not_found()
+    {
+        $this->withStandardFakeErrorViews();
+        $this->createPage('about', ['with' => ['redirect' => 'entry::about']]);
+
+        $this->get('/about')->assertStatus(404);
+    }
+
     #[Test]
     public function drafts_are_visible_if_using_live_preview()
     {

tests/Routing/ResolveRedirectTest.php

@@ -14,10 +14,15 @@
 use Statamic\Routing\ResolveRedirect;
 use Statamic\Structures\Page;
 use Statamic\Structures\Pages;
+use Tests\FakesContent;
+use Tests\PreventSavingStacheItemsToDisk;
 use Tests\TestCase;
 
 class ResolveRedirectTest extends TestCase
 {
+    use FakesContent;
+    use PreventSavingStacheItemsToDisk;
+
     #[Test]
     public function it_resolves_standard_redirects()
     {
@@ -280,4 +285,68 @@ public function it_can_resolve_an_arrayable_link()
         $this->assertEquals('/test', $resolver($arrayableLink));
         $this->assertEquals(404, $resolver($arrayableLink));
     }
+
+    #[Test]
+    public function an_entry_redirecting_to_itself_has_no_redirect()
+    {
+        $entry = $this->createPage('about', ['with' => ['redirect' => 'entry::about']]);
+
+        $this->assertNull($entry->redirectUrl());
+        $this->assertFalse($entry->isRedirect());
+        $this->assertEquals('/about', $entry->url());
+    }
+
+    #[Test]
+    public function entries_redirecting_to_each_other_have_no_redirect()
+    {
+        $a = $this->createPage('a', ['with' => ['redirect' => 'entry::b']]);
+        $b = $this->createPage('b', ['with' => ['redirect' => 'entry::a']]);
+
+        $this->assertFalse($a->isRedirect());
+        $this->assertFalse($b->isRedirect());
+        $this->assertEquals('/a', $a->url());
+        $this->assertEquals('/b', $b->url());
+    }
+
+    #[Test]
+    public function a_chain_of_redirects_up_to_the_max_depth_is_followed()
+    {
+        $first = $this->createRedirectChain(10);
+
+        $this->assertTrue($first->isRedirect());
+        $this->assertEquals('/final', $first->url());
+    }
+
+    #[Test]
+    public function a_chain_of_redirects_beyond_the_max_depth_is_treated_as_a_loop()
+    {
+        $first = $this->createRedirectChain(11);
+
+        $this->assertFalse($first->isRedirect());
+        $this->assertEquals('/chain-1', $first->url());
+    }
+
+    #[Test]
+    public function a_detected_loop_does_not_affect_later_redirects()
+    {
+        $loop = $this->createPage('loop', ['with' => ['redirect' => 'entry::loop']]);
+        $other = $this->createPage('other', ['with' => ['redirect' => '/final']]);
+
+        $this->assertFalse($loop->isRedirect());
+        $this->assertTrue($other->isRedirect());
+        $this->assertEquals('/final', $other->url());
+    }
+
+    private function createRedirectChain(int $length)
+    {
+        foreach (range(1, $length) as $i) {
+            $redirect = $i === $length ? '/final' : 'entry::chain-'.($i + 1);
+
+            $entry = $this->createPage('chain-'.$i, ['with' => ['redirect' => $redirect]]);
+
+            $first ??= $entry;
+        }
+
+        return $first;
+    }
 }