diff --git a/NEWS b/NEWS index 4441a4e984c6..d1541f6f7cb3 100644 --- a/NEWS +++ b/NEWS @@ -92,6 +92,10 @@ PHP NEWS - fgetcsv() - str_getcsv() is now deprecated. (Girgias) + . The str_getcsv() function now throws ValueErrors when the $separator and + $enclosure arguments are not one byte long, or if the $escape is not one + byte long or the empty string. This aligns the behaviour to be identical + to that of fputcsv() and fgetcsv(). (Girgias) - Streams: . Implemented GH-15155 (Stream context is lost when custom stream wrapper is diff --git a/UPGRADING b/UPGRADING index e87d2769b2b0..a1e23d148da8 100644 --- a/UPGRADING +++ b/UPGRADING @@ -184,6 +184,10 @@ PHP 8.4 UPGRADE NOTES PHP_ROUND_HALF_UP. . strcspn() with empty $characters now returns the length of the string instead of incorrectly stopping at the first NUL character. See GH-12592. + . The str_getcsv() function now throws ValueErrors when the $separator and + $enclosure arguments are not one byte long, or if the $escape is not one + byte long or the empty string. This aligns the behaviour to be identical + to that of fputcsv() and fgetcsv(). - Tidy: . Failures in the constructor now throw exceptions rather than emitting diff --git a/ext/standard/string.c b/ext/standard/string.c index ddc2d2b16458..23c34df958b5 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -5446,23 +5446,36 @@ PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, const char *allow, size_ PHP_FUNCTION(str_getcsv) { zend_string *str; - char delim = ',', enc = '"'; + char delimiter = ',', enclosure = '"'; 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; + char *delimiter_str = NULL, *enclosure_str = NULL, *escape_str = NULL; + size_t delimiter_str_len = 0, enclosure_str_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(delimiter_str, delimiter_str_len) + Z_PARAM_STRING(enclosure_str, enclosure_str_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; - - // TODO ValueError for delimiter and enclosure string being longer than 1 byte + if (delimiter_str != NULL) { + /* Make sure that there is at least one character in string */ + if (delimiter_str_len != 1) { + zend_argument_value_error(2, "must be a single character"); + RETURN_THROWS(); + } + /* use first character from string */ + delimiter = delimiter_str[0]; + } + if (enclosure_str != NULL) { + if (enclosure_str_len != 1) { + zend_argument_value_error(3, "must be a single character"); + RETURN_THROWS(); + } + /* use first character from string */ + enclosure = enclosure_str[0]; + } if (escape_str != NULL) { if (escape_str_len > 1) { zend_argument_value_error(4, "must be empty or a single character"); @@ -5480,7 +5493,7 @@ PHP_FUNCTION(str_getcsv) } } - HashTable *values = php_fgetcsv(NULL, delim, enc, escape, ZSTR_LEN(str), ZSTR_VAL(str)); + HashTable *values = php_fgetcsv(NULL, delimiter, enclosure, escape, ZSTR_LEN(str), ZSTR_VAL(str)); if (values == NULL) { values = php_bc_fgetcsv_empty_line(); } diff --git a/ext/standard/tests/strings/gh12151.phpt b/ext/standard/tests/strings/gh12151.phpt index cebdc19ae09f..dfbe3facde3e 100644 --- a/ext/standard/tests/strings/gh12151.phpt +++ b/ext/standard/tests/strings/gh12151.phpt @@ -2,7 +2,7 @@ GH-12151 (str_getcsv ending with escape zero segfualt) --FILE-- --EXPECTF-- diff --git a/ext/standard/tests/strings/str_getcsv_001.phpt b/ext/standard/tests/strings/str_getcsv_001.phpt index 7b6b5192b4c0..795ce042b27b 100644 --- a/ext/standard/tests/strings/str_getcsv_001.phpt +++ b/ext/standard/tests/strings/str_getcsv_001.phpt @@ -16,8 +16,6 @@ var_dump(str_getcsv('|f.|.|bar|.|-|-.|', '.', '|', '-')); print "-----\n"; var_dump(str_getcsv('.foo..bar.', '.', '.', '.')); print "-----\n"; -var_dump(str_getcsv('.foo. .bar.', ' ', '.', '.')); -print "-----\n"; var_dump(str_getcsv('.foo . . bar .', ' ', '.', '')); print "-----\n"; var_dump(str_getcsv('" "" "', ' ')); @@ -77,15 +75,6 @@ array(1) { 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" - [1]=> - string(3) "bar" -} ------ 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 index 00b6dca1698b..6f77ca693230 100644 --- a/ext/standard/tests/strings/str_getcsv_errors.phpt +++ b/ext/standard/tests/strings/str_getcsv_errors.phpt @@ -5,11 +5,23 @@ str_getcsv(): Invalid arguments // string input[, string delimiter[, string enclosure[, string escape]]] try { - var_dump(str_getcsv('csv_string', ',', '"', 'enclosure')); + var_dump(str_getcsv('csv_string', 'separator', '"', '')); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump(str_getcsv('csv_string', ',', 'enclosure', '')); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump(str_getcsv('csv_string', ',', '"', 'escape')); } catch (Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } ?> --EXPECT-- -ValueError: str_getcsv(): Argument #4 ($escape) must be empty or a single character \ No newline at end of file +ValueError: str_getcsv(): Argument #2 ($separator) must be a single character +ValueError: str_getcsv(): Argument #3 ($enclosure) must be a single character +ValueError: str_getcsv(): Argument #4 ($escape) must be empty or a single character