Skip to content

Fix bug #80056: SPL directory iterators lose entries on 9p filesystems - #23461

Open
denkfabrik-li wants to merge 1 commit into
php:masterfrom
denkfabrik-li:spl-directory-rewind-noop
Open

Fix bug #80056: SPL directory iterators lose entries on 9p filesystems#23461
denkfabrik-li wants to merge 1 commit into
php:masterfrom
denkfabrik-li:spl-directory-rewind-noop

Conversation

@denkfabrik-li

Copy link
Copy Markdown

Summary

FilesystemIterator, RecursiveDirectoryIterator and DirectoryIterator silently
lose directory entries on filesystems that cannot rewind a directory handle after a
partial read — most notably 9p mounts, which is what WSL2 and Docker Desktop on
Windows use to expose the Windows drive to Linux. An entire libc getdents buffer
vanishes from the iteration: ~21 entries with musl (2 KiB buffer), ~343 with glibc
(32 KiB buffer). scandir(), glob() and a plain readdir() loop over the same
directory return the complete listing.

The cause is PHP-specific behaviour, not just the kernel bug: the iterator
constructors eagerly read the first directory entry, and the implicit rewind() at
the start of the first foreach then calls rewinddir() on a stream whose position
is already past that first entry. On a healthy filesystem this seek is redundant (it
re-reads the entry the constructor already had); on 9p the seek is silently ignored
server-side while the C library discards its read buffer — everything that was in
that buffer is skipped.

This PR makes the rewind a no-op while the stream is still positioned at its first
entry, so the standard construct-then-foreach pattern never seeks at all.

History

Both reports were closed on the grounds that the underlying seek failure lives below
PHP — which is true. But no code change was ever proposed back then, and the seek
itself is avoidable: it is issued by PHP at a moment where it has nothing to do.
Every other PHP directory API (scandir(), glob(), opendir()/readdir())
works on these mounts precisely because none of them seeks a partially-read handle.

The failure mode is nasty in practice because it is silent, size-dependent (only
directories with more entries than one libc buffer are affected) and
environment-dependent (works on ext4, breaks on the very same code under Docker
Desktop on Windows). Real-world sightings include Composer autoload scans, Laravel
migration discovery and PHPStan/Larastan file collection:
laravel/framework#61336, larastan/larastan#2538, projectsend/projectsend#1680.

The mechanism

new FilesystemIterator($dir)          # constructor opens the dir
  -> readdir()                        #   ...and eagerly reads entry #1
                                      #   libc fills its getdents buffer
foreach ($it as $f)                   # foreach starts with rewind()
  -> rewinddir()                      #   seek to offset 0
                                      #   9p: seek silently ignored,
                                      #   libc buffer discarded anyway
  -> readdir(), readdir(), ...        #   continues AFTER the discarded buffer

With musl's 2 KiB buffer the first ~21 entries disappear; with glibc's 32 KiB
buffer the first ~343. Directories at or below one buffer appear complete, which is
why small test cases pass and production directories fail.

The fix

Track in spl_filesystem_object.u.dir whether the stream is still positioned at its
first (non-skipped) entry (at_initial_entry). All four rewind paths
(DirectoryIterator::rewind(), FilesystemIterator::rewind(), and the two internal
zend_object_iterator rewind handlers) are consolidated into one helper,
spl_filesystem_dir_rewind(), which skips the rewinddir() + re-read when nothing
has been consumed yet. Any actual read (next(), internal move_forward, the
clone catch-up loop, seek()) clears the flag, so a rewind after real consumption
still performs a real rewinddir() exactly as before.

Behaviour / BC analysis

Unchanged:

  • The constructor still opens the directory eagerly — an invalid path still throws
    UnexpectedValueException from the constructor.
  • The constructor still pre-reads the first entry: $it->current(),
    getFilename(), key(), hasChildren() etc. right after construction behave
    exactly as before (and now also work on 9p, since the pre-read is sequential).
  • rewind() after any entry has been consumed performs a real rewinddir() +
    re-read, as before (re-iterating an iterator, seek() backwards, clone).
  • Result sets, ordering, keys, flags handling (SKIP_DOTS, …) on healthy
    filesystems: byte-for-byte identical output; the full test suite passes.

Observable differences (deliberate):

  • While the stream is still on its first entry, rewind() no longer issues
    rewinddir() + readdir(). For userland stream wrappers this means
    dir_rewinddir() is no longer invoked at the start of the first iteration (one
    callback less; a wrapper that relied on being rewound before the first read was
    already broken, since the constructor pre-read has always happened before any
    rewind). One redundant syscall pair per iteration start is saved everywhere.
  • POSIX allows rewinddir() to pick up directory modifications made after
    opendir(). A file created between construction and the first foreach was
    previously sometimes visible (filesystem-dependent, POSIX explicitly leaves it
    unspecified); with the no-op rewind the iteration keeps the view the constructor
    started with. Code relying on that was already unreliable across filesystems.

Known limitation (unchanged, inherent to broken seeks): re-iterating the same
iterator object (second foreach, explicit rewind() after next()) still
requires a real rewinddir() and therefore still misbehaves on 9p — same as a
userland rewinddir() call. This patch fixes the overwhelmingly common
construct-then-iterate-once pattern, which is what Composer/Laravel/PHPStan & co.
use.

Tests

  • ext/spl/tests/bug80056.phpt — simulates the broken filesystem with a userland
    stream wrapper whose dir_rewinddir() pretends to succeed without resetting the
    position (exactly the observable 9p behaviour). Fails on current master (each
    iterator loses its first entry), passes with the fix. Covers DirectoryIterator,
    FilesystemIterator, RecursiveDirectoryIterator + RecursiveIteratorIterator, and
    the "entry accessed before iteration" pattern.
  • ext/spl/tests/spl_dir_iterator_rewind_noop.phpt — pins the new seek semantics
    with a counting wrapper: no dir_rewinddir() on first iteration, exactly one on
    re-iteration, exactly one for rewind() after next(), none for repeated
    rewind() without reads.
  • Existing SPL suite: green (see verification below).

Verification

Two-stage, with unpatched and patched CLI binaries built from the same master
checkout (ext/spl/spl_directory.* diff applied incrementally in the same
container build):

(a) Test suite (ext4 inside the build containers):

Full ext/spl suite with the patched CLI — identical results on
Debian bookworm (glibc) and Alpine 3.22 (musl):

Number of tests :   810               799
Tests skipped   :    11 (  1.4%)
Tests failed    :     0 (  0.0%)
Expected fail   :     1 (  0.1%)   (pre-existing XFAIL, unrelated)
Tests passed    :   798 ( 98.5%)

A second build with --enable-phar --enable-zend-test additionally runs the
SPL-over-phar and stack-limit tests (gh14687, gh17225, gh15911, gh15672 — all
pass; 802 SPL tests passed total) plus the full ext/phar suite: 382 runnable
tests, 0 failures. Relevant because Phar extends RecursiveDirectoryIterator
and embeds spl_filesystem_object; its in-memory directory stream keeps a
working seek, so behaviour is unchanged there.

Both new tests FAIL on unpatched master, each iterator losing its first entry
(array(4) instead of array(5), a.txt missing) — i.e. the regression tests
demonstrably catch the bug.

(b) Live 9p mount (Docker Desktop on Windows 11, WSL2 backend; a Windows
directory with 500 .php files — plus 1 in a subdirectory for the recursive
case — mounted into the container; stat -f -c %T reports v9fs):

API musl unpatched musl patched glibc unpatched glibc patched
scandir() 500 500 500 500
glob() 500 500 500 500
readdir() loop 500 500 500 500
FilesystemIterator 479 500 157 500
RecursiveDirectoryIterator 480 501 158 501
DirectoryIterator 479 500 157 500

The losses match the libc buffer sizes exactly: musl drops its first 2 KiB
getdents buffer (21 entries), glibc its first 32 KiB buffer (343 entries).

One-liner reproducer for anyone with Docker Desktop on Windows (stock image,
directory with a few hundred files):

> docker run --rm -v C:\some\big\dir:/mnt/d php:8.4-cli php -r \
    "echo count(scandir('/mnt/d'))-2, ' vs ', iterator_count(new FilesystemIterator('/mnt/d'));"
501 vs 158        # PHP 8.4.24, verified 2026-08-25

FilesystemIterator, RecursiveDirectoryIterator and DirectoryIterator read
the first directory entry eagerly in their constructor. The implicit
rewind() at the start of the first foreach then called rewinddir() on a
stream whose position was already past the first entry. On filesystems
that cannot seek a directory handle after a partial read - most notably
9p mounts as used by WSL2 and Docker Desktop on Windows - that seek is
silently ignored and the entries already buffered by the C library are
lost: an entire libc getdents buffer (about 21 entries with musl, about
343 with glibc) disappears from the iteration, while scandir(), glob()
and plain readdir() see the full listing.

Track whether the directory stream is still positioned at its first
entry; rewinding is a no-op in that state. The constructor still opens
the directory eagerly (an invalid path keeps throwing
UnexpectedValueException) and still pre-reads the first entry, so the
observable object state after construction is unchanged. A rewind after
any entry has been consumed keeps performing a real rewinddir() as
before.
@denkfabrik-li
denkfabrik-li force-pushed the spl-directory-rewind-noop branch from ad16a36 to ffb35f9 Compare August 25, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant