tonyair
/
silverstripe-cms
mirror of https://github.com/silverstripe/silverstripe-cms
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

NEW Return 404 when redirector page wants to link to missing page (#2663)

This commit is contained in:
Marco Hermo 2024-01-09 11:42:33 +13:00 committed by GitHub
parent 4106e98b70
commit a5d2b3bb32
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 96 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
code/Model/RedirectorPageController.php
View File

@ -3,6 +3,7 @@ namespace SilverStripe\CMS\Model;
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')) {
$this->httpError(404);
}
}
// Fall back to a message about the bad setup
return parent::handleAction($request, 'handleIndex');
}

78
tests/php/Model/RedirectorPageTest.php
View File

@ -8,6 +8,8 @@ use SilverStripe\CMS\Model\RedirectorPageController;
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
@ -59,12 +61,30 @@ class RedirectorPageTest extends FunctionalTest
);
}
public function testEmptyRedirectors()
public function provideEmptyRedirectors()
{
return [
'use 200' => [
'use404' => false,
],
'use 404' => [
'use404' => true,
],
];
}
/**
* @dataProvider provideEmptyRedirectors
*/
public function testEmptyRedirectors(bool $use404)
{
Config::modify()->set(RedirectorPageController::class, 'missing_redirect_is_404', $use404);
// If a redirector page is misconfigured, then its link method will just return the usual
// 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 +93,13 @@ class RedirectorPageTest extends FunctionalTest
// 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();
$this->assertStringContainsString('message-setupWithoutRedirect', $content);
$response = $this->get(Director::makeRelative($page2->Link()));
$content = $response->getBody();
if ($use404) {
$this->assertNull($response->getBody());
} else {
$this->assertStringContainsString('message-setupWithoutRedirect', $content);
}
}
public function testReflexiveAndTransitiveInternalRedirectors()
@ -155,4 +180,51 @@ class RedirectorPageTest extends FunctionalTest
$page = $this->objFromFixture(RedirectorPage::class, 'file');
$this->assertStringContainsString("FileTest.txt", $page->Link());
}
public function provideUnpublishedTarget()
{
return [
'use 200 with sitetree' => [
'use404' => false,
'isFile' => false,
],
'use 404 with sitetree' => [
'use404' => true,
'isFile' => false,
],
'use 200 with file' => [
'use404' => false,
'isFile' => true,
],
'use 404 with file' => [
'use404' => true,
'isFile' => true,
],
];
}
/**
* @dataProvider provideUnpublishedTarget
*/
public function testUnpublishedTarget(bool $use404, bool $isFile)
{
Config::modify()->set(RedirectorPageController::class, 'missing_redirect_is_404', $use404);
$redirectorPage = $this->objFromFixture(RedirectorPage::class, $isFile ? 'file' : 'goodinternal');
$targetModel = $isFile ? $redirectorPage->LinkToFile() : $redirectorPage->LinkTo();
$targetModel->publishSingle();
$redirectorPage->publishSingle();
$this->assertEquals(Controller::normaliseTrailingSlash($isFile ? '/filedirector' : '/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 model of this redirector page.
$targetModel->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());
}
}