Skip to content

Commit

Permalink
Fix GH-8121: SplFileObject - seek and key with csv file inconsistent
Browse files Browse the repository at this point in the history
First, we must not free the current line before we call
`spl_filesystem_file_read_csv()`, because then the `current_line` will
not be properly updated.  Since the EOF check is superfluous here, we
move that part of the code to the branch for subtypes.  This issue has
been introduced by the fix for bug 75917.

Second, we only must increase the `current_line` if we're not reading
ahead.  This issue has been introduced by the fix for bug 62004.

Closes GH-8138.
  • Loading branch information
cmb69 committed Mar 8, 2022
1 parent 15949b6 commit 1d9a1f9
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 8 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ PHP NEWS
- Pcntl:
. Fixed bug GH-8142 (Compilation error on cygwin). (David Carlier)

- SPL:
. Fixed bug GH-8121 (SplFileObject - seek and key with csv file inconsistent).
(cmb)

- Standard:
. Fixed bug GH-8048 (Force macOS to use statfs). (risner)

Expand Down
15 changes: 7 additions & 8 deletions ext/spl/spl_directory.c
Original file line number Diff line number Diff line change
Expand Up @@ -1938,7 +1938,11 @@ static int spl_filesystem_file_read_line_ex(zval * this_ptr, spl_filesystem_obje
zval retval;

/* 1) use fgetcsv? 2) overloaded call the function, 3) do it directly */
if (SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_READ_CSV) || intern->u.file.func_getCurr->common.scope != spl_ce_SplFileObject) {
if (SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_READ_CSV)) {
return spl_filesystem_file_read_csv(intern, intern->u.file.delimiter, intern->u.file.enclosure, intern->u.file.escape, NULL);
}
if (intern->u.file.func_getCurr->common.scope != spl_ce_SplFileObject) {
zend_execute_data *execute_data = EG(current_execute_data);
spl_filesystem_file_free_line(intern);

if (php_stream_eof(intern->u.file.stream)) {
Expand All @@ -1947,12 +1951,7 @@ static int spl_filesystem_file_read_line_ex(zval * this_ptr, spl_filesystem_obje
}
return FAILURE;
}
if (SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_READ_CSV)) {
return spl_filesystem_file_read_csv(intern, intern->u.file.delimiter, intern->u.file.enclosure, intern->u.file.escape, NULL);
} else {
zend_execute_data *execute_data = EG(current_execute_data);
zend_call_method_with_0_params(Z_OBJ_P(this_ptr), Z_OBJCE_P(ZEND_THIS), &intern->u.file.func_getCurr, "getCurrentLine", &retval);
}
zend_call_method_with_0_params(Z_OBJ_P(this_ptr), Z_OBJCE_P(ZEND_THIS), &intern->u.file.func_getCurr, "getCurrentLine", &retval);
if (!Z_ISUNDEF(retval)) {
if (intern->u.file.current_line || !Z_ISUNDEF(intern->u.file.current_zval)) {
intern->u.file.current_line_num++;
Expand Down Expand Up @@ -2739,8 +2738,8 @@ PHP_METHOD(SplFileObject, seek)
}
}
if (line_pos > 0) {
intern->u.file.current_line_num++;
if (!SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_READ_AHEAD)) {
intern->u.file.current_line_num++;
spl_filesystem_file_free_line(intern);
}
}
Expand Down
5 changes: 5 additions & 0 deletions ext/spl/tests/gh8121.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
id,status,on_sale,brand,name,link,meta_title,meta_desc,description
1,2,15,Samsung,M21,samsung-m21,Samsung M21,Samsung M21,Samsung M21
2,2,15,Samsung,M32,samsung-m32,Samsung M32,Samsung M32,Samsung M32
3,2,15,Samsung,M21,samsung-m21,Samsung M21,Samsung M21,Samsung M21
4,2,15,Samsung,M32,samsung-m32,Samsung M32,Samsung M32,Samsung M32
39 changes: 39 additions & 0 deletions ext/spl/tests/gh8121.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--TEST--
GH-8121 (SplFileObject - seek and key with csv file inconsistent)
--FILE--
<?php
$flagss = [
SplFileObject::READ_AHEAD | SplFileObject::READ_CSV | SplFileObject::SKIP_EMPTY | SplFileObject::DROP_NEW_LINE,
SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY | SplFileObject::DROP_NEW_LINE,
SplFileObject::SKIP_EMPTY | SplFileObject::DROP_NEW_LINE,
];
foreach ($flagss as $flags) {
$file = new SplFileObject(__DIR__ . "/gh8121.csv", "r");
echo "flags: $flags\n";
$file->setFlags($flags);
$file->seek(0);
var_dump($file->key());
$file->seek(1);
var_dump($file->key());
$file->seek(2);
var_dump($file->key());
$file->seek(3);
var_dump($file->key());
}
?>
--EXPECT--
flags: 15
int(0)
int(1)
int(2)
int(3)
flags: 7
int(0)
int(1)
int(2)
int(3)
flags: 5
int(0)
int(1)
int(2)
int(3)

0 comments on commit 1d9a1f9

Please sign in to comment.