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

Merge pull request #9265 from emteknetnz/feature/noopener 

Add noopener attribute to links with a target
This commit is contained in:
Dylan Wagstaff 2019-10-03 14:42:50 +13:00 committed by GitHub
parent 7dfc35d204 887f198b07
commit 047ac060c4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 81 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
src/Forms/HTMLEditor/HTMLEditorSanitiser.php
View File

@ -4,6 +4,7 @@ namespace SilverStripe\Forms\HTMLEditor;
use DOMAttr;
use DOMElement;
use SilverStripe\Core\Config\Configurable;
use SilverStripe\Core\Injector\Injectable;
use SilverStripe\View\Parsers\HTMLValue;
use stdClass;
@ -17,8 +18,21 @@ use stdClass;
*/
class HTMLEditorSanitiser
{
use Configurable;
use Injectable;
/**
* rel attribute to add to link elements which have a target attribute (usually "_blank")
* this is to done to prevent reverse tabnabbing - see https://www.owasp.org/index.php/Reverse_Tabnabbing
* noopener includes the behaviour we want, though some browsers don't yet support it and rely
* upon using noreferrer instead - see https://caniuse.com/rel-noopener for current browser compatibility
* set this to null if you would like to disable this behaviour
* set this to an empty string if you would like to remove rel attributes that were previously set
*
* @var string
*/
private static $link_rel_value = 'noopener noreferrer';
/** @var [stdClass] - $element => $rule hash for whitelist element rules where the element name isn't a pattern */
protected $elements = array();
/** @var [stdClass] - Sequential list of whitelist element rules where the element name is a pattern */
@ -277,6 +291,7 @@ class HTMLEditorSanitiser
return;
}
$linkRelValue = $this->config()->get('link_rel_value');
$doc = $html->getDocument();
/** @var DOMElement $el */
@ -331,6 +346,32 @@ class HTMLEditorSanitiser
$el->setAttribute($attr, $forced);
}
}
if ($el->tagName === 'a' && $linkRelValue !== null) {
$this->addRelValue($el, $linkRelValue);
}
}
}
/**
* Adds rel="noopener noreferrer" to link elements with a target attribute
*
* @param DOMElement $el
* @param string|null $linkRelValue
*/
private function addRelValue(DOMElement $el, $linkRelValue)
{
// user has checked the checkbox 'open link in new window'
if ($el->getAttribute('target') && $el->getAttribute('rel') !== $linkRelValue) {
if ($linkRelValue !== '') {
$el->setAttribute('rel', $linkRelValue);
} else {
$el->removeAttribute('rel');
}
} elseif ($el->getAttribute('rel') === $linkRelValue && !$el->getAttribute('target')) {
// user previously checked 'open link in new window' and noopener was added,
// now user has unchecked the checkbox so we can remove noopener
$el->removeAttribute('rel');
}
}
}

42
tests/php/Forms/HTMLEditor/HTMLEditorSanitiserTest.php
View File

@ -2,7 +2,7 @@
namespace SilverStripe\Forms\Tests\HTMLEditor;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\FunctionalTest;
use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
use SilverStripe\Forms\HTMLEditor\HTMLEditorSanitiser;
@ -43,7 +43,37 @@ class HTMLEditorSanitiserTest extends FunctionalTest
'<p default1="specific1" force1="specific1">Test</p>',
'<p default1="specific1" force1="force1" default2="default2" force2="force2">Test</p>',
'Default attributes are set when not present in input, forced attributes are always set'
)
),
array(
'a[href|target|rel]',
'<a href="/test" target="_blank">Test</a>',
'<a href="/test" target="_blank" rel="noopener noreferrer">Test</a>',
'noopener rel attribute is added when target attribute is set'
),
array(
'a[href|target|rel]',
'<a href="/test" target="_top">Test</a>',
'<a href="/test" target="_top" rel="noopener noreferrer">Test</a>',
'noopener rel attribute is added when target is _top instead of _blank'
),
array(
'a[href|target|rel]',
'<a href="/test" rel="noopener noreferrer">Test</a>',
'<a href="/test">Test</a>',
'noopener rel attribute is removed when target is not set'
),
array(
'a[href|target|rel]',
'<a href="/test" rel="noopener noreferrer" target="_blank">Test</a>',
'<a href="/test" target="_blank">Test</a>',
'noopener rel attribute is removed when link_rel_value is an empty string'
),
array(
'a[href|target|rel]',
'<a href="/test" target="_blank">Test</a>',
'<a href="/test" target="_blank">Test</a>',
'noopener rel attribute is unchanged when link_rel_value is null'
),
);
$config = HTMLEditorConfig::get('htmleditorsanitisertest');
@ -54,6 +84,14 @@ class HTMLEditorSanitiserTest extends FunctionalTest
$config->setOptions(array('valid_elements' => $validElements));
$sanitiser = new HtmlEditorSanitiser($config);
$value = 'noopener noreferrer';
if (strpos($desc, 'link_rel_value is an empty string') !== false) {
$value = '';
} elseif (strpos($desc, 'link_rel_value is null') !== false) {
$value = null;
}
Config::inst()->set(HTMLEditorSanitiser::class, 'link_rel_value', $value);
$htmlValue = HTMLValue::create($input);
$sanitiser->sanitise($htmlValue);