Skip to content

Commit

Permalink
[!!!][TASK] Change attr value verification to take first match
Browse files Browse the repository at this point in the history
Prior to this change, using the first match out of multiple
`AttrValueInterface` items had to be declared explicitly using
flag `Attr::MATCH_FIRST_VALUE` - thus, all values had to match.

With this change, only the first match is considered (implicit
flag `Attr::MATCH_FIRST_VALUE`). To declare that all value items
have to match, new `Attr::MATCH_ALL_VALUES` has been introduced.

Deprecations:
* `\TYPO3\HtmlSanitizer\Behavior\Attr::MATCH_FIRST_VALUE`
* `\TYPO3\HtmlSanitizer\Behavior\Attr::shallMatchFirstValue()`

New additions:
* `\TYPO3\HtmlSanitizer\Behavior\Attr::MATCH_ALL_VALUES`
* `\TYPO3\HtmlSanitizer\Behavior\Attr::shallMatchAllValues()`

Fixes: #65
  • Loading branch information
ohader committed Oct 12, 2021
1 parent db5844b commit 836ee82
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 20 deletions.
33 changes: 25 additions & 8 deletions src/Behavior/Attr.php
Expand Up @@ -34,9 +34,18 @@ class Attr
* whether the first match in `$values` shall be considered
* as indicator the attribute value is valid in general - if
* this flag is not given, all declared `$values` must match
*
* @deprecated since version 2.0.13 (it is the default behavior now)
*/
public const MATCH_FIRST_VALUE = 2;

/**
* whether all `$values` shall be considered as indicator an
* attribute value is valid - if this flag is not given, the
* first match in `$values` is taken
*/
public const MATCH_ALL_VALUES = 4;

/**
* either specific attribute name (`class`) or a prefix
* (`data-`) in case corresponding NAME_PREFIX flag is set
Expand Down Expand Up @@ -107,11 +116,19 @@ public function isPrefix(): bool
return ($this->flags & self::NAME_PREFIX) === self::NAME_PREFIX;
}

/**
* @deprecated since version 2.0.13 (it is the default behavior now)
*/
public function shallMatchFirstValue(): bool
{
return ($this->flags & self::MATCH_FIRST_VALUE) === self::MATCH_FIRST_VALUE;
}

public function shallMatchAllValues(): bool
{
return ($this->flags & self::MATCH_ALL_VALUES) === self::MATCH_ALL_VALUES;
}

public function matchesName(string $givenName): bool
{
$givenName = strtolower($givenName);
Expand All @@ -125,19 +142,19 @@ public function matchesValue(string $givenValue): bool
if ($this->values === []) {
return true;
}
$matchFirstValue = $this->shallMatchFirstValue();
$matchAllValues = $this->shallMatchAllValues();
foreach ($this->values as $value) {
// + result: false, matchFirstValue: false --> return false
// + result: true, matchFirstValue: true --> return true
// + result: false, matchAllValues: true --> return false
// + result: true, matchAllValues: false --> return true
// (anything else continues processing)
$result = $value->matches($givenValue);
if ($result === $matchFirstValue) {
return $matchFirstValue;
if ($result !== $matchAllValues) {
return !$matchAllValues;
}
}
// + matchFirstValue: false --> return true (since no other match failed before)
// + matchFirstValue: true --> return false (since no other match succeeded before)
return !$matchFirstValue;
// + matchAllValues: true --> return true (since no other match failed before)
// + matchAllValues: false --> return false (since no other match succeeded before)
return $matchAllValues;
}

protected function isDifferentValue(AttrValueInterface $a, AttrValueInterface $b): int
Expand Down
6 changes: 3 additions & 3 deletions src/Builder/CommonBuilder.php
Expand Up @@ -53,16 +53,16 @@ public function __construct()

$this->globalAttrs = $this->createGlobalAttrs();

$this->hrefAttr = (new Behavior\Attr('href', Behavior\Attr::MATCH_FIRST_VALUE))
$this->hrefAttr = (new Behavior\Attr('href'))
->addValues(...($uriAttrValueBuilders['href'] ?? $bluntUriAttrValueBuilder)->getValues());
$this->srcAttr = (new Behavior\Attr('src', Behavior\Attr::MATCH_FIRST_VALUE))
$this->srcAttr = (new Behavior\Attr('src'))
->addValues(...($uriAttrValueBuilders['src'] ?? $bluntUriAttrValueBuilder)->getValues());

// @deprecated not used anymore
$srcsetAttrValueBuilder = (new UriAttrValueBuilder())
->allowLocal(true)
->allowSchemes('http', 'https');
$this->srcsetAttr = (new Behavior\Attr('src', Behavior\Attr::MATCH_FIRST_VALUE))
$this->srcsetAttr = (new Behavior\Attr('src'))
->addValues(...$srcsetAttrValueBuilder->getValues());
}

Expand Down
20 changes: 11 additions & 9 deletions tests/Behavior/AttrTest.php
Expand Up @@ -100,20 +100,22 @@ public static function matchesValueDataProvider(): array
$equalsAorB = new DatasetAttrValue('a', 'b');

return [
[ Attr::MATCH_ALL_VALUES, [$equalsA], 'a', true ],
[ Attr::MATCH_ALL_VALUES, [$equalsA], 'b', false ],
[ Attr::MATCH_ALL_VALUES, [$equalsAorB], 'a', true ],
[ Attr::MATCH_ALL_VALUES, [$equalsA, $equalsAorB], 'a', true ],
[ Attr::MATCH_ALL_VALUES, [$equalsA, $equalsB], 'a', false ], // both `$equalsA` and `$equalsB` must match
[ Attr::MATCH_ALL_VALUES, [$equalsA, $equalsB], 'b', false ], // both `$equalsA` and `$equalsB` must match
[ Attr::MATCH_ALL_VALUES, [$equalsA, $equalsB], 'c', false ],
[ Attr::MATCH_ALL_VALUES, [$equalsA, $equalsB, $equalsAorB], 'c', false ],
[ Attr::BLUNT, [$equalsA], 'a', true ],
[ Attr::BLUNT, [$equalsA], 'b', false ],
[ Attr::BLUNT, [$equalsAorB], 'a', true ],
[ Attr::BLUNT, [$equalsA, $equalsAorB], 'a', true ],
[ Attr::BLUNT, [$equalsA, $equalsB], 'a', false ], // both `$equalsA` and `$equalsB` must match
[ Attr::BLUNT, [$equalsA, $equalsB], 'b', false ], // both `$equalsA` and `$equalsB` must match
[ Attr::BLUNT, [$equalsA, $equalsB], 'a', true ],
[ Attr::BLUNT, [$equalsA, $equalsB], 'b', true ],
[ Attr::BLUNT, [$equalsA, $equalsB], 'c', false ],
[ Attr::BLUNT, [$equalsA, $equalsB, $equalsAorB], 'c', false ],
[ Attr::MATCH_FIRST_VALUE, [$equalsA], 'a', true ],
[ Attr::MATCH_FIRST_VALUE, [$equalsAorB], 'a', true ],
[ Attr::MATCH_FIRST_VALUE, [$equalsA, $equalsAorB], 'a', true ],
[ Attr::MATCH_FIRST_VALUE, [$equalsA, $equalsB], 'a', true ],
[ Attr::MATCH_FIRST_VALUE, [$equalsA, $equalsB], 'b', true ],
[ Attr::MATCH_FIRST_VALUE, [$equalsA, $equalsB], 'c', false ],
[ Attr::MATCH_FIRST_VALUE, [$equalsA, $equalsB, $equalsAorB], 'c', false ],
];
}

Expand Down

0 comments on commit 836ee82

Please sign in to comment.