Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 5 commits into from
Jul 24, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2fefc75
`PluginOutput::render()`: Shorten the output by characterLimit before…
sukhwinder33445 Jun 17, 2024
91b3f81
PluginOutput: Remove `processHtml()` method
sukhwinder33445 Jul 23, 2024
ef96037
PluginOutput: Add method return type and visibility to constants
sukhwinder33445 Jul 24, 2024
929258c
PluginOutputTest: Migrate tests from monitoring/PluginOutputTest
sukhwinder33445 Jul 23, 2024
a40df7f
Phpstan: Update baseline files
sukhwinder33445 Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 17 additions & 96 deletions library/Icingadb/Util/PluginOutput.php
View file
Open in desktop
nilmerg marked this conversation as resolved.
Show resolved Hide resolved
nilmerg marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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(\]|\))~',
Expand All @@ -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>',
Expand All @@ -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"
];
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
}
}
10 changes: 0 additions & 10 deletions phpstan-baseline-7x.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 0 additions & 10 deletions phpstan-baseline-8x.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 7 additions & 27 deletions phpstan-baseline-standard.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
Loading