Skip to content

Commit 5ea1f21

Browse files
theodorejbclaude
andcommitted
Ask Composer's autoloaders for the file instead of letting them include it
AutoloadSourceLocator finds which file declares a class by running the registered autoloaders behind FileReadTrapStreamWrapper, which records the path an include reached for and serves an empty script in its place. That only shadows the real file while the compiler asks the wrapper for the contents. With OPcache already holding the script it does not ask, and the file runs a second time - fatal for one declaring a function, which is the function-per-file layout of php-standard-library and azjezz/psl: their files-autoload bootstrap has already loaded every path their PSR-4 prefix also resolves to. ClassLoader::findFile() answers with the same path loadClass() would include, without running anything, so ask it rather than arranging for the include to be harmless. Nothing is compiled and no cache is consulted, which is what makes this hold wherever PHP runs. Autoloaders still run in registration order: every non-Composer one runs inside the trap as before, one at a time, since an autoloader ahead of Composer's may claim a name Composer would resolve elsewhere. Running them one at a time also stops hoa/compiler's autoloader, registered ahead of the analysed project's loader, from forcing the whole probe down the include path. findFile() concatenates the mapped prefix with the rest of the name, so its answer can carry ../ segments and mixed separators, where PHP resolves an include path before the trap ever sees it. It is resolved here to match, which locateIdentifier() relies on when it compares the located path against ReflectionClass::getFileName() to tell two same-named classes in one file apart. The test drives the real locator in a subprocess under the worker's own OPcache flags: a name whose PSR-4 prefix resolves to a file the process already ran, which must not run it again, and one whose file nothing has loaded, which must still resolve without executing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 395468a commit 5ea1f21

5 files changed

Lines changed: 208 additions & 34 deletions

File tree

src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php

Lines changed: 78 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Reflection\BetterReflection\SourceLocator;
44

5+
use Composer\Autoload\ClassLoader;
56
use Override;
67
use ParseError;
78
use PhpParser\Node\Arg;
@@ -31,9 +32,11 @@
3132
use function defined;
3233
use function function_exists;
3334
use function interface_exists;
35+
use function is_array;
3436
use function is_file;
3537
use function is_string;
3638
use function opcache_invalidate;
39+
use function realpath;
3740
use function restore_error_handler;
3841
use function set_error_handler;
3942
use function spl_autoload_functions;
@@ -333,58 +336,99 @@ private function locateClassByName(string $className): ?array
333336
return null;
334337
}
335338

339+
$autoloadFunctions = spl_autoload_functions();
340+
if ($autoloadFunctions === false) {
341+
return null;
342+
}
343+
336344
$this->silenceErrors();
337345

338346
try {
339-
$result = FileReadTrapStreamWrapper::withStreamWrapperOverride(
340-
static function () use ($className): ?array {
341-
$functions = spl_autoload_functions();
342-
if ($functions === false) {
343-
return null;
344-
}
347+
return self::locateThroughAutoloaders($autoloadFunctions, $className);
348+
} finally {
349+
restore_error_handler();
350+
}
351+
}
345352

346-
foreach ($functions as $preExistingAutoloader) {
347-
try {
348-
$preExistingAutoloader($className);
349-
} catch (ParseError) {
350-
// the trap served a parse error instead of the empty
351-
// script, see FileReadTrapStreamWrapper::stream_read();
352-
// the file was recorded before the include compiled it
353-
}
353+
/**
354+
* Runs the registered autoloaders, in order, to find which file declares a
355+
* class - asking Composer's where it would look instead of letting it get
356+
* there by including the file.
357+
*
358+
* ClassLoader::findFile() answers with the same path loadClass() would
359+
* include, without running anything. Nothing is compiled, so the file cannot
360+
* execute a second time - which it otherwise does whenever OPcache already
361+
* holds the script: the include is then served from the cache without the
362+
* trap being asked for the contents at all, and a file declaring a function
363+
* fatals with "Cannot redeclare". That is the function-per-file layout of
364+
* php-standard-library and azjezz/psl, whose files-autoload bootstrap has
365+
* already loaded every path their PSR-4 prefix also resolves to.
366+
*
367+
* Every other autoloader still runs inside the trap, one at a time so that
368+
* registration order is preserved either way: an autoloader ahead of
369+
* Composer's may well claim a name Composer would resolve elsewhere.
370+
*
371+
* @param list<callable(string): void> $autoloadFunctions
372+
* @return array{string[], string, int|null}|null
373+
*/
374+
private static function locateThroughAutoloaders(array $autoloadFunctions, string $className): ?array
375+
{
376+
foreach ($autoloadFunctions as $autoloadFunction) {
377+
if (is_array($autoloadFunction) && $autoloadFunction[0] instanceof ClassLoader) {
378+
$file = $autoloadFunction[0]->findFile($className);
379+
if ($file === false) {
380+
continue;
381+
}
354382

355-
/**
356-
* This static variable is populated by the side-effect of the stream wrapper
357-
* trying to read the file path when `include()` is used by an autoloader.
358-
*
359-
* This will not be `null` when the autoloader tried to read a file.
360-
*/
361-
if (FileReadTrapStreamWrapper::$autoloadLocatedFiles !== []) {
362-
return [FileReadTrapStreamWrapper::$autoloadLocatedFiles, $className, null];
363-
}
383+
// findFile() concatenates the mapped prefix with the rest of the
384+
// name, so its answer can carry ../ segments and mixed
385+
// separators. PHP resolves an include path before the trap ever
386+
// sees it, and locateIdentifier() matches what lands here against
387+
// ReflectionClass::getFileName(), which is resolved too.
388+
$resolvedFile = realpath($file);
389+
390+
// a class map can outlive the file it points at, and loadClass()
391+
// would move on to the next autoloader just the same
392+
if ($resolvedFile === false || !is_file($resolvedFile)) {
393+
continue;
394+
}
395+
396+
return [[$resolvedFile], $className, null];
397+
}
398+
399+
$locatedFiles = FileReadTrapStreamWrapper::withStreamWrapperOverride(
400+
static function () use ($autoloadFunction, $className): array {
401+
try {
402+
$autoloadFunction($className);
403+
} catch (ParseError) {
404+
// the trap served a parse error instead of the empty
405+
// script, see FileReadTrapStreamWrapper::stream_read();
406+
// the file was recorded before the include compiled it
364407
}
365408

366-
return null;
409+
// populated by the side effect of the stream wrapper being
410+
// asked for the file an include() reached for
411+
return FileReadTrapStreamWrapper::$autoloadLocatedFiles;
367412
},
368413
);
369-
if ($result === null) {
370-
return null;
371-
}
372414

373-
if (!function_exists('opcache_invalidate')) {
374-
return $result;
415+
if ($locatedFiles === []) {
416+
continue;
375417
}
376418

377419
// the trap's empty script got compiled - and cached, with OPcache
378420
// active. Where this call cannot reach the entry, the trap served a
379421
// parse error instead, see FileReadTrapStreamWrapper::stream_read()
380-
foreach ($result[0] as $file) {
381-
opcache_invalidate($file, true);
422+
if (function_exists('opcache_invalidate')) {
423+
foreach ($locatedFiles as $locatedFile) {
424+
opcache_invalidate($locatedFile, true);
425+
}
382426
}
383427

384-
return $result;
385-
} finally {
386-
restore_error_handler();
428+
return [$locatedFiles, $className, null];
387429
}
430+
431+
return null;
388432
}
389433

390434
private function silenceErrors(): void

tests/PHPStan/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapperTest.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,16 @@
33
namespace PHPStan\Reflection\BetterReflection\SourceLocator;
44

55
use PHPUnit\Framework\Attributes\DataProvider;
6+
use PHPUnit\Framework\Attributes\Group;
67
use PHPUnit\Framework\TestCase;
8+
use function escapeshellarg;
9+
use function exec;
10+
use function explode;
11+
use function extension_loaded;
12+
use function implode;
13+
use function sprintf;
14+
use function str_contains;
15+
use const PHP_BINARY;
716

817
final class FileReadTrapStreamWrapperTest extends TestCase
918
{
@@ -29,4 +38,50 @@ public function testResolveServesParseError(int $phpVersionId, bool $opcacheEnab
2938
$this->assertSame($expected, FileReadTrapStreamWrapper::resolveServesParseError($phpVersionId, $opcacheEnabled, $path));
3039
}
3140

41+
/**
42+
* Probing a name whose PSR-4 prefix resolves to a file the process already
43+
* ran must not run it again: with OPcache holding the script, an include is
44+
* served from the cache without the trap being asked for the contents. A name
45+
* whose file nothing has loaded must still resolve, without executing it.
46+
*
47+
* Needs its own process: OPcache is only on in the processes PHPStan spawns
48+
* for itself, and the failure is a fatal error.
49+
*/
50+
#[Group('exec')]
51+
public function testTrapSurvivesOpcacheCacheHit(): void
52+
{
53+
if (!extension_loaded('Zend OPcache')) {
54+
self::markTestSkipped('OPcache is not available.');
55+
}
56+
57+
exec(sprintf(
58+
'%s -d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.validate_timestamps=0 %s 2>&1',
59+
escapeshellarg(PHP_BINARY),
60+
escapeshellarg(__DIR__ . '/data/opcache-trap/driver.php'),
61+
), $outputLines, $exitCode);
62+
$output = implode("\n", $outputLines);
63+
64+
$this->assertSame(0, $exitCode, $output);
65+
66+
$values = [];
67+
foreach ($outputLines as $outputLine) {
68+
if (!str_contains($outputLine, '=')) {
69+
continue;
70+
}
71+
[$name, $value] = explode('=', $outputLine, 2);
72+
$values[$name] = $value;
73+
}
74+
75+
// hold whether or not OPcache could be turned on
76+
$this->assertSame('1', $values['survivedLoadedProbe'] ?? null, $output);
77+
$this->assertSame('1', $values['resolvedCold'] ?? null, $output);
78+
$this->assertSame('1', $values['coldFileNotExecuted'] ?? null, $output);
79+
80+
if (($values['opcacheEnabled'] ?? '0') === '1') {
81+
return;
82+
}
83+
84+
self::markTestSkipped('OPcache could not be enabled for the CLI, so the cache hit was not exercised.');
85+
}
86+
3287
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace OpcacheTrap;
4+
5+
class ColdClass
6+
{
7+
8+
}
9+
10+
function coldThing(): string
11+
{
12+
return 'y';
13+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php declare(strict_types = 1);
2+
3+
// Driver for FileReadTrapStreamWrapperTest::testTrapSurvivesOpcacheCacheHit().
4+
//
5+
// Runs the real AutoloadSourceLocator against a Composer autoloader whose PSR-4
6+
// prefix resolves to a file the process has already loaded - the
7+
// php-standard-library / azjezz/psl shape, where a files-autoload bootstrap
8+
// loads every function file at startup and the same paths stay reachable
9+
// through the prefix. Letting the autoloader include such a path runs the file
10+
// a second time whenever OPcache already holds the script, because the include
11+
// is served from the cache without the trap being asked for the contents, and
12+
// the process dies with "Cannot redeclare function OpcacheTrap\thing()".
13+
//
14+
// Each step prints as it goes, so a failure shows how far it got.
15+
16+
use PHPStan\BetterReflection\Identifier\Identifier;
17+
use PHPStan\BetterReflection\Identifier\IdentifierType;
18+
use PHPStan\BetterReflection\Reflector\DefaultReflector;
19+
use PHPStan\Reflection\BetterReflection\SourceLocator\AutoloadSourceLocator;
20+
use PHPStan\Reflection\BetterReflection\SourceLocator\FileNodesFetcher;
21+
use PHPStan\Testing\PHPStanTestCase;
22+
23+
$loader = require __DIR__ . '/../../../../../../../vendor/autoload.php';
24+
require_once __DIR__ . '/../../../../../../phpstan-bootstrap.php';
25+
26+
$opcacheStatus = opcache_get_status(false);
27+
$opcacheEnabled = $opcacheStatus !== false && ($opcacheStatus['opcache_enabled'] ?? false) === true;
28+
echo 'opcacheEnabled=', $opcacheEnabled ? '1' : '0', "\n";
29+
30+
// what the files-autoload bootstrap of such a package does at startup
31+
require_once __DIR__ . '/thing.php';
32+
33+
// a real project registers its prefixes on the Composer loader that is
34+
// already in place, rather than adding another autoloader behind it
35+
$loader->addPsr4('OpcacheTrap\\', [__DIR__]);
36+
37+
$locator = new AutoloadSourceLocator(
38+
PHPStanTestCase::getContainer()->getByType(FileNodesFetcher::class),
39+
true,
40+
);
41+
$reflector = new DefaultReflector($locator);
42+
43+
// OpcacheTrap\thing is a function, not a class, but PHPStan probes the name as
44+
// a class the same way it does for Psl\Type\optional - and the PSR-4 prefix
45+
// sends the autoloader at the already-loaded function.php
46+
$locator->locateIdentifier($reflector, new Identifier('OpcacheTrap\thing', new IdentifierType(IdentifierType::IDENTIFIER_CLASS)));
47+
echo "survivedLoadedProbe=1\n";
48+
49+
// a name whose file nothing has loaded must still resolve
50+
$cold = $reflector->reflectClass('OpcacheTrap\ColdClass');
51+
echo 'resolvedCold=', $cold->getName() === 'OpcacheTrap\ColdClass' ? '1' : '0', "\n";
52+
echo 'coldFileNotExecuted=', !function_exists('OpcacheTrap\coldThing') ? '1' : '0', "\n";
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace OpcacheTrap;
4+
5+
// one function per file, named so that the PSR-4 prefix resolves OpcacheTrap\thing
6+
// to this very path - the php-standard-library / azjezz/psl layout
7+
function thing(): string
8+
{
9+
return 'x';
10+
}

0 commit comments

Comments
 (0)