Fix #62004: SplFileObject: fgets after seek returns wrong line

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.

diff --git a/NEWS b/NEWS
index f7c3861100c307d5f53e4597322bc8570d15b3b6..a60ed29a1c3ba83486876e9a0bb1552d4065f241 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -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)
 

diff --git a/UPGRADING b/UPGRADING
index fc7376e47f1a435a56f38b003358d25f84285cd4..1e2dcc49d491e9bcc6d8bc577d2d3231ea8c3226 100644 (file)
--- a/UPGRADING
+++ b/UPGRADING
@@ -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.

diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c
index 3d0dae243502bbf27ee14d53526bb7fff2969743..7d9ea9cc2bfaa65eeac42bca4281804db51d05f3 100644 (file)
--- a/ext/spl/spl_directory.c
+++ b/ext/spl/spl_directory.c
@@ -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();
@@ -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) */

diff --git a/ext/spl/tests/SplFileObject_key_error001.phpt b/ext/spl/tests/SplFileObject_key_error001.phpt
index b0834f00290ae482a995514d1dabd96163c5a440..0c21d0b905e95ebcfbb2840233d900d73e6e2a78 100644 (file)
--- a/ext/spl/tests/SplFileObject_key_error001.phpt
+++ b/ext/spl/tests/SplFileObject_key_error001.phpt
@@ -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)

diff --git a/ext/spl/tests/SplFileObject_next_variation002.phpt b/ext/spl/tests/SplFileObject_next_variation002.phpt
index d48ff8c22371c8f5bd9643fd95bbf4af2793367b..e4903dce0cd23d218c23c939b021c736715e96c6 100644 (file)
--- a/ext/spl/tests/SplFileObject_next_variation002.phpt
+++ b/ext/spl/tests/SplFileObject_next_variation002.phpt
@@ -26,5 +26,5 @@ echo $s->current();
 --EXPECT--
 //line 3
 //line 4
-//line 3
 //line 4
+//line 5

diff --git a/ext/spl/tests/bug46569.csv b/ext/spl/tests/bug46569.csv
new file mode 100644 (file)
index 0000000..f456a03
--- /dev/null
+++ b/ext/spl/tests/bug46569.csv
@@ -0,0 +1,5 @@
+first,line
+second,line
+third,line
+fourth,line
+fifth,line

diff --git a/ext/spl/tests/bug46569.phpt b/ext/spl/tests/bug46569.phpt
new file mode 100644 (file)
index 0000000..0c1ab6c
--- /dev/null
+++ b/ext/spl/tests/bug46569.phpt
@@ -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
+)

diff --git a/ext/spl/tests/bug62004.phpt b/ext/spl/tests/bug62004.phpt
new file mode 100644 (file)
index 0000000..4a06738
--- /dev/null
+++ b/ext/spl/tests/bug62004.phpt
@@ -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

diff --git a/ext/spl/tests/bug62004.txt b/ext/spl/tests/bug62004.txt
new file mode 100644 (file)
index 0000000..e579141
--- /dev/null
+++ b/ext/spl/tests/bug62004.txt
@@ -0,0 +1,4 @@
+Line 1
+Line 2
+Line 3
+Line 4

diff --git a/ext/spl/tests/fileobject_getcurrentline_basic.phpt b/ext/spl/tests/fileobject_getcurrentline_basic.phpt
index 607fce6640a3dce1b9c7b8a5a2e8f03ed482b5fd..47f6e28dd8b40c33287c09ed30f41d6e5bc3cf7c 100644 (file)
--- a/ext/spl/tests/fileobject_getcurrentline_basic.phpt
+++ b/ext/spl/tests/fileobject_getcurrentline_basic.phpt
@@ -15,5 +15,5 @@ echo $s->getCurrentLine();
 echo $s->getCurrentLine();
 ?>
 --EXPECT--
+//line 2
 //line 3
-//line 4