diff --git a/CHANGELOG.md b/CHANGELOG.md index 856d8613..67cf0514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # PrivateBin version history + * **1.5.1 (not yet released)** + * FIXED: Revert Filesystem purge to limited and randomized lookup (#1030) + * FIXED: Catch JSON decode errors when invalid data gets sent to the API (#1030) + * FIXED: Support sorting v1 format in mixed version comments in Filesystem backend (#1030) * **1.5 (2022-12-11)** * ADDED: script for data storage backend migrations (#1012) * ADDED: Translations for Turkish, Slovak, Greek and Thai diff --git a/CREDITS.md b/CREDITS.md index 7a19c5fd..ace41432 100644 --- a/CREDITS.md +++ b/CREDITS.md @@ -29,9 +29,8 @@ * rodehoed - option to exempt ips from the rate-limiter * Mark van Holsteijn - Google Cloud Storage backend * Austin Huang - Oracle database support -* Felix J. Ogris - S3 Storage backend +* Felix J. Ogris - S3 Storage backend, script for data backend migrations, dropped singleton behaviour of data backends * Mounir Idrassi & J. Mozdzen - secure YOURLS integration -* Felix J. Ogris - script for data backend migrations, dropped singleton behaviour of data backends ## Translations * Hexalyse - French diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 0f815414..c1b720e7 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -228,7 +228,13 @@ class Filesystem extends AbstractData $comment['parentid'] = $items[2]; // Store in array - $key = $this->getOpenSlot($comments, (int) $comment['meta']['created']); + $key = $this->getOpenSlot( + $comments, ( + (int) array_key_exists('created', $comment['meta']) ? + $comment['meta']['created'] : // v2 comments + $comment['meta']['postdate'] // v1 comments + ) + ); $comments[$key] = $comment; } } @@ -358,12 +364,12 @@ class Filesystem extends AbstractData { $pastes = array(); $count = 0; + $opened = 0; + $limit = $batchsize * 10; // try at most 10 times $batchsize pastes before giving up $time = time(); - foreach ($this->_getPasteIterator() as $file) { - if ($file->isDir()) { - continue; - } - $pasteid = $file->getBasename('.php'); + $files = $this->getAllPastes(); + shuffle($files); + foreach ($files as $pasteid) { if ($this->exists($pasteid)) { $data = $this->read($pasteid); if ( @@ -371,11 +377,13 @@ class Filesystem extends AbstractData $data['meta']['expire_date'] < $time ) { $pastes[] = $pasteid; - ++$count; - if ($count >= $batchsize) { + if (++$count >= $batchsize) { break; } } + if (++$opened >= $limit) { + break; + } } } return $pastes; @@ -387,7 +395,7 @@ class Filesystem extends AbstractData public function getAllPastes() { $pastes = array(); - foreach ($this->_getPasteIterator() as $file) { + foreach (new \GlobIterator($this->_path . self::PASTE_FILE_PATTERN) as $file) { if ($file->isFile()) { $pastes[] = $file->getBasename('.php'); } @@ -431,20 +439,6 @@ class Filesystem extends AbstractData '.discussion' . DIRECTORY_SEPARATOR; } - /** - * Get an iterator matching paste files. - * - * Note that creating the iterator issues the glob() call, so we can't pre- - * generate this object before files that should get matched exist. - * - * @access private - * @return \GlobIterator - */ - private function _getPasteIterator() - { - return new \GlobIterator($this->_path . self::PASTE_FILE_PATTERN); - } - /** * store the data * diff --git a/lib/Request.php b/lib/Request.php index 0d5f096b..5e1bb3fb 100644 --- a/lib/Request.php +++ b/lib/Request.php @@ -12,6 +12,8 @@ namespace PrivateBin; +use Exception; + /** * Request * @@ -110,9 +112,13 @@ class Request 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) - ); + try { + $this->_params = Json::decode( + file_get_contents(self::$_inputStream) + ); + } catch (Exception $e) { + // ignore error, $this->_params will remain empty + } break; default: $this->_params = $_GET; diff --git a/tst/Bootstrap.php b/tst/Bootstrap.php index 48a91cb8..bcf622f4 100644 --- a/tst/Bootstrap.php +++ b/tst/Bootstrap.php @@ -149,7 +149,7 @@ class BucketStub extends Bucket throw new BadMethodCallException('not supported by this stub'); } - public function exists() + public function exists(array $options = array()) { return true; } diff --git a/tst/ControllerTest.php b/tst/ControllerTest.php index 5c1ae2b2..8aee7829 100644 --- a/tst/ControllerTest.php +++ b/tst/ControllerTest.php @@ -450,9 +450,12 @@ class ControllerTest extends TestCase $_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REMOTE_ADDR'] = '::1'; $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste does not exists before posting data'); - $this->expectException(Exception::class); - $this->expectExceptionCode(90); + 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'); } diff --git a/tst/RequestTest.php b/tst/RequestTest.php index bf7edfb3..6734ba3b 100644 --- a/tst/RequestTest.php +++ b/tst/RequestTest.php @@ -88,7 +88,7 @@ class RequestTest extends TestCase Request::setInputStream($file); $request = new Request; unlink($file); - $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call'); + $this->assertTrue($request->isJsonApiCall(), 'is JSON API call'); $this->assertEquals('create', $request->getOperation()); $this->assertEquals('foo', $request->getParam('ct')); } @@ -102,7 +102,7 @@ class RequestTest extends TestCase file_put_contents($file, '{"ct":"foo"}'); Request::setInputStream($file); $request = new Request; - $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call'); + $this->assertTrue($request->isJsonApiCall(), 'is JSON API call'); $this->assertEquals('create', $request->getOperation()); $this->assertEquals('foo', $request->getParam('ct')); } @@ -116,7 +116,7 @@ class RequestTest extends TestCase $_SERVER['QUERY_STRING'] = $id; $_GET[$id] = ''; $request = new Request; - $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call'); + $this->assertTrue($request->isJsonApiCall(), 'is JSON API call'); $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('read', $request->getOperation()); } @@ -133,12 +133,25 @@ class RequestTest extends TestCase file_put_contents($file, '{"deletetoken":"bar"}'); Request::setInputStream($file); $request = new Request; - $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call'); + $this->assertTrue($request->isJsonApiCall(), 'is JSON API call'); $this->assertEquals('delete', $request->getOperation()); $this->assertEquals($id, $request->getParam('pasteid')); $this->assertEquals('bar', $request->getParam('deletetoken')); } + public function testPostGarbage() + { + $this->reset(); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $file = tempnam(sys_get_temp_dir(), 'FOO'); + file_put_contents($file, random_bytes(256)); + Request::setInputStream($file); + $request = new Request; + unlink($file); + $this->assertFalse($request->isJsonApiCall(), 'is HTML call'); + $this->assertEquals('create', $request->getOperation()); + } + public function testReadWithNegotiation() { $this->reset();