RedirectorPageController: Added configurable should_respond_404

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 = false;
+
     /**
      * 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');
     }
 

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,41 @@ public function testFileRedirector()
         $page = $this->objFromFixture(RedirectorPage::class, 'file');
         $this->assertStringContainsString("FileTest.txt", $page->Link());
     }
+
+    public function provideUnpublishedTargetPage()
+    {
+        return [
+            'use 200' => [
+                'use404' => false,
+            ],
+            'use 404' => [
+                'use404' => true,
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideUnpublishedTargetPage
+     */
+    public function testUnpublishedTargetPage(bool $use404)
+    {
+        Config::modify()->set(RedirectorPageController::class, 'missing_redirect_is_404', $use404);
+        $redirectorPage = $this->objFromFixture(RedirectorPage::class, 'goodinternal');
+        $targetPage = $this->objFromFixture(SiteTree::class, 'dest');
+        $targetPage->publishSingle();
+        $redirectorPage->publishSingle();
+        $this->assertEquals(Controller::normaliseTrailingSlash('/good-internal'), $redirectorPage->regularLink());
+        $redirectorPageLink = Director::makeRelative($redirectorPage->regularLink());
+
+        // redirector page should give 301 (redirection) status code
+        $response = $this->get($redirectorPageLink);
+        $this->assertEquals(301, $response->getStatusCode());
+
+        // Unpublish the target page of this redirector page.
+        $targetPage->doUnpublish();
+
+        // redirector page should give a 404 or a 200 based on config when there's no page to redirect to
+        $response = $this->get($redirectorPageLink);
+        $this->assertEquals($use404 ? 404 : 200, $response->getStatusCode());
+    }
 }