diff --git a/Magento2/Rector/Src/AddHtmlEscaperToOutput.php b/Magento2/Rector/Src/AddHtmlEscaperToOutput.php new file mode 100644 index 00000000..d58aed72 --- /dev/null +++ b/Magento2/Rector/Src/AddHtmlEscaperToOutput.php @@ -0,0 +1,186 @@ +exprs[0]; + + if ($echoContent instanceof Node\Expr\Ternary) { + + if (!$this->hasNoEscapeAttribute($echoContent->if) && $this->canEscapeOutput($echoContent->if)) { + $node->exprs[0]->if = new Node\Expr\MethodCall( + new Node\Expr\Variable('escaper'), + 'escapeHtml', + [new Node\Arg($echoContent->if)] + ); + + } + + if (!$this->hasNoEscapeAttribute($echoContent->else) && $this->canEscapeOutput($echoContent->else)) { + $node->exprs[0]->else = new Node\Expr\MethodCall( + new Node\Expr\Variable('escaper'), + 'escapeHtml', + [new Node\Arg($echoContent->else)] + ); + + } + return $node; + + } + + if ($this->hasNoEscapeAttribute($echoContent)) { + return null; + } + + if ($this->canEscapeOutput($echoContent)) { + + $node->exprs[0] = new Node\Expr\MethodCall( + new Node\Expr\Variable('escaper'), + 'escapeHtml', + [new Node\Arg($echoContent)] + ); + return $node; + } + + return null; + } + + /** + * We check if content of the echo should be escaped + * + * @param Node $echoContent + * @return bool + */ + private function canEscapeOutput(Node $echoContent):bool + { + if ($echoContent instanceof Node\Expr\Variable) { + return true; + } + + if ($echoContent instanceof Node\Expr\FuncCall + && !$this->willFunctionReturnSafeOutput($echoContent)) { + + // if string passed to __() contains html don't do anthing + if ($echoContent->name == '__' && $this->stringContainsHtml($echoContent->args[0]->value->value)) { + return false; + } + + return true; + } + + // if the echo already has escapeHtml called on $block or escaper, don't do anthing + if ($echoContent instanceof Node\Expr\MethodCall && + !$this->methodReturnsValidHtmlOrUrl($echoContent) + ) { + + // if the method is part of secureRenderer don't do anything + if ($echoContent->var->name === 'secureRenderer') { + return false; + } + + return true; + + } + + return false; + } + + /** + * We do not want to escape __() output if the inner content contains html + * + * @param string $str + * @return bool + */ + private function stringContainsHtml(string $str) + { + return strlen($str) !== strlen(strip_tags($str)); + } + + /** + * If the developer has marked the output as noEscape by using the @noEscape we want to leave that code as it is + * + * @param Node $echoContent + */ + private function hasNoEscapeAttribute(Node $echoContent):bool + { + + $comments = $echoContent->getComments(); + foreach ($comments as $comment) { + if (stripos($comment->getText(), '@noEscape') !== false) { + return true; + } + } + return false; + } + + /** + * If method contains the keyword HTML we assume developer intends to output html + * + * @param Node\Expr\MethodCall $echoContent + */ + private function methodReturnsValidHtmlOrUrl(Node\Expr\MethodCall $echoContent):bool + { + return stripos($echoContent->name->name, 'Html') !== false + || stripos($echoContent->name->name, 'Url') !== false; + } + + /** + * Some php function return safe output. count, strip_tags need not be escaped. + * + * @param Node\Expr\FuncCall $funcNode + */ + private function willFunctionReturnSafeOutput(Node\Expr\FuncCall $funcNode):bool + { + //@TODO did not handle things like $callback(); + $funcName = $funcNode->name->toCodeString(); + return in_array($funcName, $this->_phpFunctionsToIgnore); + } + + /** + * @inheritDoc + */ + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Add escaper methods like escapeHtml to html output', + [ + new CodeSample( + 'echo $productName', + 'echo $escaper->escapeHtml($productName)' + ), + ] + ); + } +} diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/AddHtmlEscaperToOutputTest.php b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/AddHtmlEscaperToOutputTest.php new file mode 100644 index 00000000..6719752d --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/AddHtmlEscaperToOutputTest.php @@ -0,0 +1,36 @@ +doTestFile($fileInfo); + } + + /** + * @return Iterator + */ + public function provideData(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/already-escaped-ouput.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/already-escaped-ouput.php.inc new file mode 100644 index 00000000..555eddf4 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/already-escaped-ouput.php.inc @@ -0,0 +1,11 @@ +escapeHtml($productName); + +?> +----- +escapeHtml($productName); + +?> diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/complex-phtml-code-mix.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/complex-phtml-code-mix.php.inc new file mode 100644 index 00000000..28dd2cc2 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/complex-phtml-code-mix.php.inc @@ -0,0 +1,27 @@ +getAllItems(); +?> +
+ 0): ?> +
%1 items found', count($items)) ?>
+ + getName(); ?> + + + + +
+----- +getAllItems(); +?> +
+ 0): ?> +
%1 items found', count($items)) ?>
+ + escapeHtml($item->getName()); ?> + + + escapeHtml(__('No items found.')); ?> + +
diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/function-call.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/function-call.php.inc new file mode 100644 index 00000000..32107eb3 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/function-call.php.inc @@ -0,0 +1,11 @@ +getName(); +echo __('Something'); +?> +----- +escapeHtml($customer->getName()); +echo $escaper->escapeHtml(__('Something')); +?> diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/message-string-contains-html.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/message-string-contains-html.php.inc new file mode 100644 index 00000000..9335a5e1 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/message-string-contains-html.php.inc @@ -0,0 +1 @@ +
%1 items found',count($items))?>
diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/method-that-return-html.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/method-that-return-html.php.inc new file mode 100644 index 00000000..973e025f --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/method-that-return-html.php.inc @@ -0,0 +1,3 @@ +getChildHtml(); ?> +getToolBarHtml();?> +getChildBlock('toolbar')->setIsBottom(true)->toHtml() ?> diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/simple-echo.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/simple-echo.php.inc new file mode 100644 index 00000000..8bb9d442 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/simple-echo.php.inc @@ -0,0 +1,11 @@ + +----- +escapeHtml($productName); + +?> diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-escaped-output-using-secure-renderer.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-escaped-output-using-secure-renderer.php.inc new file mode 100644 index 00000000..45db5f32 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-escaped-output-using-secure-renderer.php.inc @@ -0,0 +1,4 @@ +renderStyleAsTag( + $position, + 'product-item-info_' . $_product->getId() . ' div.actions-primary' +) ?> diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-output-of-count-function.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-output-of-count-function.php.inc new file mode 100644 index 00000000..7dd0e4c6 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-output-of-count-function.php.inc @@ -0,0 +1,3 @@ + + + diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-with-no-escape-annotation.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-with-no-escape-annotation.php.inc new file mode 100644 index 00000000..2696f4c3 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/skip-with-no-escape-annotation.php.inc @@ -0,0 +1,3 @@ +getProductPrice($_product) ?> + + diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/ternary-condition.php.inc b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/ternary-condition.php.inc new file mode 100644 index 00000000..20d7451d --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/Fixture/ternary-condition.php.inc @@ -0,0 +1,11 @@ +canShowInfo() ? $escaper->escapeHtml($block->getInfo()) : __('Nothing to show'); + +?> +----- +canShowInfo() ? $escaper->escapeHtml($block->getInfo()) : $escaper->escapeHtml(__('Nothing to show')); + +?> diff --git a/Magento2/Rector/Tests/AddHtmlEscaperToOutput/config/configured_rule.php b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/config/configured_rule.php new file mode 100644 index 00000000..e8366789 --- /dev/null +++ b/Magento2/Rector/Tests/AddHtmlEscaperToOutput/config/configured_rule.php @@ -0,0 +1,13 @@ +rule(\Magento2\Rector\Src\AddHtmlEscaperToOutput::class); +};