Cache type not usable if global sys_tmp_dir not writeable

[Lokilein]

BC Break Report

Q	A
Version	2.3.x-dev

Summary

When you define an own cache_dir, the system always tries the default sys_get_temp_dir first, even though you are not using it. If anything is wrong with it (like no write access), you cannot create a FileSystem Adapter at all.

Previous behavior

The sys_get_temp_dir() was only used in case that no other dir was defined.

Current behavior

I've provided the following config for the filesystem:

'name'    => 'filesystem',
'adapter' => 'filesystem',
'options' => [
    'cache_dir' => 'data/cache/',
    'ttl'       => '3600'
],
'plugins' => [
    [
        'name'    => 'exception_handler',
        'options' => [
            'throw_exceptions' => true,
        ],
    ],
    [
        'name' => 'Serializer'
    ]
]

As you can see, the cache_dir is provided in the settings.
However, since the new version, the FileSystemOptions always calls the setCacheDir with the parameter null:
$this->setCacheDir(null); (FileSystemOptions.php:115)

This results in a fallback to sys_get_temp_dir(), which will then be checked and - on my test maschine - was not writable. The Adapter throws and cannot be created then, though right after that, it would overwrite the CacheDir with the correct and working path.

How to reproduce

Define your sys_temp_dir in your php.ini to a non-existing or not-writable path.
Then try to create an adapter with a valid cache_dir configuration.
It will not work and fails with one exception of the method normalizeCacheDirectory

[Ocramius]

@Lokilein does this not affect 2.2.x?

[froschdesign]

@Lokilein

However, since the new version, the FileSystemOptions always calls the setCacheDir with the parameter null: $this->setCacheDir(null); (FileSystemOptions.php:115)

After that, your configuration is passed down a few layers:

laminas-cache-storage-adapter-filesystem/src/FilesystemOptions.php

Lines 115 to 117 in f5bc8f5

	$this->setCacheDir(null);
	parent::__construct($options);

https://github.com/laminas/laminas-stdlib/blob/9c92afc89381d9f251aff6edf7bbd5db42c82071/src/AbstractOptions.php#L38-L43

https://github.com/laminas/laminas-stdlib/blob/9c92afc89381d9f251aff6edf7bbd5db42c82071/src/AbstractOptions.php#L52-L75

https://github.com/laminas/laminas-stdlib/blob/9c92afc89381d9f251aff6edf7bbd5db42c82071/src/AbstractOptions.php#L113-L131

And if something is wrong then the adapter will throw an exception:

laminas-cache-storage-adapter-filesystem/src/FilesystemOptions.php

Lines 437 to 454 in f5bc8f5

	private function normalizeCacheDirectory(string $cacheDir): string
	{
	if (! is_dir($cacheDir)) {
	throw new Exception\InvalidArgumentException(
	"Cache directory '{$cacheDir}' not found or not a directory"
	);
	} elseif (! is_writable($cacheDir)) {
	throw new Exception\InvalidArgumentException(
	"Cache directory '{$cacheDir}' not writable"
	);
	} elseif (! is_readable($cacheDir)) {
	throw new Exception\InvalidArgumentException(
	"Cache directory '{$cacheDir}' not readable"
	);
	}
	return rtrim(realpath($cacheDir), DIRECTORY_SEPARATOR);
	}

[Lokilein]

@Lokilein does this not affect 2.2.x?

Might be so, I just updated and it occured to me with this version. We've updated from 1.2 though.

[Ocramius]

If you've updated from 1.x, then BC breaks are expected anyway: re-labeling as bug

[Lokilein]

@froschdesign Correctly. At the end, it works like it should, but as it checks the "null" aka sys_tmp_dir as well now, even though it will be overwritten soon after, it breaks where it should not break.

@Ocramius Agree!

[Ocramius]

@Lokilein are you able to provide a failing test, perhaps? 🤔

[Lokilein]

@Ocramius Hmm, not sure what you mean?

I think you just need to configure a valid Filesystem Cache with a valid cache_dir (like my example above). And if you now configure your php.ini to set your sys_tmp_dir=/does/not/exist or similar, the error should already occure.

I cannot provide my/our code as example, as we have a quite big monolith and an own Factory for the caches. That would be a overhead and not easy to extract...

[froschdesign]

@Lokilein
Write a test that illustrates the problem and send it as a pull request. Compare with the existing tests:

laminas-cache-storage-adapter-filesystem/test/unit/FilesystemOptionsTest.php

Line 36 in f5bc8f5

final class FilesystemOptionsTest extends AbstractAdapterOptionsTest

[Lokilein]

@froschdesign
hmm... I am currently working on Windows, which makes the tests difficult, but even though - I am not sure how to make sure that the sys_temp_dir is not writeable, as it is also not changable by ini_set or something and I will not change the rights of the default folder (and I hope that this would not work :D)

But, when I run the tests with an invalid php.ini setting (C:\doesnotexist), my tests are already failing with an error, while they all pass when the sys_tmp_dir is valid.

With the invalid setting, I receive:
[ErrorException] tempnam(): file created in the system's temporary directory

I think that this fails because createAdapterOptions calls the constructor and this is enough to reproduce - depending on the configured path.

[Ocramius]

docker run --rm -ti -v $(pwd):/app -w /app php:7 bash and work from there? 🤔

[tck]

Pull request to fix this issue #73