Skip to content

Follow chdir() and include_path changes made earlier in the file when resolving an include - #6472

Open
phpstan-bot wants to merge 2 commits into
phpstan:2.3.xfrom
phpstan-bot:create-pull-request/patch-f7dfift
Open

phpstan-bot wants to merge 2 commits into
phpstan:2.3.xfrom
phpstan-bot:create-pull-request/patch-f7dfift

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

require_once 'config.php' after a chdir('..') was reported as Path in require_once() "config.php" is not a file or it does not exist., because IncludedFilePathResolver always resolved a relative include against PHPStan's own working directory, include path and the analysed file's directory - ignoring that the file itself can move all three before the include runs.

The resolver now reads the calls that change include resolution and appear before the include in the same file, and follows them.

Changes

  • src/Parser/IncludePathChangingCallsVisitor.php (new): records on every Include_ node the preceding chdir(), set_include_path(), ini_set(), ini_alter() and stream_wrapper_register() calls in the file, as list<array{string, FuncCall}> (lowercased function name + call).
  • src/File/IncludedFilePathResolver.php: resolve() gained an optional ?Include_ $node parameter and now returns list<string>|null.
    • resolveWorkingDirectories() - each chdir() with a constant string argument adds its target, absolutized against the directories known so far, to the set. Earlier directories stay in the set: the call may be conditional or sit in a function that is never reached. Successive chdir()s compose.
    • resolveIncludePath() - set_include_path() and ini_set('include_path', ...)/ini_alter(...) add their PATH_SEPARATOR-separated entries to the include path. Entries are only added, for the same reason.
    • Relative include_path entries (. is there by default) are now absolutized against the working directories instead of being handed to is_file() as relative paths.
    • registersStreamWrapper() - a path through a stream wrapper that is not registered in the PHPStan process is no longer reported when the analysed file registers that scheme itself.
    • Bounded by two named constants, WORKING_DIRECTORIES_LIMIT and INCLUDE_PATH_ENTRIES_LIMIT.
  • src/Rules/Keywords/RequireFileExistsRule.php: reports nothing when resolve() returns null; doesFileExist() now takes the already-resolved candidate paths, so the rule resolves each path once instead of twice.
  • src/Dependency/DependencyResolver.php: passes the Include_ node to resolve(), so an included file found only through a chdir()/set_include_path() becomes a tracked dependency.
  • src/File/FileHelper.php: extracted isAbsolutePath() out of absolutizePath().

Analogous cases fixed alongside chdir()

The axis is "state the analysed file mutates before the include, which decides where the path resolves":

  • set_include_path() - fixed, same mechanism.
  • ini_set('include_path', ...) and its alias ini_alter() - fixed. The option name is matched case-insensitively; an unrelated option such as ini_set('memory_limit', ...) changes nothing.
  • An unreadable argument on any of them (chdir($dir), set_include_path(get_include_path() . PATH_SEPARATOR . $dir), ini_set($option, $value) where even the option name is unknown) - the working directory or include path becomes unknowable, so relative includes are no longer reported. Absolute ones still are.
  • stream_wrapper_register() - a path through a wrapper the file registers itself is no longer reported, while the unregistered-wrapper case from Do not resolve include paths through a stream wrapper that is not registered #6397 keeps reporting.

Probed and deliberately left alone: restore_include_path()/ini_restore('include_path') can only shrink the include path, so they can produce false negatives but never the false positive this issue is about; chroot() is root-only, rearranges absolute paths and cannot be modelled from a single file; chdir() in an included file is out of reach, as the issue itself notes.

Root cause

IncludedFilePathResolver treated PHPStan's own process state - getcwd(), get_include_path(), stream_get_wrappers() - as the state the analysed script will run in. That holds for a script that never touches them, but PHP lets a script change all three, and a relative include/require is resolved against them at the moment it executes. Every input to include resolution had the same defect, so the fix reads all of them out of the file rather than special-casing chdir().

Two outcomes are possible once a call is found: the new location is known (a constant string argument), in which case it is added to the candidate set rather than replacing it - the call may be conditional, so the old location stays possible - or it is not knowable, in which case a relative path could resolve anywhere and the check is skipped entirely for it. An absolute path is unaffected either way and keeps being checked, which is what keeps the skip from silently disabling the rule.

Test

tests/PHPStan/Rules/Keywords/RequireFileExistsRuleTest:

  • testBug15260 - the issue's snippet verbatim (chdir('..'); require_once('config.php');) in data/bug-15260/sub/bug-15260.php, with the working directory set to data/bug-15260/sub and config.php living one level up, where neither the working directory nor the script's own directory would find it. Reports nothing.
  • testChdir - a relative path found only through chdir(), composed chdir() calls (chdir('Keywords') then chdir('data/bug-15260') reaching config.php), a relative path still missing after the chdir(), and an absolute path still reported.
  • testChdirWithUnknownDirectory - chdir($_SERVER['CHDIR_TARGET']); the include before it is still reported, the relative one after it is not, the absolute one after it still is.
  • testIncludePathChangedAtRuntime - set_include_path(), ini_set('include_path', ...) and ini_alter('INCLUDE_PATH', ...) each make a file reachable, while ini_set('memory_limit', ...) changes nothing.
  • testIncludePathChangedToUnknownValue - the set_include_path(get_include_path() . PATH_SEPARATOR . __DIR__) idiom.
  • testIniSetWithUnknownOption - ini_set($_SERVER['OPTION'], $_SERVER['VALUE']), which could be include_path.
  • testStreamWrapperRegisteredByTheFileItself - includes through a scheme the file registers, before and after the stream_wrapper_register() call, plus a scheme nobody registers.

All seven fail on the unfixed source. make tests, make phpstan and make cs-fix are green (make name-collision fails on tests/PHPStan/Rules/Methods/data/static-call-pipe.php both with and without this change).

Fixes phpstan/phpstan#15260

…when resolving an include

- New `IncludePathChangingCallsVisitor` records, on every `Include_` node, the
  `chdir()`/`set_include_path()`/`ini_set()`/`ini_alter()`/`stream_wrapper_register()`
  calls that appear before it in the same file.
- `IncludedFilePathResolver::resolve()` takes the `Include_` node and follows those
  calls: a `chdir()` with a constant argument adds its target to the working
  directories a relative path is tried against (the previous ones stay, the call may
  be conditional), and `set_include_path()`/`ini_set('include_path', ...)` add entries
  to the include path. Relative `include_path` entries are now absolutized against the
  working directories instead of being left relative.
- `resolve()` returns null when the call's argument - or an `ini_set()` option name -
  cannot be read, meaning a relative path could resolve anywhere; `RequireFileExistsRule`
  then reports nothing. An absolute path is unaffected and still reported.
- A path through a stream wrapper the analysed file registers itself is no longer
  reported; an unregistered one still is.
- `DependencyResolver` passes the node through too, so included-file dependencies follow
  the same calls.
- Extracted `FileHelper::isAbsolutePath()` out of `absolutizePath()`.
- Bounded by `WORKING_DIRECTORIES_LIMIT` and `INCLUDE_PATH_ENTRIES_LIMIT`.

@staabm staabm left a comment

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.

not worth the complication for such a edge case problem. we need a simpler solution. no need to keep track of all these dynamic paths. just assume a plain string type instead of trying to resolve a constant string path.

…lves

Replaces following `chdir()`/`set_include_path()`/`ini_set('include_path', ...)`
calls and resolving the path against every directory they could have led to with
the simpler answer asked for in review: once such a call appears earlier in the
file, the path is treated as one that does not name a known place, so
`RequireFileExistsRule` reports nothing for it.

- `IncludePathChangingCallsVisitor` becomes `IncludeResolutionChangedVisitor`: it
  no longer records the calls themselves, only sets a flag attribute on the
  `Include_` nodes that follow one. An `ini_set()`/`ini_alter()` of a literal
  option other than `include_path` is not one of them.
- `IncludedFilePathResolver`, `FileHelper` and `DependencyResolver` are back to
  what they were: no `Include_` parameter, no nullable return, no working
  directory or include path sets, no limits.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@phpstan-bot

Copy link
Copy Markdown
Collaborator Author

Addressed — the resolution-following machinery is gone; the rule now just gives up on the path.

What the fix is now (net diff vs. the base commit: one visitor + 7 lines in the rule):

  • src/Parser/IncludeResolutionChangedVisitor.php (renamed from IncludePathChangingCallsVisitor): no longer records calls and their arguments on the node. It sets a single true attribute on every Include_ that a chdir(), set_include_path(), ini_set()/ini_alter() or stream_wrapper_register() call appears before in the same file. ini_set()/ini_alter() with a literal option other than include_path doesn't count; a non-literal option name does, since it could be include_path.
  • src/Rules/Keywords/RequireFileExistsRule.php: when that attribute is set, processNode() returns nothing — the path is treated as not naming a place PHPStan can look at, exactly as a non-constant-string path already is.
  • IncludedFilePathResolver, FileHelper and DependencyResolver are byte-identical to before the PR: no Include_ parameter, no nullable return, no working-directory/include-path sets, no *_LIMIT constants, no isAbsolutePath() extraction, no IncludedFilePathResolverTest change.

TestsRequireFileExistsRuleTest, five cases, each with an include before the call that is still reported:

  • testBug15260 — the issue's snippet verbatim, nothing reported (no working-directory override needed any more).
  • testChdir — after chdir('..') neither the relative nor the __DIR__-based include is reported; the one before it still is.
  • testSetIncludePath, testIniSetIncludePath (ini_set('memory_limit', …) and ini_alter('precision', …) change nothing, ini_set('INCLUDE_PATH', …) does), testIniSetWithUnknownOption.
  • testStreamWrapperRegisteredByTheFileItself — only the include before the stream_wrapper_register() call is reported.

All six fail with the rule's new early return disabled. make tests (21932 tests, OK), make phpstan (no errors) and make cs-fix (no violations) are green. Note one behaviour trade-off of the simpler approach: an absolute include path after such a call is no longer checked either — the whole rest of the file is given up on rather than tracked per path.

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.

require_once file existence check doesn't account for chdir.

2 participants