Skip to content

Commit bdc2e5d

Browse files
schlesseraschlessera
authored
Merge pull request #6089 from wp-cli/add/csv-escaping
2 parents 7e8b548 + 96e00a6 commit bdc2e5d

File tree

2 files changed

+181
-0
lines changed

2 files changed

+181
-0
lines changed

‎php/utils.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ function format_items( $format, $items, $fields ) {
418418
*/
419419
function write_csv( $fd, $rows, $headers = [] ) {
420420
if ( ! empty( $headers ) ) {
421+
$headers = array_map( __NAMESPACE__ . '\escape_csv_value', $headers );
421422
fputcsv( $fd, $headers, ',', '"', '\\' );
422423
}
423424

@@ -426,6 +427,7 @@ function write_csv( $fd, $rows, $headers = [] ) {
426427
$row = pick_fields( $row, $headers );
427428
}
428429

430+
$row = array_map( __NAMESPACE__ . '\escape_csv_value', $row );
429431
fputcsv( $fd, array_values( $row ), ',', '"', '\\' );
430432
}
431433
}
@@ -2032,3 +2034,33 @@ function get_hook_description( $hook ) {
20322034
}
20332035
return null;
20342036
}
2037+
2038+
/**
2039+
* Escape a value for CSV output.
2040+
*
2041+
* Values that start with the following characters are escaping with a single
2042+
* quote: =, +, -, @, TAB (0x09) and CR (0x0D).
2043+
*
2044+
* @param string $value Value to escape.
2045+
* @return string Escaped value.
2046+
*/
2047+
function escape_csv_value( $value ) {
2048+
if ( null === $value ) {
2049+
return '';
2050+
}
2051+
2052+
// Convert to string if not already
2053+
$value = (string) $value;
2054+
2055+
if (
2056+
in_array(
2057+
substr( $value, 0, 1 ),
2058+
[ '=', '+', '-', '@', "\t", "\r" ],
2059+
true
2060+
)
2061+
) {
2062+
return "'{$value}";
2063+
}
2064+
2065+
return $value;
2066+
}

‎tests/UtilsTest.php‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,155 @@ public static function dataParseUrl() {
914914
];
915915
}
916916

917+
/**
918+
* @dataProvider dataEscapeCsvValue
919+
*/
920+
public function testEscapeCsvValue( $input, $expected ) {
921+
$this->assertEquals( $expected, Utils\escape_csv_value( $input ) );
922+
}
923+
924+
public static function dataEscapeCsvValue() {
925+
return [
926+
// Values starting with special characters that should be escaped.
927+
[ '=formula', "'=formula" ],
928+
[ '+positive', "'+positive" ],
929+
[ '-negative', "'-negative" ],
930+
[ '@mention', "'@mention" ],
931+
[ "\tindented", "'\tindented" ],
932+
[ "\rcarriage", "'\rcarriage" ],
933+
934+
// Values that should not be escaped.
935+
[ 'normal text', 'normal text' ],
936+
[ 'text with = in middle', 'text with = in middle' ],
937+
[ '123', '123' ],
938+
[ '', '' ],
939+
[ ' leading space', ' leading space' ],
940+
[ 'trailing space ', 'trailing space ' ],
941+
[ '=x==y=', "'=x==y=" ], // Only escapes when the first character is special
942+
];
943+
}
944+
945+
public function testWriteCsv() {
946+
// Create a temporary file
947+
$temp_file = tmpfile();
948+
949+
// Test data with various cases that need escaping
950+
$headers = [ 'name', 'formula', 'quoted', 'comma', 'backslash' ];
951+
$rows = [
952+
[
953+
'name' => 'John Doe',
954+
'formula' => '=SUM(A1:A2)',
955+
'quoted' => 'Contains "quotes"',
956+
'comma' => 'Item 1, Item 2',
957+
'backslash' => 'C:\\path\\to\\file',
958+
],
959+
[
960+
'name' => '@username',
961+
'formula' => '+1234',
962+
'quoted' => "'Single quotes'",
963+
'comma' => '-123,45',
964+
'backslash' => 'Escape \\this',
965+
],
966+
];
967+
968+
// Write to CSV
969+
Utils\write_csv( $temp_file, $rows, $headers );
970+
971+
// Rewind file and read contents
972+
rewind( $temp_file );
973+
$csv_content = stream_get_contents( $temp_file );
974+
975+
// Normalize line endings for cross-platform testing
976+
$csv_content = str_replace( "\r\n", "\n", $csv_content );
977+
978+
// Check individual components instead of the exact string
979+
$this->assertStringContainsString( 'name,formula,quoted,comma,backslash', $csv_content );
980+
$this->assertStringContainsString( '"John Doe"', $csv_content );
981+
$this->assertStringContainsString( '\'=SUM(A1:A2)', $csv_content );
982+
$this->assertStringContainsString( '"Contains ""quotes"""', $csv_content );
983+
$this->assertStringContainsString( '"Item 1, Item 2"', $csv_content );
984+
$this->assertStringContainsString( '\'@username', $csv_content );
985+
$this->assertStringContainsString( '\'Single quotes\'', $csv_content );
986+
$this->assertStringContainsString( '\'+1234', $csv_content );
987+
$this->assertStringContainsString( '\'-123,45', $csv_content );
988+
}
989+
990+
public function testWriteCsvWithoutHeaders() {
991+
// Create a temporary file
992+
$temp_file = tmpfile();
993+
994+
// Test data without using headers
995+
$rows = [
996+
[ 'John Doe', '=SUM(A1:A2)', 'Contains "quotes"' ],
997+
[ '@username', '+1234', '-amount' ],
998+
];
999+
1000+
// Write to CSV without headers
1001+
Utils\write_csv( $temp_file, $rows );
1002+
1003+
// Rewind file and read contents
1004+
rewind( $temp_file );
1005+
$csv_content = stream_get_contents( $temp_file );
1006+
1007+
// Normalize line endings for cross-platform testing
1008+
$csv_content = str_replace( "\r\n", "\n", $csv_content );
1009+
1010+
// Check individual components instead of the exact string
1011+
$this->assertStringContainsString( '"John Doe"', $csv_content );
1012+
$this->assertStringContainsString( '\'=SUM(A1:A2)', $csv_content );
1013+
$this->assertStringContainsString( '"Contains ""quotes"""', $csv_content );
1014+
$this->assertStringContainsString( '\'@username', $csv_content );
1015+
$this->assertStringContainsString( '\'+1234', $csv_content );
1016+
$this->assertStringContainsString( '\'-amount', $csv_content );
1017+
}
1018+
1019+
public function testWriteCsvWithFieldPicking() {
1020+
// Create a temporary file
1021+
$temp_file = tmpfile();
1022+
1023+
// Test data with additional fields that should be filtered out
1024+
$rows = [
1025+
[
1026+
'id' => 1,
1027+
'name' => 'John Doe',
1028+
'email' => 'john@example.com',
1029+
'formula' => '=HYPERLINK("http://malicious.com")',
1030+
'extra' => 'Should not appear',
1031+
],
1032+
[
1033+
'id' => 2,
1034+
'name' => '@username',
1035+
'email' => 'user@example.com',
1036+
'formula' => '+1234',
1037+
'extra' => 'Should not appear',
1038+
],
1039+
];
1040+
1041+
// Only include these headers (should filter the rows accordingly)
1042+
$headers = [ 'id', 'name', 'email', 'formula' ];
1043+
1044+
// Write to CSV, which should filter fields based on headers
1045+
Utils\write_csv( $temp_file, $rows, $headers );
1046+
1047+
// Rewind file and read contents
1048+
rewind( $temp_file );
1049+
$csv_content = stream_get_contents( $temp_file );
1050+
1051+
// Normalize line endings for cross-platform testing
1052+
$csv_content = str_replace( "\r\n", "\n", $csv_content );
1053+
1054+
// Check individual components instead of the exact string
1055+
$this->assertStringContainsString( 'id,name,email,formula', $csv_content );
1056+
$this->assertStringContainsString( '1,"John Doe",john@example.com', $csv_content );
1057+
$this->assertStringContainsString( '\'=HYPERLINK', $csv_content );
1058+
$this->assertStringContainsString( '2,\'@username,user@example.com', $csv_content );
1059+
$this->assertStringContainsString( '\'+1234', $csv_content );
1060+
1061+
// Make sure 'extra' field is not in the output
1062+
$this->assertStringNotContainsString( 'extra', $csv_content );
1063+
$this->assertStringNotContainsString( 'Should not appear', $csv_content );
1064+
}
1065+
9171066
public function testReplacePathConstsAddSlashes() {
9181067
$expected = "define( 'ABSPATH', dirname( 'C:\\\\Users\\\\test\'s\\\\site' ) . '/' );";
9191068
$source = "define( 'ABSPATH', dirname( __FILE__ ) . '/' );";

0 commit comments

Comments
 (0)