Skip to content

Handle realpath()/getRealPath() failures - #1663

Open
bpfoley wants to merge 1 commit into
DistributedProofreaders:masterfrom
bpfoley:level-7-realpath
Open

bpfoley wants to merge 1 commit into
DistributedProofreaders:masterfrom
bpfoley:level-7-realpath

Conversation

@bpfoley

@bpfoley bpfoley commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Found by phpstan analyze --level 7

Found by `phpstan analyze --level 7`
Comment thread pinc/misc.inc
$files = new RecursiveIteratorIterator($iterator, RecursiveIteratorIterator::CHILD_FIRST);

foreach ($files as $file) {
$real_path = $file->getRealPath();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If $real_path is false here the file doesn't exist (which I'm unclear on how that can actually be possible given that RecursiveIteratorIterator just told us about it, but let's humor PHPstan's paranoia about race conditions) and it would be clearer to continue than try to do something with the file / directory.

Changes like this seem like we're trading readability for PHPstan's paranoia.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

PHPStan knows nothing about race conditions or the inner workings of PHP's realpath, all it knows is the function signature in one of its stub files. Unfortunately, the underlying libc function realpath() can fail for a variety of (admittedly unlikely) reasons even if the file/directory does exist. PHP returns false in those cases. See https://man7.org/linux/man-pages/man3/realpath.3.html#ERRORS

I'm not sure silently suppressing the error with if ($real_path === false) continue; is the right thing to do, especially if the reason is that something else on the system has caused the directory to be inaccessible (eg a sysadmin doing chmod 0 on the uploads directory). In that case realpath would silently fail multiple times potentially leaving an unflattened directory behind it. But given the aim is to get PHPstan level 7 working so we can detect more useful missing error handling, I don't really mind too much what way we handle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants