diff --git a/code/Model/RedirectorPageController.php b/code/Model/RedirectorPageController.php index 9d53b87dcc..637e2cfcff 100644 --- 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 index 5226fe966c..97b2f04b1e 100644 --- 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()); + } }