From 0daeb24a4734e20297ca619836c0b440e60d7fbb Mon Sep 17 00:00:00 2001 From: Ryan Rathsam Date: Tue, 14 Apr 2026 11:37:45 -0400 Subject: [PATCH 1/3] Update escape function for convertSvg This commit just updates the function used to escape the data that will ultimately passed to exif for exporting SVG versions of XDMoD charts. Tests were added for Usage and Metric Explorer tabs. --- libraries/charting.php | 12 +- .../lib/Export/ChartExportTest.php | 188 ++++++++++++++++++ 2 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 tests/integration/lib/Export/ChartExportTest.php diff --git a/libraries/charting.php b/libraries/charting.php index 777c3e3b80..2e70ed6d32 100644 --- a/libraries/charting.php +++ b/libraries/charting.php @@ -145,17 +145,17 @@ function getSvgViaChromiumHelper($html, $width, $height){ */ function convertSvg($svgData, $format, $width, $height, $docmeta){ - $author = isset($docmeta['author']) ? addcslashes($docmeta['author'], "()\n\\") : 'XDMoD'; - $subject = isset($docmeta['subject']) ? addcslashes($docmeta['subject'], "()\n\\") : 'XDMoD chart'; - $title = isset($docmeta['title']) ? addcslashes($docmeta['title'], "()\n\\") :'XDMoD PDF chart export'; - $creator = addcslashes('XDMoD ' . OPEN_XDMOD_VERSION, "()\n\\"); + $author = isset($docmeta['author']) ? escapeshellarg($docmeta['author']) : "'XDMoD'"; + $subject = isset($docmeta['subject']) ? escapeshellarg($docmeta['subject']) : "'XDMoD chart'"; + $title = isset($docmeta['title']) ? escapeshellarg($docmeta['title']) : "'XDMoD PDF chart export'"; + $creator = escapeshellarg("'XDMoD " . OPEN_XDMOD_VERSION . "'"); switch($format){ case 'png': - $exifArgs = "-Title='$title' -Author='$author' -Description='$subject' -Source='$creator'"; + $exifArgs = "-Title=$title -Author=$author -Description=$subject -Source=$creator"; break; case 'pdf': - $exifArgs = "-Title='$title' -Author='$author' -Subject='$subject' -Creator='$creator'"; + $exifArgs = "-Title=$title -Author=$author -Subject=$subject -Creator=$creator"; break; default: return $svgData; diff --git a/tests/integration/lib/Export/ChartExportTest.php b/tests/integration/lib/Export/ChartExportTest.php new file mode 100644 index 0000000000..519d62eddc --- /dev/null +++ b/tests/integration/lib/Export/ChartExportTest.php @@ -0,0 +1,188 @@ +helper = new XdmodTestHelper(); + } + + /** + * Test that proper escaping is done when exporting an svg ( which also occurs when exporting pdf's) + * @dataProvider provideChartExportEscapesCorrectly + * @return void + */ + public function testChartExportEscapesCorrectly(string $url, array $exportParams) + { + // login as the center director + $this->helper->authenticate('cd'); + + $originalLastName = $this->getPropertyFromUserProfile('last_name'); + + try { + $updateResponse = $this->helper->patch('rest/v1/users/current', [], ['last_name' => "Changed' ; id > /tmp/this_shouldnt_exist; '"]); + + // Make sure that the update was successful. + $this->assertEquals(200, $updateResponse[1]['http_code']); + $this->assertSame( + [ + 'success' => true, + 'message' => 'User profile updated successfully' + ], + $updateResponse[0] + ); + + // Make sure that the last_name that was returned actually contains the right data. + $newLastName = $this->getPropertyFromUserProfile('last_name'); + $this->assertNotFalse(strpos($newLastName, 'Changed'), 'The user last_name updated failed.'); + + $format = $exportParams['format']; + $exportResponse = $this->helper->get($url, $exportParams); + $this->assertEquals(200, $exportResponse[1]['http_code'], "Request to export in $format was unsuccessful."); + + // Make sure that the file that shouldnt' exist, does not in fact exist. + $this->assertFalse(is_file('/tmp/this_shouldnt_exist'), "Woops, chart export in $format did the bad thing. Best figure out why."); + + + } finally { + // Make sure to revert the update to centerdirector's last name. + $revertResponse = $this->helper->patch('rest/v1/users/current', [], ['last_name' => $originalLastName]); + if ($revertResponse[1]['http_code'] !== 200 || $revertResponse[0]['success'] === false) { + throw new \Exception('Unable to revert centerdirectors last name. You have been warned!'); + } + } + + // all done, logout. + $this->helper->logout(); + } + + protected function getUserProfile() + { + // Retrieve the user profile information and make sure that the last_name was updated. + $response = $this->helper->get('rest/v1/users/current'); + + // make sure that the request was successful + $this->assertEquals(200, $response[1]['http_code']); + + $responseData = $response[0]; + + // Make sure that the response is as expected. + $this->assertArrayHasKey('success', $responseData); + $this->assertTrue($responseData['success']); + $this->assertArrayHasKey('results', $responseData); + + return $responseData['results']; + } + + protected function getPropertyFromUserProfile(string $property) + { + $userProfile = $this->getUserProfile(); + + $this->assertArrayHasKey($property, $userProfile); + + return $userProfile[$property]; + } + + /** + * @return array + */ + public function provideChartExportEscapesCorrectly(): array + { + $results = []; + $urls = [ + 'controllers/user_interface.php' => [ + 'public_user' => 'true', + 'realm' => 'Jobs', + 'group_by' => 'none', + 'statistic' => 'total_cpu_hours', + 'start_date' => '2026-03-01', + 'end_date' => '2026-03-31', + 'timeframe_label' => 'Previous+month', + 'scale' => '1', + 'aggregation_unit' => 'Auto', + 'dataset_type' => 'timeseries', + 'thumbnail' => 'n', + 'query_group' => 'tg_usage', + 'display_type' => 'line', + 'combine_type' => 'stack', + 'limit' => '10', + 'offset' => '0', + 'log_scale' => 'n', + 'show_guide_lines' => 'y', + 'show_trend_line' => 'n', + 'show_error_bars' => 'n', + 'show_aggregate_labels' => 'n', + 'show_error_labels' => 'n', + 'hide_tooltip' => 'false', + 'show_title' => 'y', + 'width' => '916', + 'height' => '484', + 'legend_type' => 'bottom_center', + 'font_size' => '3', + 'format' => '', + 'inline' => 'n', + 'operation' => 'get_data' + ], + 'controllers/metric_explorer.php' => [ + 'show_title' => 'y', + 'timeseries' => 'y', + 'aggregation_unit' => 'Auto', + 'start_date' => '2016-12-22', + 'end_date' => '2017-01-01', + 'global_filters' => urldecode('%257B%2522data%2522%253A%255B%255D%252C%2522total%2522%253A0%257D'), + 'title' => 'untitled+query+1', + 'show_filters' => 'true', + 'show_warnings' => 'true', + 'show_remainder' => 'false', + 'start' => '0', + 'limit' => '10', + 'timeframe_label' => 'User+Defined', + 'operation' => 'get_data', + 'data_series' => urldecode('%255B%257B%2522id%2522%253A4860340157018481%252C%2522metric%2522%253A%2522total_cpu_hours%2522%252C%2522category%2522%253A%2522Jobs%2522%252C%2522realm%2522%253A%2522Jobs%2522%252C%2522group_by%2522%253A%2522none%2522%252C%2522x_axis%2522%253Afalse%252C%2522log_scale%2522%253Afalse%252C%2522has_std_err%2522%253Afalse%252C%2522std_err%2522%253Afalse%252C%2522std_err_labels%2522%253A%2522%2522%252C%2522value_labels%2522%253Afalse%252C%2522display_type%2522%253A%2522line%2522%252C%2522line_type%2522%253A%2522Solid%2522%252C%2522line_width%2522%253A2%252C%2522combine_type%2522%253A%2522side%2522%252C%2522sort_type%2522%253A%2522value_desc%2522%252C%2522filters%2522%253A%257B%2522data%2522%253A%255B%255D%252C%2522total%2522%253A0%257D%252C%2522ignore_global%2522%253Afalse%252C%2522long_legend%2522%253Atrue%252C%2522trend_line%2522%253Afalse%252C%2522color%2522%253A%2522auto%2522%252C%2522shadow%2522%253Afalse%252C%2522visibility%2522%253Anull%252C%2522z_index%2522%253A0%252C%2522enabled%2522%253Atrue%257D%255D'), + 'swap_xy' => 'false', + 'share_y_axis' => 'false', + 'hide_tooltip' => 'false', + 'show_guide_lines' => 'y', + 'showContextMenu' => 'y', + 'scale' => '1', + 'format' => '', + 'width' => '916', + 'height' => '484', + 'legend_type' => 'bottom_center', + 'font_size' => '3', + 'featured' => 'false', + 'trendLineEnabled' => 'undefined', + 'x_axis' => urldecode('%257B%257D'), + 'y_axis' => urldecode('%257B%257D'), + 'legend' => urldecode('%257B%257D'), + 'defaultDatasetConfig' => urldecode('%257B%257D'), + 'controller_module' => 'metric_explorer', + 'inline' => 'n' + ] + ]; + $formats = ['svg', 'png', 'pdf']; + foreach($formats as $format) { + foreach($urls as $url => $urlData) { + $urlData['format'] = $format; + $results[] = [ + $url, + $urlData + ]; + + } + } + return $results; + } +} From 49ba08b31ab90c8e914cc53b77fbe4d4b799f9b1 Mon Sep 17 00:00:00 2001 From: Ryan Rathsam Date: Tue, 5 May 2026 14:28:37 -0400 Subject: [PATCH 2/3] Actually include the decoding changes --- tests/integration/lib/Export/ChartExportTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration/lib/Export/ChartExportTest.php b/tests/integration/lib/Export/ChartExportTest.php index 519d62eddc..4bf6b60409 100644 --- a/tests/integration/lib/Export/ChartExportTest.php +++ b/tests/integration/lib/Export/ChartExportTest.php @@ -141,7 +141,7 @@ public function provideChartExportEscapesCorrectly(): array 'aggregation_unit' => 'Auto', 'start_date' => '2016-12-22', 'end_date' => '2017-01-01', - 'global_filters' => urldecode('%257B%2522data%2522%253A%255B%255D%252C%2522total%2522%253A0%257D'), + 'global_filters' => '%7B%22data%22%3A%5B%5D%2C%22total%22%3A0%7D', 'title' => 'untitled+query+1', 'show_filters' => 'true', 'show_warnings' => 'true', @@ -150,7 +150,7 @@ public function provideChartExportEscapesCorrectly(): array 'limit' => '10', 'timeframe_label' => 'User+Defined', 'operation' => 'get_data', - 'data_series' => urldecode('%255B%257B%2522id%2522%253A4860340157018481%252C%2522metric%2522%253A%2522total_cpu_hours%2522%252C%2522category%2522%253A%2522Jobs%2522%252C%2522realm%2522%253A%2522Jobs%2522%252C%2522group_by%2522%253A%2522none%2522%252C%2522x_axis%2522%253Afalse%252C%2522log_scale%2522%253Afalse%252C%2522has_std_err%2522%253Afalse%252C%2522std_err%2522%253Afalse%252C%2522std_err_labels%2522%253A%2522%2522%252C%2522value_labels%2522%253Afalse%252C%2522display_type%2522%253A%2522line%2522%252C%2522line_type%2522%253A%2522Solid%2522%252C%2522line_width%2522%253A2%252C%2522combine_type%2522%253A%2522side%2522%252C%2522sort_type%2522%253A%2522value_desc%2522%252C%2522filters%2522%253A%257B%2522data%2522%253A%255B%255D%252C%2522total%2522%253A0%257D%252C%2522ignore_global%2522%253Afalse%252C%2522long_legend%2522%253Atrue%252C%2522trend_line%2522%253Afalse%252C%2522color%2522%253A%2522auto%2522%252C%2522shadow%2522%253Afalse%252C%2522visibility%2522%253Anull%252C%2522z_index%2522%253A0%252C%2522enabled%2522%253Atrue%257D%255D'), + 'data_series' => '%5B%7B%22id%22%3A4860340157018481%2C%22metric%22%3A%22total_cpu_hours%22%2C%22category%22%3A%22Jobs%22%2C%22realm%22%3A%22Jobs%22%2C%22group_by%22%3A%22none%22%2C%22x_axis%22%3Afalse%2C%22log_scale%22%3Afalse%2C%22has_std_err%22%3Afalse%2C%22std_err%22%3Afalse%2C%22std_err_labels%22%3A%22%22%2C%22value_labels%22%3Afalse%2C%22display_type%22%3A%22line%22%2C%22line_type%22%3A%22Solid%22%2C%22line_width%22%3A2%2C%22combine_type%22%3A%22side%22%2C%22sort_type%22%3A%22value_desc%22%2C%22filters%22%3A%7B%22data%22%3A%5B%5D%2C%22total%22%3A0%7D%2C%22ignore_global%22%3Afalse%2C%22long_legend%22%3Atrue%2C%22trend_line%22%3Afalse%2C%22color%22%3A%22auto%22%2C%22shadow%22%3Afalse%2C%22visibility%22%3Anull%2C%22z_index%22%3A0%2C%22enabled%22%3Atrue%7D%5D', 'swap_xy' => 'false', 'share_y_axis' => 'false', 'hide_tooltip' => 'false', @@ -164,9 +164,9 @@ public function provideChartExportEscapesCorrectly(): array 'font_size' => '3', 'featured' => 'false', 'trendLineEnabled' => 'undefined', - 'x_axis' => urldecode('%257B%257D'), - 'y_axis' => urldecode('%257B%257D'), - 'legend' => urldecode('%257B%257D'), + 'x_axis' => '%7B%7D', + 'y_axis' => '%7B%7D', + 'legend' => '%7B%7D', 'defaultDatasetConfig' => urldecode('%257B%257D'), 'controller_module' => 'metric_explorer', 'inline' => 'n' From 6a3d05ef8fac9bd1f14aafff9907c46ae19f438a Mon Sep 17 00:00:00 2001 From: Ryan Rathsam Date: Tue, 5 May 2026 14:30:35 -0400 Subject: [PATCH 3/3] bah --- tests/integration/lib/Export/ChartExportTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/lib/Export/ChartExportTest.php b/tests/integration/lib/Export/ChartExportTest.php index 4bf6b60409..be8138b859 100644 --- a/tests/integration/lib/Export/ChartExportTest.php +++ b/tests/integration/lib/Export/ChartExportTest.php @@ -167,7 +167,7 @@ public function provideChartExportEscapesCorrectly(): array 'x_axis' => '%7B%7D', 'y_axis' => '%7B%7D', 'legend' => '%7B%7D', - 'defaultDatasetConfig' => urldecode('%257B%257D'), + 'defaultDatasetConfig' => '%7B%7D', 'controller_module' => 'metric_explorer', 'inline' => 'n' ]