SplFileObject: next() moves to inexistent indexes

[gabrielpl]

Description

The following code:

<?php

declare(strict_types=1);

const LINES = 5;
const INDEX = 10;
        
$file = new SplFileObject('/tmp/test.txt', 'w+');

// write to files
for ($i = 0; $i < LINES; $i++) {
    $file->fwrite("line {$i}" . PHP_EOL);
}

// behaviour of next()
$file->rewind();
for ($i = 0; $i < INDEX; $file->next(), $i++);
echo 'next(): ', $file->key(), PHP_EOL;

// behaviour of seek()
$file->rewind();
$file->seek(INDEX);
echo 'seek(): ', $file->key(), PHP_EOL;

Resulted in this output:

next(): 10
seek(): 5

But I expected this output instead:

next(): 5
seek(): 5

PHP Version

PHP 8.1.6

Operating System

Windows 11

[gabrielpl]

Note:

The documentation of next() is incomplete and does not define what should happen in this scenario.

May be:

1. When key() is in the last line, next() does nothing.
2. When key() is in the last line, next() moves to EOF.
   When eof() is true, next() throws an exception.

In my opinion, option 2 is the most correct.
Either way, the exact behaviour of next() must be explicit stated in the docs

[cmb69]

php-src/ext/spl/spl_directory.c

Lines 2237 to 2250 in 6beee1a

	PHP_METHOD(SplFileObject, next)
	{
	spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);
	if (zend_parse_parameters_none() == FAILURE) {
	RETURN_THROWS();
	}
	spl_filesystem_file_free_line(intern);
	if (SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_READ_AHEAD)) {
	spl_filesystem_file_read_line(ZEND_THIS, intern, 1);
	}
	intern->u.file.current_line_num++;
	} /* }}} */

So, unless ::READ_AHEAD is set, the method just increments the internal line number without checking that the line actually exists. SplFileObject::key() just returns that line number.

This also explains the behavior reported as #8562.

[cmb69]

Looking at splfileobject.inc (which is removed as of PHP 7.4.0), confirms the issue:

php-src/ext/spl/internal/splfileobject.inc

Lines 306 to 311 in 7d92153

	/** Invalidate current line buffer.
	*/
	function next()
	{
	$this->freeLine();
	}

This is fundamentally flawed. ::next() needs to read a line, or at least set a marker that a line has to be read/skipped before ::next() is called again. Apparently, the Iterator interface has been implemented with only Traversable functionality in mind. ::READ_AHEAD has been implemeted somewhat later, probably to somehow cater to this, but I'm having a hard time to claim that multiple consecutive ::next() calls without the ::READ_AHEAD flag being set are simply not supported, or that the behavior reported in this ticket makes any sense.

[Girgias]

I think the only way to fix it is to make enable the READ_AHEAD flag by default, people who want the prior behaviour can always remove it.

But this can only be fixed in 8.2

[cmb69]

I think the only way to fix it is to make enable the READ_AHEAD flag by default, […]

Frankly, I think that disabling READ_AHEAD should be deprecated. Unless I'm missing something, there is no point in not reading ahead, since sooner or later it would have to be done anyway.

[Girgias]

That's also fair, but READ_AHEAD is not the default.

[cmb69]

Yes, I know that. :)

[Girgias]

And I don't think we can make READ_AHEAD the default as it seems to break a hell of a lot of stuff

[cmb69]

Not setting READ_AHEAD could be deprecated; of course, that would require lots of code to be adapted, but it might be the only reasonable thing to do in the long run (unless there are plans to deprecate SplFileObject altogether).