From bb1182029694ba73007561b6b86441291aa20edc Mon Sep 17 00:00:00 2001 From: Tim Schmitz Date: Fri, 13 Feb 2026 15:37:46 +0100 Subject: [PATCH 1/2] UI: move escaping in charts from consumers to renderer (46966) --- .../Results/class.ilPollResultsRenderer.php | 9 ++------- .../Component/Chart/Bar/Renderer.php | 20 ++++++++++--------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/components/ILIAS/Poll/classes/BlockGUI/Results/class.ilPollResultsRenderer.php b/components/ILIAS/Poll/classes/BlockGUI/Results/class.ilPollResultsRenderer.php index d0e1a2260caf..ff9b7ca9d2a1 100755 --- a/components/ILIAS/Poll/classes/BlockGUI/Results/class.ilPollResultsRenderer.php +++ b/components/ILIAS/Poll/classes/BlockGUI/Results/class.ilPollResultsRenderer.php @@ -89,7 +89,7 @@ protected function renderStackedChart( $tooltips = []; $color_index = 0; foreach ($results->getOrderedAnswerIds() as $id) { - $label = $this->htmlSpecialCharsAsEntities($results->getAnswerText($id)); + $label = nl2br($results->getAnswerText($id)); $total_votes = $results->getAnswerTotal($id); $tooltip = $total_votes . ' (' . round($results->getAnswerPercentage($id)) . '%)'; $bar_config = new BarConfig(); @@ -135,7 +135,7 @@ protected function renderBarChart( $dataset = $this->data_factory->dataset([$votes_label => $c_dimension]); foreach ($results->getOrderedAnswerIds() as $id) { - $label = $this->htmlSpecialCharsAsEntities($results->getAnswerText($id)); + $label = nl2br($results->getAnswerText($id)); $total_votes = $results->getAnswerTotal($id); $tooltip = $total_votes . ' (' . round($results->getAnswerPercentage($id)) . '%)'; $dataset = $dataset->withPoint($label, [$votes_label => $total_votes]) @@ -151,9 +151,4 @@ protected function renderBarChart( ->withBarConfigs([$votes_label => $bar_config]); $tpl->setVariable('CHART', $this->ui_renderer->render($chart)); } - - protected function htmlSpecialCharsAsEntities(string $string): string - { - return $this->refinery->encode()->htmlSpecialCharsAsEntities()->transform(nl2br($string)); - } } diff --git a/components/ILIAS/UI/src/Implementation/Component/Chart/Bar/Renderer.php b/components/ILIAS/UI/src/Implementation/Component/Chart/Bar/Renderer.php index f80980c61f60..155a7362db42 100755 --- a/components/ILIAS/UI/src/Implementation/Component/Chart/Bar/Renderer.php +++ b/components/ILIAS/UI/src/Implementation/Component/Chart/Bar/Renderer.php @@ -113,7 +113,7 @@ function ($id) use ($options, $data, $y_labels, $tooltips) { protected function renderBasics(Bar\Bar $component, Template $tpl): void { - $tpl->setVariable("TITLE", $component->getTitle()); + $tpl->setVariable("TITLE", $this->convertSpecialCharacters($component->getTitle())); $height = ""; if ($component instanceof Bar\Horizontal) { $height = $this->determineHeightForHorizontal($component); @@ -166,10 +166,11 @@ protected function getAccessibilityList( foreach ($points_per_dimension as $dimension_name => $item_points) { $entries = []; - foreach ($item_points as $messeaurement_item_label => $point) { - if (isset($tooltips_per_dimension[$dimension_name][$messeaurement_item_label])) { + foreach ($item_points as $measurement_item_label => $point) { + // use numeric value as default + if (isset($tooltips_per_dimension[$dimension_name][$measurement_item_label])) { // use custom tooltips if defined - $entries[] = $messeaurement_item_label . ": " . $tooltips_per_dimension[$dimension_name][$messeaurement_item_label]; + $entry = $measurement_item_label . ": " . $tooltips_per_dimension[$dimension_name][$measurement_item_label]; } elseif (is_array($point)) { // handle range values $range = ""; @@ -177,19 +178,20 @@ protected function getAccessibilityList( $range .= $p . " - "; } $range = rtrim($range, " -"); - $entries[] = $messeaurement_item_label . ": " . $range; + $entry = $measurement_item_label . ": " . $range; } elseif (is_null($point)) { // handle null values - $entries[] = $messeaurement_item_label . ": -"; + $entry = $measurement_item_label . ": -"; } elseif (!empty($value_labels) && is_int($point) && !empty($value_labels[$point - $lowest])) { // use custom value labels if defined - $entries[] = $messeaurement_item_label . ": " . $value_labels[$point - $lowest]; + $entry = $measurement_item_label . ": " . $value_labels[$point - $lowest]; } else { // use numeric value for all other cases - $entries[] = $messeaurement_item_label . ": " . $point; + $entry = $measurement_item_label . ": " . $point; } + $entries[] = $this->convertSpecialCharacters($entry); } - $list_items[$dimension_name] = $ui_fac->listing()->unordered($entries); + $list_items[$this->convertSpecialCharacters($dimension_name)] = $ui_fac->listing()->unordered($entries); } $list = $ui_fac->listing()->descriptive($list_items); From b2ac21d85f58e17b9c1340d370fc903d4ef66802 Mon Sep 17 00:00:00 2001 From: Tim Schmitz Date: Thu, 26 Feb 2026 15:38:51 +0100 Subject: [PATCH 2/2] UI: unit tests for escaping in bar charts --- .../Component/Chart/Bar/ChartBarTest.php | 97 ++++++++++++++++++- 1 file changed, 94 insertions(+), 3 deletions(-) diff --git a/components/ILIAS/UI/tests/Component/Chart/Bar/ChartBarTest.php b/components/ILIAS/UI/tests/Component/Chart/Bar/ChartBarTest.php index 44677857145a..13f66e31d85e 100755 --- a/components/ILIAS/UI/tests/Component/Chart/Bar/ChartBarTest.php +++ b/components/ILIAS/UI/tests/Component/Chart/Bar/ChartBarTest.php @@ -18,9 +18,6 @@ declare(strict_types=1); -require_once(__DIR__ . "/../../../../../../../vendor/composer/vendor/autoload.php"); -require_once(__DIR__ . "/../../../Base.php"); - use ILIAS\UI\Component as C; use ILIAS\UI\Implementation as I; @@ -394,6 +391,100 @@ public function testRenderVertical(): void +EOT; + + $this->assertHTMLEquals("
" . $expected_html . "
", "
" . $html . "
"); + } + + public static function provideRiskyData(): array + { + return [ + "ampersand" => ["this&that", "this&that"], + "single quote" => ["it's a kind of magic", "it's a kind of magic"], + "double quote" => ['Dwayne "The Rock" Johnson', 'Dwayne "The Rock" Johnson'], + "tags" => ['', '<script>alert("XSS")</script>'], + ]; + } + + /** + * @dataProvider provideRiskyData + */ + public function testRenderConvertSpecialCharactersInDatasetLabel( + string $risky_datum, + string $expected_in_html + ): void { + $r = $this->getDefaultRenderer(); + $f = $this->getFactory(); + $df = $this->getDataFactory(); + + $c_dimension = $df->dimension()->cardinal(); + + $dataset = $df->dataset([$risky_datum => $c_dimension]); + $dataset = $dataset->withPoint("Item", [$risky_datum => 123]); + + $vertical = $f->vertical( + "bar123", + $dataset + ); + + $html = $r->render($vertical); + + $expected_html = << + + +
+
+
$expected_in_html
+
+
    +
  • Item: 123
    • +
+
+
+
+EOT; + + $this->assertHTMLEquals("
" . $expected_html . "
", "
" . $html . "
"); + } + + /** + * @dataProvider provideRiskyData + */ + public function testRenderConvertSpecialCharactersInItemLabel( + string $risky_datum, + string $expected_in_html + ): void { + $r = $this->getDefaultRenderer(); + $f = $this->getFactory(); + $df = $this->getDataFactory(); + + $c_dimension = $df->dimension()->cardinal(); + + $dataset = $df->dataset(["Dataset" => $c_dimension]); + $dataset = $dataset->withPoint($risky_datum, ["Dataset" => 123]); + + $vertical = $f->vertical( + "bar123", + $dataset + ); + + $html = $r->render($vertical); + + $expected_html = << + + +
+
+
Dataset
+
+
    +
  • $expected_in_html: 123
    • +
+
+
+
EOT; $this->assertHTMLEquals("
" . $expected_html . "
", "
" . $html . "
");