NEW: Ensure internal URLs are domain-relative

Domain relative URLs (i.e. those starting with “/“) are safer; less
likely to break webcrawlers. Other parts of SilverStripe output URLs
in this form by default.
This commit is contained in:
Sam Minnee 2018-06-07 18:02:19 +12:00
parent 691f5dddb8
commit f7377865d4
7 changed files with 62 additions and 48 deletions

View File

@ -418,14 +418,15 @@ class DocumentationManifest
*/ */
protected function stripLinkBase($link) protected function stripLinkBase($link)
{ {
return ltrim( // Trim baseURL
str_replace( $link = preg_replace('/^' . preg_quote(Director::baseURL(), '/') .'/', '', $link);
Config::inst()->get('DocumentationViewer', 'link_base'),
'', // Trim link_base
$link if ($linkBase = Config::inst()->get('DocumentationViewer', 'link_base')) {
), $link = preg_replace('/^' . preg_quote($linkBase, '/') .'\/?/', '', $link);
'/' }
);
return $link;
} }
/** /**
@ -569,6 +570,19 @@ class DocumentationManifest
return $output; return $output;
} }
/**
* Create a clean domain-relative URL form a docuetn URL
*/
protected function buildUrl($url)
{
return Controller::join_links(
Director::baseURL(),
Config::inst()->get('DocumentationViewer', 'link_base'),
$url,
'/'
);
}
/** /**
* Determine the next page from the given page. * Determine the next page from the given page.
* *
@ -589,7 +603,7 @@ class DocumentationManifest
if ($grabNext && strpos($page['filepath'], $entityBase) !== false) { if ($grabNext && strpos($page['filepath'], $entityBase) !== false) {
return new ArrayData( return new ArrayData(
array( array(
'Link' => Controller::join_links(Config::inst()->get('DocumentationViewer', 'link_base'), $url), 'Link' => $this->buildUrl($url),
'Title' => $page['title'] 'Title' => $page['title']
) )
); );
@ -600,7 +614,7 @@ class DocumentationManifest
} elseif (!$fallback && strpos($page['filepath'], $filepath) !== false) { } elseif (!$fallback && strpos($page['filepath'], $filepath) !== false) {
$fallback = new ArrayData( $fallback = new ArrayData(
array( array(
'Link' => Controller::join_links(Config::inst()->get('DocumentationViewer', 'link_base'), $url), 'Link' => $this->buildUrl($url),
'Title' => $page['title'], 'Title' => $page['title'],
'Fallback' => true 'Fallback' => true
) )
@ -635,7 +649,7 @@ class DocumentationManifest
if ($previousUrl) { if ($previousUrl) {
return new ArrayData( return new ArrayData(
array( array(
'Link' => Controller::join_links(Config::inst()->get('DocumentationViewer', 'link_base'), $previousUrl), 'Link' => $this->buildUrl($previousUrl),
'Title' => $previousPage['title'] 'Title' => $previousPage['title']
) )
); );
@ -684,7 +698,6 @@ class DocumentationManifest
} }
$output = new ArrayList(); $output = new ArrayList();
$base = Config::inst()->get('DocumentationViewer', 'link_base');
$entityPath = $this->normalizeUrl($entityPath); $entityPath = $this->normalizeUrl($entityPath);
$recordPath = $this->normalizeUrl($recordPath); $recordPath = $this->normalizeUrl($recordPath);
$recordParts = explode('/', trim($recordPath, '/')); $recordParts = explode('/', trim($recordPath, '/'));
@ -720,7 +733,7 @@ class DocumentationManifest
$output->push( $output->push(
new ArrayData( new ArrayData(
array( array(
'Link' => Controller::join_links($base, $url, '/'), 'Link' => $this->buildUrl($url),
'Title' => $page['title'], 'Title' => $page['title'],
'LinkingMode' => $mode, 'LinkingMode' => $mode,
'Summary' => $page['summary'], 'Summary' => $page['summary'],

View File

@ -62,7 +62,7 @@ class DocumentationViewer extends Controller implements PermissionProvider
/** /**
* @config * @config
* *
* @var string same as the routing pattern set through Director::addRules(). * @var string Site-relative root URL of documentation. Same as the routing pattern set through Director::addRules().
*/ */
private static $link_base = 'dev/docs/'; private static $link_base = 'dev/docs/';
@ -567,6 +567,7 @@ class DocumentationViewer extends Controller implements PermissionProvider
public function Link($action = '') public function Link($action = '')
{ {
$link = Controller::join_links( $link = Controller::join_links(
Director::baseURL(),
Config::inst()->get('DocumentationViewer', 'link_base'), Config::inst()->get('DocumentationViewer', 'link_base'),
$this->getLanguage(), $this->getLanguage(),
$action, $action,
@ -786,7 +787,10 @@ class DocumentationViewer extends Controller implements PermissionProvider
*/ */
public function getDocumentationBaseHref() public function getDocumentationBaseHref()
{ {
return Config::inst()->get('DocumentationViewer', 'link_base'); return Controller::join_links(
Director::baseURL(),
Config::inst()->get('DocumentationViewer', 'link_base')
);
} }
/** /**

View File

@ -137,12 +137,14 @@ class DocumentationEntity extends ViewableData
{ {
if ($this->getIsDefaultEntity()) { if ($this->getIsDefaultEntity()) {
$base = Controller::join_links( $base = Controller::join_links(
Director::baseURL(),
Config::inst()->get('DocumentationViewer', 'link_base'), Config::inst()->get('DocumentationViewer', 'link_base'),
$this->getLanguage(), $this->getLanguage(),
'/' '/'
); );
} else { } else {
$base = Controller::join_links( $base = Controller::join_links(
Director::baseURL(),
Config::inst()->get('DocumentationViewer', 'link_base'), Config::inst()->get('DocumentationViewer', 'link_base'),
$this->getLanguage(), $this->getLanguage(),
$this->getKey(), $this->getKey(),
@ -150,8 +152,6 @@ class DocumentationEntity extends ViewableData
); );
} }
$base = ltrim(str_replace('//', '/', $base), '/');
if ($short && $this->stable) { if ($short && $this->stable) {
return $base; return $base;
} }

View File

@ -262,12 +262,9 @@ class DocumentationPage extends ViewableData
*/ */
public function Link($short = false) public function Link($short = false)
{ {
return ltrim( return Controller::join_links(
Controller::join_links( $this->entity->Link($short),
$this->entity->Link($short), $this->getRelativeLink()
$this->getRelativeLink()
),
'/'
); );
} }

View File

@ -42,7 +42,7 @@ class DocumentationPageTest extends SapphireTest
// single layer // single layer
$this->assertEquals( $this->assertEquals(
'dev/docs/en/doctest/2.4/test/', Director::baseURL() . 'dev/docs/en/doctest/2.4/test/',
$page->Link(), $page->Link(),
'The page link should have no extension and have a language' 'The page link should have no extension and have a language'
); );
@ -53,7 +53,7 @@ class DocumentationPageTest extends SapphireTest
DOCSVIEWER_PATH . '/tests/docs/en/sort/' DOCSVIEWER_PATH . '/tests/docs/en/sort/'
); );
$this->assertEquals('dev/docs/en/doctest/2.4/sort/', $page->Link()); $this->assertEquals(Director::baseURL() . 'dev/docs/en/doctest/2.4/sort/', $page->Link());
$page = new DocumentationFolder( $page = new DocumentationFolder(
$this->entity, $this->entity,
@ -61,7 +61,7 @@ class DocumentationPageTest extends SapphireTest
DOCSVIEWER_PATH . '/tests/docs/en/sort/1-basic.md' DOCSVIEWER_PATH . '/tests/docs/en/sort/1-basic.md'
); );
$this->assertEquals('dev/docs/en/doctest/2.4/sort/basic/', $page->Link()); $this->assertEquals(Director::baseURL() . 'dev/docs/en/doctest/2.4/sort/basic/', $page->Link());
} }
public function testGetBreadcrumbTitle() public function testGetBreadcrumbTitle()

View File

@ -174,7 +174,7 @@ HTML;
); );
$this->assertContains( $this->assertContains(
'[link: subfolder index](dev/docs/en/documentationparsertest/2.4/subfolder/)', '[link: subfolder index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/subfolder/)',
$result $result
); );
@ -186,11 +186,11 @@ HTML;
); );
$this->assertContains( $this->assertContains(
'[link: subfolder index](dev/docs/en/documentationparsertest/2.4/subfolder/)', '[link: subfolder index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/subfolder/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: subfolder page](dev/docs/en/documentationparsertest/2.4/subfolder/subpage/)', '[link: subfolder page](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/subfolder/subpage/)',
$result $result
); );
$this->assertContains( $this->assertContains(
@ -199,12 +199,12 @@ HTML;
); );
$this->assertContains( $this->assertContains(
'[link: with anchor](dev/docs/en/documentationparsertest/2.4/test/#anchor)', '[link: with anchor](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/test/#anchor)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: relative anchor](dev/docs/en/documentationparsertest/2.4/test/#relative-anchor)', '[link: relative anchor](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/test/#relative-anchor)',
$result $result
); );
@ -215,33 +215,33 @@ HTML;
// @todo this should redirect to /subpage/ // @todo this should redirect to /subpage/
$this->assertContains( $this->assertContains(
'[link: relative](dev/docs/en/documentationparsertest/2.4/subfolder/subpage.md/)', '[link: relative](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/subfolder/subpage.md/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: absolute index](dev/docs/en/documentationparsertest/2.4/)', '[link: absolute index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/)',
$result $result
); );
// @todo this should redirect to / // @todo this should redirect to /
$this->assertContains( $this->assertContains(
'[link: absolute index with name](dev/docs/en/documentationparsertest/2.4/index/)', '[link: absolute index with name](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/index/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: relative index](dev/docs/en/documentationparsertest/2.4/)', '[link: relative index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: relative parent page](dev/docs/en/documentationparsertest/2.4/test/)', '[link: relative parent page](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/test/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: absolute parent page](dev/docs/en/documentationparsertest/2.4/test/)', '[link: absolute parent page](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/test/)',
$result $result
); );
@ -251,27 +251,27 @@ HTML;
); );
$this->assertContains( $this->assertContains(
'[link: absolute index](dev/docs/en/documentationparsertest/2.4/)', '[link: absolute index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: relative index](dev/docs/en/documentationparsertest/2.4/subfolder/)', '[link: relative index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/subfolder/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: relative parent page](dev/docs/en/documentationparsertest/2.4/subfolder/subpage/)', '[link: relative parent page](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/subfolder/subpage/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: relative grandparent page](dev/docs/en/documentationparsertest/2.4/test/)', '[link: relative grandparent page](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/test/)',
$result $result
); );
$this->assertContains( $this->assertContains(
'[link: absolute page](dev/docs/en/documentationparsertest/2.4/test/)', '[link: absolute page](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/test/)',
$result $result
); );
} }

View File

@ -157,9 +157,9 @@ class DocumentationViewerTest extends FunctionalTest
$response = $v->handleRequest(new SS_HTTPRequest('GET', 'en/doc_test/2.3/'), DataModel::inst()); $response = $v->handleRequest(new SS_HTTPRequest('GET', 'en/doc_test/2.3/'), DataModel::inst());
$expected = array( $expected = array(
'dev/docs/en/doc_test/2.3/sort/' => 'Sort', Director::baseURL() . 'dev/docs/en/doc_test/2.3/sort/' => 'Sort',
'dev/docs/en/doc_test/2.3/subfolder/' => 'Subfolder', Director::baseURL() . 'dev/docs/en/doc_test/2.3/subfolder/' => 'Subfolder',
'dev/docs/en/doc_test/2.3/test/' => 'Test' Director::baseURL() . 'dev/docs/en/doc_test/2.3/test/' => 'Test'
); );
$actual = $v->getMenu()->first()->Children->map('Link', 'Title'); $actual = $v->getMenu()->first()->Children->map('Link', 'Title');
@ -174,9 +174,9 @@ class DocumentationViewerTest extends FunctionalTest
// menu should contain all the english entities // menu should contain all the english entities
$expected = array( $expected = array(
'dev/docs/en/doc_test/2.4/' => 'Doc Test', Director::baseURL() . 'dev/docs/en/doc_test/2.4/' => 'Doc Test',
'dev/docs/en/documentationvieweraltmodule1/' => 'DocumentationViewerAltModule1', Director::baseURL() . 'dev/docs/en/documentationvieweraltmodule1/' => 'DocumentationViewerAltModule1',
'dev/docs/en/documentationvieweraltmodule2/' => 'DocumentationViewerAltModule2' Director::baseURL() . 'dev/docs/en/documentationvieweraltmodule2/' => 'DocumentationViewerAltModule2'
); );
$this->assertEquals($expected, $v->getMenu()->map('Link', 'Title')); $this->assertEquals($expected, $v->getMenu()->map('Link', 'Title'));