Skip to content
Snippets Groups Projects
Unverified Commit 1d44c8eb authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3151101 by Deepak Goyal, dww, ravi.shankar, shetpooja04, alexpott,...

Issue #3151101 by Deepak Goyal, dww, ravi.shankar, shetpooja04, alexpott, quietone: Replace use of whitelist/blacklist in Filter module
parent 244ccb28
No related branches found
No related tags found
No related merge requests found
......@@ -84,7 +84,7 @@ public static function processFormat(&$element, FormStateInterface $form_state,
// Ensure that children appear as subkeys of this element.
$element['#tree'] = TRUE;
$blacklist = [
$keys_not_to_copy = [
// Make \Drupal::formBuilder()->doBuildForm() regenerate child properties.
'#parents',
'#id',
......@@ -108,7 +108,7 @@ public static function processFormat(&$element, FormStateInterface $form_state,
// Move this element into sub-element 'value'.
unset($element['value']);
foreach (Element::properties($element) as $key) {
if (!in_array($key, $blacklist)) {
if (!in_array($key, $keys_not_to_copy)) {
$element['value'][$key] = $element[$key];
}
}
......
......@@ -304,7 +304,7 @@ public function getHtmlRestrictions() {
// with the existing set, to ensure we only end up with the tags that are
// allowed by *all* filters with an "allowed html" setting.
else {
// Track the union of forbidden (blacklisted) tags.
// Track the union of forbidden tags.
if (isset($new_restrictions['forbidden_tags'])) {
if (!isset($restrictions['forbidden_tags'])) {
$restrictions['forbidden_tags'] = $new_restrictions['forbidden_tags'];
......@@ -314,15 +314,15 @@ public function getHtmlRestrictions() {
}
}
// Track the intersection of allowed (whitelisted) tags.
// Track the intersection of allowed tags.
if (isset($restrictions['allowed'])) {
$intersection = $restrictions['allowed'];
foreach ($intersection as $tag => $attributes) {
// If the current tag is not whitelisted by the new filter, then
// it's outside of the intersection.
// If the current tag is not allowed by the new filter, then it's
// outside of the intersection.
if (!array_key_exists($tag, $new_restrictions['allowed'])) {
// The exception is the asterisk (which applies to all tags): it
// does not need to be whitelisted by every filter in order to be
// does not need to be allowed by every filter in order to be
// used; not every filter needs attribute restrictions on all tags.
if ($tag === '*') {
continue;
......@@ -375,10 +375,10 @@ public function getHtmlRestrictions() {
}
}, NULL);
// Simplification: if we have both a (intersected) whitelist and a (unioned)
// blacklist, then remove any tags from the whitelist that also exist in the
// blacklist. Now the whitelist alone expresses all tag-level restrictions,
// and we can delete the blacklist.
// Simplification: if we have both allowed (intersected) and forbidden
// (unioned) tags, then remove any allowed tags that are also forbidden.
// Once complete, the list of allowed tags expresses all tag-level
// restrictions, and the list of forbidden tags can be removed.
if (isset($restrictions['allowed']) && isset($restrictions['forbidden_tags'])) {
foreach ($restrictions['forbidden_tags'] as $tag) {
if (isset($restrictions['allowed'][$tag])) {
......@@ -388,9 +388,9 @@ public function getHtmlRestrictions() {
unset($restrictions['forbidden_tags']);
}
// Simplification: if the only remaining allowed tag is the asterisk (which
// contains attribute restrictions that apply to all tags), and only
// whitelisting filters were used, then effectively nothing is allowed.
// Simplification: if the only remaining allowed tag is the asterisk
// (which contains attribute restrictions that apply to all tags), and
// there are no forbidden tags, then effectively nothing is allowed.
if (isset($restrictions['allowed'])) {
if (count($restrictions['allowed']) === 1 && array_key_exists('*', $restrictions['allowed']) && !isset($restrictions['forbidden_tags'])) {
$restrictions['allowed'] = [];
......
......@@ -72,10 +72,10 @@ public function getFilterTypes();
*
* @return array|false
* A structured array as returned by FilterInterface::getHTMLRestrictions(),
* but with the intersection of all filters in this text format.
* Will either indicate blacklisting of tags or whitelisting of tags. In
* the latter case, it's possible that restrictions on attributes are also
* stored. FALSE means there are no HTML restrictions.
* but with the intersection of all filters in this text format. The
* restrictions will either forbid or allow a list of tags. In the latter
* case, it's possible that restrictions on attributes are also stored.
* FALSE means there are no HTML restrictions.
*/
public function getHtmlRestrictions();
......
......@@ -113,7 +113,7 @@ public function filterAttributes($text) {
$xpath = new \DOMXPath($html_dom);
foreach ($restrictions['allowed'] as $allowed_tag => $tag_attributes) {
// By default, no attributes are allowed for a tag, but due to the
// globally whitelisted attributes, it is impossible for a tag to actually
// globally allowed attributes, it is impossible for a tag to actually
// completely disallow attributes.
if ($tag_attributes === FALSE) {
$tag_attributes = [];
......@@ -149,23 +149,23 @@ public function filterAttributes($text) {
}
/**
* Filter attributes on an element by name and value according to a whitelist.
* Filters attributes on an element according to a list of allowed values.
*
* @param \DOMElement $element
* The element to be processed.
* @param array $allowed_attributes
* The attributes whitelist as an array of names and values.
* The list of allowed attributes as an array of names and values.
*/
protected function filterElementAttributes(\DOMElement $element, array $allowed_attributes) {
$modified_attributes = [];
foreach ($element->attributes as $name => $attribute) {
// Remove attributes not in the whitelist.
// Remove attributes not in the list of allowed attributes.
$allowed_value = $this->findAllowedValue($allowed_attributes, $name);
if (empty($allowed_value)) {
$modified_attributes[$name] = FALSE;
}
elseif ($allowed_value !== TRUE) {
// Check the attribute values whitelist.
// Check the list of allowed attribute values.
$attribute_values = preg_split('/\s+/', $attribute->value, -1, PREG_SPLIT_NO_EMPTY);
$modified_attributes[$name] = [];
foreach ($attribute_values as $value) {
......@@ -247,8 +247,8 @@ public function getHTMLRestrictions() {
return $this->restrictions;
}
// Parse the allowed HTML setting, and gradually make the whitelist more
// specific.
// Parse the allowed HTML setting, and gradually make the list of allowed
// tags more specific.
$restrictions = ['allowed' => []];
// Make all the tags self-closing, so they will be parsed into direct
......@@ -283,7 +283,7 @@ public function getHTMLRestrictions() {
// but one allowed attribute value that some may be tempted to use
// is specifically nonsensical: the asterisk. A prefix is required for
// allowed attribute values with a wildcard. A wildcard by itself
// would mean whitelisting all possible attribute values. But in that
// would mean allowing all possible attribute values. But in that
// case, one would not specify an attribute value at all.
$allowed_attribute_values = array_filter($allowed_attribute_values, function ($value) use ($star_protector) {
return $value !== '*';
......@@ -311,14 +311,14 @@ public function getHTMLRestrictions() {
// The 'style' and 'on*' ('onClick' etc.) attributes are always forbidden,
// and are removed by Xss::filter().
// The 'lang', and 'dir' attributes apply to all elements and are always
// allowed. The value whitelist for the 'dir' attribute is enforced by
// self::filterAttributes(). Note that those two attributes are in the
// allowed. The list of allowed values for the 'dir' attribute is enforced
// by self::filterAttributes(). Note that those two attributes are in the
// short list of globally usable attributes in HTML5. They are always
// allowed since the correct values of lang and dir may only be known to
// the content author. Of the other global attributes, they are not usually
// added by hand to content, and especially the class attribute can have
// undesired visual effects by allowing content authors to apply any
// available style, so specific values should be explicitly whitelisted.
// available style, so specific values should be explicitly allowed.
// @see http://www.w3.org/TR/html5/dom.html#global-attributes
$restrictions['allowed']['*'] = [
'style' => FALSE,
......
......@@ -406,8 +406,8 @@ public function testLineBreakFilter() {
* @todo It is possible to add script, iframe etc. to allowed tags, but this
* makes HTML filter completely ineffective.
*
* @todo Class, id, name and xmlns should be added to disallowed attributes,
* or better a whitelist approach should be used for that too.
* @todo Class, id, name and xmlns should be added to the list of forbidden
* attributes, or, better yet, use an allowed attribute list.
*/
public function testHtmlFilter() {
// Get FilterHtml object.
......@@ -462,11 +462,11 @@ public function testHtmlFilter() {
$f = (string) $filter->process('<br />', Language::LANGCODE_NOT_SPECIFIED);
$this->assertNormalized($f, '<br />', 'HTML filter should allow self-closing line breaks.');
// All attributes of whitelisted tags are stripped by default.
// All attributes of allowed tags are stripped by default.
$f = (string) $filter->process('<a kitten="cute" llama="awesome">link</a>', Language::LANGCODE_NOT_SPECIFIED);
$this->assertNormalized($f, '<a>link</a>', 'HTML filter should remove attributes that are not explicitly allowed.');
// Now whitelist the "llama" attribute on <a>.
// Now allow the "llama" attribute on <a>.
$filter->setConfiguration([
'settings' => [
'allowed_html' => '<a href llama> <em> <strong> <cite> <blockquote> <code> <ul> <ol> <li> <dl> <dt> <dd> <br>',
......@@ -477,7 +477,7 @@ public function testHtmlFilter() {
$f = (string) $filter->process('<a kitten="cute" llama="awesome">link</a>', Language::LANGCODE_NOT_SPECIFIED);
$this->assertNormalized($f, '<a llama="awesome">link</a>', 'HTML filter keeps explicitly allowed attributes, and removes attributes that are not explicitly allowed.');
// Restrict the whitelisted "llama" attribute on <a> to only allow the value
// Restrict the allowed "llama" attribute on <a> to only allow the value
// "majestical", or "epic".
$filter->setConfiguration([
'settings' => [
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment