From d08a9f8c1d58f10c953b0742b2346621727030e5 Mon Sep 17 00:00:00 2001 From: Oliver Hader Date: Wed, 19 Oct 2022 13:13:11 +0200 Subject: [PATCH] [!!!][TASK] Rename variable $node to $domNode * deprecated `\TYPO3\HtmlSanitizer\Behavior\NodeException::withNode(?DOMNode $node)`, use `\TYPO3\HtmlSanitizer\Behavior\NodeException::withDomNode(?DOMNode $domNode)` instead * deprecated `\TYPO3\HtmlSanitizer\Behavior\NodeException::getNode()`, use `\TYPO3\HtmlSanitizer\Behavior\NodeException::getDomNode()` instead Related: #89 --- UPGRADING.md | 8 ++ src/Behavior/NodeException.php | 25 +++++- src/Sanitizer.php | 26 +++--- src/Visitor/AbstractVisitor.php | 8 +- src/Visitor/CommonVisitor.php | 143 ++++++++++++++----------------- src/Visitor/VisitorInterface.php | 8 +- 6 files changed, 116 insertions(+), 102 deletions(-) create mode 100644 UPGRADING.md diff --git a/UPGRADING.md b/UPGRADING.md new file mode 100644 index 0000000..2d45d55 --- /dev/null +++ b/UPGRADING.md @@ -0,0 +1,8 @@ +# TYPO3 HTML Sanitizer notices for upgrading + +## v2.1.0 + +* deprecated `\TYPO3\HtmlSanitizer\Behavior\NodeException::withNode(?DOMNode $node)`, + use `\TYPO3\HtmlSanitizer\Behavior\NodeException::withDomNode(?DOMNode $domNode)` instead +* deprecated `\TYPO3\HtmlSanitizer\Behavior\NodeException::getNode()`, + use `\TYPO3\HtmlSanitizer\Behavior\NodeException::getDomNode()` instead diff --git a/src/Behavior/NodeException.php b/src/Behavior/NodeException.php index cb011b1..7f623a2 100644 --- a/src/Behavior/NodeException.php +++ b/src/Behavior/NodeException.php @@ -27,16 +27,33 @@ public static function create(): self /** * @var DOMNode|null */ - protected $node; + protected $domNode; - public function withNode(?DOMNode $node): self + public function withDomNode(?DOMNode $domNode): self { - $this->node = $node; + $this->domNode = $domNode; return $this; } + /** + * @deprecated since v2.1.0, use withDomNode(?DOMNode $domNode) instead + */ + public function withNode(?DOMNode $domNode): self + { + $this->domNode = $domNode; + return $this; + } + + public function getDomNode(): ?DOMNode + { + return $this->domNode; + } + + /** + * @deprecated since v2.1.0, use getDomNode() instead + */ public function getNode(): ?DOMNode { - return $this->node; + return $this->domNode; } } diff --git a/src/Sanitizer.php b/src/Sanitizer.php index a59a359..c5d12bd 100644 --- a/src/Sanitizer.php +++ b/src/Sanitizer.php @@ -89,24 +89,24 @@ protected function beforeTraverse(): void } } - protected function traverse(DOMNode $node): void + protected function traverse(DOMNode $domNode): void { foreach ($this->visitors as $visitor) { - $result = $visitor->enterNode($node); - $node = $this->replaceNode($node, $result); - if ($node === null) { + $result = $visitor->enterNode($domNode); + $domNode = $this->replaceNode($domNode, $result); + if ($domNode === null) { return; } } - if ($node->hasChildNodes()) { - $this->traverseNodeList($node->childNodes); + if ($domNode->hasChildNodes()) { + $this->traverseNodeList($domNode->childNodes); } foreach ($this->visitors as $visitor) { - $result = $visitor->leaveNode($node); - $node = $this->replaceNode($node, $result); - if ($node === null) { + $result = $visitor->leaveNode($domNode); + $domNode = $this->replaceNode($domNode, $result); + if ($domNode === null) { return; } } @@ -116,13 +116,13 @@ protected function traverse(DOMNode $node): void * Traverses node-list (child-nodes) in reverse(!) order to allow * directly removing child nodes, keeping node-list indexes. * - * @param DOMNodeList $nodeList + * @param DOMNodeList $domNodeList */ - protected function traverseNodeList(DOMNodeList $nodeList): void + protected function traverseNodeList(DOMNodeList $domNodeList): void { - for ($i = $nodeList->length - 1; $i >= 0; $i--) { + for ($i = $domNodeList->length - 1; $i >= 0; $i--) { /** @var DOMNode $item */ - $item = $nodeList->item($i); + $item = $domNodeList->item($i); $this->traverse($item); } } diff --git a/src/Visitor/AbstractVisitor.php b/src/Visitor/AbstractVisitor.php index 21ffe14..5647aa6 100644 --- a/src/Visitor/AbstractVisitor.php +++ b/src/Visitor/AbstractVisitor.php @@ -26,14 +26,14 @@ public function beforeTraverse(Context $context): void { } - public function enterNode(DOMNode $node): ?DOMNode + public function enterNode(DOMNode $domNode): ?DOMNode { - return $node; + return $domNode; } - public function leaveNode(DOMNode $node): ?DOMNode + public function leaveNode(DOMNode $domNode): ?DOMNode { - return $node; + return $domNode; } public function afterTraverse(Context $context): void diff --git a/src/Visitor/CommonVisitor.php b/src/Visitor/CommonVisitor.php index a936221..cff304b 100644 --- a/src/Visitor/CommonVisitor.php +++ b/src/Visitor/CommonVisitor.php @@ -53,174 +53,166 @@ public function beforeTraverse(Context $context): void $this->context = $context; } - public function enterNode(DOMNode $node): ?DOMNode + public function enterNode(DOMNode $domNode): ?DOMNode { - if (!$node instanceof DOMElement) { - return $node; + if (!$domNode instanceof DOMElement) { + return $domNode; } - $tag = $this->behavior->getTag($node->nodeName); + $tag = $this->behavior->getTag($domNode->nodeName); if ($tag === null) { // pass custom elements, in case it has been declared - if ($this->behavior->shallAllowCustomElements() && $this->isCustomElement($node)) { + if ($this->behavior->shallAllowCustomElements() && $this->isCustomElement($domNode)) { $this->log('Allowed custom element {nodeName}', [ 'behavior' => $this->behavior->getName(), - 'nodeName' => $node->nodeName, + 'nodeName' => $domNode->nodeName, ]); - return $node; + return $domNode; } $this->log('Found unexpected tag {nodeName}', [ 'behavior' => $this->behavior->getName(), - 'nodeName' => $node->nodeName, + 'nodeName' => $domNode->nodeName, ]); - return $this->handleInvalidTag($node); + return $this->handleInvalidTag($domNode); } - $node = $this->processAttributes($node, $tag); - $node = $this->processChildren($node, $tag); + $domNode = $this->processAttributes($domNode, $tag); + $domNode = $this->processChildren($domNode, $tag); // completely remove node, in case it is expected to exist with attributes only - if ($node instanceof DOMElement && $node->attributes->length === 0 && $tag->shallPurgeWithoutAttrs()) { + if ($domNode instanceof DOMElement && $domNode->attributes->length === 0 && $tag->shallPurgeWithoutAttrs()) { return null; } - $node = $this->handleMandatoryAttributes($node, $tag); - return $node; + $domNode = $this->handleMandatoryAttributes($domNode, $tag); + return $domNode; } - public function leaveNode(DOMNode $node): ?DOMNode + public function leaveNode(DOMNode $domNode): ?DOMNode { - if (!$node instanceof DOMElement) { - return $node; + if (!$domNode instanceof DOMElement) { + return $domNode; } - $tag = $this->behavior->getTag($node->nodeName); + $tag = $this->behavior->getTag($domNode->nodeName); if ($tag === null) { // pass custom elements, in case it has been declared - if ($this->behavior->shallAllowCustomElements() && $this->isCustomElement($node)) { - return $node; + if ($this->behavior->shallAllowCustomElements() && $this->isCustomElement($domNode)) { + return $domNode; } // unexpected node, that should have been handled in `enterNode` already return null; } // completely remove node, in case it is expected to exist with children only - if (!$this->hasNonEmptyChildren($node) && $tag->shallPurgeWithoutChildren()) { + if (!$this->hasNonEmptyChildren($domNode) && $tag->shallPurgeWithoutChildren()) { return null; } - return $node; + return $domNode; } - protected function processAttributes(?DOMNode $node, Behavior\Tag $tag): ?DOMNode + protected function processAttributes(?DOMNode $domNode, Behavior\Tag $tag): ?DOMNode { - if (!$node instanceof DOMElement) { - return $node; + if (!$domNode instanceof DOMElement) { + return $domNode; } // reverse processing of attributes, // allowing to directly remove attribute nodes - for ($i = $node->attributes->length - 1; $i >= 0; $i--) { + for ($i = $domNode->attributes->length - 1; $i >= 0; $i--) { /** @var DOMAttr $attribute */ - $attribute = $node->attributes->item($i); + $attribute = $domNode->attributes->item($i); try { - $this->processAttribute($node, $tag, $attribute); + $this->processAttribute($domNode, $tag, $attribute); } catch (Behavior\NodeException $exception) { - return $exception->getNode(); + return $exception->getDomNode(); } } - return $node; + return $domNode; } - protected function processChildren(?DOMNode $node, Behavior\Tag $tag): ?DOMNode + protected function processChildren(?DOMNode $domNode, Behavior\Tag $tag): ?DOMNode { - if (!$node instanceof DOMElement) { - return $node; + if (!$domNode instanceof DOMElement) { + return $domNode; } if (!$tag->shallAllowChildren() - && $node->childNodes->length > 0 + && $domNode->childNodes->length > 0 && $this->behavior->shallRemoveUnexpectedChildren() ) { $this->log('Found unexpected children for {nodeName}', [ 'behavior' => $this->behavior->getName(), - 'nodeName' => $node->nodeName, + 'nodeName' => $domNode->nodeName, ]); // reverse processing of children, // allowing to directly remove child nodes - for ($i = $node->childNodes->length - 1; $i >= 0; $i--) { + for ($i = $domNode->childNodes->length - 1; $i >= 0; $i--) { /** @var DOMNode $child */ - $child = $node->childNodes->item($i); - $node->removeChild($child); + $child = $domNode->childNodes->item($i); + $domNode->removeChild($child); } } - return $node; + return $domNode; } /** - * @param DOMElement $node - * @param Behavior\Tag $tag - * @param DOMAttr $attribute * @throws Behavior\NodeException */ - protected function processAttribute(DOMElement $node, Behavior\Tag $tag, DOMAttr $attribute): void + protected function processAttribute(DOMElement $domNode, Behavior\Tag $tag, DOMAttr $attribute): void { $name = strtolower($attribute->name); $attr = $tag->getAttr($name); if ($attr === null || !$attr->matchesValue($attribute->value)) { $this->log('Found invalid attribute {nodeName}.{attrName}', [ 'behavior' => $this->behavior->getName(), - 'nodeName' => $node->nodeName, + 'nodeName' => $domNode->nodeName, 'attrName' => $attribute->nodeName, ]); - $this->handleInvalidAttr($node, $name); + $this->handleInvalidAttr($domNode, $name); } } - protected function handleMandatoryAttributes(?DOMNode $node, Behavior\Tag $tag): ?DOMNode + protected function handleMandatoryAttributes(?DOMNode $domNode, Behavior\Tag $tag): ?DOMNode { - if (!$node instanceof DOMElement) { - return $node; + if (!$domNode instanceof DOMElement) { + return $domNode; } foreach ($tag->getAttrs() as $attr) { - if ($attr->isMandatory() && !$node->hasAttribute($attr->getName())) { + if ($attr->isMandatory() && !$domNode->hasAttribute($attr->getName())) { $this->log('Missing mandatory attribute {nodeName}.{attrName}', [ 'behavior' => $this->behavior->getName(), - 'nodeName' => $node->nodeName, + 'nodeName' => $domNode->nodeName, 'attrName' => $attr->getName(), ]); - return $this->handleInvalidTag($node); + return $this->handleInvalidTag($domNode); } } - return $node; + return $domNode; } - protected function handleInvalidTag(DOMNode $node): ?DOMNode + protected function handleInvalidTag(DOMNode $domNode): ?DOMNode { if ($this->behavior->shallEncodeInvalidTag()) { - return $this->convertToText($node); + return $this->convertToText($domNode); } return null; } /** - * @param DOMNode $node - * @param string $name * @throws Behavior\NodeException */ - protected function handleInvalidAttr(DOMNode $node, string $name): void + protected function handleInvalidAttr(DOMNode $domNode, string $name): void { if ($this->behavior->shallEncodeInvalidAttr()) { - throw Behavior\NodeException::create()->withNode($this->convertToText($node)); + throw Behavior\NodeException::create()->withDomNode($this->convertToText($domNode)); } - if (!$node instanceof DOMElement) { - throw Behavior\NodeException::create()->withNode(null); + if (!$domNode instanceof DOMElement) { + throw Behavior\NodeException::create()->withDomNode(null); } - $node->removeAttribute($name); + $domNode->removeAttribute($name); } /** * Converts node/element to text node, basically disarming tags. * (`` is considered empty, - * albeit `$node->childNodes->length === 1`. + * albeit `$domNode->childNodes->length === 1`. */ - protected function hasNonEmptyChildren(DOMNode $node): bool + protected function hasNonEmptyChildren(DOMNode $domNode): bool { - if ($node->childNodes->length === 0) { + if ($domNode->childNodes->length === 0) { return false; } - for ($i = $node->childNodes->length - 1; $i >= 0; $i--) { - $child = $node->childNodes->item($i); + for ($i = $domNode->childNodes->length - 1; $i >= 0; $i--) { + $child = $domNode->childNodes->item($i); if (!$child instanceof DOMText || trim($child->textContent) !== '' ) { @@ -250,14 +242,11 @@ protected function hasNonEmptyChildren(DOMNode $node): bool /** * Whether given node name can be considered as custom element. * (see https://html.spec.whatwg.org/multipage/custom-elements.html#valid-custom-element-name) - * - * @param DOMNode $node - * @return bool */ - protected function isCustomElement(DOMNode $node): bool + protected function isCustomElement(DOMNode $domNode): bool { - return $node instanceof DOMElement - && preg_match('#^[a-z][a-z0-9]*-.+#', $node->nodeName) > 0; + return $domNode instanceof DOMElement + && preg_match('#^[a-z][a-z0-9]*-.+#', $domNode->nodeName) > 0; } protected function log(string $message, array $context = [], $level = null): void diff --git a/src/Visitor/VisitorInterface.php b/src/Visitor/VisitorInterface.php index 6007a79..f6fdfaa 100644 --- a/src/Visitor/VisitorInterface.php +++ b/src/Visitor/VisitorInterface.php @@ -36,10 +36,10 @@ public function beforeTraverse(Context $context); * + returning same `DOMNode` means "keep node" * + returning different `DOMNode` means "replace node" * - * @param DOMNode $node + * @param DOMNode $domNode * @return DOMNode|null */ - public function enterNode(DOMNode $node): ?DOMNode; + public function enterNode(DOMNode $domNode): ?DOMNode; /** * Executed when leaving a node level. @@ -47,10 +47,10 @@ public function enterNode(DOMNode $node): ?DOMNode; * + returning same `DOMNode` means "keep node" * + returning different `DOMNode` means "replace node" * - * @param DOMNode $node + * @param DOMNode $domNode * @return DOMNode|null */ - public function leaveNode(DOMNode $node): ?DOMNode; + public function leaveNode(DOMNode $domNode): ?DOMNode; /** * Executed after having traversed all nodes