Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 61 additions & 0 deletions src/DependencyInjection/Configurator.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,25 @@
use PHPStan\File\FileReader;
use PHPStan\File\FileWriter;
use PHPStan\Turbo\TurboExtensionEnabler;
use function array_fill_keys;
use function array_intersect_key;
use function array_keys;
use function count;
use function error_reporting;
use function explode;
use function file_get_contents;
use function hash_file;
use function implode;
use function in_array;
use function is_dir;
use function is_file;
use function ksort;
use function preg_match;
use function preg_match_all;
use function restore_error_handler;
use function set_error_handler;
use function sprintf;
use function str_contains;
use function str_ends_with;
use function substr;
use function time;
Expand All @@ -38,6 +44,13 @@
final class Configurator extends \Nette\Bootstrap\Configurator
{

/**
* Env vars PHPStan reads at build time in a CompilerExtension (FnsrExtension reads PHPSTAN_FNSR),
* so the container depends on them and they must stay in the cache key. ContainerCacheKeyEnvGuardTest
* fails if a CompilerExtension reads an env var not listed here or referenced via %env.*%.
*/
public const BUILD_TIME_ENV_VARIABLES = ['PHPSTAN_FNSR'];

/** @var string[] */
private array $allConfigFiles = [];

Expand Down Expand Up @@ -97,6 +110,19 @@ public function loadContainer(): string
// make sure invocations via blackfire use the same container
unset($staticParameters['env']['BLACKFIRE_AGENT_SOCKET']);

// Keep only the env vars the container actually depends on in the cache key, so unrelated env
// changes (CI/shell) don't force a full recompile - phpstan/phpstan#14072. The full env stays
// in the container parameters, so %env.*% resolution is unaffected; this only narrows the key.
if (isset($staticParameters['env'])) {
$relevantEnvVariableNames = $this->relevantEnvVariableNamesForCacheKey();
if ($relevantEnvVariableNames !== null) {
$staticParameters['env'] = array_intersect_key(
$staticParameters['env'],
array_fill_keys($relevantEnvVariableNames, true),
);
}
}

$containerKey = [
$staticParameters,
array_keys($this->dynamicParameters),
Expand Down Expand Up @@ -231,6 +257,41 @@ public function createContainer(bool $initialize = true): OriginalNetteContainer
return $container;
}

/**
* Env vars that can change the generated container, so they must stay in the cache key: every
* %env.NAME% referenced across the loaded configs, plus BUILD_TIME_ENV_VARIABLES. Returns null
* when a config references the whole %env% array, in which case all of it must be kept.
*
* @return list<string>|null
*/
private function relevantEnvVariableNamesForCacheKey(): ?array
{
$names = self::BUILD_TIME_ENV_VARIABLES;
foreach ($this->allConfigFiles as $file) {

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.

This isn't a great way to do this. I think the tracking of used %env... should be somewhere deeper in the DIC compiler. I'm not entirely sure how to implement it. This would have to be researched where Nette DI actually expands this parameters and whether we can efficiently tap into it.

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.

@ondrejmirtes shall I apply the suggestion in this comment below, or do you have another direction you want me to explore?

$contents = @file_get_contents($file);
if ($contents === false || !str_contains($contents, '%env')) {
continue;
}

// A bare %env% reference exposes the whole environment to the container; keep all of it.
if (preg_match('~%env%~', $contents) === 1) {
return null;
}

// Match Nette's parameter-name grammar (%([\w.-]*)% in NeonAdapter) so a valid reference like
// %env.MY-VAR% is not missed, which would drop MY-VAR from the key and reuse a stale container.
if (preg_match_all('~%env\.([\w.-]+)%~', $contents, $matches) === 0) {
continue;
}

foreach ($matches[1] as $name) {
$names[] = $name;
}
}

return $names;
}

/**
* @return string[]
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php declare(strict_types = 1);

namespace PHPStan\DependencyInjection;

use PHPUnit\Framework\TestCase;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
use SplFileInfo;
use function file_get_contents;
use function implode;
use function in_array;
use function preg_match_all;
use function sort;
use function str_contains;
use const PHP_EOL;

/**
* An env var a CompilerExtension reads at build time must be declared in
* Configurator::BUILD_TIME_ENV_VARIABLES, or narrowing the container cache key would silently reuse a
* stale container when it changes (phpstan/phpstan#14072). This fails on any undeclared build-time
* read. Detection is deliberately simple: literal getenv()/$_ENV names in classes that directly
* extend CompilerExtension.
*/
final class ContainerCacheKeyEnvGuardTest extends TestCase
{

public function testBuildTimeEnvReadsInCompilerExtensionsAreDeclared(): void
{
$srcDir = __DIR__ . '/../../../src';
$offenders = [];

/** @var SplFileInfo $file */
foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($srcDir, RecursiveDirectoryIterator::SKIP_DOTS)) as $file) {
if (!$file->isFile() || $file->getExtension() !== 'php') {
continue;
}

$contents = file_get_contents($file->getPathname());
if ($contents === false || !str_contains($contents, 'extends CompilerExtension')) {
continue;
}

if (preg_match_all('~(?:getenv\(|\$_ENV\[)\s*[\'"]([A-Za-z_][A-Za-z0-9_]*)[\'"]~', $contents, $matches) === 0) {
continue;
}

foreach ($matches[1] as $envName) {
if (in_array($envName, Configurator::BUILD_TIME_ENV_VARIABLES, true)) {
continue;
}

$offenders[] = $file->getFilename() . ': ' . $envName;
}
}

sort($offenders);

$this->assertSame(
[],
$offenders,
'A CompilerExtension reads an environment variable at build time that is not declared in '
. 'Configurator::BUILD_TIME_ENV_VARIABLES. Add it there (so it stays in the container cache '
. 'key and a change recompiles), or read it via %env.* in a config instead:' . PHP_EOL
. implode(PHP_EOL, $offenders),
);
}

}
176 changes: 176 additions & 0 deletions tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
<?php declare(strict_types = 1);

namespace PHPStan\DependencyInjection;

use Override;
use PHPStan\File\FileHelper;
use PHPUnit\Framework\TestCase;
use function count;
use function getenv;
use function glob;
use function is_dir;
use function md5;
use function putenv;
use function rmdir;
use function sprintf;
use function sys_get_temp_dir;
use function uniqid;
use function unlink;
use const DIRECTORY_SEPARATOR;

final class ContainerCacheKeyEnvTest extends TestCase
{

private const ENV_VAR = 'PHPSTAN_TEST_W1_CACHE_KEY';

private string $tmpDir;

private string|false $envBackup;

#[Override]
protected function setUp(): void
{
$this->envBackup = getenv(self::ENV_VAR);
$this->tmpDir = sys_get_temp_dir() . '/phpstan-container-cache-key-' . md5(uniqid());
$this->removeDirectory($this->tmpDir);
}

#[Override]
protected function tearDown(): void
{
if ($this->envBackup === false) {
putenv(self::ENV_VAR);
} else {
putenv(sprintf('%s=%s', self::ENV_VAR, $this->envBackup));
}

$this->removeDirectory($this->tmpDir);
}

public function testEnvironmentIsDroppedFromCacheKeyWhenNoConfigReferencesIt(): void
{
// No loaded config uses %env.*%, so an unrelated env change must NOT recompile the container.
putenv(sprintf('%s=first', self::ENV_VAR));
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');

putenv(sprintf('%s=second', self::ENV_VAR));
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');

$this->assertSame(
1,
$this->countCompiledContainers(),
'Changing an unrelated environment variable should reuse the cached container.',
);
}

public function testEnvironmentStaysInCacheKeyWhenAConfigReferencesIt(): void
{
// A loaded config uses %env.*%, so the environment is relevant and a change must recompile.
putenv(sprintf('%s=first', self::ENV_VAR));
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv.neon');

putenv(sprintf('%s=second', self::ENV_VAR));
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv.neon');

$this->assertSame(
2,
$this->countCompiledContainers(),
'When a config references %env.*%, changing it must recompile the container.',
);
}

public function testDashedEnvVariableReferenceStaysInCacheKey(): void
{
// %env.MY-VAR% is a valid Nette reference (grammar %([\w.-]*)%): extraction must keep dashed
// names too, otherwise changing the referenced var would reuse a stale container.
$envName = 'PHPSTAN_TEST_W1-DASH';
$backup = getenv($envName);
try {
putenv(sprintf('%s=first', $envName));
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv-dashed.neon');

putenv(sprintf('%s=second', $envName));
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv-dashed.neon');

$this->assertSame(
2,
$this->countCompiledContainers(),
'A %env.* reference whose name contains a dash must keep that var in the cache key.',
);
} finally {
if ($backup === false) {
putenv($envName);
} else {
putenv(sprintf('%s=%s', $envName, $backup));
}
}
}

public function testBuildTimeEnvVariableStaysInCacheKey(): void
{
// FnsrExtension reads getenv('PHPSTAN_FNSR') at build time and rewires the container, so it
// depends on the var even with no %env.*% in config; toggling it must recompile.
$backup = getenv('PHPSTAN_FNSR');
try {
putenv('PHPSTAN_FNSR=0');
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');

putenv('PHPSTAN_FNSR=1');
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');

$this->assertSame(
2,
$this->countCompiledContainers(),
'Changing PHPSTAN_FNSR (read at build time by FnsrExtension) must recompile the container.',
);
} finally {
if ($backup === false) {
putenv('PHPSTAN_FNSR');
} else {
putenv(sprintf('PHPSTAN_FNSR=%s', $backup));
}
}
}

private function buildContainer(string $additionalConfigFile): void
{
$rootDir = __DIR__ . '/../../..';
$fileHelper = new FileHelper($rootDir);
$rootDir = $fileHelper->normalizePath($rootDir, '/');
$containerFactory = new ContainerFactory($rootDir);
$containerFactory->create(
$this->tmpDir,
[
$containerFactory->getConfigDirectory() . '/config.level8.neon',
$additionalConfigFile,
],
[],
);
}

private function countCompiledContainers(): int
{
$containers = glob($this->tmpDir . '/cache/nette.configurator/Container_*.php');

return $containers === false ? 0 : count($containers);
}

private function removeDirectory(string $directory): void
{
if (!is_dir($directory)) {
return;
}

$entries = glob($directory . DIRECTORY_SEPARATOR . '*');
foreach ($entries === false ? [] : $entries as $entry) {
if (is_dir($entry)) {
$this->removeDirectory($entry);
} else {
@unlink($entry);
}
}

@rmdir($directory);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
parameters:
customRulesetUsed: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# References an env var whose name contains a dash (%env.PHPSTAN_TEST_W1-DASH%) - a valid Nette
# reference (Nette's parameter-name grammar is %([\w.-]*)%). The env-name extraction must keep such
# names in the container cache key too, otherwise changing this var would reuse a stale container.
parameters:
tmpDir: %env.PHPSTAN_TEST_W1-DASH%
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# References an environment variable via %env.<name>%, so that env var must stay in the container
# cache key - otherwise changing it would wrongly reuse a stale container. The resolved tmpDir value
# is irrelevant here (ContainerFactory::create() overrides tmpDir with its own argument); the
# reference itself is what keeps PHPSTAN_TEST_W1_CACHE_KEY in the cache key.
parameters:
tmpDir: %env.PHPSTAN_TEST_W1_CACHE_KEY%
Loading