Skip to content

Commit

Permalink
Fix #62004: SplFileObject: fgets after seek returns wrong line
Browse files Browse the repository at this point in the history
As it is, `::seek(0)` sets the file pointer to the beginning of the
file, but `::seek($n)` where `$n > 0` sets the file pointer to the
beginning of the following line, having line `$n` already read into the
line buffer.  This is pretty inconsistent; we fix it by always seeking
to the beginning of the line.

We also add a test case for the duplicate bug #46569.

Closes GH-6434.
  • Loading branch information
cmb69 committed Nov 30, 2020
1 parent 841b00f commit f1d11c1
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 7 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ PHP NEWS
- Phpdbg:
. Fixed bug #76813 (Access violation near NULL on source operand). (cmb)

- SPL:
. Fixed #62004 (SplFileObject: fgets after seek returns wrong line). (cmb)

- Standard:
. Fixed bug #80366 (Return Value of zend_fstat() not Checked). (sagpant, cmb)

Expand Down
2 changes: 2 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,8 @@ PHP 8.0 UPGRADE NOTES

- SPL:
. SplFileObject::fgetss() has been removed.
. SplFileObject::seek() now always seeks to the beginning of the line.
Previously, positions >=1 sought to the beginning of the next line.
. SplHeap::compare($a, $b) now specifies a method signature. Inheriting
classes implementing this method will now have to use a compatible
method signature.
Expand Down
10 changes: 7 additions & 3 deletions ext/spl/spl_directory.c
Original file line number Diff line number Diff line change
Expand Up @@ -2727,7 +2727,7 @@ PHP_METHOD(SplFileObject, ftruncate)
PHP_METHOD(SplFileObject, seek)
{
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);
zend_long line_pos;
zend_long line_pos, i;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &line_pos) == FAILURE) {
RETURN_THROWS();
Expand All @@ -2742,11 +2742,15 @@ PHP_METHOD(SplFileObject, seek)

spl_filesystem_file_rewind(ZEND_THIS, intern);

while(intern->u.file.current_line_num < line_pos) {
for (i = 0; i < line_pos; i++) {
if (spl_filesystem_file_read_line(ZEND_THIS, intern, 1) == FAILURE) {
break;
return;
}
}
if (line_pos > 0) {
intern->u.file.current_line_num++;
spl_filesystem_file_free_line(intern);
}
} /* }}} */

/* {{{ PHP_MINIT_FUNCTION(spl_directory) */
Expand Down
4 changes: 2 additions & 2 deletions ext/spl/tests/SplFileObject_key_error001.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ Erwin Poeze <erwin.poeze@gmail.com>
//line 5
$s = new SplFileObject(__FILE__);

$s->seek(12);
$s->seek(13);
$s->next();
var_dump($s->key());
var_dump($s->valid());
?>
--EXPECT--
int(13)
int(14)
bool(false)
2 changes: 1 addition & 1 deletion ext/spl/tests/SplFileObject_next_variation002.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ echo $s->current();
--EXPECT--
//line 3
//line 4
//line 3
//line 4
//line 5
5 changes: 5 additions & 0 deletions ext/spl/tests/bug46569.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
first,line
second,line
third,line
fourth,line
fifth,line
14 changes: 14 additions & 0 deletions ext/spl/tests/bug46569.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Bug #46569 (SplFileObject: fgetcsv after seek returns wrong line)
--FILE--
<?php
$file = new SplFileObject(__DIR__ . '/bug46569.csv');
$file->seek(1);
print_r($file->fgetcsv());
?>
--EXPECT--
Array
(
[0] => second
[1] => line
)
17 changes: 17 additions & 0 deletions ext/spl/tests/bug62004.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--TEST--
Bug #62004 (SplFileObject: fgets after seek returns wrong line)
--FILE--
<?php
$f = new SplFileObject(__DIR__ . '/bug62004.txt');
$f->setFlags(SplFileObject::SKIP_EMPTY);
$f->seek(0);
echo $f->fgets();
$f->seek(1);
echo $f->fgets();
$f->seek(2);
echo $f->fgets();
?>
--EXPECT--
Line 1
Line 2
Line 3
4 changes: 4 additions & 0 deletions ext/spl/tests/bug62004.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Line 1
Line 2
Line 3
Line 4
2 changes: 1 addition & 1 deletion ext/spl/tests/fileobject_getcurrentline_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ echo $s->getCurrentLine();
echo $s->getCurrentLine();
?>
--EXPECT--
//line 2
//line 3
//line 4

0 comments on commit f1d11c1

Please sign in to comment.