From 62c11fc7825e5f8a80a381ae39a35219cff4742b Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 12 Dec 2022 20:45:46 +0100 Subject: [PATCH 1/7] duplications --- CREDITS.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 From 38574f0196d041597e25046740b644d1a2def892 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 12 Dec 2022 20:46:47 +0100 Subject: [PATCH 2/7] return invalid data error on API instead of exception --- lib/Request.php | 12 +++++++++--- tst/ControllerTest.php | 7 +++++-- tst/RequestTest.php | 21 +++++++++++++++++---- 3 files changed, 31 insertions(+), 9 deletions(-) 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/ControllerTest.php b/tst/ControllerTest.php index 698d5f86..c1876f7f 100644 --- a/tst/ControllerTest.php +++ b/tst/ControllerTest.php @@ -436,8 +436,6 @@ class ControllerTest extends PHPUnit_Framework_TestCase * silently removed, check that this case is handled * * @runInSeparateProcess - * @expectedException Exception - * @expectedExceptionCode 90 */ public function testCreateBrokenUpload() { @@ -449,7 +447,12 @@ class ControllerTest extends PHPUnit_Framework_TestCase $_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REMOTE_ADDR'] = '::1'; $this->assertFalse($this->_data->exists(Helper::getPasteId()), 'paste does not exists before posting data'); + 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 9b440be0..38501c56 100644 --- a/tst/RequestTest.php +++ b/tst/RequestTest.php @@ -97,7 +97,7 @@ class RequestTest extends PHPUnit_Framework_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')); } @@ -111,7 +111,7 @@ class RequestTest extends PHPUnit_Framework_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')); } @@ -125,7 +125,7 @@ class RequestTest extends PHPUnit_Framework_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()); } @@ -142,12 +142,25 @@ class RequestTest extends PHPUnit_Framework_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(); From e54277f0145337dec8228ea1eb5301a2fc926300 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 12 Dec 2022 20:48:36 +0100 Subject: [PATCH 3/7] re-add 10 * batch size limit in filesystem purge and support v1 dates for sorting mixed versioned comments --- lib/Data/Filesystem.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 0f815414..3266f010 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,6 +364,8 @@ 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()) { @@ -371,11 +379,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; From 23a2c1829ff4dffd66f5117cbcc9d93842855046 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 12 Dec 2022 20:49:04 +0100 Subject: [PATCH 4/7] deal with annotation reported in github actions --- tst/Bootstrap.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; } From 53ab57627e0f42e467e8d2ec3188b3c25159c206 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 12 Dec 2022 21:28:38 +0100 Subject: [PATCH 5/7] re-add shuffling paste list --- lib/Data/Filesystem.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 3266f010..19292c8b 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -367,11 +367,9 @@ class Filesystem extends AbstractData $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 ( From a93c8ceccb5230eaef569fd9d1f4cd5f8887072a Mon Sep 17 00:00:00 2001 From: El RIDO Date: Tue, 13 Dec 2022 06:21:37 +0100 Subject: [PATCH 6/7] fold extracted function back into the one remaining place calling it --- lib/Data/Filesystem.php | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 19292c8b..c1b720e7 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -395,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'); } @@ -439,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 * From 30fec3e2ebbde4b77b0f02a9d46e33115257f267 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Tue, 13 Dec 2022 18:45:41 +0100 Subject: [PATCH 7/7] document changes --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) 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