From 22f1ec3fa1b923fcafcb10f0372db97f6ba607ce Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 27 May 2022 13:13:50 +0100 Subject: [PATCH 1/2] Fix GH-8563 With memory streams if we get a NULL buffer we must not instantiate an empty line --- ext/spl/spl_directory.c | 23 +++++---- .../SplFileObject_fgetcsv_basic.phpt | 2 + .../SplFileObject_key_error001.phpt | 2 +- .../SplFileObject_key_error002.phpt | 2 +- ext/spl/tests/SplFileObject/gh8563.phpt | 48 +++++++++++++++++++ ext/spl/tests/bug81477.phpt | 1 - ext/spl/tests/fileobject_001.phpt | 2 +- 7 files changed, 64 insertions(+), 16 deletions(-) rename ext/spl/tests/{ => SplFileObject}/SplFileObject_fgetcsv_basic.phpt (93%) rename ext/spl/tests/{ => SplFileObject}/SplFileObject_key_error001.phpt (97%) rename ext/spl/tests/{ => SplFileObject}/SplFileObject_key_error002.phpt (97%) create mode 100644 ext/spl/tests/SplFileObject/gh8563.phpt diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 8646ba20b0e7f..0302e3c4ffb6a 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -1883,22 +1883,21 @@ static zend_result spl_filesystem_file_read_ex(spl_filesystem_object *intern, bo } if (!buf) { - intern->u.file.current_line = estrdup(""); - intern->u.file.current_line_len = 0; - } else { - if (SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_DROP_NEW_LINE)) { - if (line_len > 0 && buf[line_len - 1] == '\n') { + return FAILURE; + } + + if (SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_DROP_NEW_LINE)) { + if (line_len > 0 && buf[line_len - 1] == '\n') { + line_len--; + if (line_len > 0 && buf[line_len - 1] == '\r') { line_len--; - if (line_len > 0 && buf[line_len - 1] == '\r') { - line_len--; - } - buf[line_len] = '\0'; } + buf[line_len] = '\0'; } - - intern->u.file.current_line = buf; - intern->u.file.current_line_len = line_len; } + + intern->u.file.current_line = buf; + intern->u.file.current_line_len = line_len; intern->u.file.current_line_num += line_add; return SUCCESS; diff --git a/ext/spl/tests/SplFileObject_fgetcsv_basic.phpt b/ext/spl/tests/SplFileObject/SplFileObject_fgetcsv_basic.phpt similarity index 93% rename from ext/spl/tests/SplFileObject_fgetcsv_basic.phpt rename to ext/spl/tests/SplFileObject/SplFileObject_fgetcsv_basic.phpt index 2580021426fd3..166a0d7fe0205 100644 --- a/ext/spl/tests/SplFileObject_fgetcsv_basic.phpt +++ b/ext/spl/tests/SplFileObject/SplFileObject_fgetcsv_basic.phpt @@ -13,6 +13,7 @@ fclose($fp); $fo = new SplFileObject('SplFileObject__fgetcsv1.csv'); var_dump($fo->fgetcsv()); +var_dump($fo->fgetcsv()); ?> --CLEAN-- string(1) "5" } +NULL diff --git a/ext/spl/tests/SplFileObject_key_error001.phpt b/ext/spl/tests/SplFileObject/SplFileObject_key_error001.phpt similarity index 97% rename from ext/spl/tests/SplFileObject_key_error001.phpt rename to ext/spl/tests/SplFileObject/SplFileObject_key_error001.phpt index 0c21d0b905e95..7d0e3ae8d9698 100644 --- a/ext/spl/tests/SplFileObject_key_error001.phpt +++ b/ext/spl/tests/SplFileObject/SplFileObject_key_error001.phpt @@ -18,5 +18,5 @@ var_dump($s->key()); var_dump($s->valid()); ?> --EXPECT-- -int(14) +int(12) bool(false) diff --git a/ext/spl/tests/SplFileObject_key_error002.phpt b/ext/spl/tests/SplFileObject/SplFileObject_key_error002.phpt similarity index 97% rename from ext/spl/tests/SplFileObject_key_error002.phpt rename to ext/spl/tests/SplFileObject/SplFileObject_key_error002.phpt index 8fc9b7fef0a58..0834dbc0524fc 100644 --- a/ext/spl/tests/SplFileObject_key_error002.phpt +++ b/ext/spl/tests/SplFileObject/SplFileObject_key_error002.phpt @@ -18,5 +18,5 @@ var_dump($s->key()); var_dump($s->valid()); ?> --EXPECT-- -int(13) +int(12) bool(false) diff --git a/ext/spl/tests/SplFileObject/gh8563.phpt b/ext/spl/tests/SplFileObject/gh8563.phpt new file mode 100644 index 0000000000000..11309ae8101fa --- /dev/null +++ b/ext/spl/tests/SplFileObject/gh8563.phpt @@ -0,0 +1,48 @@ +--TEST-- +Bug GH-8563: Different results for seek() on SplFileObject and SplTempFileObject +--FILE-- +fwrite("line {$i}" . PHP_EOL); + $file_02->fwrite("line {$i}" . PHP_EOL); + $file_03->fwrite("line {$i}" . PHP_EOL); + $file_04->fwrite("line {$i}" . PHP_EOL); +} + +// reset +$file_01->rewind(); +$file_02->rewind(); +$file_03->rewind(); +$file_04->rewind(); + +// seek +$file_01->seek(INDEX); +$file_02->seek(INDEX); +$file_03->seek(INDEX); +$file_04->seek(INDEX); + +// show results +echo 'file_01: ' . $file_01->key(), PHP_EOL; +echo 'file_02: ' . $file_02->key(), PHP_EOL; +echo 'file_03: ' . $file_03->key(), PHP_EOL; +echo 'file_04: ' . $file_04->key(), PHP_EOL; +?> +--CLEAN-- + +--EXPECT-- +file_01: 4 +file_02: 4 +file_03: 4 +file_04: 4 diff --git a/ext/spl/tests/bug81477.phpt b/ext/spl/tests/bug81477.phpt index f7730a791aa03..421c74dc4d68e 100644 --- a/ext/spl/tests/bug81477.phpt +++ b/ext/spl/tests/bug81477.phpt @@ -21,7 +21,6 @@ string(8) "baz,bat " string(10) "more,data " -string(0) "" --CLEAN-- Date: Wed, 8 Jun 2022 11:59:06 +0100 Subject: [PATCH 2/2] Add more tests to ensure nothing was broken --- .../fgetcsv_file_empty_lines.phpt | 38 +++++++++++++++++++ .../foreach_file_empty_lines.phpt | 37 ++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 ext/spl/tests/SplFileObject/fgetcsv_file_empty_lines.phpt create mode 100644 ext/spl/tests/SplFileObject/foreach_file_empty_lines.phpt diff --git a/ext/spl/tests/SplFileObject/fgetcsv_file_empty_lines.phpt b/ext/spl/tests/SplFileObject/fgetcsv_file_empty_lines.phpt new file mode 100644 index 0000000000000..464c98fc505b5 --- /dev/null +++ b/ext/spl/tests/SplFileObject/fgetcsv_file_empty_lines.phpt @@ -0,0 +1,38 @@ +--TEST-- +SplFileObject::fgetcsv with empty lines +--FILE-- +fwrite("foo,bar\n"); +$file->fwrite("\n"); +$file->fwrite("baz,qux"); + +$file->rewind(); + + +var_dump($file->fgetcsv()); +var_dump($file->fgetcsv()); +var_dump($file->fgetcsv()); +var_dump($file->fgetcsv()); + +?> +--EXPECT-- +array(2) { + [0]=> + string(3) "foo" + [1]=> + string(3) "bar" +} +array(1) { + [0]=> + NULL +} +array(2) { + [0]=> + string(3) "baz" + [1]=> + string(3) "qux" +} +NULL diff --git a/ext/spl/tests/SplFileObject/foreach_file_empty_lines.phpt b/ext/spl/tests/SplFileObject/foreach_file_empty_lines.phpt new file mode 100644 index 0000000000000..5fefdf0fddd5a --- /dev/null +++ b/ext/spl/tests/SplFileObject/foreach_file_empty_lines.phpt @@ -0,0 +1,37 @@ +--TEST-- +Iterate over SplFileObject with empty lines with CSV flags +--FILE-- +fwrite("foo,bar\n"); +$file->fwrite("\n"); +$file->fwrite("baz,qux"); + +$file->rewind(); + +$file->setFlags(SplFileObject::READ_CSV | SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY /* | SplFileObject::DROP_NEW_LINE */); + + +foreach ($file as $line) { + var_dump($line); +} +?> +--EXPECT-- +array(2) { + [0]=> + string(3) "foo" + [1]=> + string(3) "bar" +} +array(1) { + [0]=> + NULL +} +array(2) { + [0]=> + string(3) "baz" + [1]=> + string(3) "qux" +}