Skip to content

[HttpKernel] Make Kernel::getShareDir() nullable #62204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nicolas-grekas merged 1 commit into from
Oct 31, 2025
Merged

[HttpKernel] Make Kernel::getShareDir() nullable #62204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/Command/AboutCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
['Charset', $kernel->getCharset()],
['Cache directory', self::formatPath($kernel->getCacheDir(), $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($kernel->getCacheDir()).'</>)'],
['Build directory', self::formatPath($buildDir, $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($buildDir).'</>)'],
['Share directory', self::formatPath($shareDir, $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($shareDir).'</>)'],
['Share directory', null === $shareDir ? 'none' : self::formatPath($shareDir, $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($shareDir).'</>)'],
['Log directory', self::formatPath($kernel->getLogDir(), $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($kernel->getLogDir()).'</>)'],
new TableSeparator(),
['<info>PHP</>'],
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/HttpCache/HttpCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,6 @@ protected function createSurrogate(): SurrogateInterface

protected function createStore(): StoreInterface
{
return $this->store ?? new Store($this->cacheDir ?: $this->kernel->getShareDir().'/http_cache');
return $this->store ?? new Store($this->cacheDir ?: ($this->kernel->getShareDir() ?? $this->kernel->getCacheDir()).'/http_cache');
}
}
9 changes: 7 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,15 @@ public function getBuildDir(): string
return parent::getBuildDir();
}

public function getShareDir(): string
public function getShareDir(): ?string
{
if (isset($_SERVER['APP_SHARE_DIR'])) {
return $_SERVER['APP_SHARE_DIR'].'/'.$this->environment;
if (false === $dir = filter_var($_SERVER['APP_SHARE_DIR'], \FILTER_VALIDATE_BOOL, \FILTER_NULL_ON_FAILURE) ?? $_SERVER['APP_SHARE_DIR']) {
return null;
}
if (\is_string($dir)) {
return $dir.'/'.$this->environment;
}
}

return parent::getShareDir();
Expand Down
43 changes: 43 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/MicroKernelTraitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,49 @@ protected function tearDown(): void
}
}

public function testGetShareDirDisabledByEnv()
{
$previous = $_SERVER['APP_SHARE_DIR'] ?? null;
Copy link
Member

@xabbuh xabbuh Oct 31, 2025

Choose a reason for hiding this comment

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

Is this needed? PHPUnit does restore the $_SERVER array after each test if the backup globale feature isn’t disabled, doesn’t it?

Copy link
Member Author

@nicolas-grekas nicolas-grekas Oct 31, 2025

Choose a reason for hiding this comment

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

that's quite possible 😬

Copy link
Member

@xabbuh xabbuh Nov 2, 2025

Choose a reason for hiding this comment

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

FTR, it's the opposite: #62273

$_SERVER['APP_SHARE_DIR'] = 'false';

try {
$kernel = $this->kernel = new ConcreteMicroKernel('test', false);

$this->assertNull($kernel->getShareDir());

$parameters = $kernel->getKernelParameters();
$this->assertArrayNotHasKey('kernel.share_dir', $parameters);
} finally {
if (null === $previous) {
unset($_SERVER['APP_SHARE_DIR']);
} else {
$_SERVER['APP_SHARE_DIR'] = $previous;
}
}
}

public function testGetShareDirCustomPathFromEnv()
{
$previous = $_SERVER['APP_SHARE_DIR'] ?? null;
$_SERVER['APP_SHARE_DIR'] = sys_get_temp_dir();

try {
$kernel = $this->kernel = new ConcreteMicroKernel('test', false);

$expected = rtrim(sys_get_temp_dir(), '/').'/test';
$this->assertSame($expected, $kernel->getShareDir());

$parameters = $kernel->getKernelParameters();
$this->assertSame($expected, $parameters['kernel.share_dir'] ?? null);
} finally {
if (null === $previous) {
unset($_SERVER['APP_SHARE_DIR']);
} else {
$_SERVER['APP_SHARE_DIR'] = $previous;
}
}
}

public function test()
{
$kernel = $this->kernel = new ConcreteMicroKernel('test', false);
Expand Down
5 changes: 2 additions & 3 deletions src/Symfony/Component/HttpKernel/Kernel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ public function getBuildDir(): string
return $this->getCacheDir();
}

public function getShareDir(): string
public function getShareDir(): ?string
{
// Returns $this->getCacheDir() for backward compatibility
return $this->getCacheDir();
Comment on lines 308 to 309
Copy link
Contributor

@alexander-schranz alexander-schranz Oct 31, 2025

Choose a reason for hiding this comment

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

should this not be now return null; ?

Copy link
Member Author

@nicolas-grekas nicolas-grekas Oct 31, 2025

Choose a reason for hiding this comment

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

nope: we want to null to be opt-in

Expand Down Expand Up @@ -586,13 +586,12 @@ protected function getKernelParameters(): array
'kernel.debug' => $this->debug,
'kernel.build_dir' => realpath($dir = $this->warmupDir ?: $this->getBuildDir()) ?: $dir,
'kernel.cache_dir' => realpath($dir = ($this->getCacheDir() === $this->getBuildDir() ? ($this->warmupDir ?: $this->getCacheDir()) : $this->getCacheDir())) ?: $dir,
'kernel.share_dir' => realpath($dir = $this->getShareDir()) ?: $dir,
'kernel.logs_dir' => realpath($dir = $this->getLogDir()) ?: $dir,
'kernel.bundles' => $bundles,
'kernel.bundles_metadata' => $bundlesMetadata,
'kernel.charset' => $this->getCharset(),
'kernel.container_class' => $this->getContainerClass(),
];
] + (null !== ($dir = $this->getShareDir()) ? ['kernel.share_dir' => realpath($dir) ?: $dir] : []);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/HttpKernel/KernelInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
*
* It manages an environment made of application kernel and bundles.
*
* @method string getShareDir() Returns the share directory - not implementing it is deprecated since Symfony 7.4.
* This directory should be used to store data that is shared between all front-end servers.
* This typically fits application caches.
* @method string|null getShareDir() Returns the share directory - not implementing it is deprecated since Symfony 7.4.
* This directory should be used to store data that is shared between all front-end servers; this typically fits application caches.
* `null` should be returned if the application shall not use a share directory.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
Expand Down
Loading