From c818d944cf998b3151e4b312d655c51223612c4e Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 12 Aug 2024 16:09:56 +0100 Subject: [PATCH] ext/(standard|spl): Deprecate passing a non-empty string as the $enclosure parameter (#15362) --- NEWS | 10 +++ UPGRADING | 12 +++ ext/spl/spl_directory.c | 12 +++ .../SplFileObject_fgetcsv_escape_basic.phpt | 3 +- .../SplFileObject_setCsvControl_basic.phpt | 15 ++-- ext/standard/file.c | 8 ++ ext/standard/string.c | 29 ++++++-- ext/standard/tests/file/bug40501.phpt | 1 + ext/standard/tests/file/bug72330.phpt | 3 +- .../tests/file/fputcsv_variation15.phpt | 22 ++---- .../tests/file/fputcsv_variation17.phpt | 2 +- .../tests/file/fputcsv_variation18.phpt | 73 +++++++++++++++++++ ext/standard/tests/strings/gh12151.phpt | 7 +- .../tests/strings/str_getcsv_001.phpt | 17 ++--- .../tests/strings/str_getcsv_errors.phpt | 15 ++++ 15 files changed, 184 insertions(+), 45 deletions(-) create mode 100644 ext/standard/tests/file/fputcsv_variation18.phpt create mode 100644 ext/standard/tests/strings/str_getcsv_errors.phpt diff --git a/NEWS b/NEWS index dc28d6abdc3..71f58978d14 100644 --- a/NEWS +++ b/NEWS @@ -77,10 +77,20 @@ PHP NEWS . The SplFixedArray::__wakeup() method has been deprecated as it implements __serialize() and __unserialize() which need to be overwritten instead. (TysonAndre) + . Passing a non-empty string for the $enclosure parameter of: + - SplFileObject::setCsvControl() + - SplFileObject::fputcsv() + - SplFileObject::fgetcsv() + is now deprecated. (Girgias) - Standard: . Unserializing the uppercase 'S' tag is now deprecated. (timwolla) . Enables crc32 auxiliary detection on OpenBSD. (David Carlier) + . Passing a non-empty string for the $enclosure parameter of: + - fputcsv() + - fgetcsv() + - str_getcsv() + is now deprecated. (Girgias) - Streams: . Implemented GH-15155 (Stream context is lost when custom stream wrapper is diff --git a/UPGRADING b/UPGRADING index 29dc2d2b171..5a3d170adf0 100644 --- a/UPGRADING +++ b/UPGRADING @@ -506,6 +506,12 @@ PHP 8.4 UPGRADE NOTES - SPL: . The SplFixedArray::__wakeup() method has been deprecated as it implements __serialize() and __unserialize() which need to be overwritten instead. + . Passing a non-empty string for the $enclosure parameter of: + - SplFileObject::setCsvControl() + - SplFileObject::fputcsv() + - SplFileObject::fgetcsv() + is now deprecated. + RFC: https://wiki.php.net/rfc/deprecations_php_8_4#deprecate_proprietary_csv_escaping_mechanism - Standard: . Calling stream_context_set_option() with 2 arguments is deprecated. @@ -514,6 +520,12 @@ PHP 8.4 UPGRADE NOTES RFC: https://wiki.php.net/rfc/raising_zero_to_power_of_negative_number . Unserializing strings using the uppercase 'S' tag is deprecated. RFC: https://wiki.php.net/rfc/deprecations_php_8_4 + . Passing a non-empty string for the $enclosure parameter of: + - fputcsv() + - fgetcsv() + - str_getcsv() + is now deprecated. + RFC: https://wiki.php.net/rfc/deprecations_php_8_4#deprecate_proprietary_csv_escaping_mechanism - XML: . The xml_set_object() function has been deprecated. diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 0521087da30..15273cb6c46 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -2310,6 +2310,10 @@ PHP_METHOD(SplFileObject, fgetcsv) if (esc_len == 0) { escape = PHP_CSV_NO_ESCAPE; } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a non-empty string to the $escape parameter is deprecated since 8.4"); + if (UNEXPECTED(EG(exception))) { + RETURN_THROWS(); + } escape = (unsigned char) esc[0]; } } @@ -2358,6 +2362,10 @@ PHP_METHOD(SplFileObject, fputcsv) if (esc_len == 0) { escape = PHP_CSV_NO_ESCAPE; } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a non-empty string to the $escape parameter is deprecated since 8.4"); + if (UNEXPECTED(EG(exception))) { + RETURN_THROWS(); + } escape = (unsigned char) esc[0]; } } @@ -2405,6 +2413,10 @@ PHP_METHOD(SplFileObject, setCsvControl) if (esc_len == 0) { escape = PHP_CSV_NO_ESCAPE; } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a non-empty string to the $escape parameter is deprecated since 8.4"); + if (UNEXPECTED(EG(exception))) { + RETURN_THROWS(); + } escape = (unsigned char) esc[0]; } } diff --git a/ext/spl/tests/SplFileObject/SplFileObject_fgetcsv_escape_basic.phpt b/ext/spl/tests/SplFileObject/SplFileObject_fgetcsv_escape_basic.phpt index f6a68ff1fae..df66463db78 100644 --- a/ext/spl/tests/SplFileObject/SplFileObject_fgetcsv_escape_basic.phpt +++ b/ext/spl/tests/SplFileObject/SplFileObject_fgetcsv_escape_basic.phpt @@ -13,7 +13,8 @@ var_dump($fo->fgetcsv(',', '"', '"')); ---EXPECT-- +--EXPECTF-- +Deprecated: SplFileObject::fgetcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d array(3) { [0]=> string(3) "aaa" diff --git a/ext/spl/tests/SplFileObject/SplFileObject_setCsvControl_basic.phpt b/ext/spl/tests/SplFileObject/SplFileObject_setCsvControl_basic.phpt index c2eb623be91..a8b7cb730d0 100644 --- a/ext/spl/tests/SplFileObject/SplFileObject_setCsvControl_basic.phpt +++ b/ext/spl/tests/SplFileObject/SplFileObject_setCsvControl_basic.phpt @@ -4,16 +4,17 @@ SPL: SplFileObject::setCsvControl basic Erwin Poeze --FILE-- setFlags(SplFileObject::READ_CSV); -$s->setCsvControl('|', '\'', '/'); +$s->setCsvControl('|', '\'', ''); foreach ($s as $row) { list($fruit, $quantity) = $row; echo "$fruit : $quantity\n"; diff --git a/ext/standard/file.c b/ext/standard/file.c index 8a3decd5125..677fd74d225 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -1734,6 +1734,10 @@ PHP_FUNCTION(fputcsv) if (escape_str_len < 1) { escape_char = PHP_CSV_NO_ESCAPE; } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a non-empty string to the $escape parameter is deprecated since 8.4"); + if (UNEXPECTED(EG(exception))) { + RETURN_THROWS(); + } /* use first character from string */ escape_char = (unsigned char) *escape_str; } @@ -1875,6 +1879,10 @@ PHP_FUNCTION(fgetcsv) if (escape_str_len < 1) { escape = PHP_CSV_NO_ESCAPE; } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a non-empty string to the $escape parameter is deprecated since 8.4"); + if (UNEXPECTED(EG(exception))) { + RETURN_THROWS(); + } escape = (unsigned char) escape_str[0]; } } diff --git a/ext/standard/string.c b/ext/standard/string.c index a5d8827edf5..ddc2d2b1645 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -5447,25 +5447,40 @@ PHP_FUNCTION(str_getcsv) { zend_string *str; char delim = ',', enc = '"'; - int esc = (unsigned char) '\\'; - char *delim_str = NULL, *enc_str = NULL, *esc_str = NULL; - size_t delim_len = 0, enc_len = 0, esc_len = 0; + int escape = (unsigned char) '\\'; + char *delim_str = NULL, *enc_str = NULL, *escape_str = NULL; + size_t delim_len = 0, enc_len = 0, escape_str_len = 0; ZEND_PARSE_PARAMETERS_START(1, 4) Z_PARAM_STR(str) Z_PARAM_OPTIONAL Z_PARAM_STRING(delim_str, delim_len) Z_PARAM_STRING(enc_str, enc_len) - Z_PARAM_STRING(esc_str, esc_len) + Z_PARAM_STRING(escape_str, escape_str_len) ZEND_PARSE_PARAMETERS_END(); delim = delim_len ? delim_str[0] : delim; enc = enc_len ? enc_str[0] : enc; - if (esc_str != NULL) { - esc = esc_len ? (unsigned char) esc_str[0] : PHP_CSV_NO_ESCAPE; + + // TODO ValueError for delimiter and enclosure string being longer than 1 byte + if (escape_str != NULL) { + if (escape_str_len > 1) { + zend_argument_value_error(4, "must be empty or a single character"); + RETURN_THROWS(); + } + + if (escape_str_len < 1) { + escape = PHP_CSV_NO_ESCAPE; + } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a non-empty string to the $escape parameter is deprecated since 8.4"); + if (UNEXPECTED(EG(exception))) { + RETURN_THROWS(); + } + escape = (unsigned char) escape_str[0]; + } } - HashTable *values = php_fgetcsv(NULL, delim, enc, esc, ZSTR_LEN(str), ZSTR_VAL(str)); + HashTable *values = php_fgetcsv(NULL, delim, enc, escape, ZSTR_LEN(str), ZSTR_VAL(str)); if (values == NULL) { values = php_bc_fgetcsv_empty_line(); } diff --git a/ext/standard/tests/file/bug40501.phpt b/ext/standard/tests/file/bug40501.phpt index 42fcd296cfa..e278609acab 100644 --- a/ext/standard/tests/file/bug40501.phpt +++ b/ext/standard/tests/file/bug40501.phpt @@ -11,6 +11,7 @@ fclose($h); var_dump($data); ?> --EXPECTF-- +Deprecated: fgetcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d array(2) { [0]=> string(%d) "this element contains the delimiter, and ends with an odd number of diff --git a/ext/standard/tests/file/bug72330.phpt b/ext/standard/tests/file/bug72330.phpt index 843032ae2d9..d869b7171dc 100644 --- a/ext/standard/tests/file/bug72330.phpt +++ b/ext/standard/tests/file/bug72330.phpt @@ -17,7 +17,8 @@ $string = '"first #' . $utf_1 . $utf_2 . '";"second"'; $fields = str_getcsv($string, ';', '"', "#"); var_dump($fields); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: str_getcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d array(2) { [0]=> string(11) "first #с؀" diff --git a/ext/standard/tests/file/fputcsv_variation15.phpt b/ext/standard/tests/file/fputcsv_variation15.phpt index 150fdd69d53..c9fa9932c9e 100644 --- a/ext/standard/tests/file/fputcsv_variation15.phpt +++ b/ext/standard/tests/file/fputcsv_variation15.phpt @@ -22,18 +22,13 @@ $list = array ( 13 => 'aaa,"bbb "', 14 => 'aaa"aaa","bbb"bbb', 15 => 'aaa"aaa""",bbb', - 16 => 'aaa,"/"bbb,ccc', - 17 => 'aaa"/"a","bbb"', - 18 => '"/"","aaa"', - 19 => '"/""",aaa', ); $file = __DIR__ . '/fputcsv_variation15.csv'; -@unlink($file); $fp = fopen($file, "w"); foreach ($list as $v) { - fputcsv($fp, explode(',', $v), ',', '"', '/'); + fputcsv($fp, explode(',', $v), ',', '"', ''); } fclose($fp); @@ -46,7 +41,7 @@ echo '$list = ';var_export($res);echo ";\n"; $fp = fopen($file, "r"); $res = array(); -while($l=fgetcsv($fp, 0, ',', '"', '/')) +while($l=fgetcsv($fp, 0, ',', '"', '')) { $res[] = join(',',$l); } @@ -54,8 +49,11 @@ fclose($fp); echo '$list = ';var_export($res);echo ";\n"; +?> +--CLEAN-- + --EXPECT-- $list = array ( @@ -75,10 +73,6 @@ $list = array ( 13 => 'aaa,"""bbb """', 14 => '"aaa""aaa""","""bbb""bbb"', 15 => '"aaa""aaa""""""",bbb', - 16 => 'aaa,"""/"bbb",ccc', - 17 => '"aaa""/"a""","""bbb"""', - 18 => '"""/"""","""aaa"""', - 19 => '"""/"""""",aaa', ); $list = array ( 0 => 'aaa,bbb', @@ -97,8 +91,4 @@ $list = array ( 13 => 'aaa,"bbb "', 14 => 'aaa"aaa","bbb"bbb', 15 => 'aaa"aaa""",bbb', - 16 => 'aaa,"/"bbb,ccc', - 17 => 'aaa"/"a","bbb"', - 18 => '"/"","aaa"', - 19 => '"/""",aaa', ); diff --git a/ext/standard/tests/file/fputcsv_variation17.phpt b/ext/standard/tests/file/fputcsv_variation17.phpt index 9592154f425..786b0649467 100644 --- a/ext/standard/tests/file/fputcsv_variation17.phpt +++ b/ext/standard/tests/file/fputcsv_variation17.phpt @@ -12,7 +12,7 @@ $eol_chars = ['||', '|', '\n', "\n", "\0"]; foreach ($eol_chars as $eol_char) { $stream = fopen('php://memory', 'w+'); foreach ($data as $record) { - fputcsv($stream, $record, ',', '"', '\\', $eol_char); + fputcsv($stream, $record, ',', '"', '', $eol_char); } rewind($stream); echo stream_get_contents($stream), "\n"; diff --git a/ext/standard/tests/file/fputcsv_variation18.phpt b/ext/standard/tests/file/fputcsv_variation18.phpt new file mode 100644 index 00000000000..7a6d84c21b0 --- /dev/null +++ b/ext/standard/tests/file/fputcsv_variation18.phpt @@ -0,0 +1,73 @@ +--TEST-- +fputcsv() variant where escape parameter matters +--FILE-- + 'aaa,"/"bbb,ccc', + 2 => 'aaa"/"a","bbb"', + 3 => '"/"","aaa"', + 4 => '"/""",aaa', +]; + +$file = __DIR__ . '/fputcsv_variation18.csv'; + +$fp = fopen($file, "w"); +foreach ($list as $v) { + fputcsv($fp, explode(',', $v), ',', '"', '/'); +} +fclose($fp); + +$res = file($file); +foreach($res as &$val) +{ + $val = substr($val, 0, -1); +} +echo '$list = ';var_export($res);echo ";\n"; + +$fp = fopen($file, "r"); +$res = array(); +while($l=fgetcsv($fp, 0, ',', '"', '/')) +{ + $res[] = join(',',$l); +} +fclose($fp); + +echo '$list = ';var_export($res);echo ";\n"; + +?> +--CLEAN-- + +--EXPECTF-- +Deprecated: fputcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d + +Deprecated: fputcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d + +Deprecated: fputcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d + +Deprecated: fputcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d +$list = array ( + 0 => 'aaa,"""/"bbb",ccc', + 1 => '"aaa""/"a""","""bbb"""', + 2 => '"""/"""","""aaa"""', + 3 => '"""/"""""",aaa', +); + +Deprecated: fgetcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d + +Deprecated: fgetcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d + +Deprecated: fgetcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d + +Deprecated: fgetcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d + +Deprecated: fgetcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d +$list = array ( + 0 => 'aaa,"/"bbb,ccc', + 1 => 'aaa"/"a","bbb"', + 2 => '"/"","aaa"', + 3 => '"/""",aaa', +); diff --git a/ext/standard/tests/strings/gh12151.phpt b/ext/standard/tests/strings/gh12151.phpt index eeb21ea5bdb..cebdc19ae09 100644 --- a/ext/standard/tests/strings/gh12151.phpt +++ b/ext/standard/tests/strings/gh12151.phpt @@ -5,10 +5,13 @@ GH-12151 (str_getcsv ending with escape zero segfualt) var_export(str_getcsv("y","","y","\000")); var_export(str_getcsv("\0yy","y","y","\0")); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: str_getcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d array ( 0 => '' . "\0" . '', -)array ( +) +Deprecated: str_getcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d +array ( 0 => '' . "\0" . '', 1 => '' . "\0" . '', ) diff --git a/ext/standard/tests/strings/str_getcsv_001.phpt b/ext/standard/tests/strings/str_getcsv_001.phpt index 8bad30b2be5..7b6b5192b4c 100644 --- a/ext/standard/tests/strings/str_getcsv_001.phpt +++ b/ext/standard/tests/strings/str_getcsv_001.phpt @@ -18,8 +18,6 @@ var_dump(str_getcsv('.foo..bar.', '.', '.', '.')); print "-----\n"; var_dump(str_getcsv('.foo. .bar.', ' ', '.', '.')); print "-----\n"; -var_dump(str_getcsv('1foo1 1bar111', ' ', '1 ', '\ ')); -print "-----\n"; var_dump(str_getcsv('.foo . . bar .', ' ', '.', '')); print "-----\n"; var_dump(str_getcsv('" "" "', ' ')); @@ -28,7 +26,7 @@ var_dump(str_getcsv('')); print "-----\n"; ?> ---EXPECT-- +--EXPECTF-- array(3) { [0]=> string(1) "f" @@ -61,6 +59,8 @@ array(2) { string(3) "bar" } ----- + +Deprecated: str_getcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d array(3) { [0]=> string(2) "f." @@ -70,11 +70,15 @@ array(3) { string(4) "-|-." } ----- + +Deprecated: str_getcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d array(1) { [0]=> string(7) "foo.bar" } ----- + +Deprecated: str_getcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 in %s on line %d array(2) { [0]=> string(3) "foo" @@ -82,13 +86,6 @@ array(2) { string(3) "bar" } ----- -array(2) { - [0]=> - string(3) "foo" - [1]=> - string(4) "bar1" -} ------ array(2) { [0]=> string(5) "foo " diff --git a/ext/standard/tests/strings/str_getcsv_errors.phpt b/ext/standard/tests/strings/str_getcsv_errors.phpt new file mode 100644 index 00000000000..00b6dca1698 --- /dev/null +++ b/ext/standard/tests/strings/str_getcsv_errors.phpt @@ -0,0 +1,15 @@ +--TEST-- +str_getcsv(): Invalid arguments +--FILE-- +getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +ValueError: str_getcsv(): Argument #4 ($escape) must be empty or a single character \ No newline at end of file