RedirectorPageController: Added configurable should_respond_404

silverstripe · Dec 22, 2023 · 14c34cf · 14c34cf
1 parent fb1cc3c
commit 14c34cf
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 2 deletions.
diff --git a/code/Model/RedirectorPageController.php b/code/Model/RedirectorPageController.php
@@ -3,6 +3,7 @@
 
 use SilverStripe\Control\HTTPRequest;
 use PageController;
+use SilverStripe\Control\HTTPResponse_Exception;
 
 /**
  * Controller for the {@link RedirectorPage}.
@@ -11,19 +12,38 @@ class RedirectorPageController extends PageController
 {
     private static $allowed_actions = ['index'];
 
+    /**
+     * Should respond with HTTP 404 if the page or file being redirected to is missing
+     */
+    private static bool $missing_redirect_is_404 = true;
+
     /**
      * Check we don't already have a redirect code set
      *
      * @param  HTTPRequest $request
      * @return \SilverStripe\Control\HTTPResponse
+     * @throws HTTPResponse_Exception
      */
     public function index(HTTPRequest $request)
     {
         /** @var RedirectorPage $page */
         $page = $this->data();
+
+        // Redirect if we can
         if (!$this->getResponse()->isFinished() && $link = $page->redirectionLink()) {
-            $this->redirect($link, 301);
+            return $this->redirect($link, 301);
         }
+
+        // Respond with 404 if redirecting to a missing file or page
+        if (($this->RedirectionType === 'Internal' && !$page->LinkTo()?->exists())
+            || ($this->RedirectionType === 'File' && !$page->LinkToFile()?->exists())
+        ) {
+            if (static::config()->get('missing_redirect_is_404')) {
+                return $this->httpError(404);
+            }
+        }
+
+        // Fall back to a message about the bad setup
         return parent::handleAction($request, 'handleIndex');
     }
 

diff --git a/tests/php/Model/RedirectorPageTest.php b/tests/php/Model/RedirectorPageTest.php
@@ -8,6 +8,8 @@
 use SilverStripe\Control\Director;
 use SilverStripe\Assets\File;
 use SilverStripe\Assets\Dev\TestAssetStore;
+use SilverStripe\Control\Controller;
+use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\FunctionalTest;
 
 class RedirectorPageTest extends FunctionalTest
@@ -65,6 +67,8 @@ public function testEmptyRedirectors()
         // URLSegment-generated value
         $page1 = $this->objFromFixture(RedirectorPage::class, 'badexternal');
         $this->assertEquals('/bad-external', $page1->Link());
+        $response = $this->get($page1->Link());
+        $this->assertEquals(200, $response->getStatusCode());
 
         // An error message will be shown if you visit it
         $content = $this->get(Director::makeRelative($page1->Link()))->getBody();
@@ -73,8 +77,10 @@ public function testEmptyRedirectors()
         // This also applies for internal links
         $page2 = $this->objFromFixture(RedirectorPage::class, 'badinternal');
         $this->assertEquals('/bad-internal', $page2->Link());
-        $content = $this->get(Director::makeRelative($page2->Link()))->getBody();
+        $response = $this->get(Director::makeRelative($page2->Link()));
+        $content = $response->getBody();
         $this->assertStringContainsString('message-setupWithoutRedirect', $content);
+        $this->assertEquals(200, $response->getStatusCode());
     }
 
     public function testReflexiveAndTransitiveInternalRedirectors()
@@ -155,4 +161,33 @@ public function testFileRedirector()
         $page = $this->objFromFixture(RedirectorPage::class, 'file');
         $this->assertStringContainsString("FileTest.txt", $page->Link());
     }
+
+    public function testOnUnpublishedTargetPage()
+    {
+        Config::modify()->set(RedirectorPageController::class, 'missing_redirect_is_404', false);
+        $redirectorPage = $this->objFromFixture(RedirectorPage::class, 'goodinternal');
+        $this->assertEquals(Controller::normaliseTrailingSlash('/redirection-dest'), $redirectorPage->Link());
+        $redirectorPageLink = Director::makeRelative($redirectorPage->Link());
+        $targetPage = $this->objFromFixture(SiteTree::class, 'dest');
+        $this->assertTrue($targetPage->isPublished());
+        $this->assertTrue($redirectorPage->isPublished());
+
+        // redirector page should display ???
+        $response = $this->get($redirectorPageLink);
+        $this->assertEquals(200, $response->getStatusCode());
+
+        // Unpublish the target page of this redirector page.
+        $targetPage->doUnpublish();
+
+        // redirector page should display ???
+        $response = $this->get($redirectorPageLink);
+        $this->assertEquals(200, $response->getStatusCode());
+
+        // Override to display 404
+        Config::modify()->set(RedirectorPageController::class, 'missing_redirect_is_404', true);
+
+        // redirector page should show 404
+        $response = $this->get($redirectorPageLink);
+        $this->assertEquals(404, $response->getStatusCode());
+    }
 }