Merge pull request #148 from sminnee/safe-urls

NEW: Ensure internal URLs are domain-relative
This commit is contained in:
Ingo Schommer 2018-06-08 13:26:29 +12:00 committed by GitHub
commit 8f7ebf42a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 62 additions and 48 deletions

View File

@ -418,14 +418,15 @@ class DocumentationManifest
*/
protected function stripLinkBase($link)
{
return ltrim(
str_replace(
Config::inst()->get('DocumentationViewer', 'link_base'),
'',
$link
),
'/'
);
// Trim baseURL
$link = preg_replace('/^' . preg_quote(Director::baseURL(), '/') .'/', '', $link);
// Trim link_base
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;
}
/**
* 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.
*
@ -589,7 +603,7 @@ class DocumentationManifest
if ($grabNext && strpos($page['filepath'], $entityBase) !== false) {
return new ArrayData(
array(
'Link' => Controller::join_links(Config::inst()->get('DocumentationViewer', 'link_base'), $url),
'Link' => $this->buildUrl($url),
'Title' => $page['title']
)
);
@ -600,7 +614,7 @@ class DocumentationManifest
} elseif (!$fallback && strpos($page['filepath'], $filepath) !== false) {
$fallback = new ArrayData(
array(
'Link' => Controller::join_links(Config::inst()->get('DocumentationViewer', 'link_base'), $url),
'Link' => $this->buildUrl($url),
'Title' => $page['title'],
'Fallback' => true
)
@ -635,7 +649,7 @@ class DocumentationManifest
if ($previousUrl) {
return new ArrayData(
array(
'Link' => Controller::join_links(Config::inst()->get('DocumentationViewer', 'link_base'), $previousUrl),
'Link' => $this->buildUrl($previousUrl),
'Title' => $previousPage['title']
)
);
@ -684,7 +698,6 @@ class DocumentationManifest
}
$output = new ArrayList();
$base = Config::inst()->get('DocumentationViewer', 'link_base');
$entityPath = $this->normalizeUrl($entityPath);
$recordPath = $this->normalizeUrl($recordPath);
$recordParts = explode('/', trim($recordPath, '/'));
@ -720,7 +733,7 @@ class DocumentationManifest
$output->push(
new ArrayData(
array(
'Link' => Controller::join_links($base, $url, '/'),
'Link' => $this->buildUrl($url),
'Title' => $page['title'],
'LinkingMode' => $mode,
'Summary' => $page['summary'],

View File

@ -62,7 +62,7 @@ class DocumentationViewer extends Controller implements PermissionProvider
/**
* @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/';
@ -567,6 +567,7 @@ class DocumentationViewer extends Controller implements PermissionProvider
public function Link($action = '')
{
$link = Controller::join_links(
Director::baseURL(),
Config::inst()->get('DocumentationViewer', 'link_base'),
$this->getLanguage(),
$action,
@ -786,7 +787,10 @@ class DocumentationViewer extends Controller implements PermissionProvider
*/
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()) {
$base = Controller::join_links(
Director::baseURL(),
Config::inst()->get('DocumentationViewer', 'link_base'),
$this->getLanguage(),
'/'
);
} else {
$base = Controller::join_links(
Director::baseURL(),
Config::inst()->get('DocumentationViewer', 'link_base'),
$this->getLanguage(),
$this->getKey(),
@ -150,8 +152,6 @@ class DocumentationEntity extends ViewableData
);
}
$base = ltrim(str_replace('//', '/', $base), '/');
if ($short && $this->stable) {
return $base;
}

View File

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

View File

@ -42,7 +42,7 @@ class DocumentationPageTest extends SapphireTest
// single layer
$this->assertEquals(
'dev/docs/en/doctest/2.4/test/',
Director::baseURL() . 'dev/docs/en/doctest/2.4/test/',
$page->Link(),
'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/'
);
$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(
$this->entity,
@ -61,7 +61,7 @@ class DocumentationPageTest extends SapphireTest
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()

View File

@ -174,7 +174,7 @@ HTML;
);
$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
);
@ -186,11 +186,11 @@ HTML;
);
$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
);
$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
);
$this->assertContains(
@ -199,12 +199,12 @@ HTML;
);
$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
);
$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
);
@ -215,33 +215,33 @@ HTML;
// @todo this should redirect to /subpage/
$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
);
$this->assertContains(
'[link: absolute index](dev/docs/en/documentationparsertest/2.4/)',
'[link: absolute index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/)',
$result
);
// @todo this should redirect to /
$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
);
$this->assertContains(
'[link: relative index](dev/docs/en/documentationparsertest/2.4/)',
'[link: relative index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/)',
$result
);
$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
);
$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
);
@ -251,27 +251,27 @@ HTML;
);
$this->assertContains(
'[link: absolute index](dev/docs/en/documentationparsertest/2.4/)',
'[link: absolute index](' . Director::baseURL() . 'dev/docs/en/documentationparsertest/2.4/)',
$result
);
$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
);
$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
);
$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
);
$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
);
}

View File

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