Feedback: Please let us know if #21679 fixes in SplFileObject will be backported or not

[rene-hermenau]

Description

Related to #21679

@Girgias, I saw that this is currently not planned for backporting, but it has already raised some concerns on our side.

Could you please clarify which branch this change is expected to land in: 8.5.x, 8.6.x, or 9.x?

From our perspective, it would be much easier if this does not get backported, since it would introduce a backward compatibility break in a particularly complex area for us to address.

PHP Version

PHP 8.6

Operating System

No response

[Girgias]

This is merged into master so it will be in 8.6.

Anyhow, what is the problem with the bugfixes and what change of behaviour is it causing?

@iliaal as expected, touching SPL always causes issues.

[iliaal]

@iliaal as expected, touching SPL always causes issues.

Yeah :/ I wonder what the actual use case is, those were genuine bugs though so maintaining broken/invalid behavior doesn't seem right, but 100% right call on not back-porting to earlier versions. @rene-hermenau would it be possible to see a small use-case that causes regression?

[rene-hermenau]

Thanks for confirming it's 8.6 only.

@iliaal — you authored the fix so you probably know the behavior best, but here's the concrete problem from a library author's perspective. I wrote a test script and ran it against every PHP version from 7.0 to 8.6-dev:

<?php
$file = tempnam(sys_get_temp_dir(), 'spl');
file_put_contents($file, "Line 0\nLine 1\nLine 2\nLine 3\nLine 4\n");

// Test A: seek() then fgets() — what line does fgets return?
$spl = new SplFileObject($file, 'r');
$spl->setFlags(SplFileObject::DROP_NEW_LINE);
$spl->seek(2);
echo $spl->fgets();  // PHP 7.x: "Line 3" | PHP 8.0-8.6: "Line 2"

// Test B: next() without preceding current() — does current() reflect the advance?
$spl2 = new SplFileObject($file, 'r');
$spl2->rewind();
$spl2->next();
echo $spl2->current();  // PHP 7.0-8.5: "Line 0" | PHP 8.6: "Line 1"

// Test C: fgets() then current() — stale cache?
$spl3 = new SplFileObject($file, 'r');
$spl3->rewind();
$spl3->fgets();
echo $spl3->current();  // PHP 7.0-8.5: "Line 0" | PHP 8.6: "Line 1"

// Test D: next() past EOF — does key() keep incrementing?
$spl4 = new SplFileObject($file, 'r');
$spl4->rewind();
for ($i = 0; $i < 10; $spl4->next(), $i++);
echo $spl4->key();  // PHP 7.0-8.5: 10 | PHP 8.6: 5

// Test E: seek() + current() + next() + current() — the workaround pattern
$spl5 = new SplFileObject($file, 'r');
$spl5->setFlags(SplFileObject::DROP_NEW_LINE);
$spl5->seek(2);
$first = $spl5->current();
$spl5->next();
$second = $spl5->current();
echo "$first → $second key=" . $spl5->key();
// ALL versions 7.0-8.6: "Line 2" → "Line 3" key=3

unlink($file);

Results across all tested versions:

Test	7.0	7.2	7.4	8.0	8.1	8.2	8.3	8.4	8.5	8.6-dev
A: seek(2)+fgets()	Line 3	Line 3	Line 3	Line 2	Line 2	Line 2	Line 2	Line 2	Line 2	Line 2
B: next()+current()	Line 0	Line 0	Line 0	Line 0	Line 0	Line 0	Line 0	Line 0	Line 0	Line 1
C: fgets()+current()	Line 0	Line 0	Line 0	Line 0	Line 0	Line 0	Line 0	Line 0	Line 0	Line 1
D: next() 10x key()	10	10	10	10	10	10	10	10	10	5
E: seek+current+next+current	Line 2→3	Line 2→3	Line 2→3	Line 2→3	Line 2→3	Line 2→3	Line 2→3	Line 2→3	Line 2→3	Line 2→3

Tests B, C, and D are genuine bug fixes — 8.6 is the first version to get these right. No argument there.

Test E shows the seek() + current() + next() + current() pattern works identically across ALL versions — this is the workaround pattern we use in production.

The compatibility problem: Code that adapted to 8.0+'s broken behavior over the past 5 years will break on 8.6 if it uses patterns from tests B, C, or D. We maintain a custom SplFileObject wrapper for a WordPress plugin (200k+ installs) that processes large database dumps and file lists line-by-line with seek/resume. The wrapper ensures consistent behavior across PHP 7.0 through 8.5, with version-specific workarounds for behavioral changes introduced in PHP 8.0.1, 8.0.19, 8.1.6, and 8.2.0.

For developers who don't maintain custom wrappers, the impact depends on which methods they combine. Test E (seek + current + next + current) is safe across all versions. But code relying on next() without current(), or on fgets() caching, will see different behavior on 8.6.

We've already added a runtime behavior probe that detects the fix and adapts accordingly, so we're prepared. Our concern was only about potential backporting to patch releases.

[iliaal]

@rene-hermenau First all, big thanks for deep analysis and reproduce code.

Looks like A is aligned across 8.X series and E is unchanged. B&C are semantically the same issue, old code advances forward, but current() doesn't see it, which is a bug. D is also very weird old behavior, where looping via next() doubles the number of iterations vs correctly going through 5 records.

I completely recognize that over the years code was adopted to work on around this and perhaps even in some cases to rely on it in case of B/C , hard to see how D would be anything other than unexpected behavior. IMHO maintaining broken functionality seems wrong, so fix was needed, that being said @Girgias punting this to 8.6 is/was absolutely right move, as it gives everyone time to adapt.

It sounds like you already handled it in your code, hopefully was not too much work I put on your plate 🤞

I do wonder if perhaps we need to add note to UPGRADING for PHP 8.6 to clearly state the impact of this fix, @Girgias happy to draft a PR with addition to the file into "Backward Incompatible Changes" section if you think it is appropriate.

[rene-hermenau]

@iliaal I need to investigate this further, as my previous assessment may not have been fully correct or complete.

I found additional behavior that I want to verify before following up, so you can make a decision based on the most complete test results possible. This will take a bit of time because I need to run it across all PHP versions in our CI pipeline, but I’ll get back to you as soon as I can.

[iliaal]

Sounds good, please keep tagging me so I don't miss it 🙏