From 6d2fec9bcec46167a0fef3b08fce9707efac788c Mon Sep 17 00:00:00 2001 From: MGatner Date: Thu, 14 Jan 2021 19:30:17 +0000 Subject: [PATCH 1/5] Add Cache File mode --- app/Config/Cache.php | 16 +++++ system/Cache/Handlers/FileHandler.php | 29 ++++++-- .../system/Cache/Handlers/FileHandlerTest.php | 66 ++++++++++++++----- user_guide_src/source/libraries/caching.rst | 7 +- 4 files changed, 92 insertions(+), 26 deletions(-) diff --git a/app/Config/Cache.php b/app/Config/Cache.php index 8c7900a84926..a3b3bbcf895d 100644 --- a/app/Config/Cache.php +++ b/app/Config/Cache.php @@ -46,6 +46,8 @@ class Cache extends BaseConfig * system. * * @var string + * + * @deprecated Use the driver-specific variant under $file */ public $storePath = WRITEPATH . 'cache/'; @@ -80,6 +82,20 @@ class Cache extends BaseConfig */ public $prefix = ''; + /** + * -------------------------------------------------------------------------- + * File settings + * -------------------------------------------------------------------------- + * Your file storage preferences can be specified below, if you are using + * the File driver. + * + * @var array + */ + public $file = [ + 'storePath' => WRITEPATH . 'cache/', + 'mode' => 0640, + ]; + /** * ------------------------------------------------------------------------- * Memcached settings diff --git a/system/Cache/Handlers/FileHandler.php b/system/Cache/Handlers/FileHandler.php index e99874e9d173..88b1eb72cf5e 100644 --- a/system/Cache/Handlers/FileHandler.php +++ b/system/Cache/Handlers/FileHandler.php @@ -34,6 +34,16 @@ class FileHandler implements CacheInterface */ protected $path; + /** + * Mode for the stored files. + * Must be chmod-safe (octal). + * + * @var int + * + * @see https://www.php.net/manual/en/function.chmod.php + */ + protected $mode; + //-------------------------------------------------------------------- /** @@ -44,14 +54,23 @@ class FileHandler implements CacheInterface */ public function __construct(Cache $config) { - $path = ! empty($config->storePath) ? $config->storePath : WRITEPATH . 'cache'; - if (! is_really_writable($path)) + if (isset($config->file)) + { + $this->path = $config->file['storePath'] ?? WRITEPATH . 'cache'; + $this->mode = $config->file['mode'] ?? 0640; + } + else + { + $this->path = ! empty($config->storePath) ? $config->storePath : WRITEPATH . 'cache'; + } + + if (! is_really_writable($this->path)) { - throw CacheException::forUnableToWrite($path); + throw CacheException::forUnableToWrite($this->path); } $this->prefix = $config->prefix ?: ''; - $this->path = rtrim($path, '/') . '/'; + $this->path = rtrim($this->path, '/') . '/'; } //-------------------------------------------------------------------- @@ -105,7 +124,7 @@ public function save(string $key, $value, int $ttl = 60) if ($this->writeFile($this->path . $key, serialize($contents))) { - chmod($this->path . $key, 0640); + chmod($this->path . $key, $this->mode); return true; } diff --git a/tests/system/Cache/Handlers/FileHandlerTest.php b/tests/system/Cache/Handlers/FileHandlerTest.php index bb77935abf10..61269efacceb 100644 --- a/tests/system/Cache/Handlers/FileHandlerTest.php +++ b/tests/system/Cache/Handlers/FileHandlerTest.php @@ -26,13 +26,13 @@ protected function setUp(): void { parent::setUp(); - //Initialize path - $this->config = new \Config\Cache(); - $this->config->storePath .= self::$directory; + // Initialize path + $this->config = new \Config\Cache(); + $this->config->file['storePath'] .= self::$directory; - if (! is_dir($this->config->storePath)) + if (! is_dir($this->config->file['storePath'])) { - mkdir($this->config->storePath, 0777, true); + mkdir($this->config->file['storePath'], 0777, true); } $this->fileHandler = new FileHandler($this->config); @@ -41,20 +41,20 @@ protected function setUp(): void public function tearDown(): void { - if (is_dir($this->config->storePath)) + if (is_dir($this->config->file['storePath'])) { - chmod($this->config->storePath, 0777); + chmod($this->config->file['storePath'], 0777); foreach (self::getKeyArray() as $key) { - if (is_file($this->config->storePath . DIRECTORY_SEPARATOR . $key)) + if (is_file($this->config->file['storePath'] . DIRECTORY_SEPARATOR . $key)) { - chmod($this->config->storePath . DIRECTORY_SEPARATOR . $key, 0777); - unlink($this->config->storePath . DIRECTORY_SEPARATOR . $key); + chmod($this->config->file['storePath'] . DIRECTORY_SEPARATOR . $key, 0777); + unlink($this->config->file['storePath'] . DIRECTORY_SEPARATOR . $key); } } - rmdir($this->config->storePath); + rmdir($this->config->file['storePath']); } } @@ -67,15 +67,15 @@ public function testNewWithNonWritablePath() { $this->expectException('CodeIgniter\Cache\Exceptions\CacheException'); - chmod($this->config->storePath, 0444); + chmod($this->config->file['storePath'], 0444); new FileHandler($this->config); } public function testSetDefaultPath() { - //Initialize path - $config = new \Config\Cache(); - $config->storePath = null; + // Initialize path + $config = new \Config\Cache(); + $config->file['storePath'] = null; $this->fileHandler = new FileHandler($config); $this->fileHandler->initialize(); @@ -98,7 +98,7 @@ public function testSave() { $this->assertTrue($this->fileHandler->save(self::$key1, 'value')); - chmod($this->config->storePath, 0444); + chmod($this->config->file['storePath'], 0444); $this->assertFalse($this->fileHandler->save(self::$key2, 'value')); } @@ -173,6 +173,36 @@ public function testIsSupported() $this->assertTrue($this->fileHandler->isSupported()); } + /** + * @dataProvider modeProvider + */ + public function testSaveMode($int, $string) + { + // Initialize mode + $config = new \Config\Cache(); + $config->file['mode'] = $int; + + $this->fileHandler = new FileHandler($config); + $this->fileHandler->initialize(); + + $this->fileHandler->save(self::$key1, 'value'); + + $file = $config->file['storePath'] . DIRECTORY_SEPARATOR . self::$key1; + $mode = substr(sprintf('%o', fileperms($file)), -4); + + $this->assertEquals($string, $mode); + } + + public function modeProvider() + { + return [ + [0640, '0640'], + [0600, '0600'], + [0660, '0660'], + [0777, '0777'], + ]; + } + //-------------------------------------------------------------------- public function testFileHandler() @@ -200,8 +230,8 @@ final class BaseTestFileHandler extends FileHandler public function __construct() { - $this->config = new \Config\Cache(); - $this->config->storePath .= self::$directory; + $this->config = new \Config\Cache(); + $this->config->file['storePath'] .= self::$directory; parent::__construct($this->config); } diff --git a/user_guide_src/source/libraries/caching.rst b/user_guide_src/source/libraries/caching.rst index 368a34498c09..3108ec1dc700 100644 --- a/user_guide_src/source/libraries/caching.rst +++ b/user_guide_src/source/libraries/caching.rst @@ -59,9 +59,9 @@ more complex, multi-server setups. If you have more than one application using the same cache storage, you can add a custom prefix here that is prepended to all key names. -**$path** +**$file** -This is used by the ``file`` handler to show where it should save the cache files to. +This is an array of settings specific to the ``File`` handler determine how it should save the cache files. **$memcached** @@ -212,7 +212,8 @@ File-based Caching Unlike caching from the Output Class, the driver file-based caching allows for pieces of view files to be cached. Use this with care, and make sure to benchmark your application, as a point can come where disk -I/O will negate positive gains by caching. This requires a writable cache directory to be really writable (0777). +I/O will negate positive gains by caching. This requires a cache +directory to be really writable by the application. ================= Memcached Caching From ee4df3ea0e72b2c2820683d2ba44c00b97cfd06b Mon Sep 17 00:00:00 2001 From: MGatner Date: Thu, 14 Jan 2021 19:33:02 +0000 Subject: [PATCH 2/5] Add deprecation note --- user_guide_src/source/changelogs/v4.0.5.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.0.5.rst b/user_guide_src/source/changelogs/v4.0.5.rst index 9a55cf6d6e60..90a8ac7ab62e 100644 --- a/user_guide_src/source/changelogs/v4.0.5.rst +++ b/user_guide_src/source/changelogs/v4.0.5.rst @@ -14,6 +14,7 @@ Enhancements: - Support for setting SameSite attribute on Session and CSRF cookies has been added. For security and compatibility with latest browser versions, the default setting is ``Lax``. - Guessing file extensions from mime type in ``Config\Mimes::guessExtensionFromType`` now only reverse searches the ``$mimes`` array if no extension is proposed (i.e., usually not for uploaded files). - The getter functions for file extensions of uploaded files now have different fallback values (``$this->getClientExtension()`` for ``UploadedFile->getExtension()`` and ``''`` for ``UploadedFile->guessExtension()``). This is a security fix and makes the process less dependent on the client. +- The Cache ``FileHandler`` now allows setting the file permissions mode via ``Config\Cache`` Changes: @@ -44,3 +45,4 @@ Deprecations: - Deprecated ``CodeIgniter\Config\Config`` in favor of ``CodeIgniter\Config\Factories::config()`` - Deprecated ``CodeIgniter\HTTP\Message::getHeader`` in favor of ``header()`` to prepare for PSR-7 - Deprecated ``CodeIgniter\HTTP\Message::getHeaders`` in favor of ``headers()`` to prepare for PSR-7 +- Deprecated ``Config\Cache::$storePath`` in favor of ``Config\Cache::$file['storePath']`` From 7985cea37afa304a4186e2e83966392127756dfb Mon Sep 17 00:00:00 2001 From: MGatner Date: Fri, 15 Jan 2021 02:11:53 +0000 Subject: [PATCH 3/5] Improve test, fix typo --- tests/system/Cache/Handlers/FileHandlerTest.php | 15 ++++++++++----- user_guide_src/source/libraries/caching.rst | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/system/Cache/Handlers/FileHandlerTest.php b/tests/system/Cache/Handlers/FileHandlerTest.php index 61269efacceb..232b3e71b5f9 100644 --- a/tests/system/Cache/Handlers/FileHandlerTest.php +++ b/tests/system/Cache/Handlers/FileHandlerTest.php @@ -26,6 +26,11 @@ protected function setUp(): void { parent::setUp(); + if (! function_exists('octal_permissions')) + { + helper('filesystem'); + } + // Initialize path $this->config = new \Config\Cache(); $this->config->file['storePath'] .= self::$directory; @@ -188,7 +193,7 @@ public function testSaveMode($int, $string) $this->fileHandler->save(self::$key1, 'value'); $file = $config->file['storePath'] . DIRECTORY_SEPARATOR . self::$key1; - $mode = substr(sprintf('%o', fileperms($file)), -4); + $mode = octal_permissions(fileperms($file)); $this->assertEquals($string, $mode); } @@ -196,10 +201,10 @@ public function testSaveMode($int, $string) public function modeProvider() { return [ - [0640, '0640'], - [0600, '0600'], - [0660, '0660'], - [0777, '0777'], + [0640, '640'], + [0600, '600'], + [0660, '660'], + [0777, '777'], ]; } diff --git a/user_guide_src/source/libraries/caching.rst b/user_guide_src/source/libraries/caching.rst index 3108ec1dc700..887c08d472af 100644 --- a/user_guide_src/source/libraries/caching.rst +++ b/user_guide_src/source/libraries/caching.rst @@ -61,7 +61,7 @@ here that is prepended to all key names. **$file** -This is an array of settings specific to the ``File`` handler determine how it should save the cache files. +This is an array of settings specific to the ``File`` handler to determine how it should save the cache files. **$memcached** From 9b9bc39683542db9e003fd86652c21fec2717831 Mon Sep 17 00:00:00 2001 From: MGatner Date: Fri, 15 Jan 2021 14:26:39 +0000 Subject: [PATCH 4/5] Implement review property check --- system/Cache/Handlers/FileHandler.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/system/Cache/Handlers/FileHandler.php b/system/Cache/Handlers/FileHandler.php index 88b1eb72cf5e..05a5ba3cba85 100644 --- a/system/Cache/Handlers/FileHandler.php +++ b/system/Cache/Handlers/FileHandler.php @@ -38,7 +38,7 @@ class FileHandler implements CacheInterface * Mode for the stored files. * Must be chmod-safe (octal). * - * @var int + * @var integer * * @see https://www.php.net/manual/en/function.chmod.php */ @@ -54,23 +54,24 @@ class FileHandler implements CacheInterface */ public function __construct(Cache $config) { - if (isset($config->file)) + if (! property_exists($config, 'file')) { - $this->path = $config->file['storePath'] ?? WRITEPATH . 'cache'; - $this->mode = $config->file['mode'] ?? 0640; - } - else - { - $this->path = ! empty($config->storePath) ? $config->storePath : WRITEPATH . 'cache'; + $config->file = [ + 'storePath' => $config->storePath ?? WRITEPATH . 'cache', + 'mode' => 0640, + ]; } + $this->path = ! empty($config->file['storePath']) ? $config->file['storePath'] : WRITEPATH . 'cache'; + $this->path = rtrim($this->path, '/') . '/'; + if (! is_really_writable($this->path)) { throw CacheException::forUnableToWrite($this->path); } + $this->mode = $config->file['mode'] ?? 0640; $this->prefix = $config->prefix ?: ''; - $this->path = rtrim($this->path, '/') . '/'; } //-------------------------------------------------------------------- From 06b92259cfc1747a6c0eabfefc8532896d5d69a8 Mon Sep 17 00:00:00 2001 From: MGatner Date: Fri, 15 Jan 2021 14:28:42 +0000 Subject: [PATCH 5/5] Allow '0' cache prefix --- system/Cache/Handlers/FileHandler.php | 2 +- system/Cache/Handlers/MemcachedHandler.php | 2 +- system/Cache/Handlers/PredisHandler.php | 2 +- system/Cache/Handlers/RedisHandler.php | 2 +- system/Cache/Handlers/WincacheHandler.php | 2 +- user_guide_src/source/libraries/caching.rst | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/system/Cache/Handlers/FileHandler.php b/system/Cache/Handlers/FileHandler.php index 05a5ba3cba85..d4ed24aecd6f 100644 --- a/system/Cache/Handlers/FileHandler.php +++ b/system/Cache/Handlers/FileHandler.php @@ -71,7 +71,7 @@ public function __construct(Cache $config) } $this->mode = $config->file['mode'] ?? 0640; - $this->prefix = $config->prefix ?: ''; + $this->prefix = (string) $config->prefix; } //-------------------------------------------------------------------- diff --git a/system/Cache/Handlers/MemcachedHandler.php b/system/Cache/Handlers/MemcachedHandler.php index 84d6539bcc81..8a883e26823b 100644 --- a/system/Cache/Handlers/MemcachedHandler.php +++ b/system/Cache/Handlers/MemcachedHandler.php @@ -58,7 +58,7 @@ class MemcachedHandler implements CacheInterface */ public function __construct(Cache $config) { - $this->prefix = $config->prefix ?: ''; + $this->prefix = (string) $config->prefix; if (! empty($config)) { diff --git a/system/Cache/Handlers/PredisHandler.php b/system/Cache/Handlers/PredisHandler.php index 81d982dd5f1b..2a062e17f2fa 100644 --- a/system/Cache/Handlers/PredisHandler.php +++ b/system/Cache/Handlers/PredisHandler.php @@ -58,7 +58,7 @@ class PredisHandler implements CacheInterface */ public function __construct(Cache $config) { - $this->prefix = $config->prefix ?: ''; + $this->prefix = (string) $config->prefix; if (isset($config->redis)) { diff --git a/system/Cache/Handlers/RedisHandler.php b/system/Cache/Handlers/RedisHandler.php index 5af9f1f07fbf..d18efb1e93d5 100644 --- a/system/Cache/Handlers/RedisHandler.php +++ b/system/Cache/Handlers/RedisHandler.php @@ -58,7 +58,7 @@ class RedisHandler implements CacheInterface */ public function __construct(Cache $config) { - $this->prefix = $config->prefix ?: ''; + $this->prefix = (string) $config->prefix; if (! empty($config)) { diff --git a/system/Cache/Handlers/WincacheHandler.php b/system/Cache/Handlers/WincacheHandler.php index a37aba2556be..4c445d4f0076 100644 --- a/system/Cache/Handlers/WincacheHandler.php +++ b/system/Cache/Handlers/WincacheHandler.php @@ -37,7 +37,7 @@ class WincacheHandler implements CacheInterface */ public function __construct(Cache $config) { - $this->prefix = $config->prefix ?: ''; + $this->prefix = (string) $config->prefix; } //-------------------------------------------------------------------- diff --git a/user_guide_src/source/libraries/caching.rst b/user_guide_src/source/libraries/caching.rst index 887c08d472af..8e0f38d3a6e0 100644 --- a/user_guide_src/source/libraries/caching.rst +++ b/user_guide_src/source/libraries/caching.rst @@ -57,7 +57,7 @@ more complex, multi-server setups. **$prefix** If you have more than one application using the same cache storage, you can add a custom prefix -here that is prepended to all key names. +string here that is prepended to all key names. **$file**