Follow chdir() and include_path changes made earlier in the file when resolving an include - #6472
phpstan-bot wants to merge 2 commits into
Conversation
…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
left a comment
There was a problem hiding this comment.
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>
|
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):
Tests —
All six fail with the rule's new early return disabled. |
Summary
require_once 'config.php'after achdir('..')was reported asPath in require_once() "config.php" is not a file or it does not exist., becauseIncludedFilePathResolveralways 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 everyInclude_node the precedingchdir(),set_include_path(),ini_set(),ini_alter()andstream_wrapper_register()calls in the file, aslist<array{string, FuncCall}>(lowercased function name + call).src/File/IncludedFilePathResolver.php:resolve()gained an optional?Include_ $nodeparameter and now returnslist<string>|null.resolveWorkingDirectories()- eachchdir()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. Successivechdir()s compose.resolveIncludePath()-set_include_path()andini_set('include_path', ...)/ini_alter(...)add theirPATH_SEPARATOR-separated entries to the include path. Entries are only added, for the same reason.include_pathentries (.is there by default) are now absolutized against the working directories instead of being handed tois_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.WORKING_DIRECTORIES_LIMITandINCLUDE_PATH_ENTRIES_LIMIT.src/Rules/Keywords/RequireFileExistsRule.php: reports nothing whenresolve()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 theInclude_node toresolve(), so an included file found only through achdir()/set_include_path()becomes a tracked dependency.src/File/FileHelper.php: extractedisAbsolutePath()out ofabsolutizePath().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 aliasini_alter()- fixed. The option name is matched case-insensitively; an unrelated option such asini_set('memory_limit', ...)changes nothing.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
IncludedFilePathResolvertreated 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 relativeinclude/requireis 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-casingchdir().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');) indata/bug-15260/sub/bug-15260.php, with the working directory set todata/bug-15260/subandconfig.phpliving 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 throughchdir(), composedchdir()calls (chdir('Keywords')thenchdir('data/bug-15260')reachingconfig.php), a relative path still missing after thechdir(), 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', ...)andini_alter('INCLUDE_PATH', ...)each make a file reachable, whileini_set('memory_limit', ...)changes nothing.testIncludePathChangedToUnknownValue- theset_include_path(get_include_path() . PATH_SEPARATOR . __DIR__)idiom.testIniSetWithUnknownOption-ini_set($_SERVER['OPTION'], $_SERVER['VALUE']), which could beinclude_path.testStreamWrapperRegisteredByTheFileItself- includes through a scheme the file registers, before and after thestream_wrapper_register()call, plus a scheme nobody registers.All seven fail on the unfixed source.
make tests,make phpstanandmake cs-fixare green (make name-collisionfails ontests/PHPStan/Rules/Methods/data/static-call-pipe.phpboth with and without this change).Fixes phpstan/phpstan#15260