Skip to content

Commit

Permalink
Merge pull request #4103 from MGatner/cache-permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
MGatner authored Jan 16, 2021
2 parents 2020c2d + 06b9225 commit 2a0d39f
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 32 deletions.
16 changes: 16 additions & 0 deletions app/Config/Cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class Cache extends BaseConfig
* system.
*
* @var string
*
* @deprecated Use the driver-specific variant under $file
*/
public $storePath = WRITEPATH . 'cache/';

Expand Down Expand Up @@ -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<string, string|int|null>
*/
public $file = [
'storePath' => WRITEPATH . 'cache/',
'mode' => 0640,
];

/**
* -------------------------------------------------------------------------
* Memcached settings
Expand Down
32 changes: 26 additions & 6 deletions system/Cache/Handlers/FileHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ class FileHandler implements CacheInterface
*/
protected $path;

/**
* Mode for the stored files.
* Must be chmod-safe (octal).
*
* @var integer
*
* @see https://www.php.net/manual/en/function.chmod.php
*/
protected $mode;

//--------------------------------------------------------------------

/**
Expand All @@ -44,14 +54,24 @@ class FileHandler implements CacheInterface
*/
public function __construct(Cache $config)
{
$path = ! empty($config->storePath) ? $config->storePath : WRITEPATH . 'cache';
if (! is_really_writable($path))
if (! property_exists($config, 'file'))
{
$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($path);
throw CacheException::forUnableToWrite($this->path);
}

$this->prefix = $config->prefix ?: '';
$this->path = rtrim($path, '/') . '/';
$this->mode = $config->file['mode'] ?? 0640;
$this->prefix = (string) $config->prefix;
}

//--------------------------------------------------------------------
Expand Down Expand Up @@ -105,7 +125,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;
}
Expand Down
2 changes: 1 addition & 1 deletion system/Cache/Handlers/MemcachedHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
2 changes: 1 addition & 1 deletion system/Cache/Handlers/PredisHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
2 changes: 1 addition & 1 deletion system/Cache/Handlers/RedisHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
2 changes: 1 addition & 1 deletion system/Cache/Handlers/WincacheHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class WincacheHandler implements CacheInterface
*/
public function __construct(Cache $config)
{
$this->prefix = $config->prefix ?: '';
$this->prefix = (string) $config->prefix;
}

//--------------------------------------------------------------------
Expand Down
71 changes: 53 additions & 18 deletions tests/system/Cache/Handlers/FileHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,18 @@ protected function setUp(): void
{
parent::setUp();

//Initialize path
$this->config = new \Config\Cache();
$this->config->storePath .= self::$directory;
if (! function_exists('octal_permissions'))
{
helper('filesystem');
}

// 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);
Expand All @@ -41,20 +46,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']);
}
}

Expand All @@ -67,15 +72,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();
Expand All @@ -98,7 +103,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'));
}

Expand Down Expand Up @@ -173,6 +178,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 = octal_permissions(fileperms($file));

$this->assertEquals($string, $mode);
}

public function modeProvider()
{
return [
[0640, '640'],
[0600, '600'],
[0660, '660'],
[0777, '777'],
];
}

//--------------------------------------------------------------------

public function testFileHandler()
Expand Down Expand Up @@ -200,8 +235,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);
}
Expand Down
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.0.5.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down Expand Up @@ -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']``
9 changes: 5 additions & 4 deletions user_guide_src/source/libraries/caching.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ 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.

**$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 to determine how it should save the cache files.

**$memcached**

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2a0d39f

Please sign in to comment.