From 0419e8a7f99b5fda19852b5d34ca95ad272c2426 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Wed, 21 Oct 2026 17:24:51 +0530 Subject: [PATCH] fix: enhance CSV handling with escape character support and sanitize input --- classes/Visualizer/Module.php | 6 +-- classes/Visualizer/Module/Abilities.php | 2 +- classes/Visualizer/Module/Chart.php | 6 +-- classes/Visualizer/Source/Csv.php | 8 +-- index.php | 3 ++ tests/php/static-analysis-stubs/symbols.php | 3 ++ tests/test-ajax.php | 39 ++++++++++++++ tests/test-csv-escape.php | 57 +++++++++++++++++++++ 8 files changed, 113 insertions(+), 11 deletions(-) create mode 100644 tests/test-csv-escape.php diff --git a/classes/Visualizer/Module.php b/classes/Visualizer/Module.php index ae7993917..ff9fa2c33 100644 --- a/classes/Visualizer/Module.php +++ b/classes/Visualizer/Module.php @@ -285,11 +285,11 @@ private function _getCSV( $rows, $filename, $enclose ) { // support for MS Excel fprintf( $fp, $bom ); foreach ( $rows as $row ) { - fputcsv( $fp, $row ); + fputcsv( $fp, $row, ',', '"', VISUALIZER_CSV_ESCAPE ); } rewind( $fp ); $csv = ''; - $array = fgetcsv( $fp ); + $array = fgetcsv( $fp, 0, ',', '"', VISUALIZER_CSV_ESCAPE ); while ( $array !== false ) { if ( strlen( $csv ) > 0 ) { $csv .= PHP_EOL; @@ -307,7 +307,7 @@ private function _getCSV( $rows, $filename, $enclose ) { $array = $temp_array; } $csv .= implode( ',', $array ); - $array = fgetcsv( $fp ); + $array = fgetcsv( $fp, 0, ',', '"', VISUALIZER_CSV_ESCAPE ); } fclose( $fp ); diff --git a/classes/Visualizer/Module/Abilities.php b/classes/Visualizer/Module/Abilities.php index 54bb221a1..95c2cb430 100644 --- a/classes/Visualizer/Module/Abilities.php +++ b/classes/Visualizer/Module/Abilities.php @@ -1209,7 +1209,7 @@ private function buildManualSource( $input ) { return new WP_Error( 'visualizer_tmp_file', __( 'A temporary file could not be created.', 'visualizer' ) ); } foreach ( $lines as $line ) { - fputcsv( $handle, $line, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); + fputcsv( $handle, $line, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE, VISUALIZER_CSV_ESCAPE ); } fclose( $handle ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fclose diff --git a/classes/Visualizer/Module/Chart.php b/classes/Visualizer/Module/Chart.php index 4ca5cd06f..bbe126e0a 100644 --- a/classes/Visualizer/Module/Chart.php +++ b/classes/Visualizer/Module/Chart.php @@ -346,10 +346,10 @@ public function getCharts() { if ( ! current_user_can( 'edit_others_posts' ) ) { $query_args['author'] = get_current_user_id(); } - $filter = filter_input( INPUT_GET, 's', FILTER_SANITIZE_STRING ); + $filter = isset( $_GET['s'] ) ? sanitize_text_field( wp_unslash( $_GET['s'] ) ) : ''; if ( empty( $filter ) ) { // 'filter' is from the modal from the add media button. - $filter = filter_input( INPUT_GET, 'filter', FILTER_SANITIZE_STRING ); + $filter = isset( $_GET['filter'] ) ? sanitize_text_field( wp_unslash( $_GET['filter'] ) ) : ''; } if ( $filter && in_array( $filter, Visualizer_Plugin::getChartTypes(), true ) ) { @@ -1259,7 +1259,7 @@ private function handleTabularData() { foreach ( $exclude as $j ) { unset( $row[ $j ] ); } - fputcsv( $handle, $row ); + fputcsv( $handle, $row, ',', '"', VISUALIZER_CSV_ESCAPE ); } } $source = new Visualizer_Source_Csv( $tmpfile ); diff --git a/classes/Visualizer/Source/Csv.php b/classes/Visualizer/Source/Csv.php index 679714e58..f4fb5b65a 100644 --- a/classes/Visualizer/Source/Csv.php +++ b/classes/Visualizer/Source/Csv.php @@ -61,7 +61,7 @@ public function __construct( $filename = '' ) { */ private function _fetchSeries( &$handle ) { // read column titles - $labels = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); + $labels = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE, VISUALIZER_CSV_ESCAPE ); $types = null; if ( false !== strpos( $this->_filename, 'tqx=out:csv' ) ) { @@ -74,7 +74,7 @@ private function _fetchSeries( &$handle ) { if ( is_null( $types ) ) { // read series types - $types = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); + $types = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE, VISUALIZER_CSV_ESCAPE ); } if ( ! is_array( $labels ) || ! is_array( $types ) ) { @@ -155,10 +155,10 @@ public function fetch() { } // fetch data - $data = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); + $data = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE, VISUALIZER_CSV_ESCAPE ); while ( $data !== false ) { $this->_data[] = $this->_normalizeData( $data ); - $data = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); + $data = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE, VISUALIZER_CSV_ESCAPE ); } // close file handle diff --git a/index.php b/index.php index 05b0725db..177bebe7b 100644 --- a/index.php +++ b/index.php @@ -81,6 +81,9 @@ function visualizer_launch() { if ( ! defined( 'VISUALIZER_CSV_ENCLOSURE' ) ) { define( 'VISUALIZER_CSV_ENCLOSURE', '"' ); } + if ( ! defined( 'VISUALIZER_CSV_ESCAPE' ) ) { + define( 'VISUALIZER_CSV_ESCAPE', '\\' ); + } if ( ! defined( 'VISUALIZER_DEBUG' ) ) { define( 'VISUALIZER_DEBUG', false ); } diff --git a/tests/php/static-analysis-stubs/symbols.php b/tests/php/static-analysis-stubs/symbols.php index 4a32429d1..0e17329a9 100644 --- a/tests/php/static-analysis-stubs/symbols.php +++ b/tests/php/static-analysis-stubs/symbols.php @@ -18,6 +18,9 @@ if ( ! defined( 'VISUALIZER_CSV_ENCLOSURE' ) ) { define( 'VISUALIZER_CSV_ENCLOSURE', '"' ); } +if ( ! defined( 'VISUALIZER_CSV_ESCAPE' ) ) { + define( 'VISUALIZER_CSV_ESCAPE', '\\' ); +} if ( ! defined( 'VISUALIZER_DEBUG' ) ) { define( 'VISUALIZER_DEBUG', false ); } diff --git a/tests/test-ajax.php b/tests/test-ajax.php index 386734002..20a0ed8c4 100644 --- a/tests/test-ajax.php +++ b/tests/test-ajax.php @@ -988,6 +988,45 @@ public function test_get_charts_only_lists_editable_scope_for_contributor() { $this->assertNotContains( $other_chart, $chart_ids ); } + /** + * Request params the chart library reads the chart type filter from. + * + * @return array + */ + public function chart_type_filter_param_provider(): array { + return array( + 'search box' => array( 's' ), + 'media modal' => array( 'filter' ), + ); + } + + /** + * The chart library filters by the requested chart type. + * + * @dataProvider chart_type_filter_param_provider + * + * @param string $param Request param carrying the chart type. + */ + public function test_get_charts_filters_by_chart_type( string $param ): void { + $this->create_chart_for_user( $this->admin_user_id, 'publish' ); + $pie_chart = $this->create_chart_for_user( $this->admin_user_id, 'publish' ); + update_post_meta( $pie_chart, Visualizer_Plugin::CF_CHART_TYPE, 'pie' ); + $_GET = array( + 'nonce' => wp_create_nonce( Visualizer_Plugin::ACTION_GET_CHARTS ), + $param => 'pie', + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_GET_CHARTS ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after the JSON response. + } + + $response = json_decode( $this->_last_response, true ); + $this->assertTrue( $response['success'] ); + $this->assertSame( array( $pie_chart ), wp_list_pluck( $response['data'], 'id' ) ); + } + /** * A valid nonce does not permit deleting another user's chart. */ diff --git a/tests/test-csv-escape.php b/tests/test-csv-escape.php new file mode 100644 index 000000000..a033f65be --- /dev/null +++ b/tests/test-csv-escape.php @@ -0,0 +1,57 @@ +file ) { + wp_delete_file( $this->file ); + } + parent::tearDown(); + } + + /** + * CSV import keeps the backslash escape and logs no deprecation. + */ + public function test_csv_import_uses_backslash_escape(): void { + $this->file = wp_tempnam( 'visualizer' ); + file_put_contents( $this->file, "Label,Value\nstring,number\n\"say \\\"hi\\\"\",1\n" ); + + $deprecations = array(); + set_error_handler( + static function ( int $errno, string $errstr ) use ( &$deprecations ): bool { + $deprecations[] = $errstr; + return true; + }, + E_DEPRECATED + ); + try { + $source = new Visualizer_Source_Csv( $this->file ); + $this->assertTrue( $source->fetch() ); + } finally { + restore_error_handler(); + } + + $this->assertSame( array(), $deprecations ); + $this->assertSame( 'say "hi"', $source->getRawData()[0][0] ); + } +}