From b60ab19b7c727b2c1b43851a2de2a98a9f5429cf Mon Sep 17 00:00:00 2001 From: Stanimir Stoyanov Date: Mon, 11 Jul 2016 15:54:15 +0300 Subject: [PATCH] Fixes #11759 and deletes expired keys --- phalcon/cache/backend/file.zep | 222 +++++++++++++++++++++++++++------ unit-tests/CacheTest.php | 68 ++++++++++ 2 files changed, 253 insertions(+), 37 deletions(-) diff --git a/phalcon/cache/backend/file.zep b/phalcon/cache/backend/file.zep index d0acf6a8235..fd2bc3b1869 100644 --- a/phalcon/cache/backend/file.zep +++ b/phalcon/cache/backend/file.zep @@ -100,9 +100,14 @@ class File extends Backend /** * Returns a cached content */ - public function get(string keyName, int lifetime = null) -> var | null + public function get(string keyName, var lifetime = null) -> var | null { - var prefixedKey, cacheDir, cacheFile, frontend, lastLifetime, ttl, cachedContent, ret, modifiedTime; + var prefixedKey, cacheDir, cacheFile, frontend, lastLifetime, cachedContent, ret; + int createdTime, ttl; + + if lifetime != null && lifetime < 1 { + throw new Exception("The lifetime must be at least 1 second"); + } let prefixedKey = this->_prefix . this->getKey(keyName); let this->_lastKey = prefixedKey; @@ -117,33 +122,49 @@ class File extends Backend let frontend = this->_frontend; + let cachedContent = json_decode(file_get_contents(cacheFile), true); + /** * Take the lifetime from the frontend or read it from the set in start() + * if the cachedContent is not a valid json array */ if !lifetime { let lastLifetime = this->_lastLifetime; - if !lastLifetime { - let ttl = (int) frontend->getLifeTime(); + if this->isValidArray(cachedContent, "lifetime") { + let ttl = (int) cachedContent["lifetime"]; } else { - let ttl = (int) lastLifetime; + let lastLifetime = this->_lastLifetime; + if !lastLifetime { + let ttl = (int) frontend->getLifeTime(); + } else { + let ttl = (int) lastLifetime; + } } } else { let ttl = (int) lifetime; } clearstatcache(true, cacheFile); - let modifiedTime = (int) filemtime(cacheFile); + if !this->isValidArray(cachedContent, "created") { + let createdTime = (int) filemtime(cacheFile); + } else { + let createdTime = (int) cachedContent["created"]; + } /** * Check if the file has expired * The content is only retrieved if the content has not expired */ - if modifiedTime + ttl > time() { + if !(time() - ttl > createdTime) { /** * Use file-get-contents to control that the openbase_dir can't be skipped */ - let cachedContent = file_get_contents(cacheFile); + if !this->isValidArray(cachedContent, "content") { + let cachedContent = file_get_contents(cacheFile); + } else { + let cachedContent = cachedContent["content"]; + } if cachedContent === false { throw new Exception("Cache file ". cacheFile. " could not be opened"); } @@ -157,7 +178,15 @@ class File extends Backend let ret = frontend->afterRetrieve(cachedContent); return ret; } + } else { + /** + * As the content is expired, deleting the cache file + */ + this->delete(keyName); + return null; } + } else { + return null; } return null; @@ -170,9 +199,16 @@ class File extends Backend * @param string content * @param int lifetime */ - public function save(var keyName = null, var content = null, lifetime = null, boolean stopBuffer = true) -> boolean + public function save(var keyName = null, var content = null, var lifetime = null, boolean stopBuffer = true) -> boolean { - var lastKey, frontend, cacheDir, isBuffering, cacheFile, cachedContent, preparedContent, status; + var lastKey, frontend, cacheDir, isBuffering, cacheFile, cachedContent, preparedContent, status, + finalContent, lastLifetime; + + int ttl; + + if lifetime != null && lifetime < 1 { + throw new Exception("The lifetime must be at least 1 second"); + } if keyName === null { let lastKey = this->_lastKey; @@ -205,10 +241,27 @@ class File extends Backend let preparedContent = cachedContent; } + let preparedContent = frontend->beforeStore(cachedContent); + if !lifetime { + let lastLifetime = this->_lastLifetime; + if !lastLifetime { + let ttl = (int) frontend->getLifeTime(); + } else { + let ttl = (int) lastLifetime; + } + } else { + let ttl = (int) lifetime; + } /** * We use file_put_contents to respect open-base-dir directive */ let status = file_put_contents(cacheFile, preparedContent); + if !is_numeric(cachedContent) { + let finalContent = json_encode(["created": time(), "lifetime": ttl, "content": preparedContent]); + } else { + let finalContent = json_encode(["created": time(), "lifetime": ttl, "content": cachedContent]); + } + let status = file_put_contents(cacheFile, finalContent); if status === false { throw new Exception("Cache file ". cacheFile . " could not be written"); @@ -300,7 +353,9 @@ class File extends Backend */ public function exists(var keyName = null, int lifetime = null) -> boolean { - var lastKey, prefix, cacheFile, ttl, modifiedTime; + var lastKey, prefix, cacheFile, cachedContent; + int ttl; + bool cacheFileExists; if !keyName { let lastKey = this->_lastKey; @@ -313,29 +368,57 @@ class File extends Backend let cacheFile = this->_options["cacheDir"] . lastKey; - if file_exists(cacheFile) { + let cacheFileExists = file_exists(cacheFile); + + if cacheFileExists { /** * Check if the file has expired */ + let cachedContent = json_decode(file_get_contents(cacheFile), true); if !lifetime { - let ttl = (int) this->_frontend->getLifeTime(); + if this->isValidArray(cachedContent, "lifetime") { + let ttl = (int) cachedContent["lifetime"]; + } else { + let ttl = (int) this->_frontend->getLifeTime(); + } } else { let ttl = (int) lifetime; } clearstatcache(true, cacheFile); - let modifiedTime = (int) filemtime(cacheFile); - - if modifiedTime + ttl > time() { + if !this->isValidArray(cachedContent, "created") && filemtime(cacheFile) + ttl > time() { return true; + } else { + if (cachedContent["created"] + ttl > time()) { + return true; + } } } } + /** + * If the cache file exists and is expired, delete it + */ + if cacheFileExists { + this->delete(keyName); + } + return false; } + /** + * Check if given variable is array, containing the key $cacheKey + * + * @param array|null cachedContent + * @param string|null cacheKey + * @return bool + */ + private function isValidArray(var cachedContent = null, var cacheKey = null) + { + return (is_array(cachedContent) && array_key_exists(cacheKey, cachedContent)); + } + /** * Increment of a given key, by number $value * @@ -343,8 +426,9 @@ class File extends Backend */ public function increment(var keyName = null, int value = 1) -> int | null { - var prefixedKey, cacheFile, frontend, lifetime, ttl, - cachedContent, result, modifiedTime; + var prefixedKey, cacheFile, frontend, + cachedContent, result, lastLifetime, newValue; + int modifiedTime, ttl; let prefixedKey = this->_prefix . this->getKey(keyName), this->_lastKey = prefixedKey, @@ -354,44 +438,73 @@ class File extends Backend let frontend = this->_frontend; + /** + * Check if the file has expired + */ + let cachedContent = json_decode(file_get_contents(cacheFile), true); + /** * Take the lifetime from the frontend or read it from the set in start() + * if the cachedContent is not a valid array */ - let lifetime = this->_lastLifetime; - if !lifetime { - let ttl = frontend->getLifeTime(); + if this->isValidArray(cachedContent, "lifetime") { + let ttl = (int) cachedContent["lifetime"]; } else { - let ttl = lifetime; + let lastLifetime = this->_lastLifetime; + if !lastLifetime { + let ttl = (int) frontend->getLifeTime(); + } else { + let ttl = (int) lastLifetime; + } } clearstatcache(true, cacheFile); - let modifiedTime = (int) filemtime(cacheFile); + + if !this->isValidArray(cachedContent, "created") { + let modifiedTime = (int) filemtime(cacheFile); + } else { + let modifiedTime = (int) cachedContent["created"]; + } /** * Check if the file has expired * The content is only retrieved if the content has not expired */ - if modifiedTime + ttl > time() { + if !(time() - ttl > modifiedTime) { /** * Use file-get-contents to control that the openbase_dir can't be skipped */ - let cachedContent = file_get_contents(cacheFile); + if !this->isValidArray(cachedContent, "content") { + let cachedContent = file_get_contents(cacheFile); + } else { + let cachedContent = cachedContent["content"]; + } if cachedContent === false { throw new Exception("Cache file " . cacheFile . " could not be opened"); } if is_numeric(cachedContent) { + let newValue = cachedContent + value; + let result = json_encode(["created": time(), "lifetime": ttl, "content": newValue]); - let result = cachedContent + value; if file_put_contents(cacheFile, result) === false { throw new Exception("Cache directory could not be written"); } - return result; + return newValue; + } else { + throw new Exception("The cache value is not numeric, therefore could not be incremented"); } + } else { + /** + * The cache file is expired, so we're removing it + */ + this->delete(keyName); + return null; } + return null; } return null; @@ -404,7 +517,8 @@ class File extends Backend */ public function decrement(var keyName = null, int value = 1) -> int | null { - var prefixedKey, cacheFile, lifetime, ttl, cachedContent, result, modifiedTime; + var prefixedKey, cacheFile, cachedContent, result, lastLifetime, newValue; + int ttl, modifiedTime, lifetime; let prefixedKey = this->_prefix . this->getKey(keyName), this->_lastKey = prefixedKey, @@ -412,44 +526,77 @@ class File extends Backend if file_exists(cacheFile) { + /** + * Check if the file has expired + */ + let cachedContent = json_decode(file_get_contents(cacheFile), true); + /** * Take the lifetime from the frontend or read it from the set in start() + * if the cachedContent is not a valid array */ - let lifetime = this->_lastLifetime; if !lifetime { - let ttl = this->_frontend->getLifeTime(); + if this->isValidArray(cachedContent, "lifetime") { + let ttl = (int) cachedContent["lifetime"]; + } else { + let lastLifetime = this->_lastLifetime; + if !lastLifetime { + let ttl = (int) this->_frontend->getLifeTime(); + } else { + let ttl = (int) lastLifetime; + } + } + } else { + let ttl = (int) lifetime; + } + + if !this->isValidArray(cachedContent, "created") { + let modifiedTime = (int) filemtime(cacheFile); + let ttl = (int) lifetime; } else { - let ttl = lifetime; + let modifiedTime = (int) cachedContent["created"]; } clearstatcache(true, cacheFile); - let modifiedTime = (int) filemtime(cacheFile); /** * Check if the file has expired * The content is only retrieved if the content has not expired */ - if modifiedTime + ttl > time() { + if !(time() - ttl > modifiedTime) { /** * Use file-get-contents to control that the openbase_dir can't be skipped */ - let cachedContent = file_get_contents(cacheFile); + if !this->isValidArray(cachedContent, "content") { + let cachedContent = file_get_contents(cacheFile); + } else { + let cachedContent = cachedContent["content"]; + } if cachedContent === false { throw new Exception("Cache file " . cacheFile . " could not be opened"); } if is_numeric(cachedContent) { - - let result = cachedContent - value; + let newValue = cachedContent - value; + let result = json_encode(["created": time(), "lifetime": ttl, "content": newValue]); if file_put_contents(cacheFile, result) === false { throw new Exception("Cache directory can't be written"); } - return result; + return newValue; + } else { + throw new Exception("The cache value is not numeric, therefore could not decrement it"); } + } else { + /** + * The cache file is expired, so we're removing it + */ + this->delete(keyName); + return null; } + return null; } return null; @@ -507,3 +654,4 @@ class File extends Backend return this; } } + diff --git a/unit-tests/CacheTest.php b/unit-tests/CacheTest.php index 4600480ce38..bc902b593cb 100644 --- a/unit-tests/CacheTest.php +++ b/unit-tests/CacheTest.php @@ -70,6 +70,74 @@ public function testDataFileCacheDecrement() $this->assertEquals(95, $cache->decrement('foo', 4)); } + /** + * @expectedException \Exception + */ + public function testDataFileCacheSaveNegativeLifetime() + { + $frontCache = new Phalcon\Cache\Frontend\Data(); + + $cache = new Phalcon\Cache\Backend\File($frontCache, array( + 'cacheDir' => 'unit-tests/cache/' + )); + $cache->save('foo', "1", -1); + } + + /** + * @expectedException \Exception + */ + public function testDataFileCacheGetNegativeLifetime() + { + $frontCache = new Phalcon\Cache\Frontend\Data(); + + $cache = new Phalcon\Cache\Backend\File($frontCache, array( + 'cacheDir' => 'unit-tests/cache/' + )); + $cache->save('foo', "1"); + $cache->get('foo', -1); + } + + public function testDataFileCacheGetNonexistentCache() + { + $frontCache = new Phalcon\Cache\Frontend\Data(); + + $cache = new Phalcon\Cache\Backend\File($frontCache, array( + 'cacheDir' => 'unit-tests/cache/' + )); + if ($cache->exists('foo')) { + $cache->delete('foo'); + } + $this->assertEquals($cache->get('foo'), null); + } + + /** + * @expectedException \Exception + */ + public function testDataFileCacheIncrementNonNumeric() + { + $frontCache = new Phalcon\Cache\Frontend\Data(); + + $cache = new Phalcon\Cache\Backend\File($frontCache, array( + 'cacheDir' => 'unit-tests/cache/' + )); + $cache->save('foo', "a"); + $cache->increment('foo', 1); + } + + /** + * @expectedException \Exception + */ + public function testDataFileCacheDecrementNonNumeric() + { + $frontCache = new Phalcon\Cache\Frontend\Data(); + + $cache = new Phalcon\Cache\Backend\File($frontCache, array( + 'cacheDir' => 'unit-tests/cache/' + )); + $cache->save('foo', "a"); + $cache->decrement('foo', 1); + } + /** * @expectedException \Exception */