Skip to content

Commit 0353557

Browse files
committed
Fix GH-8563, GH-8564: SplFileObject EOF handling for seek() and next()
spl_filesystem_file_read_ex() treated a NULL buffer from php_stream_get_line as a successful read of an empty line, creating a phantom cached line at EOF. This caused seek() to give inconsistent results between SplFileObject and SplTempFileObject (GH-8563), and next() to increment the line counter indefinitely past EOF (GH-8564). Return FAILURE when php_stream_get_line returns NULL, matching the behavior of the existing php_stream_eof check. In seek(), break out of the loop on EOF instead of returning, so the post-loop cleanup runs consistently. In next(), return early at EOF without incrementing. Make __toString() return empty string at EOF instead of throwing. Closes GH-8563 Closes GH-8564
1 parent 6035b0a commit 0353557

File tree

8 files changed

+74
-25
lines changed

8 files changed

+74
-25
lines changed

ext/spl/spl_directory.c

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1813,21 +1813,24 @@ static zend_result spl_filesystem_file_read_ex(spl_filesystem_object *intern, bo
18131813
}
18141814

18151815
if (!buf) {
1816-
intern->u.file.current_line = ZSTR_EMPTY_ALLOC();
1817-
} else {
1818-
if (!csv && SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_DROP_NEW_LINE)) {
1819-
if (line_len > 0 && buf[line_len - 1] == '\n') {
1816+
if (!silent) {
1817+
spl_filesystem_file_cannot_read(intern);
1818+
}
1819+
return FAILURE;
1820+
}
1821+
1822+
if (!csv && SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_DROP_NEW_LINE)) {
1823+
if (line_len > 0 && buf[line_len - 1] == '\n') {
1824+
line_len--;
1825+
if (line_len > 0 && buf[line_len - 1] == '\r') {
18201826
line_len--;
1821-
if (line_len > 0 && buf[line_len - 1] == '\r') {
1822-
line_len--;
1823-
}
1824-
buf[line_len] = '\0';
18251827
}
1828+
buf[line_len] = '\0';
18261829
}
1827-
1828-
intern->u.file.current_line = zend_string_init(buf, line_len, /* persistent */ false);
1829-
efree(buf);
18301830
}
1831+
1832+
intern->u.file.current_line = zend_string_init(buf, line_len, /* persistent */ false);
1833+
efree(buf);
18311834
intern->u.file.current_line_num += line_add;
18321835

18331836
return SUCCESS;
@@ -2144,7 +2147,9 @@ PHP_METHOD(SplFileObject, next)
21442147
ZEND_PARSE_PARAMETERS_NONE();
21452148

21462149
if (!intern->u.file.current_line && Z_ISUNDEF(intern->u.file.current_zval)) {
2147-
spl_filesystem_file_read_line(ZEND_THIS, intern, true);
2150+
if (spl_filesystem_file_read_line(ZEND_THIS, intern, true) == FAILURE) {
2151+
return;
2152+
}
21482153
}
21492154

21502155
spl_filesystem_file_free_line(intern);
@@ -2634,7 +2639,7 @@ PHP_METHOD(SplFileObject, seek)
26342639

26352640
for (i = 0; i < line_pos; i++) {
26362641
if (spl_filesystem_file_read_line(ZEND_THIS, intern, true) == FAILURE) {
2637-
return;
2642+
break;
26382643
}
26392644
}
26402645
if (line_pos > 0 && !SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_READ_AHEAD)) {
@@ -2653,9 +2658,8 @@ PHP_METHOD(SplFileObject, __toString)
26532658

26542659
if (!intern->u.file.current_line) {
26552660
ZEND_ASSERT(Z_ISUNDEF(intern->u.file.current_zval));
2656-
zend_result result = spl_filesystem_file_read_line(ZEND_THIS, intern, false);
2657-
if (UNEXPECTED(result != SUCCESS)) {
2658-
RETURN_THROWS();
2661+
if (spl_filesystem_file_read_line(ZEND_THIS, intern, true) == FAILURE) {
2662+
RETURN_EMPTY_STRING();
26592663
}
26602664
}
26612665

ext/spl/tests/SplFileObject/SplFileObject_key_error001.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ var_dump($s->key());
1818
var_dump($s->valid());
1919
?>
2020
--EXPECT--
21-
int(14)
21+
int(12)
2222
bool(false)

ext/spl/tests/SplFileObject/SplFileObject_key_error002.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ var_dump($s->key());
1818
var_dump($s->valid());
1919
?>
2020
--EXPECT--
21-
int(13)
21+
int(12)
2222
bool(false)

ext/spl/tests/SplFileObject/bug81477.phpt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ string(8) "baz,bat
2121
"
2222
string(10) "more,data
2323
"
24-
string(0) ""
2524
--CLEAN--
2625
<?php
2726
@unlink(__DIR__ . '/bug81477.csv');

ext/spl/tests/SplFileObject/fgetcsv_blank_file.phpt

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,5 @@ $file->rewind();
1818
var_dump($file->fgetcsv());
1919
?>
2020
--EXPECT--
21-
array(1) {
22-
[0]=>
23-
NULL
24-
}
21+
bool(false)
2522
bool(false)
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GH-8563 (Different results for seek() on SplFileObject and SplTempFileObject)
3+
--FILE--
4+
<?php
5+
$path = __DIR__ . '/gh8563.txt';
6+
$file_01 = new SplFileObject($path, 'w+');
7+
$file_02 = new SplTempFileObject();
8+
9+
for ($i = 0; $i < 5; $i++) {
10+
$file_01->fwrite("line {$i}" . PHP_EOL);
11+
$file_02->fwrite("line {$i}" . PHP_EOL);
12+
}
13+
14+
$file_01->rewind();
15+
$file_02->rewind();
16+
17+
$file_01->seek(10);
18+
$file_02->seek(10);
19+
20+
echo 'SplFileObject: ' . $file_01->key() . "\n";
21+
echo 'SplTempFileObject: ' . $file_02->key() . "\n";
22+
?>
23+
--CLEAN--
24+
<?php
25+
unlink(__DIR__ . '/gh8563.txt');
26+
?>
27+
--EXPECT--
28+
SplFileObject: 5
29+
SplTempFileObject: 5
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
GH-8564 (SplFileObject: next() moves to nonexistent indexes)
3+
--FILE--
4+
<?php
5+
$file = new SplTempFileObject();
6+
for ($i = 0; $i < 5; $i++) {
7+
$file->fwrite("line {$i}" . PHP_EOL);
8+
}
9+
10+
$file->rewind();
11+
for ($i = 0; $i < 10; $file->next(), $i++);
12+
echo "next() 10x: key=" . $file->key() . " valid=" . var_export($file->valid(), true) . "\n";
13+
14+
$file->rewind();
15+
$file->seek(10);
16+
echo "seek(10): key=" . $file->key() . " valid=" . var_export($file->valid(), true) . "\n";
17+
?>
18+
--EXPECT--
19+
next() 10x: key=5 valid=false
20+
seek(10): key=5 valid=false

ext/spl/tests/gh13685.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ try {
4444
string(14) ""A", "B", "C"
4545
"
4646
string(13) ""D", "E", "F""
47-
Cannot read from file php://temp
47+
string(0) ""
4848
--- Use csv control ---
4949
string(14) ""A", "B", "C"
5050
"
5151
string(13) ""D", "E", "F""
52-
Cannot read from file php://temp
52+
string(0) ""

0 commit comments

Comments
 (0)