diff --git a/library/Icingadb/Util/PluginOutput.php b/library/Icingadb/Util/PluginOutput.php index 03055a9ef..463ea36db 100644 --- a/library/Icingadb/Util/PluginOutput.php +++ b/library/Icingadb/Util/PluginOutput.php @@ -4,24 +4,19 @@ namespace Icinga\Module\Icingadb\Util; -use DOMDocument; -use DOMNode; -use DOMText; use Icinga\Module\Icingadb\Hook\PluginOutputHook; use Icinga\Module\Icingadb\Model\Host; use Icinga\Module\Icingadb\Model\Service; -use Icinga\Web\Dom\DomNodeIterator; use Icinga\Web\Helper\HtmlPurifier; use InvalidArgumentException; use ipl\Html\HtmlString; use ipl\Orm\Model; use LogicException; -use RecursiveIteratorIterator; class PluginOutput extends HtmlString { /** @var string[] Patterns to be replaced in plain text plugin output */ - const TEXT_PATTERNS = [ + protected const TEXT_PATTERNS = [ '~\\\t~', '~\\\n~', '~(\[|\()OK(\]|\))~', @@ -34,7 +29,7 @@ class PluginOutput extends HtmlString ]; /** @var string[] Replacements for {@see PluginOutput::TEXT_PATTERNS} */ - const TEXT_REPLACEMENTS = [ + protected const TEXT_REPLACEMENTS = [ "\t", "\n", '', @@ -47,13 +42,13 @@ class PluginOutput extends HtmlString ]; /** @var string[] Patterns to be replaced in html plugin output */ - const HTML_PATTERNS = [ + protected const HTML_PATTERNS = [ '~\\\t~', '~\\\n~' ]; /** @var string[] Replacements for {@see PluginOutput::HTML_PATTERNS} */ - const HTML_REPLACEMENTS = [ + protected const HTML_REPLACEMENTS = [ "\t", "\n" ]; @@ -159,7 +154,7 @@ public static function fromObject(Model $object): self ->setCommandName($object->checkcommand_name); } - public function render() + public function render(): string { if ($this->renderedOutput !== null) { return $this->renderedOutput; @@ -174,22 +169,24 @@ public function render() $output = PluginOutputHook::processOutput($output, $this->commandName, $this->enrichOutput); } - if (preg_match('~<\w+(?>\s\w+=[^>]*)?>~', $output)) { - // HTML - $output = HtmlPurifier::process(preg_replace( - self::HTML_PATTERNS, - self::HTML_REPLACEMENTS, - $output - )); - $this->isHtml = true; + $output = substr($output, 0, $this->characterLimit); + + $this->isHtml = (bool) preg_match('~<\w+(?>\s\w+=[^>]*)?>~', $output); + + if ($this->isHtml) { + if ($this->enrichOutput) { + $output = preg_replace(self::TEXT_PATTERNS, self::TEXT_REPLACEMENTS, $output); + } else { + $output = preg_replace(self::HTML_PATTERNS, self::HTML_REPLACEMENTS, $output); + } + + $output = HtmlPurifier::process($output); } else { - // Plaintext $output = preg_replace( self::TEXT_PATTERNS, self::TEXT_REPLACEMENTS, htmlspecialchars($output, ENT_COMPAT | ENT_SUBSTITUTE | ENT_HTML5, null, false) ); - $this->isHtml = false; } $output = trim($output); @@ -198,84 +195,8 @@ public function render() // in oder to help browsers to break words in plugin output $output = preg_replace('/,(?=[^\s])/', ',​', $output); - if ($this->enrichOutput && $this->isHtml) { - $output = $this->processHtml($output); - } - - if ($this->characterLimit) { - $output = substr($output, 0, $this->characterLimit); - } - $this->renderedOutput = $output; return $output; } - - /** - * Replace color state information, if any - * - * @param string $html - * - * @todo Do we really need to create a DOM here? Or is a preg_replace like we do it for text also feasible? - * @return string - */ - protected function processHtml(string $html): string - { - $pattern = '/[([](OK|WARNING|CRITICAL|UNKNOWN|UP|DOWN)[)\]]/'; - $doc = new DOMDocument(); - $doc->loadXML('
' . $html . '
', LIBXML_NOERROR | LIBXML_NOWARNING); - $dom = new RecursiveIteratorIterator(new DomNodeIterator($doc), RecursiveIteratorIterator::SELF_FIRST); - - $nodesToRemove = []; - foreach ($dom as $node) { - /** @var DOMNode $node */ - if ($node->nodeType !== XML_TEXT_NODE) { - continue; - } - - $start = 0; - while (preg_match($pattern, $node->nodeValue, $match, PREG_OFFSET_CAPTURE, $start)) { - $offsetLeft = $match[0][1]; - $matchLength = strlen($match[0][0]); - $leftLength = $offsetLeft - $start; - - // if there is text before the match - if ($leftLength) { - // create node for leading text - $text = new DOMText(substr($node->nodeValue, $start, $leftLength)); - $node->parentNode->insertBefore($text, $node); - } - - // create the state ball for the match - $span = $doc->createElement('span'); - $span->setAttribute( - 'class', - 'state-ball ball-size-m state-' . strtolower($match[1][0]) - ); - $node->parentNode->insertBefore($span, $node); - - // start for next match - $start = $offsetLeft + $matchLength; - } - - if ($start) { - // is there text left? - if (strlen($node->nodeValue) > $start) { - // create node for trailing text - $text = new DOMText(substr($node->nodeValue, $start)); - $node->parentNode->insertBefore($text, $node); - } - - // delete the old node later - $nodesToRemove[] = $node; - } - } - - foreach ($nodesToRemove as $node) { - /** @var DOMNode $node */ - $node->parentNode->removeChild($node); - } - - return substr($doc->saveHTML(), 5, -7); - } } diff --git a/phpstan-baseline-7x.neon b/phpstan-baseline-7x.neon index 6da352556..95668ced4 100644 --- a/phpstan-baseline-7x.neon +++ b/phpstan-baseline-7x.neon @@ -80,11 +80,6 @@ parameters: count: 1 path: library/Icingadb/Util/PerfData.php - - - message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PluginOutput\\:\\:render\\(\\) should return string but returns string\\|false\\|null\\.$#" - count: 1 - path: library/Icingadb/Util/PluginOutput.php - - message: "#^Parameter \\#1 \\$str of function trim expects string, string\\|null given\\.$#" count: 1 @@ -95,11 +90,6 @@ parameters: count: 1 path: library/Icingadb/Util/PluginOutput.php - - - message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PluginOutput\\:\\:\\$renderedOutput \\(string\\) does not accept string\\|false\\|null\\.$#" - count: 1 - path: library/Icingadb/Util/PluginOutput.php - - message: "#^Parameter \\#1 \\$str of function trim expects string, mixed given\\.$#" count: 1 diff --git a/phpstan-baseline-8x.neon b/phpstan-baseline-8x.neon index b5b85f7f9..d27ed7950 100644 --- a/phpstan-baseline-8x.neon +++ b/phpstan-baseline-8x.neon @@ -80,21 +80,11 @@ parameters: count: 1 path: library/Icingadb/Util/PerfData.php - - - message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PluginOutput\\:\\:render\\(\\) should return string but returns string\\|null\\.$#" - count: 1 - path: library/Icingadb/Util/PluginOutput.php - - message: "#^Parameter \\#1 \\$string of function trim expects string, string\\|null given\\.$#" count: 1 path: library/Icingadb/Util/PluginOutput.php - - - message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PluginOutput\\:\\:\\$renderedOutput \\(string\\) does not accept string\\|null\\.$#" - count: 1 - path: library/Icingadb/Util/PluginOutput.php - - message: "#^Parameter \\#1 \\$string of function trim expects string, mixed given\\.$#" count: 1 diff --git a/phpstan-baseline-standard.neon b/phpstan-baseline-standard.neon index c974c5e11..cc744dfed 100644 --- a/phpstan-baseline-standard.neon +++ b/phpstan-baseline-standard.neon @@ -4936,17 +4936,7 @@ parameters: path: library/Icingadb/Util/PluginOutput.php - - message: "#^Cannot call method insertBefore\\(\\) on DOMNode\\|null\\.$#" - count: 3 - path: library/Icingadb/Util/PluginOutput.php - - - - message: "#^Cannot call method removeChild\\(\\) on DOMNode\\|null\\.$#" - count: 1 - path: library/Icingadb/Util/PluginOutput.php - - - - message: "#^Parameter \\#1 \\$html of method Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PluginOutput\\:\\:processHtml\\(\\) expects string, string\\|null given\\.$#" + message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PluginOutput\\:\\:render\\(\\) should return string but returns string\\|null\\.$#" count: 1 path: library/Icingadb/Util/PluginOutput.php @@ -4956,22 +4946,7 @@ parameters: path: library/Icingadb/Util/PluginOutput.php - - message: "#^Parameter \\#1 \\$string of function strlen expects string, string\\|null given\\.$#" - count: 1 - path: library/Icingadb/Util/PluginOutput.php - - - - message: "#^Parameter \\#1 \\$string of function substr expects string, string\\|false given\\.$#" - count: 1 - path: library/Icingadb/Util/PluginOutput.php - - - - message: "#^Parameter \\#1 \\$string of function substr expects string, string\\|null given\\.$#" - count: 3 - path: library/Icingadb/Util/PluginOutput.php - - - - message: "#^Parameter \\#2 \\$subject of function preg_match expects string, string\\|null given\\.$#" + message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PluginOutput\\:\\:\\$renderedOutput \\(string\\) does not accept string\\|null\\.$#" count: 1 path: library/Icingadb/Util/PluginOutput.php @@ -6635,6 +6610,11 @@ parameters: count: 1 path: library/Icingadb/Widget/Detail/UserDetail.php + - + message: "#^Cannot access property \\$display_name on mixed\\.$#" + count: 1 + path: library/Icingadb/Widget/Detail/UserDetail.php + - message: "#^Cannot call method getModel\\(\\) on mixed\\.$#" count: 1 diff --git a/test/php/library/Icingadb/Util/PluginOutputTest.php b/test/php/library/Icingadb/Util/PluginOutputTest.php new file mode 100644 index 000000000..80d9f8d76 --- /dev/null +++ b/test/php/library/Icingadb/Util/PluginOutputTest.php @@ -0,0 +1,199 @@ +setCharacterLimit($characterLimit); + } + + $this->assertSame($expected, $p->render(), 'PluginOutput::render does not return expected values'); + } + + + public function testRenderPlainText(): void + { + $input = 'This is a plain text'; + + $this->checkOutput($input, $input); + } + + public function testRenderTextWithStates(): void + { + $input = <<<'INPUT' +[OK] Dummy state + \_ [OK] Fake "state" + \_ [WARNING] Fake state again +INPUT; + + $expectedOutput = <<<'EXPECTED_OUTPUT' + Dummy state + \_ Fake "state" + \_ Fake state again +EXPECTED_OUTPUT; + + $this->checkOutput($expectedOutput, $input); + } + + public function testRenderTextWithStatesAndCharacterLimit(): void + { + $input = <<<'INPUT' +[OK] Dummy state + \_ [OK] Fake "state" + \_ [WARNING] Fake state again +INPUT; + + $expectedOutput = <<<'EXPECTED_OUTPUT' + Dummy +EXPECTED_OUTPUT; + + $this->checkOutput($expectedOutput, $input, 10); + } + + public function testRenderTextWithHtml(): void + { + $input = <<<'INPUT' +Hello

World

, this "is" 'a test. +INPUT; + + $expectedOutput = <<<'EXPECTED_OUTPUT' +Hello

World

, this "is" 'a test. +EXPECTED_OUTPUT; + + $this->checkOutput($expectedOutput, $input); + } + + public function testRenderTextWithHtmlAndStates(): void + { + $input = <<<'INPUT' +Hello

World

, this "is" a test. +[OK] Dummy state + \_ [OK] Fake "state" + \_ [WARNING] Fake state again +text ends here +INPUT; + + $expectedOutput = <<<'EXPECTED_OUTPUT' +Hello

World

, this "is" a test. + Dummy state + \_ Fake "state" + \_ Fake state again +text ends here +EXPECTED_OUTPUT; + + $this->checkOutput($expectedOutput, $input); + } + + public function testRenderTextWithHtmlIncludingStatesAndSpecialChars(): void + { + $input = <<<'INPUT' +Hello

World

, this "is" a test. +[OK] Dummy state + special chars: !@#$%^&*()_+{}|:"<>?`-=[]\;',./ +text ends here +INPUT; + + $expectedOutput = <<<'EXPECTED_OUTPUT' +Hello

World

, this "is" a test. + Dummy state + special chars: !@#$%^&*()_+{}|:"<>?`-=[]\;',​./ +text ends here +EXPECTED_OUTPUT; + + $this->checkOutput($expectedOutput, $input); + } + + public function testOutputWithNewlines(): void + { + $input = 'foo\nbar\n\nraboof'; + $expectedOutput = "foo\nbar\n\nraboof"; + + $this->checkOutput($expectedOutput, $input); + } + + public function testOutputWithHtmlEntities(): void + { + $input = 'foo & bar'; + $expectedOutput = $input; + + $this->checkOutput($expectedOutput, $input); + } + + public function testSimpleHtmlOutput(): void + { + $input = <<<'INPUT' +OK - Teststatus Info +INPUT; + + $expectedOutput = <<<'EXPECTED_OUTPUT' +OK - Teststatus Info +EXPECTED_OUTPUT; + + $this->checkOutput($expectedOutput, $input); + } + + public function testTextStatusTags(): void + { + foreach (['OK', 'WARNING', 'CRITICAL', 'UNKNOWN', 'UP', 'DOWN'] as $s) { + $l = strtolower($s); + + $input = sprintf('[%s] Test', $s); + $expectedOutput = sprintf(' Test', $l); + + $this->checkOutput($expectedOutput, $input); + + $input = sprintf('(%s) Test', $s); + + $this->checkOutput($expectedOutput, $input); + } + } + + public function testHtmlStatusTags(): void + { + $dummyHtml = ''; + + foreach (['OK', 'WARNING', 'CRITICAL', 'UNKNOWN', 'UP', 'DOWN'] as $s) { + $l = strtolower($s); + + $input = sprintf('%s [%s] Test', $dummyHtml, $s); + $expectedOutput = sprintf('%s Test', $dummyHtml, $l); + + $this->checkOutput($expectedOutput, $input); + + $input = sprintf('%s (%s) Test', $dummyHtml, $s); + + $this->checkOutput($expectedOutput, $input); + } + } + + public function testNewlineProcessingInHtmlOutput(): void + { + $input = 'This is plugin output\n\n\n\n' + . 'and more text that\nis split onto multiple\n\nlines'; + + $expectedOutput = <<<'EXPECTED_OUTPUT' +This is plugin output + + + +and more text that +is split onto multiple + +lines +EXPECTED_OUTPUT; + + $this->checkOutput($expectedOutput, $input); + } +}