From 7901ec74a7167c6fb65866826d7c21e936427c4c Mon Sep 17 00:00:00 2001 From: El RIDO Date: Tue, 8 Jun 2021 22:01:29 +0200 Subject: [PATCH] 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 095bbebb..4a1aa0d7 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 3a481d92..8b443ad4 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 73f0ab2e..4e61a8ee 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 329a8ef2..93f5486e 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 f6bc07fb..11468614 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 92683ced..165ae089 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 17b699f1..6d9732a5 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 d5c4074f..478d4c10 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 ecdc0f83..3db5f7d7 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 41013016..2e81d83a 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 afcda562..abfb8c49 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()