From 1a7d0799c00de0e2944e0e15e29c64e64d89e4a3 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 7 Jun 2021 06:53:15 +0200 Subject: [PATCH 01/30] scaffolding interface for AbstractData key/value storage, folding Persistance\DataStore into Data\Filesystem --- lib/Data/AbstractData.php | 35 ++++++- lib/Data/Filesystem.php | 170 ++++++++++++++++++++++++++++++---- lib/Persistence/DataStore.php | 96 ------------------- 3 files changed, 185 insertions(+), 116 deletions(-) delete mode 100644 lib/Persistence/DataStore.php diff --git a/lib/Data/AbstractData.php b/lib/Data/AbstractData.php index 077864e..8a52d1d 100644 --- a/lib/Data/AbstractData.php +++ b/lib/Data/AbstractData.php @@ -20,7 +20,7 @@ namespace PrivateBin\Data; abstract class AbstractData { /** - * singleton instance + * Singleton instance * * @access protected * @static @@ -28,8 +28,14 @@ abstract class AbstractData */ protected static $_instance = null; + protected static $_namespaces = array( + 'purge_limiter', + 'salt', + 'traffic_limiter', + ); + /** - * enforce singleton, disable constructor + * Enforce singleton, disable constructor * * Instantiate using {@link getInstance()}, privatebin is a singleton object. * @@ -40,7 +46,7 @@ abstract class AbstractData } /** - * enforce singleton, disable cloning + * Enforce singleton, disable cloning * * Instantiate using {@link getInstance()}, privatebin is a singleton object. * @@ -51,7 +57,7 @@ abstract class AbstractData } /** - * get instance of singleton + * Get instance of singleton * * @access public * @static @@ -130,6 +136,27 @@ abstract class AbstractData */ abstract public function existsComment($pasteid, $parentid, $commentid); + /** + * Save a value. + * + * @access public + * @param string $value + * @param string $namespace + * @param string $key + * @return bool + */ + abstract public function setValue($value, $namespace, $key = ''); + + /** + * Load a value. + * + * @access public + * @param string $namespace + * @param string $key + * @return string + */ + abstract public function getValue($namespace, $key = ''); + /** * Returns up to batch size number of paste ids that have expired * diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 96ee691..fee61c1 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -12,7 +12,8 @@ namespace PrivateBin\Data; -use PrivateBin\Persistence\DataStore; +use Exception; +use PrivateBin\Json; /** * Filesystem @@ -21,6 +22,22 @@ use PrivateBin\Persistence\DataStore; */ class Filesystem extends AbstractData { + /** + * first line in file, to protect its contents + * + * @const string + */ + const PROTECTION_LINE = 'read())) { if (substr($filename, -4) !== '.php' && strlen($filename) >= 16) { $commentFilename = $discdir . $filename . '.php'; - DataStore::prependRename($discdir . $filename, $commentFilename); + self::_prependRename($discdir . $filename, $commentFilename); } } $dir->close(); @@ -165,7 +182,7 @@ class Filesystem extends AbstractData if (!is_dir($storagedir)) { mkdir($storagedir, 0700, true); } - return DataStore::store($file, $comment); + return self::_store($file, $comment); } /** @@ -187,7 +204,7 @@ class Filesystem extends AbstractData // - commentid is the comment identifier itself. // - parentid is the comment this comment replies to (It can be pasteid) if (is_file($discdir . $filename)) { - $comment = DataStore::get($discdir . $filename); + $comment = self::_get($discdir . $filename); $items = explode('.', $filename); // Add some meta information not contained in file. $comment['id'] = $items[1]; @@ -223,6 +240,33 @@ class Filesystem extends AbstractData ); } + /** + * Save a value. + * + * @access public + * @param string $value + * @param string $namespace + * @param string $key + * @return bool + */ + public function setValue($value, $namespace, $key = '') + { + + } + + /** + * Load a value. + * + * @access public + * @param string $namespace + * @param string $key + * @return string + */ + public function getValue($namespace, $key = '') + { + + } + /** * Returns up to batch size number of paste ids that have expired * @@ -233,9 +277,8 @@ class Filesystem extends AbstractData protected function _getExpiredPastes($batchsize) { $pastes = array(); - $mainpath = DataStore::getPath(); $firstLevel = array_filter( - scandir($mainpath), + scandir(self::$_path), 'self::_isFirstLevelDir' ); if (count($firstLevel) > 0) { @@ -243,7 +286,7 @@ class Filesystem extends AbstractData for ($i = 0, $max = $batchsize * 10; $i < $max; ++$i) { $firstKey = array_rand($firstLevel); $secondLevel = array_filter( - scandir($mainpath . DIRECTORY_SEPARATOR . $firstLevel[$firstKey]), + scandir(self::$_path . DIRECTORY_SEPARATOR . $firstLevel[$firstKey]), 'self::_isSecondLevelDir' ); @@ -254,7 +297,7 @@ class Filesystem extends AbstractData } $secondKey = array_rand($secondLevel); - $path = $mainpath . DIRECTORY_SEPARATOR . + $path = self::$_path . DIRECTORY_SEPARATOR . $firstLevel[$firstKey] . DIRECTORY_SEPARATOR . $secondLevel[$secondKey]; if (!is_dir($path)) { @@ -314,10 +357,9 @@ class Filesystem extends AbstractData */ private static function _dataid2path($dataid) { - return DataStore::getPath( + return self::$_path . DIRECTORY_SEPARATOR . substr($dataid, 0, 2) . DIRECTORY_SEPARATOR . - substr($dataid, 2, 2) . DIRECTORY_SEPARATOR - ); + substr($dataid, 2, 2) . DIRECTORY_SEPARATOR; } /** @@ -347,7 +389,7 @@ class Filesystem extends AbstractData private static function _isFirstLevelDir($element) { return self::_isSecondLevelDir($element) && - is_dir(DataStore::getPath($element)); + is_dir(self::$_path . DIRECTORY_SEPARATOR . $element); } /** @@ -362,4 +404,100 @@ class Filesystem extends AbstractData { return (bool) preg_match('/^[a-f0-9]{2}$/', $element); } + + /** + * store the data + * + * @access public + * @static + * @param string $filename + * @param array $data + * @return bool + */ + private static function _store($filename, $data) + { + if (strpos($filename, self::$_path) === 0) { + $filename = substr($filename, strlen(self::$_path)); + } + + // Create storage directory if it does not exist. + if (!is_dir(self::$_path)) { + if (!@mkdir(self::$_path, 0700)) { + throw new Exception('unable to create directory ' . self::$_path, 10); + } + } + $file = self::$_path . DIRECTORY_SEPARATOR . '.htaccess'; + if (!is_file($file)) { + $writtenBytes = 0; + if ($fileCreated = @touch($file)) { + $writtenBytes = @file_put_contents( + $file, + 'Require all denied' . PHP_EOL, + LOCK_EX + ); + } + if ($fileCreated === false || $writtenBytes === false || $writtenBytes < 19) { + return false; + } + } + + try { + $data = self::PROTECTION_LINE . PHP_EOL . Json::encode($data); + } catch (Exception $e) { + return false; + } + $file = self::$_path . DIRECTORY_SEPARATOR . $filename; + $fileCreated = true; + $writtenBytes = 0; + if (!is_file($file)) { + $fileCreated = @touch($file); + } + if ($fileCreated) { + $writtenBytes = @file_put_contents($file, $data, LOCK_EX); + } + if ($fileCreated === false || $writtenBytes === false || $writtenBytes < strlen($data)) { + return false; + } + @chmod($file, 0640); // protect file access + return true; + } + + /** + * get the data + * + * @access public + * @static + * @param string $filename + * @return array|false $data + */ + private static function _get($filename) + { + return Json::decode( + substr( + file_get_contents($filename), + strlen(self::PROTECTION_LINE . PHP_EOL) + ) + ); + } + + /** + * rename a file, prepending the protection line at the beginning + * + * @access public + * @static + * @param string $srcFile + * @param string $destFile + * @return void + */ + private static function _prependRename($srcFile, $destFile) + { + // don't overwrite already converted file + if (!is_readable($destFile)) { + $handle = fopen($srcFile, 'r', false, stream_context_create()); + file_put_contents($destFile, self::PROTECTION_LINE . PHP_EOL); + file_put_contents($destFile, $handle, FILE_APPEND); + fclose($handle); + } + unlink($srcFile); + } } diff --git a/lib/Persistence/DataStore.php b/lib/Persistence/DataStore.php deleted file mode 100644 index d17e5fa..0000000 --- a/lib/Persistence/DataStore.php +++ /dev/null @@ -1,96 +0,0 @@ - Date: Mon, 7 Jun 2021 07:02:47 +0200 Subject: [PATCH 02/30] conclude scaffolding of AbstractData key/value storage, missing implementation --- lib/Data/AbstractData.php | 6 ----- lib/Data/Database.php | 40 +++++++++++++++++++++++++++++++++ lib/Data/Filesystem.php | 15 ++++++++++++- lib/Data/GoogleCloudStorage.php | 40 +++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 7 deletions(-) diff --git a/lib/Data/AbstractData.php b/lib/Data/AbstractData.php index 8a52d1d..0508bc0 100644 --- a/lib/Data/AbstractData.php +++ b/lib/Data/AbstractData.php @@ -28,12 +28,6 @@ abstract class AbstractData */ protected static $_instance = null; - protected static $_namespaces = array( - 'purge_limiter', - 'salt', - 'traffic_limiter', - ); - /** * Enforce singleton, disable constructor * diff --git a/lib/Data/Database.php b/lib/Data/Database.php index 607013b..5a0f369 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -423,6 +423,46 @@ class Database extends AbstractData ); } + /** + * Save a value. + * + * @access public + * @param string $value + * @param string $namespace + * @param string $key + * @return bool + */ + public function setValue($value, $namespace, $key = '') + { + switch ($namespace) { + case 'purge_limiter': + ; + break; + case 'salt': + ; + break; + case 'traffic_limiter': + ; + break; + default: + return false; + break; + } + } + + /** + * Load a value. + * + * @access public + * @param string $namespace + * @param string $key + * @return string + */ + public function getValue($namespace, $key = '') + { + + } + /** * Returns up to batch size number of paste ids that have expired * diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index fee61c1..76c2600 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -251,7 +251,20 @@ class Filesystem extends AbstractData */ public function setValue($value, $namespace, $key = '') { - + switch ($namespace) { + case 'purge_limiter': + ; + break; + case 'salt': + ; + break; + case 'traffic_limiter': + ; + break; + default: + return false; + break; + } } /** diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index 1a1d8bf..81fcce6 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -217,6 +217,46 @@ class GoogleCloudStorage extends AbstractData return $o->exists(); } + /** + * Save a value. + * + * @access public + * @param string $value + * @param string $namespace + * @param string $key + * @return bool + */ + public function setValue($value, $namespace, $key = '') + { + switch ($namespace) { + case 'purge_limiter': + ; + break; + case 'salt': + ; + break; + case 'traffic_limiter': + ; + break; + default: + return false; + break; + } + } + + /** + * Load a value. + * + * @access public + * @param string $namespace + * @param string $key + * @return string + */ + public function getValue($namespace, $key = '') + { + + } + /** * @inheritDoc */ From 55efc858b5d523c65581b999a2ab0def7e540c9b Mon Sep 17 00:00:00 2001 From: Mark van Holsteijn Date: Mon, 7 Jun 2021 09:11:24 +0200 Subject: [PATCH 03/30] simplest implementation of kv support on gcs --- lib/Data/GoogleCloudStorage.php | 59 ++++++++++++++++------------- tst/Data/GoogleCloudStorageTest.php | 25 ++++++++++-- 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index 81fcce6..ca1b340 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -218,43 +218,48 @@ class GoogleCloudStorage extends AbstractData } /** - * Save a value. - * - * @access public - * @param string $value - * @param string $namespace - * @param string $key - * @return bool + * This is the simplest thing that could possibly work. + * will be to tested for runtime performance. + * @inheritDoc */ public function setValue($value, $namespace, $key = '') { - switch ($namespace) { - case 'purge_limiter': - ; - break; - case 'salt': - ; - break; - case 'traffic_limiter': - ; - break; - default: - return false; - break; + $key = 'config/' . $namespace . '/' . $key; + $data = Json::encode($value); + + try { + $this->_bucket->upload($data, array( + 'name' => $key, + 'chunkSize' => 262144, + 'predefinedAcl' => 'private', + 'metadata' => array( + 'content-type' => 'application/json', + 'metadata' => array('namespace' => $namespace), + ), + )); + } catch (Exception $e) { + error_log('failed to set key ' . $key . ' to ' . $this->_bucket->name() . ', ' . + trim(preg_replace('/\s\s+/', ' ', $e->getMessage()))); + return false; } + return true; } /** - * Load a value. - * - * @access public - * @param string $namespace - * @param string $key - * @return string + * This is the simplest thing that could possibly work. + * will be to tested for runtime performance. + * @inheritDoc */ public function getValue($namespace, $key = '') { - + $key = 'config/' . $namespace . '/' . $key; + try { + $o = $this->_bucket->object($key); + $data = $o->downloadAsString(); + return Json::decode($data); + } catch (NotFoundException $e) { + return false; + } } /** diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php index 6905f04..30a7d5e 100644 --- a/tst/Data/GoogleCloudStorageTest.php +++ b/tst/Data/GoogleCloudStorageTest.php @@ -31,9 +31,6 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase public function setUp() { - // do not report E_NOTICE as fsouza/fake-gcs-server does not return a `generation` value in the response - // which the Google Cloud Storage PHP library expects. - error_reporting(E_ERROR | E_WARNING | E_PARSE); ini_set('error_log', stream_get_meta_data(tmpfile())['uri']); $this->_model = GoogleCloudStorage::getInstance(array( 'bucket' => self::$_bucket->name(), @@ -138,6 +135,28 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'unable to store broken comment'); $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does still not exist'); } + + /** + * @throws Exception + */ + public function testKeyValueStore() + { + $salt = bin2hex(random_bytes(256)); + $this->_model->setValue($salt, 'salt', 'master'); + $storedSalt = $this->_model->getValue('salt', 'master'); + $this->assertEquals($salt, $storedSalt); + + $client = hash_hmac('sha512', '127.0.0.1', $salt); + $expire = time(); + $this->_model->setValue($expire, 'traffic_limiter', $client); + $storedExpired = $this->_model->getValue('traffic_limiter', $client); + $this->assertEquals($expire, $storedExpired); + + $purgeAt = $expire + (15 * 60); + $this->_model->setValue($purgeAt, 'purge_limiter', 'at'); + $storedPurgedAt = $this->_model->getValue('purge_limiter', 'at'); + $this->assertEquals($purgeAt, $storedPurgedAt); + } } /** From ae486d651be3a622dd1307bc0c681ecf5a894058 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 7 Jun 2021 21:53:42 +0200 Subject: [PATCH 04/30] folding Persistance\PurgeLimiter into Data\Filesystem --- lib/Data/Filesystem.php | 99 ++++++++++++++++--------- lib/Model.php | 1 + lib/Persistence/AbstractPersistence.php | 22 ++++++ lib/Persistence/PurgeLimiter.php | 14 +--- tst/Persistence/PurgeLimiterTest.php | 4 + 5 files changed, 94 insertions(+), 46 deletions(-) diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 76c2600..f24691b 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -253,7 +253,10 @@ class Filesystem extends AbstractData { switch ($namespace) { case 'purge_limiter': - ; + return self::_storeString( + self::$_path . DIRECTORY_SEPARATOR . 'purge_limiter.php', + '_conf); + PurgeLimiter::setStore($this->_getStore()); if (PurgeLimiter::canPurge()) { $this->_getStore()->purge($this->_conf->getKey('batchsize', 'purge')); } diff --git a/lib/Persistence/AbstractPersistence.php b/lib/Persistence/AbstractPersistence.php index 489836d..73f0ab2 100644 --- a/lib/Persistence/AbstractPersistence.php +++ b/lib/Persistence/AbstractPersistence.php @@ -13,6 +13,7 @@ namespace PrivateBin\Persistence; use Exception; +use PrivateBin\Data\AbstractData; /** * AbstractPersistence @@ -30,6 +31,15 @@ abstract class AbstractPersistence */ private static $_path = 'data'; + /** + * data storage to use to persist something + * + * @access private + * @static + * @var AbstractData + */ + protected static $_store; + /** * set the path * @@ -42,6 +52,18 @@ abstract class AbstractPersistence self::$_path = $path; } + /** + * set the path + * + * @access public + * @static + * @param AbstractData $store + */ + public static function setStore(AbstractData $store) + { + self::$_store = $store; + } + /** * get the path * diff --git a/lib/Persistence/PurgeLimiter.php b/lib/Persistence/PurgeLimiter.php index ea07e32..19b83e2 100644 --- a/lib/Persistence/PurgeLimiter.php +++ b/lib/Persistence/PurgeLimiter.php @@ -71,17 +71,11 @@ class PurgeLimiter extends AbstractPersistence } $now = time(); - $file = 'purge_limiter.php'; - if (self::_exists($file)) { - require self::getPath($file); - $pl = $GLOBALS['purge_limiter']; - if ($pl + self::$_limit >= $now) { - return false; - } + $pl = (int) self::$_store->getValue('purge_limiter'); + if ($pl + self::$_limit >= $now) { + return false; } - - $content = 'setValue((string) $now, 'purge_limiter'); return true; } } diff --git a/tst/Persistence/PurgeLimiterTest.php b/tst/Persistence/PurgeLimiterTest.php index 391a840..e8cedc0 100644 --- a/tst/Persistence/PurgeLimiterTest.php +++ b/tst/Persistence/PurgeLimiterTest.php @@ -1,5 +1,6 @@ _path); } PurgeLimiter::setPath($this->_path); + PurgeLimiter::setStore( + Filesystem::getInstance(array('dir' => $this->_path)) + ); } public function tearDown() From 3429d293d3ba0426924b5da488147e4f3b223c9f Mon Sep 17 00:00:00 2001 From: El RIDO Date: Tue, 8 Jun 2021 06:37:27 +0200 Subject: [PATCH 05/30] remove configurable dir for traffic & purge limiters --- CHANGELOG.md | 1 + cfg/conf.sample.php | 6 ------ lib/Configuration.php | 2 -- lib/Controller.php | 1 - lib/Persistence/PurgeLimiter.php | 3 +-- lib/Persistence/TrafficLimiter.php | 1 - tst/ConfigurationTest.php | 2 -- tst/ConfigurationTestGenerator.php | 4 ---- tst/ControllerTest.php | 3 --- tst/JsonApiTest.php | 2 -- tst/Persistence/PurgeLimiterTest.php | 1 - 11 files changed, 2 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c4321a..c546bd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ * CHANGED: Language selection cookie only transmitted over HTTPS (#472) * CHANGED: Upgrading libraries to: random_compat 2.0.20 * CHANGED: Removed automatic `.ini` configuration file migration (#808) + * CHANGED: Removed configurable `dir` for `traffic` & `purge` limiters (#419) * **1.3.5 (2021-04-05)** * ADDED: Translation for Hebrew, Lithuanian, Indonesian and Catalan * ADDED: Make the project info configurable (#681) diff --git a/cfg/conf.sample.php b/cfg/conf.sample.php index a4b7f6b..d362f3f 100644 --- a/cfg/conf.sample.php +++ b/cfg/conf.sample.php @@ -143,9 +143,6 @@ limit = 10 ; set the HTTP header containing the visitors IP address, i.e. X_FORWARDED_FOR ; header = "X_FORWARDED_FOR" -; directory to store the traffic limits in -dir = PATH "data" - [purge] ; minimum time limit between two purgings of expired pastes, it is only ; triggered when pastes are created @@ -157,9 +154,6 @@ limit = 300 ; site batchsize = 10 -; directory to store the purge limit in -dir = PATH "data" - [model] ; name of data model class to load and directory for storage ; the default model "Filesystem" stores everything in the filesystem diff --git a/lib/Configuration.php b/lib/Configuration.php index 1185440..7c4eb10 100644 --- a/lib/Configuration.php +++ b/lib/Configuration.php @@ -80,13 +80,11 @@ class Configuration 'traffic' => array( 'limit' => 10, 'header' => null, - 'dir' => 'data', 'exemptedIp' => null, ), 'purge' => array( 'limit' => 300, 'batchsize' => 10, - 'dir' => 'data', ), 'model' => array( 'class' => 'Filesystem', diff --git a/lib/Controller.php b/lib/Controller.php index 2df522a..72bd5b2 100644 --- a/lib/Controller.php +++ b/lib/Controller.php @@ -162,7 +162,6 @@ class Controller $this->_model = new Model($this->_conf); $this->_request = new Request; $this->_urlBase = $this->_request->getRequestUri(); - ServerSalt::setPath($this->_conf->getKey('dir', 'traffic')); // set default language $lang = $this->_conf->getKey('languagedefault'); diff --git a/lib/Persistence/PurgeLimiter.php b/lib/Persistence/PurgeLimiter.php index 19b83e2..6b60817 100644 --- a/lib/Persistence/PurgeLimiter.php +++ b/lib/Persistence/PurgeLimiter.php @@ -52,7 +52,6 @@ class PurgeLimiter extends AbstractPersistence public static function setConfiguration(Configuration $conf) { self::setLimit($conf->getKey('limit', 'purge')); - self::setPath($conf->getKey('dir', 'purge')); } /** @@ -71,7 +70,7 @@ class PurgeLimiter extends AbstractPersistence } $now = time(); - $pl = (int) self::$_store->getValue('purge_limiter'); + $pl = (int) self::$_store->getValue('purge_limiter'); if ($pl + self::$_limit >= $now) { return false; } diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index be76b8c..299c7a6 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -85,7 +85,6 @@ class TrafficLimiter extends AbstractPersistence public static function setConfiguration(Configuration $conf) { self::setLimit($conf->getKey('limit', 'traffic')); - self::setPath($conf->getKey('dir', 'traffic')); self::setExemptedIp($conf->getKey('exemptedIp', 'traffic')); if (($option = $conf->getKey('header', 'traffic')) !== null) { diff --git a/tst/ConfigurationTest.php b/tst/ConfigurationTest.php index 246618c..312b799 100644 --- a/tst/ConfigurationTest.php +++ b/tst/ConfigurationTest.php @@ -17,8 +17,6 @@ class ConfigurationTest extends PHPUnit_Framework_TestCase $this->_minimalConfig = '[main]' . PHP_EOL . '[model]' . PHP_EOL . '[model_options]'; $this->_options = Configuration::getDefaults(); $this->_options['model_options']['dir'] = PATH . $this->_options['model_options']['dir']; - $this->_options['traffic']['dir'] = PATH . $this->_options['traffic']['dir']; - $this->_options['purge']['dir'] = PATH . $this->_options['purge']['dir']; $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_cfg'; if (!is_dir($this->_path)) { mkdir($this->_path); diff --git a/tst/ConfigurationTestGenerator.php b/tst/ConfigurationTestGenerator.php index 284fa5f..945fc47 100755 --- a/tst/ConfigurationTestGenerator.php +++ b/tst/ConfigurationTestGenerator.php @@ -428,8 +428,6 @@ class ConfigurationCombinationsTest extends PHPUnit_Framework_TestCase Helper::confBackup(); $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_data'; $this->_model = Filesystem::getInstance(array('dir' => $this->_path)); - ServerSalt::setPath($this->_path); - TrafficLimiter::setPath($this->_path); $this->reset(); } @@ -449,8 +447,6 @@ class ConfigurationCombinationsTest extends PHPUnit_Framework_TestCase if ($this->_model->exists(Helper::getPasteId())) $this->_model->delete(Helper::getPasteId()); $configuration['model_options']['dir'] = $this->_path; - $configuration['traffic']['dir'] = $this->_path; - $configuration['purge']['dir'] = $this->_path; Helper::createIniFile(CONF, $configuration); } diff --git a/tst/ControllerTest.php b/tst/ControllerTest.php index b00f2ce..6e7ec4c 100644 --- a/tst/ControllerTest.php +++ b/tst/ControllerTest.php @@ -37,11 +37,8 @@ class ControllerTest extends PHPUnit_Framework_TestCase $this->_data->delete(Helper::getPasteId()); } $options = parse_ini_file(CONF_SAMPLE, true); - $options['purge']['dir'] = $this->_path; - $options['traffic']['dir'] = $this->_path; $options['model_options']['dir'] = $this->_path; Helper::createIniFile(CONF, $options); - ServerSalt::setPath($this->_path); } /** diff --git a/tst/JsonApiTest.php b/tst/JsonApiTest.php index 9655e60..17b699f 100644 --- a/tst/JsonApiTest.php +++ b/tst/JsonApiTest.php @@ -25,8 +25,6 @@ class JsonApiTest extends PHPUnit_Framework_TestCase $this->_model->delete(Helper::getPasteId()); } $options = parse_ini_file(CONF_SAMPLE, true); - $options['purge']['dir'] = $this->_path; - $options['traffic']['dir'] = $this->_path; $options['model_options']['dir'] = $this->_path; Helper::confBackup(); Helper::createIniFile(CONF, $options); diff --git a/tst/Persistence/PurgeLimiterTest.php b/tst/Persistence/PurgeLimiterTest.php index e8cedc0..adb96ff 100644 --- a/tst/Persistence/PurgeLimiterTest.php +++ b/tst/Persistence/PurgeLimiterTest.php @@ -14,7 +14,6 @@ class PurgeLimiterTest extends PHPUnit_Framework_TestCase if (!is_dir($this->_path)) { mkdir($this->_path); } - PurgeLimiter::setPath($this->_path); PurgeLimiter::setStore( Filesystem::getInstance(array('dir' => $this->_path)) ); From b5a6ce323e04318fba7a45a089bd6c872cd5b89c Mon Sep 17 00:00:00 2001 From: El RIDO Date: Tue, 8 Jun 2021 07:49:22 +0200 Subject: [PATCH 06/30] folding Persistance\TrafficLimiter into Data\Filesystem --- lib/Controller.php | 1 + lib/Data/AbstractData.php | 10 +++++++ lib/Data/Database.php | 17 +++++++++++ lib/Data/Filesystem.php | 45 ++++++++++++++++++++++++++++-- lib/Data/GoogleCloudStorage.php | 17 +++++++++++ lib/Model.php | 8 +++--- lib/Persistence/TrafficLimiter.php | 31 +++++--------------- tst/ControllerTest.php | 1 + 8 files changed, 100 insertions(+), 30 deletions(-) diff --git a/lib/Controller.php b/lib/Controller.php index 72bd5b2..095bbeb 100644 --- a/lib/Controller.php +++ b/lib/Controller.php @@ -198,6 +198,7 @@ class Controller try { // Ensure last paste from visitors IP address was more than configured amount of seconds ago. TrafficLimiter::setConfiguration($this->_conf); + TrafficLimiter::setStore($this->_model->getStore()); if (!TrafficLimiter::canPass()) { $this->_return_message( 1, I18n::_( diff --git a/lib/Data/AbstractData.php b/lib/Data/AbstractData.php index 0508bc0..3de4bab 100644 --- a/lib/Data/AbstractData.php +++ b/lib/Data/AbstractData.php @@ -130,6 +130,16 @@ abstract class AbstractData */ abstract public function existsComment($pasteid, $parentid, $commentid); + /** + * Purge outdated entries. + * + * @access public + * @param string $namespace + * @param int $time + * @return void + */ + abstract public function purgeValues($namespace, $time); + /** * Save a value. * diff --git a/lib/Data/Database.php b/lib/Data/Database.php index 5a0f369..2a1bbcb 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -423,6 +423,23 @@ class Database extends AbstractData ); } + /** + * Purge outdated entries. + * + * @access public + * @param string $namespace + * @param int $time + * @return void + */ + public function purgeValues($namespace, $time) + { + switch ($namespace) { + case 'traffic_limiter': + ; + break; + } + } + /** * Save a value. * diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index f24691b..3a481d9 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -38,6 +38,15 @@ class Filesystem extends AbstractData */ private static $_path = 'data'; + /** + * cache for the traffic limiter + * + * @access private + * @static + * @var array + */ + private static $_traffic_limiter_cache = array(); + /** * get instance of singleton * @@ -240,6 +249,27 @@ class Filesystem extends AbstractData ); } + /** + * Purge outdated entries. + * + * @access public + * @param string $namespace + * @param int $time + * @return void + */ + public function purgeValues($namespace, $time) + { + switch ($namespace) { + case 'traffic_limiter': + foreach (self::$_traffic_limiter_cache as $key => $last_access) { + if ($last_access <= $time) { + unset(self::$_traffic_limiter_cache[$key]); + } + } + break; + } + } + /** * Save a value. * @@ -262,7 +292,11 @@ class Filesystem extends AbstractData ; break; case 'traffic_limiter': - ; + self::$_traffic_limiter_cache[$key] = $value; + return self::_storeString( + self::$_path . DIRECTORY_SEPARATOR . 'traffic_limiter.php', + 'exists(); } + /** + * Purge outdated entries. + * + * @access public + * @param string $namespace + * @param int $time + * @return void + */ + public function purgeValues($namespace, $time) + { + switch ($namespace) { + case 'traffic_limiter': + ; + break; + } + } + /** * This is the simplest thing that could possibly work. * will be to tested for runtime performance. diff --git a/lib/Model.php b/lib/Model.php index 1a23653..8aebd79 100644 --- a/lib/Model.php +++ b/lib/Model.php @@ -54,7 +54,7 @@ class Model */ public function getPaste($pasteId = null) { - $paste = new Paste($this->_conf, $this->_getStore()); + $paste = new Paste($this->_conf, $this->getStore()); if ($pasteId !== null) { $paste->setId($pasteId); } @@ -67,9 +67,9 @@ class Model public function purge() { PurgeLimiter::setConfiguration($this->_conf); - PurgeLimiter::setStore($this->_getStore()); + PurgeLimiter::setStore($this->getStore()); if (PurgeLimiter::canPurge()) { - $this->_getStore()->purge($this->_conf->getKey('batchsize', 'purge')); + $this->getStore()->purge($this->_conf->getKey('batchsize', 'purge')); } } @@ -78,7 +78,7 @@ class Model * * @return Data\AbstractData */ - private function _getStore() + public function getStore() { if ($this->_store === null) { $this->_store = forward_static_call( diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index 299c7a6..f6bc07f 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -169,35 +169,18 @@ class TrafficLimiter extends AbstractPersistence } } - $file = 'traffic_limiter.php'; - if (self::_exists($file)) { - require self::getPath($file); - $tl = $GLOBALS['traffic_limiter']; - } else { - $tl = array(); - } - - // purge file of expired hashes to keep it small - $now = time(); - foreach ($tl as $key => $time) { - if ($time + self::$_limit < $now) { - unset($tl[$key]); - } - } - // this hash is used as an array key, hence a shorter algo is used $hash = self::getHash('sha256'); - if (array_key_exists($hash, $tl) && ($tl[$hash] + self::$_limit >= $now)) { + $now = time(); + $tl = self::$_store->getValue('traffic_limiter', $hash); + self::$_store->purgeValues('traffic_limiter', $now - self::$_limit); + if ($tl > 0 && ($tl + self::$_limit >= $now)) { $result = false; } else { - $tl[$hash] = time(); - $result = true; + $tl = time(); + $result = true; } - self::_store( - $file, - 'setValue((string) $tl, 'traffic_limiter'); return $result; } } diff --git a/tst/ControllerTest.php b/tst/ControllerTest.php index 6e7ec4c..92683ce 100644 --- a/tst/ControllerTest.php +++ b/tst/ControllerTest.php @@ -17,6 +17,7 @@ class ControllerTest extends PHPUnit_Framework_TestCase /* Setup Routine */ $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_data'; $this->_data = Filesystem::getInstance(array('dir' => $this->_path)); + TrafficLimiter::setStore($this->_data); $this->reset(); } From 7901ec74a7167c6fb65866826d7c21e936427c4c Mon Sep 17 00:00:00 2001 From: El RIDO Date: Tue, 8 Jun 2021 22:01:29 +0200 Subject: [PATCH 07/30] folding Persistance\ServerSalt into Data\Filesystem --- lib/Controller.php | 1 + lib/Data/Filesystem.php | 22 +++-- lib/Model/Paste.php | 2 +- lib/Persistence/AbstractPersistence.php | 113 ------------------------ lib/Persistence/ServerSalt.php | 23 ++--- lib/Persistence/TrafficLimiter.php | 4 +- tst/ControllerTest.php | 22 ----- tst/JsonApiTest.php | 2 +- tst/ModelTest.php | 2 +- tst/Persistence/ServerSaltTest.php | 64 ++++++++------ tst/Persistence/TrafficLimiterTest.php | 18 +++- tst/Vizhash16x16Test.php | 3 +- 12 files changed, 80 insertions(+), 196 deletions(-) diff --git a/lib/Controller.php b/lib/Controller.php index 095bbeb..4a1aa0d 100644 --- a/lib/Controller.php +++ b/lib/Controller.php @@ -197,6 +197,7 @@ class Controller { try { // Ensure last paste from visitors IP address was more than configured amount of seconds ago. + ServerSalt::setStore($this->_model->getStore()); TrafficLimiter::setConfiguration($this->_conf); TrafficLimiter::setStore($this->_model->getStore()); if (!TrafficLimiter::canPass()) { diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 3a481d9..8b443ad 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -287,17 +287,17 @@ class Filesystem extends AbstractData self::$_path . DIRECTORY_SEPARATOR . 'purge_limiter.php', '_data['meta']['created'] = time(); - $this->_data['meta']['salt'] = serversalt::generate(); + $this->_data['meta']['salt'] = ServerSalt::generate(); // store paste if ( diff --git a/lib/Persistence/AbstractPersistence.php b/lib/Persistence/AbstractPersistence.php index 73f0ab2..4e61a8e 100644 --- a/lib/Persistence/AbstractPersistence.php +++ b/lib/Persistence/AbstractPersistence.php @@ -12,7 +12,6 @@ namespace PrivateBin\Persistence; -use Exception; use PrivateBin\Data\AbstractData; /** @@ -22,15 +21,6 @@ use PrivateBin\Data\AbstractData; */ abstract class AbstractPersistence { - /** - * path in which to persist something - * - * @access private - * @static - * @var string - */ - private static $_path = 'data'; - /** * data storage to use to persist something * @@ -40,18 +30,6 @@ abstract class AbstractPersistence */ protected static $_store; - /** - * set the path - * - * @access public - * @static - * @param string $path - */ - public static function setPath($path) - { - self::$_path = $path; - } - /** * set the path * @@ -63,95 +41,4 @@ abstract class AbstractPersistence { self::$_store = $store; } - - /** - * get the path - * - * @access public - * @static - * @param string $filename - * @return string - */ - public static function getPath($filename = null) - { - if (strlen($filename)) { - return self::$_path . DIRECTORY_SEPARATOR . $filename; - } else { - return self::$_path; - } - } - - /** - * checks if the file exists - * - * @access protected - * @static - * @param string $filename - * @return bool - */ - protected static function _exists($filename) - { - self::_initialize(); - return is_file(self::$_path . DIRECTORY_SEPARATOR . $filename); - } - - /** - * prepares path for storage - * - * @access protected - * @static - * @throws Exception - */ - protected static function _initialize() - { - // Create storage directory if it does not exist. - if (!is_dir(self::$_path)) { - if (!@mkdir(self::$_path, 0700)) { - throw new Exception('unable to create directory ' . self::$_path, 10); - } - } - $file = self::$_path . DIRECTORY_SEPARATOR . '.htaccess'; - if (!is_file($file)) { - $writtenBytes = 0; - if ($fileCreated = @touch($file)) { - $writtenBytes = @file_put_contents( - $file, - 'Require all denied' . PHP_EOL, - LOCK_EX - ); - } - if ($fileCreated === false || $writtenBytes === false || $writtenBytes < 19) { - throw new Exception('unable to write to file ' . $file, 11); - } - } - } - - /** - * store the data - * - * @access protected - * @static - * @param string $filename - * @param string $data - * @throws Exception - * @return string - */ - protected static function _store($filename, $data) - { - self::_initialize(); - $file = self::$_path . DIRECTORY_SEPARATOR . $filename; - $fileCreated = true; - $writtenBytes = 0; - if (!is_file($file)) { - $fileCreated = @touch($file); - } - if ($fileCreated) { - $writtenBytes = @file_put_contents($file, $data, LOCK_EX); - } - if ($fileCreated === false || $writtenBytes === false || $writtenBytes < strlen($data)) { - throw new Exception('unable to write to file ' . $file, 13); - } - @chmod($file, 0640); // protect file access - return $file; - } } diff --git a/lib/Persistence/ServerSalt.php b/lib/Persistence/ServerSalt.php index 329a8ef..93f5486 100644 --- a/lib/Persistence/ServerSalt.php +++ b/lib/Persistence/ServerSalt.php @@ -13,6 +13,7 @@ namespace PrivateBin\Persistence; use Exception; +use PrivateBin\Data\AbstractData; /** * ServerSalt @@ -71,20 +72,12 @@ class ServerSalt extends AbstractPersistence return self::$_salt; } - if (self::_exists(self::$_file)) { - if (is_readable(self::getPath(self::$_file))) { - $items = explode('|', file_get_contents(self::getPath(self::$_file))); - } - if (!isset($items) || !is_array($items) || count($items) != 3) { - throw new Exception('unable to read file ' . self::getPath(self::$_file), 20); - } - self::$_salt = $items[1]; + $salt = self::$_store->getValue('salt'); + if ($salt) { + self::$_salt = $salt; } else { self::$_salt = self::generate(); - self::_store( - self::$_file, - 'setValue(self::$_salt, 'salt'); } return self::$_salt; } @@ -94,11 +87,11 @@ class ServerSalt extends AbstractPersistence * * @access public * @static - * @param string $path + * @param AbstractData $store */ - public static function setPath($path) + public static function setStore(AbstractData $store) { self::$_salt = ''; - parent::setPath($path); + parent::setStore($store); } } diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index f6bc07f..1146861 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -172,7 +172,7 @@ class TrafficLimiter extends AbstractPersistence // this hash is used as an array key, hence a shorter algo is used $hash = self::getHash('sha256'); $now = time(); - $tl = self::$_store->getValue('traffic_limiter', $hash); + $tl = (int) self::$_store->getValue('traffic_limiter', $hash); self::$_store->purgeValues('traffic_limiter', $now - self::$_limit); if ($tl > 0 && ($tl + self::$_limit >= $now)) { $result = false; @@ -180,7 +180,7 @@ class TrafficLimiter extends AbstractPersistence $tl = time(); $result = true; } - self::$_store->setValue((string) $tl, 'traffic_limiter'); + self::$_store->setValue((string) $tl, 'traffic_limiter', $hash); return $result; } } diff --git a/tst/ControllerTest.php b/tst/ControllerTest.php index 92683ce..165ae08 100644 --- a/tst/ControllerTest.php +++ b/tst/ControllerTest.php @@ -125,28 +125,6 @@ class ControllerTest extends PHPUnit_Framework_TestCase ); } - /** - * @runInSeparateProcess - */ - public function testHtaccess() - { - $htaccess = $this->_path . DIRECTORY_SEPARATOR . '.htaccess'; - @unlink($htaccess); - - $paste = Helper::getPasteJson(); - $file = tempnam(sys_get_temp_dir(), 'FOO'); - file_put_contents($file, $paste); - Request::setInputStream($file); - $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest'; - $_SERVER['REQUEST_METHOD'] = 'POST'; - $_SERVER['REMOTE_ADDR'] = '::1'; - ob_start(); - new Controller; - ob_end_clean(); - - $this->assertFileExists($htaccess, 'htaccess recreated'); - } - /** * @expectedException Exception * @expectedExceptionCode 2 diff --git a/tst/JsonApiTest.php b/tst/JsonApiTest.php index 17b699f..6d9732a 100644 --- a/tst/JsonApiTest.php +++ b/tst/JsonApiTest.php @@ -16,7 +16,7 @@ class JsonApiTest extends PHPUnit_Framework_TestCase /* Setup Routine */ $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_data'; $this->_model = Filesystem::getInstance(array('dir' => $this->_path)); - ServerSalt::setPath($this->_path); + ServerSalt::setStore($this->_model); $_POST = array(); $_GET = array(); diff --git a/tst/ModelTest.php b/tst/ModelTest.php index d5c4074..478d4c1 100644 --- a/tst/ModelTest.php +++ b/tst/ModelTest.php @@ -25,7 +25,6 @@ class ModelTest extends PHPUnit_Framework_TestCase if (!is_dir($this->_path)) { mkdir($this->_path); } - ServerSalt::setPath($this->_path); $options = parse_ini_file(CONF_SAMPLE, true); $options['purge']['limit'] = 0; $options['model'] = array( @@ -39,6 +38,7 @@ class ModelTest extends PHPUnit_Framework_TestCase ); Helper::confBackup(); Helper::createIniFile(CONF, $options); + ServerSalt::setStore(Database::getInstance($options['model_options'])); $this->_conf = new Configuration; $this->_model = new Model($this->_conf); $_SERVER['REMOTE_ADDR'] = '::1'; diff --git a/tst/Persistence/ServerSaltTest.php b/tst/Persistence/ServerSaltTest.php index ecdc0f8..3db5f7d 100644 --- a/tst/Persistence/ServerSaltTest.php +++ b/tst/Persistence/ServerSaltTest.php @@ -1,5 +1,6 @@ _path)) { mkdir($this->_path); } - ServerSalt::setPath($this->_path); + ServerSalt::setStore( + Filesystem::getInstance(array('dir' => $this->_path)) + ); $this->_otherPath = $this->_path . DIRECTORY_SEPARATOR . 'foo'; @@ -40,46 +43,46 @@ class ServerSaltTest extends PHPUnit_Framework_TestCase public function testGeneration() { // generating new salt - ServerSalt::setPath($this->_path); + ServerSalt::setStore( + Filesystem::getInstance(array('dir' => $this->_path)) + ); $salt = ServerSalt::get(); // try setting a different path and resetting it - ServerSalt::setPath($this->_otherPath); + ServerSalt::setStore( + Filesystem::getInstance(array('dir' => $this->_otherPath)) + ); $this->assertNotEquals($salt, ServerSalt::get()); - ServerSalt::setPath($this->_path); + ServerSalt::setStore( + Filesystem::getInstance(array('dir' => $this->_path)) + ); $this->assertEquals($salt, ServerSalt::get()); } - /** - * @expectedException Exception - * @expectedExceptionCode 11 - */ public function testPathShenanigans() { // try setting an invalid path chmod($this->_invalidPath, 0000); - ServerSalt::setPath($this->_invalidPath); - ServerSalt::get(); + $store = Filesystem::getInstance(array('dir' => $this->_invalidPath)); + ServerSalt::setStore($store); + $salt = ServerSalt::get(); + ServerSalt::setStore($store); + $this->assertNotEquals($salt, ServerSalt::get()); } - /** - * @expectedException Exception - * @expectedExceptionCode 20 - */ public function testFileRead() { // try setting an invalid file chmod($this->_invalidPath, 0700); file_put_contents($this->_invalidFile, ''); chmod($this->_invalidFile, 0000); - ServerSalt::setPath($this->_invalidPath); - ServerSalt::get(); + $store = Filesystem::getInstance(array('dir' => $this->_invalidPath)); + ServerSalt::setStore($store); + $salt = ServerSalt::get(); + ServerSalt::setStore($store); + $this->assertNotEquals($salt, ServerSalt::get()); } - /** - * @expectedException Exception - * @expectedExceptionCode 13 - */ public function testFileWrite() { // try setting an invalid file @@ -90,19 +93,24 @@ class ServerSaltTest extends PHPUnit_Framework_TestCase } file_put_contents($this->_invalidPath . DIRECTORY_SEPARATOR . '.htaccess', ''); chmod($this->_invalidPath, 0500); - ServerSalt::setPath($this->_invalidPath); - ServerSalt::get(); + $store = Filesystem::getInstance(array('dir' => $this->_invalidPath)); + ServerSalt::setStore($store); + $salt = ServerSalt::get(); + ServerSalt::setStore($store); + $this->assertNotEquals($salt, ServerSalt::get()); } - /** - * @expectedException Exception - * @expectedExceptionCode 10 - */ public function testPermissionShenanigans() { // try creating an invalid path chmod($this->_invalidPath, 0000); - ServerSalt::setPath($this->_invalidPath . DIRECTORY_SEPARATOR . 'baz'); - ServerSalt::get(); + ServerSalt::setStore( + Filesystem::getInstance(array('dir' => $this->_invalidPath . DIRECTORY_SEPARATOR . 'baz')) + ); + $store = Filesystem::getInstance(array('dir' => $this->_invalidPath)); + ServerSalt::setStore($store); + $salt = ServerSalt::get(); + ServerSalt::setStore($store); + $this->assertNotEquals($salt, ServerSalt::get()); } } diff --git a/tst/Persistence/TrafficLimiterTest.php b/tst/Persistence/TrafficLimiterTest.php index 4101301..2e81d83 100644 --- a/tst/Persistence/TrafficLimiterTest.php +++ b/tst/Persistence/TrafficLimiterTest.php @@ -1,5 +1,7 @@ _path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'trafficlimit'; - TrafficLimiter::setPath($this->_path); + $store = Filesystem::getInstance(array('dir' => $this->_path)); + ServerSalt::setStore($store); + TrafficLimiter::setStore($store); } public function tearDown() @@ -19,11 +23,17 @@ class TrafficLimiterTest extends PHPUnit_Framework_TestCase Helper::rmDir($this->_path . DIRECTORY_SEPARATOR); } + public function testHtaccess() + { + $htaccess = $this->_path . DIRECTORY_SEPARATOR . '.htaccess'; + @unlink($htaccess); + $_SERVER['REMOTE_ADDR'] = 'foobar'; + TrafficLimiter::canPass(); + $this->assertFileExists($htaccess, 'htaccess recreated'); + } + public function testTrafficGetsLimited() { - $this->assertEquals($this->_path, TrafficLimiter::getPath()); - $file = 'baz'; - $this->assertEquals($this->_path . DIRECTORY_SEPARATOR . $file, TrafficLimiter::getPath($file)); TrafficLimiter::setLimit(4); $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; $this->assertTrue(TrafficLimiter::canPass(), 'first request may pass'); diff --git a/tst/Vizhash16x16Test.php b/tst/Vizhash16x16Test.php index afcda56..abfb8c4 100644 --- a/tst/Vizhash16x16Test.php +++ b/tst/Vizhash16x16Test.php @@ -1,5 +1,6 @@ _path); } $this->_file = $this->_path . DIRECTORY_SEPARATOR . 'vizhash.png'; - ServerSalt::setPath($this->_path); + ServerSalt::setStore(Filesystem::getInstance(array('dir' => $this->_path))); } public function tearDown() From a203e6322b16823b126f0538313d79855ced7560 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 9 Jun 2021 07:47:40 +0200 Subject: [PATCH 08/30] implementing key/value store of Persistance in Database storage --- CHANGELOG.md | 1 + lib/Data/AbstractData.php | 20 +++++++++- lib/Data/Database.php | 71 +++++++++++++++++++-------------- lib/Data/Filesystem.php | 30 -------------- lib/Data/GoogleCloudStorage.php | 9 +++-- tst/ControllerTest.php | 1 + tst/ControllerWithDbTest.php | 4 ++ tst/Data/DatabaseTest.php | 15 +++++++ 8 files changed, 86 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c546bd1..d964a57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * CHANGED: Upgrading libraries to: random_compat 2.0.20 * CHANGED: Removed automatic `.ini` configuration file migration (#808) * CHANGED: Removed configurable `dir` for `traffic` & `purge` limiters (#419) + * CHANGED: Server salt, traffic and purge limiter now stored in the storage backend (#419) * **1.3.5 (2021-04-05)** * ADDED: Translation for Hebrew, Lithuanian, Indonesian and Catalan * ADDED: Make the project info configurable (#681) diff --git a/lib/Data/AbstractData.php b/lib/Data/AbstractData.php index 3de4bab..6f2dee5 100644 --- a/lib/Data/AbstractData.php +++ b/lib/Data/AbstractData.php @@ -28,6 +28,15 @@ abstract class AbstractData */ protected static $_instance = null; + /** + * cache for the traffic limiter + * + * @access private + * @static + * @var array + */ + protected static $_traffic_limiter_cache = array(); + /** * Enforce singleton, disable constructor * @@ -138,7 +147,16 @@ abstract class AbstractData * @param int $time * @return void */ - abstract public function purgeValues($namespace, $time); + public function purgeValues($namespace, $time) + { + if ($namespace === 'traffic_limiter') { + foreach (self::$_traffic_limiter_cache as $key => $last_access) { + if ($last_access <= $time) { + unset(self::$_traffic_limiter_cache[$key]); + } + } + } + } /** * Save a value. diff --git a/lib/Data/Database.php b/lib/Data/Database.php index 2a1bbcb..ca0cc79 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -423,23 +423,6 @@ class Database extends AbstractData ); } - /** - * Purge outdated entries. - * - * @access public - * @param string $namespace - * @param int $time - * @return void - */ - public function purgeValues($namespace, $time) - { - switch ($namespace) { - case 'traffic_limiter': - ; - break; - } - } - /** * Save a value. * @@ -451,20 +434,19 @@ class Database extends AbstractData */ public function setValue($value, $namespace, $key = '') { - switch ($namespace) { - case 'purge_limiter': - ; - break; - case 'salt': - ; - break; - case 'traffic_limiter': - ; - break; - default: + if ($namespace === 'traffic_limiter') { + self::$_traffic_limiter_cache[$key] = $value; + try { + $value = Json::encode(self::$_traffic_limiter_cache); + } catch (Exception $e) { return false; - break; + } } + return self::_exec( + 'UPDATE ' . self::_sanitizeIdentifier('config') . + ' SET value = ? WHERE id = ?', + array($value, strtoupper($namespace)) + ); } /** @@ -477,7 +459,36 @@ class Database extends AbstractData */ public function getValue($namespace, $key = '') { + $configKey = strtoupper($namespace); + $value = $this->_getConfig($configKey); + if ($value === '') { + // initialize the row, so that setValue can rely on UPDATE queries + self::_exec( + 'INSERT INTO ' . self::_sanitizeIdentifier('config') . + ' VALUES(?,?)', + array($configKey, '') + ); + // migrate filesystem based salt into database + $file = 'data' . DIRECTORY_SEPARATOR . 'salt.php'; + if ($namespace === 'salt' && is_readable($file)) { + $value = Filesystem::getInstance(array('dir' => 'data'))->getValue('salt'); + $this->setValue($value, 'salt'); + @unlink($file); + return $value; + } + } + if ($value && $namespace === 'traffic_limiter') { + try { + self::$_traffic_limiter_cache = Json::decode($value); + } catch (Exception $e) { + self::$_traffic_limiter_cache = array(); + } + if (array_key_exists($key, self::$_traffic_limiter_cache)) { + return self::$_traffic_limiter_cache[$key]; + } + } + return (string) $value; } /** @@ -629,7 +640,7 @@ class Database extends AbstractData 'SELECT value FROM ' . self::_sanitizeIdentifier('config') . ' WHERE id = ?', array($key), true ); - return $row['value']; + return $row ? $row['value']: ''; } /** diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 8b443ad..384c72d 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -38,15 +38,6 @@ class Filesystem extends AbstractData */ private static $_path = 'data'; - /** - * cache for the traffic limiter - * - * @access private - * @static - * @var array - */ - private static $_traffic_limiter_cache = array(); - /** * get instance of singleton * @@ -249,27 +240,6 @@ class Filesystem extends AbstractData ); } - /** - * Purge outdated entries. - * - * @access public - * @param string $namespace - * @param int $time - * @return void - */ - public function purgeValues($namespace, $time) - { - switch ($namespace) { - case 'traffic_limiter': - foreach (self::$_traffic_limiter_cache as $key => $last_access) { - if ($last_access <= $time) { - unset(self::$_traffic_limiter_cache[$key]); - } - } - break; - } - } - /** * Save a value. * diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index f6fc489..2521957 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -227,10 +227,11 @@ class GoogleCloudStorage extends AbstractData */ public function purgeValues($namespace, $time) { - switch ($namespace) { - case 'traffic_limiter': - ; - break; + if ($namespace === 'traffic_limiter') { + // TODO implement purging of keys in namespace that are <= $time + // if GCS has no easy way to iterate all keys, consider using the + // self::$_traffic_limiter_cache in a similar way as the other + // implementations. } } diff --git a/tst/ControllerTest.php b/tst/ControllerTest.php index 165ae08..0aa7d79 100644 --- a/tst/ControllerTest.php +++ b/tst/ControllerTest.php @@ -17,6 +17,7 @@ class ControllerTest extends PHPUnit_Framework_TestCase /* Setup Routine */ $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_data'; $this->_data = Filesystem::getInstance(array('dir' => $this->_path)); + ServerSalt::setStore($this->_data); TrafficLimiter::setStore($this->_data); $this->reset(); } diff --git a/tst/ControllerWithDbTest.php b/tst/ControllerWithDbTest.php index 90dee92..bc8cd7b 100644 --- a/tst/ControllerWithDbTest.php +++ b/tst/ControllerWithDbTest.php @@ -1,6 +1,8 @@ _options['dsn'] = 'sqlite:' . $this->_path . DIRECTORY_SEPARATOR . 'tst.sq3'; $this->_data = Database::getInstance($this->_options); + ServerSalt::setStore($this->_data); + TrafficLimiter::setStore($this->_data); $this->reset(); } diff --git a/tst/Data/DatabaseTest.php b/tst/Data/DatabaseTest.php index 0d48eb6..f9cd265 100644 --- a/tst/Data/DatabaseTest.php +++ b/tst/Data/DatabaseTest.php @@ -2,6 +2,8 @@ use PrivateBin\Controller; use PrivateBin\Data\Database; +use PrivateBin\Data\Filesystem; +use PrivateBin\Persistence\ServerSalt; class DatabaseTest extends PHPUnit_Framework_TestCase { @@ -31,6 +33,19 @@ class DatabaseTest extends PHPUnit_Framework_TestCase } } + public function testSaltMigration() + { + ServerSalt::setStore(Filesystem::getInstance(array('dir' => 'data'))); + $salt = ServerSalt::get(); + $file = 'data' . DIRECTORY_SEPARATOR . 'salt.php'; + $this->assertFileExists($file, 'ServerSalt got initialized and stored on disk'); + $this->assertNotEquals($salt, ''); + ServerSalt::setStore($this->_model); + ServerSalt::get(); + $this->assertFileNotExists($file, 'legacy ServerSalt got removed'); + $this->assertEquals($salt, ServerSalt::get(), 'ServerSalt got preserved & migrated'); + } + public function testDatabaseBasedDataStoreWorks() { $this->_model->delete(Helper::getPasteId()); From 7b2f0ff302a24c15a7597c23e3bf03adfabc2dca Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 9 Jun 2021 19:16:22 +0200 Subject: [PATCH 09/30] apply StyleCI recommendation --- lib/Data/Database.php | 4 ++-- lib/Persistence/TrafficLimiter.php | 4 ++-- tst/Persistence/TrafficLimiterTest.php | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Data/Database.php b/lib/Data/Database.php index ca0cc79..ae4da19 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -460,7 +460,7 @@ class Database extends AbstractData public function getValue($namespace, $key = '') { $configKey = strtoupper($namespace); - $value = $this->_getConfig($configKey); + $value = $this->_getConfig($configKey); if ($value === '') { // initialize the row, so that setValue can rely on UPDATE queries self::_exec( @@ -640,7 +640,7 @@ class Database extends AbstractData 'SELECT value FROM ' . self::_sanitizeIdentifier('config') . ' WHERE id = ?', array($key), true ); - return $row ? $row['value']: ''; + return $row ? $row['value'] : ''; } /** diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index 1146861..930272d 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -171,8 +171,8 @@ class TrafficLimiter extends AbstractPersistence // this hash is used as an array key, hence a shorter algo is used $hash = self::getHash('sha256'); - $now = time(); - $tl = (int) self::$_store->getValue('traffic_limiter', $hash); + $now = time(); + $tl = (int) self::$_store->getValue('traffic_limiter', $hash); self::$_store->purgeValues('traffic_limiter', $now - self::$_limit); if ($tl > 0 && ($tl + self::$_limit >= $now)) { $result = false; diff --git a/tst/Persistence/TrafficLimiterTest.php b/tst/Persistence/TrafficLimiterTest.php index 2e81d83..aedbf88 100644 --- a/tst/Persistence/TrafficLimiterTest.php +++ b/tst/Persistence/TrafficLimiterTest.php @@ -12,7 +12,7 @@ class TrafficLimiterTest extends PHPUnit_Framework_TestCase { /* Setup Routine */ $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'trafficlimit'; - $store = Filesystem::getInstance(array('dir' => $this->_path)); + $store = Filesystem::getInstance(array('dir' => $this->_path)); ServerSalt::setStore($store); TrafficLimiter::setStore($store); } From 1232717334aa2ff65edf3381a21152bb6e1b02cd Mon Sep 17 00:00:00 2001 From: Mark van Holsteijn Date: Wed, 9 Jun 2021 22:27:34 +0200 Subject: [PATCH 10/30] added purgeValues to GCS --- lib/Data/GoogleCloudStorage.php | 37 ++++++++++++++++++++--------- tst/Data/GoogleCloudStorageTest.php | 22 ++++++++++++----- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index 2521957..75bb8a6 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -2,6 +2,7 @@ namespace PrivateBin\Data; +use DateTime; use Exception; use Google\Cloud\Core\Exception\NotFoundException; use Google\Cloud\Storage\StorageClient; @@ -9,6 +10,8 @@ use PrivateBin\Json; class GoogleCloudStorage extends AbstractData { + const DATETIME_FORMAT = 'Y-m-d\TH:i:s.u\Z'; + /** * returns a Google Cloud Storage data backend. * @@ -218,20 +221,32 @@ class GoogleCloudStorage extends AbstractData } /** - * Purge outdated entries. - * - * @access public - * @param string $namespace - * @param int $time - * @return void + * @inheritDoc */ public function purgeValues($namespace, $time) { - if ($namespace === 'traffic_limiter') { - // TODO implement purging of keys in namespace that are <= $time - // if GCS has no easy way to iterate all keys, consider using the - // self::$_traffic_limiter_cache in a similar way as the other - // implementations. + $prefix = 'config/' . $namespace . '/'; + try { + foreach ($this->_bucket->objects(array('prefix' => $prefix)) as $object) { + $info = $object->info(); + $timeCreated = false; + if (key_exists('timeCreated', $info)) { + $timeCreated = DateTime::createFromFormat(GoogleCloudStorage::DATETIME_FORMAT, $info['timeCreated']); + } + if ($timeCreated && ($timeCreated->getTimestamp() < $time)) { + try { + $object->delete(); + } catch (NotFoundException $e) { + // deleted by another instance. + } + } else { + if (!$timeCreated) { + error_log('failed to parse create timestamp ' . $info['timeCreated'] . ' of object ' . $object->name()); + } + } + } + } catch (NotFoundException $e) { + // no objects in the bucket yet } } diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php index 30a7d5e..1c5190d 100644 --- a/tst/Data/GoogleCloudStorageTest.php +++ b/tst/Data/GoogleCloudStorageTest.php @@ -43,7 +43,6 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase foreach (self::$_bucket->objects() as $object) { $object->delete(); } - error_reporting(E_ALL); } public static function tearDownAfterClass() @@ -145,17 +144,26 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $this->_model->setValue($salt, 'salt', 'master'); $storedSalt = $this->_model->getValue('salt', 'master'); $this->assertEquals($salt, $storedSalt); + $this->_model->purgeValues('salt', time() + 60); + $this->assertFalse($this->_model->getValue('salt', 'master')); $client = hash_hmac('sha512', '127.0.0.1', $salt); $expire = time(); $this->_model->setValue($expire, 'traffic_limiter', $client); $storedExpired = $this->_model->getValue('traffic_limiter', $client); $this->assertEquals($expire, $storedExpired); + $this->assertEquals($expire, $storedExpired); + $this->_model->purgeValues('traffic_limiter', time() - 60); + $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); + $this->_model->purgeValues('traffic_limiter', time() + 60); + $this->assertFalse($this->_model->getValue('traffic_limiter', $client)); $purgeAt = $expire + (15 * 60); $this->_model->setValue($purgeAt, 'purge_limiter', 'at'); $storedPurgedAt = $this->_model->getValue('purge_limiter', 'at'); $this->assertEquals($purgeAt, $storedPurgedAt); + $this->_model->purgeValues('purge_limiter', time() + 60); + $this->assertFalse($this->_model->getValue('purge_limiter', 'at')); } } @@ -431,11 +439,13 @@ class StorageObjectStub extends StorageObject public function __construct(ConnectionInterface $connection, $name, $bucket, $generation = null, array $info = array(), $encryptionKey = null, $encryptionKeySHA256 = null) { - $this->_name = $name; - $this->_bucket = $bucket; - $this->_generation = $generation; - $this->_info = $info; - $this->_connection = $connection; + $this->_name = $name; + $this->_bucket = $bucket; + $this->_generation = $generation; + $this->_info = $info; + $this->_connection = $connection; + $timeCreated = new Datetime(); + $this->_info['metadata']['timeCreated'] = $timeCreated->format(GoogleCloudStorage::DATETIME_FORMAT); } public function acl() From 1b88eef3569f39c0596d2fb5b39d23301f65af0f Mon Sep 17 00:00:00 2001 From: Mark van Holsteijn Date: Thu, 10 Jun 2021 21:39:15 +0200 Subject: [PATCH 11/30] improved implementation of GoogleStorageBucket --- lib/Data/GoogleCloudStorage.php | 55 +++++++++++++++++------------ tst/Data/GoogleCloudStorageTest.php | 43 +++++++++++++++++++--- 2 files changed, 71 insertions(+), 27 deletions(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index 75bb8a6..6bbea60 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -2,7 +2,6 @@ namespace PrivateBin\Data; -use DateTime; use Exception; use Google\Cloud\Core\Exception\NotFoundException; use Google\Cloud\Storage\StorageClient; @@ -225,23 +224,22 @@ class GoogleCloudStorage extends AbstractData */ public function purgeValues($namespace, $time) { - $prefix = 'config/' . $namespace . '/'; + $path = 'config/' . $namespace; try { - foreach ($this->_bucket->objects(array('prefix' => $prefix)) as $object) { - $info = $object->info(); - $timeCreated = false; - if (key_exists('timeCreated', $info)) { - $timeCreated = DateTime::createFromFormat(GoogleCloudStorage::DATETIME_FORMAT, $info['timeCreated']); + foreach ($this->_bucket->objects(array('prefix' => $path)) as $object) { + $name = $object->name(); + if (strlen($name) > strlen($path) && substr($name, strlen($path), 1) !== '/') { + continue; } - if ($timeCreated && ($timeCreated->getTimestamp() < $time)) { - try { - $object->delete(); - } catch (NotFoundException $e) { - // deleted by another instance. - } - } else { - if (!$timeCreated) { - error_log('failed to parse create timestamp ' . $info['timeCreated'] . ' of object ' . $object->name()); + $info = $object->info(); + if (key_exists('metadata', $info) && key_exists('value', $info['metadata'])) { + $value = $info['metadata']['value']; + if (is_numeric($value) && intval($value) < $time) { + try { + $object->delete(); + } catch (NotFoundException $e) { + // deleted by another instance. + } } } } @@ -251,15 +249,24 @@ class GoogleCloudStorage extends AbstractData } /** - * This is the simplest thing that could possibly work. - * will be to tested for runtime performance. + * For GoogleCloudStorage, the value will also be stored in the metadata for the + * namespaces traffic_limiter and purge_limiter. * @inheritDoc */ public function setValue($value, $namespace, $key = '') { - $key = 'config/' . $namespace . '/' . $key; + if ($key === '') { + $key = 'config/' . $namespace; + } else { + $key = 'config/' . $namespace . '/' . $key; + } + $data = Json::encode($value); + $metadata = array('namespace' => $namespace); + if ($namespace != 'salt') { + $metadata['value'] = strval($value); + } try { $this->_bucket->upload($data, array( 'name' => $key, @@ -267,7 +274,7 @@ class GoogleCloudStorage extends AbstractData 'predefinedAcl' => 'private', 'metadata' => array( 'content-type' => 'application/json', - 'metadata' => array('namespace' => $namespace), + 'metadata' => $metadata, ), )); } catch (Exception $e) { @@ -279,13 +286,15 @@ class GoogleCloudStorage extends AbstractData } /** - * This is the simplest thing that could possibly work. - * will be to tested for runtime performance. * @inheritDoc */ public function getValue($namespace, $key = '') { - $key = 'config/' . $namespace . '/' . $key; + if ($key === '') { + $key = 'config/' . $namespace; + } else { + $key = 'config/' . $namespace . '/' . $key; + } try { $o = $this->_bucket->object($key); $data = $o->downloadAsString(); diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php index 1c5190d..7945e2b 100644 --- a/tst/Data/GoogleCloudStorageTest.php +++ b/tst/Data/GoogleCloudStorageTest.php @@ -141,8 +141,8 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase public function testKeyValueStore() { $salt = bin2hex(random_bytes(256)); - $this->_model->setValue($salt, 'salt', 'master'); - $storedSalt = $this->_model->getValue('salt', 'master'); + $this->_model->setValue($salt, 'salt', ''); + $storedSalt = $this->_model->getValue('salt', ''); $this->assertEquals($salt, $storedSalt); $this->_model->purgeValues('salt', time() + 60); $this->assertFalse($this->_model->getValue('salt', 'master')); @@ -159,12 +159,47 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $this->assertFalse($this->_model->getValue('traffic_limiter', $client)); $purgeAt = $expire + (15 * 60); - $this->_model->setValue($purgeAt, 'purge_limiter', 'at'); - $storedPurgedAt = $this->_model->getValue('purge_limiter', 'at'); + $this->_model->setValue($purgeAt, 'purge_limiter', ''); + $storedPurgedAt = $this->_model->getValue('purge_limiter', ''); $this->assertEquals($purgeAt, $storedPurgedAt); $this->_model->purgeValues('purge_limiter', time() + 60); $this->assertFalse($this->_model->getValue('purge_limiter', 'at')); } + + /** + * @throws Exception + */ + public function testKeyValuePurgeTrafficLimiter() + { + $salt = bin2hex(random_bytes(256)); + $client = hash_hmac('sha512', '127.0.0.1', $salt); + $expire = time(); + $this->_model->setValue($expire, 'traffic_limiter', $client); + $storedExpired = $this->_model->getValue('traffic_limiter', $client); + $this->assertEquals($expire, $storedExpired); + + $this->_model->purgeValues('traffic_limiter', time() - 60); + $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); + + $this->_model->purgeValues('traffic_limiter', time() + 60); + $this->assertFalse($this->_model->getValue('traffic_limiter', $client)); + } + + public function testKeyValuePurgeTrafficLimiterWithKey() + { + $salt = bin2hex(random_bytes(256)); + $client = hash_hmac('sha512', '127.0.0.1', $salt); + $expire = time(); + $this->_model->setValue($expire, 'traffic_limiter', $client); + $storedExpired = $this->_model->getValue('traffic_limiter', $client); + $this->assertEquals($expire, $storedExpired); + + $this->_model->purgeValues('traffic_limiter', time() - 60); + $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); + + $this->_model->purgeValues('traffic_limiter', time() + 60); + $this->assertFalse($this->_model->getValue('traffic_limiter', $client)); + } } /** From e294145a2bdf52753ad0ca55d8091e8160068490 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 08:26:05 +0200 Subject: [PATCH 12/30] ip-lib doesn't except on the matches interfaces --- lib/Persistence/TrafficLimiter.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index 930272d..4f11ec7 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -13,7 +13,6 @@ namespace PrivateBin\Persistence; -use Exception; use IPLib\Factory; use PrivateBin\Configuration; @@ -133,13 +132,7 @@ class TrafficLimiter extends AbstractPersistence return false; } - // Ip-lib throws an exception when something goes wrong, if so we want to catch it and set contained to false - try { - return $address->matches($range); - } catch (Exception $e) { - // If something is wrong with matching the ip, we assume it doesn't match - return false; - } + return $address->matches($range); } /** @@ -149,7 +142,6 @@ class TrafficLimiter extends AbstractPersistence * * @access public * @static - * @throws Exception * @return bool */ public static function canPass() From 93135e0abf92ebce8cafee5299769e1434f24bf2 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 10:44:26 +0200 Subject: [PATCH 13/30] improving code coverage --- lib/Controller.php | 27 +- lib/Data/Database.php | 78 ++-- lib/Data/GoogleCloudStorage.php | 17 +- lib/FormatV2.php | 5 + lib/Persistence/PurgeLimiter.php | 1 - lib/Persistence/ServerSalt.php | 5 +- lib/Request.php | 13 +- tst/Bootstrap.php | 595 +++++++++++++++++++++++++++- tst/ControllerTest.php | 27 +- tst/ControllerWithGcsTest.php | 59 +++ tst/Data/DatabaseTest.php | 42 ++ tst/Data/FilesystemTest.php | 1 + tst/Data/GoogleCloudStorageTest.php | 585 +-------------------------- tst/ModelTest.php | 154 +++++++ 14 files changed, 949 insertions(+), 660 deletions(-) create mode 100644 tst/ControllerWithGcsTest.php diff --git a/lib/Controller.php b/lib/Controller.php index 4a1aa0d..fb919ca 100644 --- a/lib/Controller.php +++ b/lib/Controller.php @@ -195,22 +195,17 @@ class Controller */ private function _create() { - try { - // Ensure last paste from visitors IP address was more than configured amount of seconds ago. - ServerSalt::setStore($this->_model->getStore()); - TrafficLimiter::setConfiguration($this->_conf); - TrafficLimiter::setStore($this->_model->getStore()); - if (!TrafficLimiter::canPass()) { - $this->_return_message( - 1, I18n::_( - 'Please wait %d seconds between each post.', - $this->_conf->getKey('limit', 'traffic') - ) - ); - return; - } - } catch (Exception $e) { - $this->_return_message(1, I18n::_($e->getMessage())); + // Ensure last paste from visitors IP address was more than configured amount of seconds ago. + ServerSalt::setStore($this->_model->getStore()); + TrafficLimiter::setConfiguration($this->_conf); + TrafficLimiter::setStore($this->_model->getStore()); + if (!TrafficLimiter::canPass()) { + $this->_return_message( + 1, I18n::_( + 'Please wait %d seconds between each post.', + $this->_conf->getKey('limit', 'traffic') + ) + ); return; } diff --git a/lib/Data/Database.php b/lib/Data/Database.php index ae4da19..a15b862 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -198,21 +198,25 @@ class Database extends AbstractData $opendiscussion = $paste['adata'][2]; $burnafterreading = $paste['adata'][3]; } - return self::_exec( - 'INSERT INTO ' . self::_sanitizeIdentifier('paste') . - ' VALUES(?,?,?,?,?,?,?,?,?)', - array( - $pasteid, - $isVersion1 ? $paste['data'] : Json::encode($paste), - $created, - $expire_date, - (int) $opendiscussion, - (int) $burnafterreading, - Json::encode($meta), - $attachment, - $attachmentname, - ) - ); + try { + return self::_exec( + 'INSERT INTO ' . self::_sanitizeIdentifier('paste') . + ' VALUES(?,?,?,?,?,?,?,?,?)', + array( + $pasteid, + $isVersion1 ? $paste['data'] : Json::encode($paste), + $created, + $expire_date, + (int) $opendiscussion, + (int) $burnafterreading, + Json::encode($meta), + $attachment, + $attachmentname, + ) + ); + } catch (Exception $e) { + return false; + } } /** @@ -348,19 +352,23 @@ class Database extends AbstractData $meta[$key] = null; } } - return self::_exec( - 'INSERT INTO ' . self::_sanitizeIdentifier('comment') . - ' VALUES(?,?,?,?,?,?,?)', - array( - $commentid, - $pasteid, - $parentid, - $data, - $meta['nickname'], - $meta[$iconKey], - $meta[$createdKey], - ) - ); + try { + return self::_exec( + 'INSERT INTO ' . self::_sanitizeIdentifier('comment') . + ' VALUES(?,?,?,?,?,?,?)', + array( + $commentid, + $pasteid, + $parentid, + $data, + $meta['nickname'], + $meta[$iconKey], + $meta[$createdKey], + ) + ); + } catch (Exception $e) { + return false; + } } /** @@ -416,11 +424,15 @@ class Database extends AbstractData */ public function existsComment($pasteid, $parentid, $commentid) { - return (bool) self::_select( - 'SELECT dataid FROM ' . self::_sanitizeIdentifier('comment') . - ' WHERE pasteid = ? AND parentid = ? AND dataid = ?', - array($pasteid, $parentid, $commentid), true - ); + try { + return (bool) self::_select( + 'SELECT dataid FROM ' . self::_sanitizeIdentifier('comment') . + ' WHERE pasteid = ? AND parentid = ? AND dataid = ?', + array($pasteid, $parentid, $commentid), true + ); + } catch (Exception $e) { + return false; + } } /** diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index 6bbea60..2066769 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -34,12 +34,9 @@ class GoogleCloudStorage extends AbstractData if (is_array($options) && array_key_exists('prefix', $options)) { $prefix = $options['prefix']; } - if (is_array($options) && array_key_exists('client', $options)) { - $client = $options['client']; - } if (!(self::$_instance instanceof self)) { - self::$_instance = new self($bucket, $prefix, $client); + self::$_instance = new self($bucket, $prefix); } return self::$_instance; } @@ -48,16 +45,12 @@ class GoogleCloudStorage extends AbstractData protected $_bucket = null; protected $_prefix = 'pastes'; - public function __construct($bucket, $prefix, $client = null) + public function __construct($bucket, $prefix) { parent::__construct(); - if ($client == null) { - $this->_client = new StorageClient(array('suppressKeyFileNotice' => true)); - } else { - // use given client for test purposes - $this->_client = $client; - } - + $this->_client = class_exists('StorageClientStub', false) ? + new \StorageClientStub(array()) : + new StorageClient(array('suppressKeyFileNotice' => true)); $this->_bucket = $this->_client->bucket($bucket); if ($prefix != null) { $this->_prefix = $prefix; diff --git a/lib/FormatV2.php b/lib/FormatV2.php index a06aa5d..d2055f3 100644 --- a/lib/FormatV2.php +++ b/lib/FormatV2.php @@ -52,6 +52,11 @@ class FormatV2 } } + // Make sure adata is an array. + if (!is_array($message['adata'])) { + return false; + } + $cipherParams = $isComment ? $message['adata'] : $message['adata'][0]; // Make sure some fields are base64 data: diff --git a/lib/Persistence/PurgeLimiter.php b/lib/Persistence/PurgeLimiter.php index 6b60817..89d5d60 100644 --- a/lib/Persistence/PurgeLimiter.php +++ b/lib/Persistence/PurgeLimiter.php @@ -59,7 +59,6 @@ class PurgeLimiter extends AbstractPersistence * * @access public * @static - * @throws \Exception * @return bool */ public static function canPurge() diff --git a/lib/Persistence/ServerSalt.php b/lib/Persistence/ServerSalt.php index 93f5486..14f7bd1 100644 --- a/lib/Persistence/ServerSalt.php +++ b/lib/Persistence/ServerSalt.php @@ -12,7 +12,6 @@ namespace PrivateBin\Persistence; -use Exception; use PrivateBin\Data\AbstractData; /** @@ -54,8 +53,7 @@ class ServerSalt extends AbstractPersistence */ public static function generate() { - $randomSalt = bin2hex(random_bytes(256)); - return $randomSalt; + return bin2hex(random_bytes(256)); } /** @@ -63,7 +61,6 @@ class ServerSalt extends AbstractPersistence * * @access public * @static - * @throws Exception * @return string */ public static function get() diff --git a/lib/Request.php b/lib/Request.php index 5776cab..df517bb 100644 --- a/lib/Request.php +++ b/lib/Request.php @@ -108,6 +108,8 @@ class Request case 'DELETE': case 'PUT': case 'POST': + // it might be a creation or a deletion, the latter is detected below + $this->_operation = 'create'; $this->_params = Json::decode( file_get_contents(self::$_inputStream) ); @@ -125,15 +127,10 @@ class Request } // prepare operation, depending on current parameters - if ( - array_key_exists('ct', $this->_params) && - !empty($this->_params['ct']) - ) { - $this->_operation = 'create'; - } elseif (array_key_exists('pasteid', $this->_params) && !empty($this->_params['pasteid'])) { + if (array_key_exists('pasteid', $this->_params) && !empty($this->_params['pasteid'])) { if (array_key_exists('deletetoken', $this->_params) && !empty($this->_params['deletetoken'])) { $this->_operation = 'delete'; - } else { + } else if ($this->_operation != 'create') { $this->_operation = 'read'; } } elseif (array_key_exists('jsonld', $this->_params) && !empty($this->_params['jsonld'])) { @@ -172,7 +169,7 @@ class Request $data['meta'] = $meta; } foreach ($required_keys as $key) { - $data[$key] = $this->getParam($key); + $data[$key] = $this->getParam($key, $key == 'v' ? 1 : ''); } // forcing a cast to int or float $data['v'] = $data['v'] + 0; diff --git a/tst/Bootstrap.php b/tst/Bootstrap.php index b539351..bdf8a38 100644 --- a/tst/Bootstrap.php +++ b/tst/Bootstrap.php @@ -1,5 +1,14 @@ _config = $config; + $this->_connection = new ConnectionInterfaceStub(); + } + + public function bucket($name, $userProject = false) + { + if (!key_exists($name, $this->_buckets)) { + $b = new BucketStub($this->_connection, $name, array(), $this); + $this->_buckets[$name] = $b; + } + return $this->_buckets[$name]; + } + + /** + * @throws \Google\Cloud\Core\Exception\NotFoundException + */ + public function deleteBucket($name) + { + if (key_exists($name, $this->_buckets)) { + unset($this->_buckets[$name]); + } else { + throw new NotFoundException(); + } + } + + public function buckets(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function registerStreamWrapper($protocol = null) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function unregisterStreamWrapper($protocol = null) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function signedUrlUploader($uri, $data, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function timestamp(\DateTimeInterface $timestamp, $nanoSeconds = null) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getServiceAccount(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function hmacKeys(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function hmacKey($accessId, $projectId = null, array $metadata = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function createHmacKey($serviceAccountEmail, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function createBucket($name, array $options = array()) + { + if (key_exists($name, $this->_buckets)) { + throw new BadRequestException('already exists'); + } + $b = new BucketStub($this->_connection, $name, array(), $this); + $this->_buckets[$name] = $b; + return $b; + } +} + +/** + * Class BucketStub stubs a GCS bucket. + */ +class BucketStub extends Bucket +{ + public $_objects; + private $_name; + private $_info; + private $_connection; + private $_client; + + public function __construct(ConnectionInterface $connection, $name, array $info = array(), $client = null) + { + $this->_name = $name; + $this->_info = $info; + $this->_connection = $connection; + $this->_objects = array(); + $this->_client = $client; + } + + public function acl() + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function defaultAcl() + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function exists() + { + return true; + } + + public function upload($data, array $options = array()) + { + if (!is_string($data) || !key_exists('name', $options)) { + throw new BadMethodCallException('not supported by this stub'); + } + + $name = $options['name']; + $generation = '1'; + $o = new StorageObjectStub($this->_connection, $name, $this, $generation, $options); + $this->_objects[$options['name']] = $o; + $o->setData($data); + } + + public function uploadAsync($data, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getResumableUploader($data, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getStreamableUploader($data, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function object($name, array $options = array()) + { + if (key_exists($name, $this->_objects)) { + return $this->_objects[$name]; + } else { + return new StorageObjectStub($this->_connection, $name, $this, null, $options); + } + } + + public function objects(array $options = array()) + { + $prefix = key_exists('prefix', $options) ? $options['prefix'] : ''; + + return new CallbackFilterIterator( + new ArrayIterator($this->_objects), + function ($current, $key, $iterator) use ($prefix) { + return substr($key, 0, strlen($prefix)) == $prefix; + } + ); + } + + public function createNotification($topic, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function notification($id) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function notifications(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function delete(array $options = array()) + { + $this->_client->deleteBucket($this->_name); + } + + public function update(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function compose(array $sourceObjects, $name, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function info(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function reload(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function name() + { + return $this->_name; + } + + public static function lifecycle(array $lifecycle = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function currentLifecycle(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function isWritable($file = null) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function iam() + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function lockRetentionPolicy(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function signedUrl($expires, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function generateSignedPostPolicyV4($objectName, $expires, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } +} + +/** + * Class StorageObjectStub stubs a GCS storage object. + */ +class StorageObjectStub extends StorageObject +{ + private $_name; + private $_data; + private $_info; + private $_bucket; + private $_generation; + private $_exists = false; + private $_connection; + + public function __construct(ConnectionInterface $connection, $name, $bucket, $generation = null, array $info = array(), $encryptionKey = null, $encryptionKeySHA256 = null) + { + $this->_name = $name; + $this->_bucket = $bucket; + $this->_generation = $generation; + $this->_info = $info; + $this->_connection = $connection; + $timeCreated = new Datetime(); + $this->_info['metadata']['timeCreated'] = $timeCreated->format(GoogleCloudStorage::DATETIME_FORMAT); + } + + public function acl() + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function exists(array $options = array()) + { + return key_exists($this->_name, $this->_bucket->_objects); + } + + /** + * @throws NotFoundException + */ + public function delete(array $options = array()) + { + if (key_exists($this->_name, $this->_bucket->_objects)) { + unset($this->_bucket->_objects[$this->_name]); + } else { + throw new NotFoundException('key ' . $this->_name . ' not found.'); + } + } + + /** + * @throws NotFoundException + */ + public function update(array $metadata, array $options = array()) + { + if (!$this->_exists) { + throw new NotFoundException('key ' . $this->_name . ' not found.'); + } + $this->_info = $metadata; + } + + public function copy($destination, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function rewrite($destination, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function rename($name, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + /** + * @throws NotFoundException + */ + public function downloadAsString(array $options = array()) + { + if (!$this->_exists) { + throw new NotFoundException('key ' . $this->_name . ' not found.'); + } + return $this->_data; + } + + public function downloadToFile($path, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function downloadAsStream(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function downloadAsStreamAsync(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function signedUrl($expires, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function signedUploadUrl($expires, array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function beginSignedUploadSession(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function info(array $options = array()) + { + return key_exists('metadata',$this->_info) ? $this->_info['metadata'] : array(); + } + + public function reload(array $options = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function name() + { + return $this->_name; + } + + public function identity() + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function gcsUri() + { + return sprintf( + 'gs://%s/%s', + $this->_bucket->name(), + $this->_name + ); + } + + public function setData($data) + { + $this->_data = $data; + $this->_exists = true; + } +} + +/** + * Class ConnectionInterfaceStub required for the stubs. + */ +class ConnectionInterfaceStub implements ConnectionInterface +{ + public function deleteAcl(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getAcl(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function listAcl(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function insertAcl(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function patchAcl(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function deleteBucket(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getBucket(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function listBuckets(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function insertBucket(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getBucketIamPolicy(array $args) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function setBucketIamPolicy(array $args) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function testBucketIamPermissions(array $args) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function patchBucket(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function deleteObject(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function copyObject(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function rewriteObject(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function composeObject(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getObject(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function listObjects(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function patchObject(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function downloadObject(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function insertObject(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getNotification(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function deleteNotification(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function insertNotification(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function listNotifications(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getServiceAccount(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function lockRetentionPolicy(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function createHmacKey(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function deleteHmacKey(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function getHmacKey(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function updateHmacKey(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } + + public function listHmacKeys(array $args = array()) + { + throw new BadMethodCallException('not supported by this stub'); + } +} + +/** + * Class Helper provides unit tests pastes and comments of various formats + */ class Helper { /** @@ -155,7 +744,11 @@ class Helper public static function getPastePost($version = 2, array $meta = array()) { $example = self::getPaste($version, $meta); - $example['meta'] = array('expire' => $example['meta']['expire']); + if ($version == 2) { + $example['meta'] = array('expire' => $example['meta']['expire']); + } else { + unset($example['meta']['postdate']); + } return $example; } diff --git a/tst/ControllerTest.php b/tst/ControllerTest.php index 0aa7d79..5c95127 100644 --- a/tst/ControllerTest.php +++ b/tst/ControllerTest.php @@ -48,6 +48,8 @@ class ControllerTest extends PHPUnit_Framework_TestCase */ public function testView() { + $_SERVER['QUERY_STRING'] = Helper::getPasteId(); + $_GET[Helper::getPasteId()] = ''; ob_start(); new Controller; $content = ob_get_contents(); @@ -470,6 +472,29 @@ class ControllerTest extends PHPUnit_Framework_TestCase $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste exists after posting data'); } + /** + * @runInSeparateProcess + */ + public function testCreateInvalidFormat() + { + $options = parse_ini_file(CONF, true); + $options['traffic']['limit'] = 0; + Helper::createIniFile(CONF, $options); + $file = tempnam(sys_get_temp_dir(), 'FOO'); + file_put_contents($file, Helper::getPasteJson(1)); + Request::setInputStream($file); + $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest'; + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_SERVER['REMOTE_ADDR'] = '::1'; + ob_start(); + new Controller; + $content = ob_get_contents(); + ob_end_clean(); + $response = json_decode($content, true); + $this->assertEquals(1, $response['status'], 'outputs error status'); + $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste exists after posting data'); + } + /** * @runInSeparateProcess */ @@ -518,7 +543,7 @@ class ControllerTest extends PHPUnit_Framework_TestCase ob_end_clean(); $response = json_decode($content, true); $this->assertEquals(1, $response['status'], 'outputs error status'); - $this->assertFalse($this->_data->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'paste exists after posting data'); + $this->assertFalse($this->_data->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment exists after posting data'); } /** diff --git a/tst/ControllerWithGcsTest.php b/tst/ControllerWithGcsTest.php new file mode 100644 index 0000000..3983341 --- /dev/null +++ b/tst/ControllerWithGcsTest.php @@ -0,0 +1,59 @@ +false)); + $handler = HttpHandlerFactory::build($httpClient); + + $name = 'pb-'; + $alphabet = 'abcdefghijklmnopqrstuvwxyz'; + for ($i = 0; $i < 29; ++$i) { + $name .= $alphabet[rand(0, strlen($alphabet) - 1)]; + } + self::$_client = new StorageClientStub(array()); + self::$_bucket = self::$_client->createBucket($name); + } + + public function setUp() + { + /* Setup Routine */ + $this->_path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_data'; + if (!is_dir($this->_path)) { + mkdir($this->_path); + } + $this->_options = array( + 'bucket' => self::$_bucket->name(), + 'prefix' => 'pastes', + ); + $this->_data = GoogleCloudStorage::getInstance($this->_options); + ServerSalt::setStore($this->_data); + TrafficLimiter::setStore($this->_data); + $this->reset(); + } + + public function reset() + { + parent::reset(); + // but then inject a db config + $options = parse_ini_file(CONF, true); + $options['model'] = array( + 'class' => 'GoogleCloudStorage', + ); + $options['model_options'] = $this->_options; + Helper::createIniFile(CONF, $options); + } +} diff --git a/tst/Data/DatabaseTest.php b/tst/Data/DatabaseTest.php index f9cd265..1cfc0be 100644 --- a/tst/Data/DatabaseTest.php +++ b/tst/Data/DatabaseTest.php @@ -302,6 +302,48 @@ class DatabaseTest extends PHPUnit_Framework_TestCase Helper::rmDir($this->_path); } + public function testCorruptMeta() + { + mkdir($this->_path); + $path = $this->_path . DIRECTORY_SEPARATOR . 'meta-test.sq3'; + if (is_file($path)) { + unlink($path); + } + $this->_options['dsn'] = 'sqlite:' . $path; + $this->_options['tbl'] = 'baz_'; + $model = Database::getInstance($this->_options); + $paste = Helper::getPaste(1, array('expire_date' => 1344803344)); + unset($paste['meta']['formatter'], $paste['meta']['opendiscussion'], $paste['meta']['salt']); + $model->delete(Helper::getPasteId()); + + $db = new PDO( + $this->_options['dsn'], + $this->_options['usr'], + $this->_options['pwd'], + $this->_options['opt'] + ); + $statement = $db->prepare('INSERT INTO baz_paste VALUES(?,?,?,?,?,?,?,?,?)'); + $statement->execute( + array( + Helper::getPasteId(), + $paste['data'], + $paste['meta']['postdate'], + $paste['meta']['expire_date'], + 0, + 0, + '{', + null, + null, + ) + ); + $statement->closeCursor(); + + $this->assertTrue($model->exists(Helper::getPasteId()), 'paste exists after storing it'); + $this->assertEquals($paste, $model->read(Helper::getPasteId())); + + Helper::rmDir($this->_path); + } + public function testTableUpgrade() { mkdir($this->_path); diff --git a/tst/Data/FilesystemTest.php b/tst/Data/FilesystemTest.php index 37e03f3..684c294 100644 --- a/tst/Data/FilesystemTest.php +++ b/tst/Data/FilesystemTest.php @@ -117,6 +117,7 @@ class FilesystemTest extends PHPUnit_Framework_TestCase $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste does not yet exist'); $this->assertFalse($this->_model->create(Helper::getPasteId(), $paste), 'unable to store broken paste'); $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste does still not exist'); + $this->assertFalse($this->_model->setValue('foo', 'non existing namespace'), 'rejects setting value in non existing namespace'); } public function testCommentErrorDetection() diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php index 7945e2b..557984e 100644 --- a/tst/Data/GoogleCloudStorageTest.php +++ b/tst/Data/GoogleCloudStorageTest.php @@ -1,12 +1,6 @@ _model = GoogleCloudStorage::getInstance(array( 'bucket' => self::$_bucket->name(), 'prefix' => 'pastes', - 'client' => self::$_client, )); + )); } public function tearDown() @@ -201,580 +195,3 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $this->assertFalse($this->_model->getValue('traffic_limiter', $client)); } } - -/** - * Class StorageClientStub provides a limited stub for performing the unit test - */ -class StorageClientStub extends StorageClient -{ - private $_config = null; - private $_connection = null; - private $_buckets = array(); - - public function __construct(array $config = array()) - { - $this->_config = $config; - $this->_connection = new ConnectionInterfaceStub(); - } - - public function bucket($name, $userProject = false) - { - if (!key_exists($name, $this->_buckets)) { - $b = new BucketStub($this->_connection, $name, array(), $this); - $this->_buckets[$name] = $b; - } - return $this->_buckets[$name]; - } - - /** - * @throws \Google\Cloud\Core\Exception\NotFoundException - */ - public function deleteBucket($name) - { - if (key_exists($name, $this->_buckets)) { - unset($this->_buckets[$name]); - } else { - throw new NotFoundException(); - } - } - - public function buckets(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function registerStreamWrapper($protocol = null) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function unregisterStreamWrapper($protocol = null) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function signedUrlUploader($uri, $data, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function timestamp(\DateTimeInterface $timestamp, $nanoSeconds = null) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getServiceAccount(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function hmacKeys(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function hmacKey($accessId, $projectId = null, array $metadata = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function createHmacKey($serviceAccountEmail, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function createBucket($name, array $options = array()) - { - if (key_exists($name, $this->_buckets)) { - throw new BadRequestException('already exists'); - } - $b = new BucketStub($this->_connection, $name, array(), $this); - $this->_buckets[$name] = $b; - return $b; - } -} - -/** - * Class BucketStub stubs a GCS bucket. - */ -class BucketStub extends Bucket -{ - public $_objects; - private $_name; - private $_info; - private $_connection; - private $_client; - - public function __construct(ConnectionInterface $connection, $name, array $info = array(), $client = null) - { - $this->_name = $name; - $this->_info = $info; - $this->_connection = $connection; - $this->_objects = array(); - $this->_client = $client; - } - - public function acl() - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function defaultAcl() - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function exists() - { - return true; - } - - public function upload($data, array $options = array()) - { - if (!is_string($data) || !key_exists('name', $options)) { - throw new BadMethodCallException('not supported by this stub'); - } - - $name = $options['name']; - $generation = '1'; - $o = new StorageObjectStub($this->_connection, $name, $this, $generation, $options); - $this->_objects[$options['name']] = $o; - $o->setData($data); - } - - public function uploadAsync($data, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getResumableUploader($data, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getStreamableUploader($data, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function object($name, array $options = array()) - { - if (key_exists($name, $this->_objects)) { - return $this->_objects[$name]; - } else { - return new StorageObjectStub($this->_connection, $name, $this, null, $options); - } - } - - public function objects(array $options = array()) - { - $prefix = key_exists('prefix', $options) ? $options['prefix'] : ''; - - return new CallbackFilterIterator( - new ArrayIterator($this->_objects), - function ($current, $key, $iterator) use ($prefix) { - return substr($key, 0, strlen($prefix)) == $prefix; - } - ); - } - - public function createNotification($topic, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function notification($id) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function notifications(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function delete(array $options = array()) - { - $this->_client->deleteBucket($this->_name); - } - - public function update(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function compose(array $sourceObjects, $name, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function info(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function reload(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function name() - { - return $this->_name; - } - - public static function lifecycle(array $lifecycle = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function currentLifecycle(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function isWritable($file = null) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function iam() - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function lockRetentionPolicy(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function signedUrl($expires, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function generateSignedPostPolicyV4($objectName, $expires, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } -} - -/** - * Class StorageObjectStub stubs a GCS storage object. - */ -class StorageObjectStub extends StorageObject -{ - private $_name; - private $_data; - private $_info; - private $_bucket; - private $_generation; - private $_exists = false; - private $_connection; - - public function __construct(ConnectionInterface $connection, $name, $bucket, $generation = null, array $info = array(), $encryptionKey = null, $encryptionKeySHA256 = null) - { - $this->_name = $name; - $this->_bucket = $bucket; - $this->_generation = $generation; - $this->_info = $info; - $this->_connection = $connection; - $timeCreated = new Datetime(); - $this->_info['metadata']['timeCreated'] = $timeCreated->format(GoogleCloudStorage::DATETIME_FORMAT); - } - - public function acl() - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function exists(array $options = array()) - { - return key_exists($this->_name, $this->_bucket->_objects); - } - - /** - * @throws NotFoundException - */ - public function delete(array $options = array()) - { - if (key_exists($this->_name, $this->_bucket->_objects)) { - unset($this->_bucket->_objects[$this->_name]); - } else { - throw new NotFoundException('key ' . $this->_name . ' not found.'); - } - } - - /** - * @throws NotFoundException - */ - public function update(array $metadata, array $options = array()) - { - if (!$this->_exists) { - throw new NotFoundException('key ' . $this->_name . ' not found.'); - } - $this->_info = $metadata; - } - - public function copy($destination, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function rewrite($destination, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function rename($name, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - /** - * @throws NotFoundException - */ - public function downloadAsString(array $options = array()) - { - if (!$this->_exists) { - throw new NotFoundException('key ' . $this->_name . ' not found.'); - } - return $this->_data; - } - - public function downloadToFile($path, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function downloadAsStream(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function downloadAsStreamAsync(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function signedUrl($expires, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function signedUploadUrl($expires, array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function beginSignedUploadSession(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function info(array $options = array()) - { - return key_exists('metadata',$this->_info) ? $this->_info['metadata'] : array(); - } - - public function reload(array $options = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function name() - { - return $this->_name; - } - - public function identity() - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function gcsUri() - { - return sprintf( - 'gs://%s/%s', - $this->_bucket->name(), - $this->_name - ); - } - - public function setData($data) - { - $this->_data = $data; - $this->_exists = true; - } -} - -/** - * Class ConnectionInterfaceStub required for the stubs. - */ -class ConnectionInterfaceStub implements ConnectionInterface -{ - public function deleteAcl(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getAcl(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function listAcl(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function insertAcl(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function patchAcl(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function deleteBucket(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getBucket(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function listBuckets(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function insertBucket(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getBucketIamPolicy(array $args) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function setBucketIamPolicy(array $args) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function testBucketIamPermissions(array $args) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function patchBucket(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function deleteObject(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function copyObject(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function rewriteObject(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function composeObject(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getObject(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function listObjects(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function patchObject(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function downloadObject(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function insertObject(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getNotification(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function deleteNotification(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function insertNotification(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function listNotifications(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getServiceAccount(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function lockRetentionPolicy(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function createHmacKey(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function deleteHmacKey(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function getHmacKey(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function updateHmacKey(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } - - public function listHmacKeys(array $args = array()) - { - throw new BadMethodCallException('not supported by this stub'); - } -} diff --git a/tst/ModelTest.php b/tst/ModelTest.php index 478d4c1..fb82442 100644 --- a/tst/ModelTest.php +++ b/tst/ModelTest.php @@ -102,6 +102,58 @@ class ModelTest extends PHPUnit_Framework_TestCase $this->assertEquals(array(), $paste->getComments(), 'comment was deleted with paste'); } + public function testPasteV1() + { + $pasteData = Helper::getPaste(1); + unset($pasteData['meta']['formatter']); + + $path = $this->_path . DIRECTORY_SEPARATOR . 'v1-test.sq3'; + if (is_file($path)) { + unlink($path); + } + $options = parse_ini_file(CONF_SAMPLE, true); + $options['purge']['limit'] = 0; + $options['model'] = array( + 'class' => 'Database', + ); + $options['model_options'] = array( + 'dsn' => 'sqlite:' . $path, + 'usr' => null, + 'pwd' => null, + 'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION), + ); + Helper::createIniFile(CONF, $options); + $model = new Model(new Configuration); + $model->getPaste('0000000000000000')->exists(); // triggers database table creation + $model->getPaste(Helper::getPasteId())->delete(); // deletes the cache + + $db = new PDO( + $options['model_options']['dsn'], + $options['model_options']['usr'], + $options['model_options']['pwd'], + $options['model_options']['opt'] + ); + $statement = $db->prepare('INSERT INTO paste VALUES(?,?,?,?,?,?,?,?,?)'); + $statement->execute( + array( + Helper::getPasteId(), + $pasteData['data'], + $pasteData['meta']['postdate'], + 0, + 0, + 0, + json_encode($pasteData['meta']), + null, + null, + ) + ); + $statement->closeCursor(); + + $paste = $model->getPaste(Helper::getPasteId()); + $paste->getDeleteToken(); + $this->assertEquals('plaintext', $paste->get()['meta']['formatter'], 'paste got created with default formatter'); + } + public function testCommentDefaults() { $comment = new Comment( @@ -133,6 +185,96 @@ class ModelTest extends PHPUnit_Framework_TestCase $paste->store(); } + /** + * @expectedException Exception + * @expectedExceptionCode 76 + */ + public function testStoreFail() + { + $path = $this->_path . DIRECTORY_SEPARATOR . 'model-store-test.sq3'; + if (is_file($path)) { + unlink($path); + } + $options = parse_ini_file(CONF_SAMPLE, true); + $options['purge']['limit'] = 0; + $options['model'] = array( + 'class' => 'Database', + ); + $options['model_options'] = array( + 'dsn' => 'sqlite:' . $path, + 'usr' => null, + 'pwd' => null, + 'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION), + ); + Helper::createIniFile(CONF, $options); + $model = new Model(new Configuration); + + $pasteData = Helper::getPastePost(); + $model->getPaste(Helper::getPasteId())->delete(); + $model->getPaste(Helper::getPasteId())->exists(); + + $db = new PDO( + $options['model_options']['dsn'], + $options['model_options']['usr'], + $options['model_options']['pwd'], + $options['model_options']['opt'] + ); + $statement = $db->prepare('DROP TABLE paste'); + $statement->execute(); + $statement->closeCursor(); + + $paste = $model->getPaste(); + $paste->setData($pasteData); + $paste->store(); + } + + /** + * @expectedException Exception + * @expectedExceptionCode 70 + */ + public function testCommentStoreFail() + { + $path = $this->_path . DIRECTORY_SEPARATOR . 'model-test.sq3'; + if (is_file($path)) { + unlink($path); + } + $options = parse_ini_file(CONF_SAMPLE, true); + $options['purge']['limit'] = 0; + $options['model'] = array( + 'class' => 'Database', + ); + $options['model_options'] = array( + 'dsn' => 'sqlite:' . $path, + 'usr' => null, + 'pwd' => null, + 'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION), + ); + Helper::createIniFile(CONF, $options); + $model = new Model(new Configuration); + + $pasteData = Helper::getPastePost(); + $commentData = Helper::getCommentPost(); + $model->getPaste(Helper::getPasteId())->delete(); + + $paste = $model->getPaste(); + $paste->setData($pasteData); + $paste->store(); + + $db = new PDO( + $options['model_options']['dsn'], + $options['model_options']['usr'], + $options['model_options']['pwd'], + $options['model_options']['opt'] + ); + $statement = $db->prepare('DROP TABLE comment'); + $statement->execute(); + $statement->closeCursor(); + + $comment = $paste->getComment(Helper::getPasteId()); + $comment->setData($commentData); + $comment->store(); + } + /** * @expectedException Exception * @expectedExceptionCode 69 @@ -195,6 +337,18 @@ class ModelTest extends PHPUnit_Framework_TestCase $paste->get(); } + /** + * @expectedException Exception + * @expectedExceptionCode 75 + */ + public function testInvalidPasteFormat() + { + $pasteData = Helper::getPastePost(); + $pasteData['adata'][1] = 'format does not exist'; + $paste = $this->_model->getPaste(); + $paste->setData($pasteData); + } + /** * @expectedException Exception * @expectedExceptionCode 60 From 1f2dddd9d85c3feb0dd4c210a41bc369252de3c1 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 10:53:01 +0200 Subject: [PATCH 14/30] address Codacy issues --- lib/Data/AbstractData.php | 8 ++++---- lib/Data/Database.php | 12 ++++++------ lib/Data/Filesystem.php | 10 +++++----- lib/Persistence/ServerSalt.php | 9 --------- 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/lib/Data/AbstractData.php b/lib/Data/AbstractData.php index 6f2dee5..52a85a4 100644 --- a/lib/Data/AbstractData.php +++ b/lib/Data/AbstractData.php @@ -35,7 +35,7 @@ abstract class AbstractData * @static * @var array */ - protected static $_traffic_limiter_cache = array(); + protected static $_last_cache = array(); /** * Enforce singleton, disable constructor @@ -150,9 +150,9 @@ abstract class AbstractData public function purgeValues($namespace, $time) { if ($namespace === 'traffic_limiter') { - foreach (self::$_traffic_limiter_cache as $key => $last_access) { - if ($last_access <= $time) { - unset(self::$_traffic_limiter_cache[$key]); + foreach (self::$_last_cache as $key => $last_submission) { + if ($last_submission <= $time) { + unset(self::$_last_cache[$key]); } } } diff --git a/lib/Data/Database.php b/lib/Data/Database.php index a15b862..dea1be6 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -447,9 +447,9 @@ class Database extends AbstractData public function setValue($value, $namespace, $key = '') { if ($namespace === 'traffic_limiter') { - self::$_traffic_limiter_cache[$key] = $value; + self::$_last_cache[$key] = $value; try { - $value = Json::encode(self::$_traffic_limiter_cache); + $value = Json::encode(self::$_last_cache); } catch (Exception $e) { return false; } @@ -492,12 +492,12 @@ class Database extends AbstractData } if ($value && $namespace === 'traffic_limiter') { try { - self::$_traffic_limiter_cache = Json::decode($value); + self::$_last_cache = Json::decode($value); } catch (Exception $e) { - self::$_traffic_limiter_cache = array(); + self::$_last_cache = array(); } - if (array_key_exists($key, self::$_traffic_limiter_cache)) { - return self::$_traffic_limiter_cache[$key]; + if (array_key_exists($key, self::$_last_cache)) { + return self::$_last_cache[$key]; } } return (string) $value; diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 384c72d..9b8e7ba 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -263,10 +263,10 @@ class Filesystem extends AbstractData ' Date: Sun, 13 Jun 2021 11:02:53 +0200 Subject: [PATCH 15/30] address Scrutinizer issues --- lib/Data/GoogleCloudStorage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index 2066769..d678db3 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -224,7 +224,7 @@ class GoogleCloudStorage extends AbstractData if (strlen($name) > strlen($path) && substr($name, strlen($path), 1) !== '/') { continue; } - $info = $object->info(); + $info = $object->info(); if (key_exists('metadata', $info) && key_exists('value', $info['metadata'])) { $value = $info['metadata']['value']; if (is_numeric($value) && intval($value) < $time) { @@ -293,7 +293,7 @@ class GoogleCloudStorage extends AbstractData $data = $o->downloadAsString(); return Json::decode($data); } catch (NotFoundException $e) { - return false; + return ''; } } From bbcf57de0ef1d1ae9b4a9281f630b25712f3b129 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 11:08:38 +0200 Subject: [PATCH 16/30] address Scrutinizer issues --- tst/Data/GoogleCloudStorageTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php index 557984e..2489743 100644 --- a/tst/Data/GoogleCloudStorageTest.php +++ b/tst/Data/GoogleCloudStorageTest.php @@ -139,7 +139,7 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $storedSalt = $this->_model->getValue('salt', ''); $this->assertEquals($salt, $storedSalt); $this->_model->purgeValues('salt', time() + 60); - $this->assertFalse($this->_model->getValue('salt', 'master')); + $this->assertEquals('', $this->_model->getValue('salt', 'master')); $client = hash_hmac('sha512', '127.0.0.1', $salt); $expire = time(); @@ -150,14 +150,14 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $this->_model->purgeValues('traffic_limiter', time() - 60); $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); $this->_model->purgeValues('traffic_limiter', time() + 60); - $this->assertFalse($this->_model->getValue('traffic_limiter', $client)); + $this->assertEquals('', $this->_model->getValue('traffic_limiter', $client)); $purgeAt = $expire + (15 * 60); $this->_model->setValue($purgeAt, 'purge_limiter', ''); $storedPurgedAt = $this->_model->getValue('purge_limiter', ''); $this->assertEquals($purgeAt, $storedPurgedAt); $this->_model->purgeValues('purge_limiter', time() + 60); - $this->assertFalse($this->_model->getValue('purge_limiter', 'at')); + $this->assertEquals('', $this->_model->getValue('purge_limiter', 'at')); } /** From fa4fe2852d09e39234bdf5618fb884a4c7160486 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 11:12:19 +0200 Subject: [PATCH 17/30] address Scrutinizer issues --- tst/Data/GoogleCloudStorageTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php index 2489743..827297a 100644 --- a/tst/Data/GoogleCloudStorageTest.php +++ b/tst/Data/GoogleCloudStorageTest.php @@ -176,7 +176,7 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); $this->_model->purgeValues('traffic_limiter', time() + 60); - $this->assertFalse($this->_model->getValue('traffic_limiter', $client)); + $this->assertEquals('', $this->_model->getValue('traffic_limiter', $client)); } public function testKeyValuePurgeTrafficLimiterWithKey() @@ -192,6 +192,6 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); $this->_model->purgeValues('traffic_limiter', time() + 60); - $this->assertFalse($this->_model->getValue('traffic_limiter', $client)); + $this->assertEquals('', $this->_model->getValue('traffic_limiter', $client)); } } From 68b097087d8bb6af219967031447859019d8af56 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 11:16:29 +0200 Subject: [PATCH 18/30] apply StyleCI recommendation --- lib/Request.php | 4 ++-- tst/Bootstrap.php | 2 -- tst/ModelTest.php | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/Request.php b/lib/Request.php index df517bb..858f131 100644 --- a/lib/Request.php +++ b/lib/Request.php @@ -110,7 +110,7 @@ class Request case 'POST': // it might be a creation or a deletion, the latter is detected below $this->_operation = 'create'; - $this->_params = Json::decode( + $this->_params = Json::decode( file_get_contents(self::$_inputStream) ); break; @@ -130,7 +130,7 @@ class Request if (array_key_exists('pasteid', $this->_params) && !empty($this->_params['pasteid'])) { if (array_key_exists('deletetoken', $this->_params) && !empty($this->_params['deletetoken'])) { $this->_operation = 'delete'; - } else if ($this->_operation != 'create') { + } elseif ($this->_operation != 'create') { $this->_operation = 'read'; } } elseif (array_key_exists('jsonld', $this->_params) && !empty($this->_params['jsonld'])) { diff --git a/tst/Bootstrap.php b/tst/Bootstrap.php index bdf8a38..97f9405 100644 --- a/tst/Bootstrap.php +++ b/tst/Bootstrap.php @@ -1,13 +1,11 @@ _model->getPaste(); + $paste = $this->_model->getPaste(); $paste->setData($pasteData); } From 078c5785ddf09dfaac3c3c01e7848664685a69ee Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 12:40:06 +0200 Subject: [PATCH 19/30] fix unit tests on php < 7.3 --- lib/Data/Database.php | 26 ++++++++++++++++---------- tst/ModelTest.php | 1 + 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/Data/Database.php b/lib/Data/Database.php index dea1be6..1d1327f 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -233,11 +233,14 @@ class Database extends AbstractData } self::$_cache[$pasteid] = false; - $paste = self::_select( - 'SELECT * FROM ' . self::_sanitizeIdentifier('paste') . - ' WHERE dataid = ?', array($pasteid), true - ); - + try { + $paste = self::_select( + 'SELECT * FROM ' . self::_sanitizeIdentifier('paste') . + ' WHERE dataid = ?', array($pasteid), true + ); + } catch (Exception $e) { + $paste = false; + } if ($paste === false) { return false; } @@ -643,15 +646,18 @@ class Database extends AbstractData * @access private * @static * @param string $key - * @throws PDOException * @return string */ private static function _getConfig($key) { - $row = self::_select( - 'SELECT value FROM ' . self::_sanitizeIdentifier('config') . - ' WHERE id = ?', array($key), true - ); + try { + $row = self::_select( + 'SELECT value FROM ' . self::_sanitizeIdentifier('config') . + ' WHERE id = ?', array($key), true + ); + } catch (PDOException $e) { + return ''; + } return $row ? $row['value'] : ''; } diff --git a/tst/ModelTest.php b/tst/ModelTest.php index 327e701..9432124 100644 --- a/tst/ModelTest.php +++ b/tst/ModelTest.php @@ -259,6 +259,7 @@ class ModelTest extends PHPUnit_Framework_TestCase $paste = $model->getPaste(); $paste->setData($pasteData); $paste->store(); + $paste->exists(); $db = new PDO( $options['model_options']['dsn'], From d0248d55d356914479e670c76342b9daaa535634 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 12:43:18 +0200 Subject: [PATCH 20/30] address Scrutinizer issues --- lib/Data/GoogleCloudStorage.php | 2 +- lib/Json.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index d678db3..c7dfcb9 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -291,7 +291,7 @@ class GoogleCloudStorage extends AbstractData try { $o = $this->_bucket->object($key); $data = $o->downloadAsString(); - return Json::decode($data); + return (string) Json::decode($data); } catch (NotFoundException $e) { return ''; } diff --git a/lib/Json.php b/lib/Json.php index b6567ed..5f4efcf 100644 --- a/lib/Json.php +++ b/lib/Json.php @@ -44,13 +44,13 @@ class Json * @static * @param string $input * @throws Exception - * @return array + * @return mixed */ public static function decode($input) { - $array = json_decode($input, true); + $output = json_decode($input, true); self::_detectError(); - return $array; + return $output; } /** From 9357f122b7c72bbf27ed0d018355b8d762decb97 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sun, 13 Jun 2021 12:49:59 +0200 Subject: [PATCH 21/30] address Scrutinizer issues --- lib/Data/GoogleCloudStorage.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index c7dfcb9..3f5c8e2 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -21,7 +21,6 @@ class GoogleCloudStorage extends AbstractData */ public static function getInstance(array $options) { - $client = null; $bucket = null; $prefix = 'pastes'; From b4c75b541ba37896896d0b01d2ecdf4bb4b036ca Mon Sep 17 00:00:00 2001 From: Mark van Holsteijn Date: Sun, 13 Jun 2021 21:16:30 +0200 Subject: [PATCH 22/30] removed json encoding from get/setValue --- lib/Data/GoogleCloudStorage.php | 9 +++----- tst/Data/GoogleCloudStorageTest.php | 33 ++++++++--------------------- 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index 3f5c8e2..1c26a14 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -253,14 +253,12 @@ class GoogleCloudStorage extends AbstractData $key = 'config/' . $namespace . '/' . $key; } - $data = Json::encode($value); - $metadata = array('namespace' => $namespace); if ($namespace != 'salt') { $metadata['value'] = strval($value); } try { - $this->_bucket->upload($data, array( + $this->_bucket->upload($value, array( 'name' => $key, 'chunkSize' => 262144, 'predefinedAcl' => 'private', @@ -288,9 +286,8 @@ class GoogleCloudStorage extends AbstractData $key = 'config/' . $namespace . '/' . $key; } try { - $o = $this->_bucket->object($key); - $data = $o->downloadAsString(); - return (string) Json::decode($data); + $o = $this->_bucket->object($key); + return $o->downloadAsString(); } catch (NotFoundException $e) { return ''; } diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php index 827297a..3b101c4 100644 --- a/tst/Data/GoogleCloudStorageTest.php +++ b/tst/Data/GoogleCloudStorageTest.php @@ -143,20 +143,21 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $client = hash_hmac('sha512', '127.0.0.1', $salt); $expire = time(); - $this->_model->setValue($expire, 'traffic_limiter', $client); + $this->_model->setValue(strval($expire), 'traffic_limiter', $client); $storedExpired = $this->_model->getValue('traffic_limiter', $client); - $this->assertEquals($expire, $storedExpired); - $this->assertEquals($expire, $storedExpired); + $this->assertEquals(strval($expire), $storedExpired); + $this->_model->purgeValues('traffic_limiter', time() - 60); $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); $this->_model->purgeValues('traffic_limiter', time() + 60); $this->assertEquals('', $this->_model->getValue('traffic_limiter', $client)); $purgeAt = $expire + (15 * 60); - $this->_model->setValue($purgeAt, 'purge_limiter', ''); + $this->_model->setValue(strval($purgeAt), 'purge_limiter', ''); $storedPurgedAt = $this->_model->getValue('purge_limiter', ''); - $this->assertEquals($purgeAt, $storedPurgedAt); - $this->_model->purgeValues('purge_limiter', time() + 60); + $this->assertEquals(strval($purgeAt), $storedPurgedAt); + $this->_model->purgeValues('purge_limiter', $purgeAt + 60); + $this->assertEquals('', $this->_model->getValue('purge_limiter', '')); $this->assertEquals('', $this->_model->getValue('purge_limiter', 'at')); } @@ -168,25 +169,9 @@ class GoogleCloudStorageTest extends PHPUnit_Framework_TestCase $salt = bin2hex(random_bytes(256)); $client = hash_hmac('sha512', '127.0.0.1', $salt); $expire = time(); - $this->_model->setValue($expire, 'traffic_limiter', $client); + $this->_model->setValue(strval($expire), 'traffic_limiter', $client); $storedExpired = $this->_model->getValue('traffic_limiter', $client); - $this->assertEquals($expire, $storedExpired); - - $this->_model->purgeValues('traffic_limiter', time() - 60); - $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); - - $this->_model->purgeValues('traffic_limiter', time() + 60); - $this->assertEquals('', $this->_model->getValue('traffic_limiter', $client)); - } - - public function testKeyValuePurgeTrafficLimiterWithKey() - { - $salt = bin2hex(random_bytes(256)); - $client = hash_hmac('sha512', '127.0.0.1', $salt); - $expire = time(); - $this->_model->setValue($expire, 'traffic_limiter', $client); - $storedExpired = $this->_model->getValue('traffic_limiter', $client); - $this->assertEquals($expire, $storedExpired); + $this->assertEquals(strval($expire), $storedExpired); $this->_model->purgeValues('traffic_limiter', time() - 60); $this->assertEquals($storedExpired, $this->_model->getValue('traffic_limiter', $client)); From 3327645fd40bc0791b6111087b3e2b66399514ff Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 14 Jun 2021 06:44:30 +0200 Subject: [PATCH 23/30] updated doc blocks, comments, fixed indentations, moved some constant strings --- lib/Data/AbstractData.php | 6 +++--- lib/Data/Database.php | 2 +- lib/Data/Filesystem.php | 27 +++++++++++++++++++-------- lib/Data/GoogleCloudStorage.php | 2 -- tst/Bootstrap.php | 2 +- 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/lib/Data/AbstractData.php b/lib/Data/AbstractData.php index 52a85a4..591b91f 100644 --- a/lib/Data/AbstractData.php +++ b/lib/Data/AbstractData.php @@ -15,7 +15,7 @@ namespace PrivateBin\Data; /** * AbstractData * - * Abstract model for PrivateBin data access, implemented as a singleton. + * Abstract model for data access, implemented as a singleton. */ abstract class AbstractData { @@ -40,7 +40,7 @@ abstract class AbstractData /** * Enforce singleton, disable constructor * - * Instantiate using {@link getInstance()}, privatebin is a singleton object. + * Instantiate using {@link getInstance()}, this object implements the singleton pattern. * * @access protected */ @@ -51,7 +51,7 @@ abstract class AbstractData /** * Enforce singleton, disable cloning * - * Instantiate using {@link getInstance()}, privatebin is a singleton object. + * Instantiate using {@link getInstance()}, this object implements the singleton pattern. * * @access private */ diff --git a/lib/Data/Database.php b/lib/Data/Database.php index 1d1327f..0c66d33 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -234,7 +234,7 @@ class Database extends AbstractData self::$_cache[$pasteid] = false; try { - $paste = self::_select( + $paste = self::_select( 'SELECT * FROM ' . self::_sanitizeIdentifier('paste') . ' WHERE dataid = ?', array($pasteid), true ); diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 9b8e7ba..25cca45 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -23,12 +23,19 @@ use PrivateBin\Json; class Filesystem extends AbstractData { /** - * first line in file, to protect its contents + * first line in paste or comment files, to protect their contents from browsing exposed data directories * * @const string */ const PROTECTION_LINE = '_info = $info; $this->_connection = $connection; $timeCreated = new Datetime(); - $this->_info['metadata']['timeCreated'] = $timeCreated->format(GoogleCloudStorage::DATETIME_FORMAT); + $this->_info['metadata']['timeCreated'] = $timeCreated->format('Y-m-d\TH:i:s.u\Z'); } public function acl() From af54e70359ae5994abc9f505e33cea6fdccf639c Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 14 Jun 2021 06:48:46 +0200 Subject: [PATCH 24/30] apply StyleCI recommendation --- tst/Bootstrap.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tst/Bootstrap.php b/tst/Bootstrap.php index 9d9f07c..70aafdd 100644 --- a/tst/Bootstrap.php +++ b/tst/Bootstrap.php @@ -6,7 +6,6 @@ use Google\Cloud\Storage\Bucket; use Google\Cloud\Storage\Connection\ConnectionInterface; use Google\Cloud\Storage\StorageClient; use Google\Cloud\Storage\StorageObject; -use PrivateBin\Data\GoogleCloudStorage; use PrivateBin\Persistence\ServerSalt; error_reporting(E_ALL | E_STRICT); From ae1e4e3edbd583a472eb5ad683b7bab28ad596e0 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 16 Jun 2021 04:39:24 +0200 Subject: [PATCH 25/30] clarify use of getDeleteToken() method in unit test --- tst/ModelTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tst/ModelTest.php b/tst/ModelTest.php index 9432124..a88e029 100644 --- a/tst/ModelTest.php +++ b/tst/ModelTest.php @@ -150,7 +150,7 @@ class ModelTest extends PHPUnit_Framework_TestCase $statement->closeCursor(); $paste = $model->getPaste(Helper::getPasteId()); - $paste->getDeleteToken(); + $this->assertNotEmpty($paste->getDeleteToken(), 'excercise the condition to load the data from storage'); $this->assertEquals('plaintext', $paste->get()['meta']['formatter'], 'paste got created with default formatter'); } From 3d9ba10fcb10d0222956110dd9f170a80fff1a7e Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 16 Jun 2021 05:19:45 +0200 Subject: [PATCH 26/30] more consistent AbstractData implementation --- lib/Data/GoogleCloudStorage.php | 112 +++++++++++++++++++------------- 1 file changed, 66 insertions(+), 46 deletions(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index d6189e1..cc4a4c6 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -9,6 +9,33 @@ use PrivateBin\Json; class GoogleCloudStorage extends AbstractData { + /** + * GCS client + * + * @access private + * @static + * @var StorageClient + */ + private static $_client = null; + + /** + * GCS bucket + * + * @access private + * @static + * @var Bucket + */ + private static $_bucket = null; + + /** + * object prefix + * + * @access private + * @static + * @var string + */ + private static $_prefix = 'pastes'; + /** * returns a Google Cloud Storage data backend. * @@ -19,9 +46,12 @@ class GoogleCloudStorage extends AbstractData */ public static function getInstance(array $options) { - $bucket = null; - $prefix = 'pastes'; + // if needed initialize the singleton + if (!(self::$_instance instanceof self)) { + self::$_instance = new self; + } + $bucket = null; if (getenv('PRIVATEBIN_GCS_BUCKET')) { $bucket = getenv('PRIVATEBIN_GCS_BUCKET'); } @@ -29,46 +59,36 @@ class GoogleCloudStorage extends AbstractData $bucket = $options['bucket']; } if (is_array($options) && array_key_exists('prefix', $options)) { - $prefix = $options['prefix']; + self::$_prefix = $options['prefix']; } - if (!(self::$_instance instanceof self)) { - self::$_instance = new self($bucket, $prefix); + if (empty(self::$_client)) { + self::$_client = class_exists('StorageClientStub', false) ? + new \StorageClientStub(array()) : + new StorageClient(array('suppressKeyFileNotice' => true)); } + self::$_bucket = self::$_client->bucket($bucket); + return self::$_instance; } - protected $_client = null; - protected $_bucket = null; - protected $_prefix = 'pastes'; - - public function __construct($bucket, $prefix) - { - parent::__construct(); - $this->_client = class_exists('StorageClientStub', false) ? - new \StorageClientStub(array()) : - new StorageClient(array('suppressKeyFileNotice' => true)); - $this->_bucket = $this->_client->bucket($bucket); - if ($prefix != null) { - $this->_prefix = $prefix; - } - } - /** - * returns the google storage object key for $pasteid in $this->_bucket. + * returns the google storage object key for $pasteid in self::$_bucket. + * + * @access private * @param $pasteid string to get the key for * @return string */ private function _getKey($pasteid) { - if ($this->_prefix != '') { - return $this->_prefix . '/' . $pasteid; + if (self::$_prefix != '') { + return self::$_prefix . '/' . $pasteid; } return $pasteid; } /** - * Uploads the payload in the $this->_bucket under the specified key. + * Uploads the payload in the self::$_bucket under the specified key. * The entire payload is stored as a JSON document. The metadata is replicated * as the GCS object's metadata except for the fields attachment, attachmentname * and salt. @@ -77,7 +97,7 @@ class GoogleCloudStorage extends AbstractData * @param $payload array to store * @return bool true if successful, otherwise false. */ - private function upload($key, $payload) + private function _upload($key, $payload) { $metadata = array_key_exists('meta', $payload) ? $payload['meta'] : array(); unset($metadata['attachment'], $metadata['attachmentname'], $metadata['salt']); @@ -85,7 +105,7 @@ class GoogleCloudStorage extends AbstractData $metadata[$k] = strval($v); } try { - $this->_bucket->upload(Json::encode($payload), array( + self::$_bucket->upload(Json::encode($payload), array( 'name' => $key, 'chunkSize' => 262144, 'predefinedAcl' => 'private', @@ -95,7 +115,7 @@ class GoogleCloudStorage extends AbstractData ), )); } catch (Exception $e) { - error_log('failed to upload ' . $key . ' to ' . $this->_bucket->name() . ', ' . + error_log('failed to upload ' . $key . ' to ' . self::$_bucket->name() . ', ' . trim(preg_replace('/\s\s+/', ' ', $e->getMessage()))); return false; } @@ -111,7 +131,7 @@ class GoogleCloudStorage extends AbstractData return false; } - return $this->upload($this->_getKey($pasteid), $paste); + return $this->_upload($this->_getKey($pasteid), $paste); } /** @@ -120,13 +140,13 @@ class GoogleCloudStorage extends AbstractData public function read($pasteid) { try { - $o = $this->_bucket->object($this->_getKey($pasteid)); + $o = self::$_bucket->object($this->_getKey($pasteid)); $data = $o->downloadAsString(); return Json::decode($data); } catch (NotFoundException $e) { return false; } catch (Exception $e) { - error_log('failed to read ' . $pasteid . ' from ' . $this->_bucket->name() . ', ' . + error_log('failed to read ' . $pasteid . ' from ' . self::$_bucket->name() . ', ' . trim(preg_replace('/\s\s+/', ' ', $e->getMessage()))); return false; } @@ -140,9 +160,9 @@ class GoogleCloudStorage extends AbstractData $name = $this->_getKey($pasteid); try { - foreach ($this->_bucket->objects(array('prefix' => $name . '/discussion/')) as $comment) { + foreach (self::$_bucket->objects(array('prefix' => $name . '/discussion/')) as $comment) { try { - $this->_bucket->object($comment->name())->delete(); + self::$_bucket->object($comment->name())->delete(); } catch (NotFoundException $e) { // ignore if already deleted. } @@ -152,7 +172,7 @@ class GoogleCloudStorage extends AbstractData } try { - $this->_bucket->object($name)->delete(); + self::$_bucket->object($name)->delete(); } catch (NotFoundException $e) { // ignore if already deleted } @@ -163,7 +183,7 @@ class GoogleCloudStorage extends AbstractData */ public function exists($pasteid) { - $o = $this->_bucket->object($this->_getKey($pasteid)); + $o = self::$_bucket->object($this->_getKey($pasteid)); return $o->exists(); } @@ -176,7 +196,7 @@ class GoogleCloudStorage extends AbstractData return false; } $key = $this->_getKey($pasteid) . '/discussion/' . $parentid . '/' . $commentid; - return $this->upload($key, $comment); + return $this->_upload($key, $comment); } /** @@ -187,8 +207,8 @@ class GoogleCloudStorage extends AbstractData $comments = array(); $prefix = $this->_getKey($pasteid) . '/discussion/'; try { - foreach ($this->_bucket->objects(array('prefix' => $prefix)) as $key) { - $comment = JSON::decode($this->_bucket->object($key->name())->downloadAsString()); + foreach (self::$_bucket->objects(array('prefix' => $prefix)) as $key) { + $comment = JSON::decode(self::$_bucket->object($key->name())->downloadAsString()); $comment['id'] = basename($key->name()); $slot = $this->getOpenSlot($comments, (int) $comment['meta']['created']); $comments[$slot] = $comment; @@ -205,7 +225,7 @@ class GoogleCloudStorage extends AbstractData public function existsComment($pasteid, $parentid, $commentid) { $name = $this->_getKey($pasteid) . '/discussion/' . $parentid . '/' . $commentid; - $o = $this->_bucket->object($name); + $o = self::$_bucket->object($name); return $o->exists(); } @@ -216,7 +236,7 @@ class GoogleCloudStorage extends AbstractData { $path = 'config/' . $namespace; try { - foreach ($this->_bucket->objects(array('prefix' => $path)) as $object) { + foreach (self::$_bucket->objects(array('prefix' => $path)) as $object) { $name = $object->name(); if (strlen($name) > strlen($path) && substr($name, strlen($path), 1) !== '/') { continue; @@ -256,7 +276,7 @@ class GoogleCloudStorage extends AbstractData $metadata['value'] = strval($value); } try { - $this->_bucket->upload($value, array( + self::$_bucket->upload($value, array( 'name' => $key, 'chunkSize' => 262144, 'predefinedAcl' => 'private', @@ -266,7 +286,7 @@ class GoogleCloudStorage extends AbstractData ), )); } catch (Exception $e) { - error_log('failed to set key ' . $key . ' to ' . $this->_bucket->name() . ', ' . + error_log('failed to set key ' . $key . ' to ' . self::$_bucket->name() . ', ' . trim(preg_replace('/\s\s+/', ' ', $e->getMessage()))); return false; } @@ -284,7 +304,7 @@ class GoogleCloudStorage extends AbstractData $key = 'config/' . $namespace . '/' . $key; } try { - $o = $this->_bucket->object($key); + $o = self::$_bucket->object($key); return $o->downloadAsString(); } catch (NotFoundException $e) { return ''; @@ -299,12 +319,12 @@ class GoogleCloudStorage extends AbstractData $expired = array(); $now = time(); - $prefix = $this->_prefix; + $prefix = self::$_prefix; if ($prefix != '') { - $prefix = $prefix . '/'; + $prefix .= '/'; } try { - foreach ($this->_bucket->objects(array('prefix' => $prefix)) as $object) { + foreach (self::$_bucket->objects(array('prefix' => $prefix)) as $object) { $metadata = $object->info()['metadata']; if ($metadata != null && array_key_exists('expire_date', $metadata)) { $expire_at = intval($metadata['expire_date']); From fd08d991fe073cce7aee254ebaf48c246c833225 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 16 Jun 2021 05:32:45 +0200 Subject: [PATCH 27/30] log errors storing persistance --- lib/Persistence/PurgeLimiter.php | 7 +++++-- lib/Persistence/ServerSalt.php | 4 +++- lib/Persistence/TrafficLimiter.php | 4 +++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/Persistence/PurgeLimiter.php b/lib/Persistence/PurgeLimiter.php index 89d5d60..ef37770 100644 --- a/lib/Persistence/PurgeLimiter.php +++ b/lib/Persistence/PurgeLimiter.php @@ -73,7 +73,10 @@ class PurgeLimiter extends AbstractPersistence if ($pl + self::$_limit >= $now) { return false; } - self::$_store->setValue((string) $now, 'purge_limiter'); - return true; + $hasStored = self::$_store->setValue((string) $now, 'purge_limiter'); + if (!$hasStored) { + error_log('failed to store the purge limiter, skipping purge cycle to avoid getting stuck in a purge loop'); + } + return $hasStored; } } diff --git a/lib/Persistence/ServerSalt.php b/lib/Persistence/ServerSalt.php index 50e1cd6..1095498 100644 --- a/lib/Persistence/ServerSalt.php +++ b/lib/Persistence/ServerSalt.php @@ -65,7 +65,9 @@ class ServerSalt extends AbstractPersistence self::$_salt = $salt; } else { self::$_salt = self::generate(); - self::$_store->setValue(self::$_salt, 'salt'); + if (!self::$_store->setValue(self::$_salt, 'salt')) { + error_log('failed to store the server salt, delete tokens, traffic limiter and user icons won\'t work'); + } } return self::$_salt; } diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index 4f11ec7..9e896c1 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -172,7 +172,9 @@ class TrafficLimiter extends AbstractPersistence $tl = time(); $result = true; } - self::$_store->setValue((string) $tl, 'traffic_limiter', $hash); + if (!self::$_store->setValue((string) $tl, 'traffic_limiter', $hash)) { + error_log('failed to store the traffic limiter, it probably contains outdated information'); + } return $result; } } From be164bb6a939e0293db4a869f6a3eb238fb6a1cb Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 16 Jun 2021 05:43:18 +0200 Subject: [PATCH 28/30] apply StyleCI recommendation --- lib/Data/GoogleCloudStorage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index cc4a4c6..9cd7f25 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -74,7 +74,7 @@ class GoogleCloudStorage extends AbstractData /** * returns the google storage object key for $pasteid in self::$_bucket. - * + * * @access private * @param $pasteid string to get the key for * @return string From 9c09018e6ed30d51b99b3647cae2905c1ab391da Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 16 Jun 2021 05:50:41 +0200 Subject: [PATCH 29/30] address Scrutinizer issues --- lib/Data/Filesystem.php | 9 +++++---- lib/Data/GoogleCloudStorage.php | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 25cca45..577ad34 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -99,12 +99,13 @@ class Filesystem extends AbstractData */ public function read($pasteid) { - if (!$this->exists($pasteid)) { + if ( + !$this->exists($pasteid) || + !$paste = self::_get(self::_dataid2path($pasteid) . $pasteid . '.php') + ) { return false; } - return self::upgradePreV1Format( - self::_get(self::_dataid2path($pasteid) . $pasteid . '.php') - ); + return self::upgradePreV1Format($paste); } /** diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index 9cd7f25..f110230 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -23,7 +23,7 @@ class GoogleCloudStorage extends AbstractData * * @access private * @static - * @var Bucket + * @var \Google\Cloud\Storage\Bucket */ private static $_bucket = null; From 1fd998f325d72ad0273c714ad39a441a41492730 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Wed, 16 Jun 2021 05:57:26 +0200 Subject: [PATCH 30/30] address Scrutinizer issues --- lib/Data/GoogleCloudStorage.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index f110230..2e8e2c5 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -4,6 +4,7 @@ namespace PrivateBin\Data; use Exception; use Google\Cloud\Core\Exception\NotFoundException; +use Google\Cloud\Storage\Bucket; use Google\Cloud\Storage\StorageClient; use PrivateBin\Json; @@ -23,7 +24,7 @@ class GoogleCloudStorage extends AbstractData * * @access private * @static - * @var \Google\Cloud\Storage\Bucket + * @var Bucket */ private static $_bucket = null;