PluginOutput::render(): Return correctly cut off output (#1026)

fixes #1012
Icinga · Jul 24, 2024 · 092dca7 · 092dca7
2 parents cdbaea0 + a40df7f
commit 092dca7
Show file tree

Hide file tree

Showing 5 changed files with 223 additions and 143 deletions.
diff --git 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",
         '<span class="state-ball ball-size-m state-ok"></span>',
@@ -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])/', ',&#8203;', $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('<div>' . $html . '</div>', 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
@@ -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
@@ -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
@@ -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