Skip to content

Fix GH-25232: ftruncate() does not invalidate stat cache - #25233

Open
lazerg wants to merge 1 commit into
php:PHP-8.4from
lazerg:fix/gh-25232-ftruncate-stat-cache
Open

lazerg wants to merge 1 commit into
php:PHP-8.4from
lazerg:fix/gh-25232-ftruncate-stat-cache

Conversation

@lazerg

@lazerg lazerg commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

The plain wrapper already clears the stat cache after reads, writes and flushes, but not after truncating, so filesize() kept returning the old size after ftruncate() or SplFileObject::ftruncate(). It now clears it on a successful truncate too.

Fixes #25232

}
#endif
if (EG(active)) {
/* clear stat cache as size, mtime and ctime got changed */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearing the global stat cache here fixes filesize() for external lookups, but leaves the open stream's internal buffer (stream->readbuf) untouched when truncation occurs below the buffer horizon.

Unlike _php_stream_seek() which invalidates stream read/write positions (stream->readpos = stream->writepos = 0;), calling ftruncate() on a stream that has already performed buffered reads leaves stale bytes in memory, resulting in ghost reads past EOF:

$f = tempnam(sys_get_temp_dir(), 'trunc_');
file_put_contents($f, str_repeat('A', 100));

$h = fopen($f, 'r+');
fread($h, 10);      // stream pre-buffers 8KB into stream->readbuf
ftruncate($h, 10);  // truncated to 10 bytes on disk

var_dump(filesize($f));  // int(10)
var_dump(fread($h, 50)); // returns 50 bytes of stale buffer past EOF

fclose($h);
unlink($f);

Should stream buffer invalidation be handled in _php_stream_truncate_set_size() in main/streams/streams.c alongside this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to keep this PR to the stat cache. The stale read buffer is real, but stock 8.4.27 shows the same output, so it is a separate bug and should get its own issue and fix.

GH-25232: ftruncate() does not invalidate stat cache
--FILE--
<?php
$file = __DIR__ . '/gh25232.tmp';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing directly to __DIR__ breaks out-of-tree builds where the source tree is mounted read-only (e.g. CI packaging setups or --srcdir testing). If the test crashes or aborts before reaching --CLEAN--, it also leaves untracked temporary files in the worktree.

Using tempnam(sys_get_temp_dir(), ...) (as in the original GH-25232 issue report) would be safer and keep the source tree clean.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept __DIR__ because most tests in ext/standard/tests/file write there and clean up in --CLEAN--. For a read-only source tree, run-tests has --temp-source and --temp-target.

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.

2 participants