Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lazerg wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
+38 −2
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions ext/standard/tests/file/gh25232.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
--TEST--
GH-25232: ftruncate() does not invalidate stat cache
--FILE--
<?php
$file = __DIR__ . '/gh25232.tmp';

@prateekbhujel prateekbhujel Nov 6, 2026

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.

@lazerg lazerg Nov 6, 2026

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.

file_put_contents($file, 'abcdefgh');
$handle = fopen($file, 'r+');

var_dump(filesize($file));
var_dump(ftruncate($handle, 3));
var_dump(filesize($file));

fclose($handle);

$obj = new SplFileObject($file, 'r+');
var_dump(filesize($file));
var_dump($obj->ftruncate(1));
var_dump(filesize($file));
?>
--CLEAN--
<?php
@unlink(__DIR__ . '/gh25232.tmp');
?>
--EXPECT--
int(8)
bool(true)
int(3)
int(3)
bool(true)
int(1)
10 changes: 8 additions & 2 deletions main/streams/plain_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,10 +1018,16 @@ static int php_stdiop_set_option(php_stream *stream, int option, int value, void
if (!SetFilePointerEx(h, old_sz, NULL, FILE_BEGIN)) {
return PHP_STREAM_OPTION_RETURN_ERR;
}
return PHP_STREAM_OPTION_RETURN_OK;
#else
return ftruncate(fd, new_size) == 0 ? PHP_STREAM_OPTION_RETURN_OK : PHP_STREAM_OPTION_RETURN_ERR;
if (ftruncate(fd, new_size) != 0) {
return PHP_STREAM_OPTION_RETURN_ERR;
}
#endif
if (EG(active)) {
/* clear stat cache as size, mtime and ctime got changed */

@prateekbhujel prateekbhujel Nov 6, 2026

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?

@lazerg lazerg Nov 6, 2026

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.

php_clear_stat_cache(0, NULL, 0);
}
return PHP_STREAM_OPTION_RETURN_OK;
}
}
return PHP_STREAM_OPTION_RETURN_NOTIMPL;
Expand Down
Loading