From 8ad6300c1cf9c0123d78d303cd6ae3b31a63e639 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Thu, 13 Mar 2025 09:22:27 +0100 Subject: [PATCH] pass by reference, closes #858 --- lib/Data/AbstractData.php | 6 ++--- lib/Data/Database.php | 12 ++++++---- lib/Data/Filesystem.php | 4 ++-- lib/Data/GoogleCloudStorage.php | 6 ++--- lib/Data/S3Storage.php | 6 ++--- tst/ControllerTest.php | 27 ++++++++++++++------- tst/Data/DatabaseTest.php | 37 +++++++++++++++++++++++++---- tst/Data/FilesystemTest.php | 14 ++++++----- tst/Data/GoogleCloudStorageTest.php | 14 ++++++----- tst/JsonApiTest.php | 8 ++++--- tst/MigrateTest.php | 6 +++-- 11 files changed, 94 insertions(+), 46 deletions(-) diff --git a/lib/Data/AbstractData.php b/lib/Data/AbstractData.php index 8e8c8bd5..bc038be8 100644 --- a/lib/Data/AbstractData.php +++ b/lib/Data/AbstractData.php @@ -34,7 +34,7 @@ abstract class AbstractData * @param array $paste * @return bool */ - abstract public function create($pasteid, array $paste); + abstract public function create($pasteid, array &$paste); /** * Read a paste. @@ -72,7 +72,7 @@ abstract class AbstractData * @param array $comment * @return bool */ - abstract public function createComment($pasteid, $parentid, $commentid, array $comment); + abstract public function createComment($pasteid, $parentid, $commentid, array &$comment); /** * Read all comments of paste. @@ -199,7 +199,7 @@ abstract class AbstractData * @param array $paste * @return array */ - protected static function upgradePreV1Format(array $paste) + protected static function upgradePreV1Format(array &$paste) { if (array_key_exists('attachment', $paste['meta'])) { $paste['attachment'] = $paste['meta']['attachment']; diff --git a/lib/Data/Database.php b/lib/Data/Database.php index f9e0b8b5..508555a2 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -140,7 +140,7 @@ class Database extends AbstractData * @param array $paste * @return bool */ - public function create($pasteid, array $paste) + public function create($pasteid, array &$paste) { $expire_date = 0; $opendiscussion = $burnafterreading = false; @@ -297,14 +297,18 @@ class Database extends AbstractData * @param array $comment * @return bool */ - public function createComment($pasteid, $parentid, $commentid, array $comment) + public function createComment($pasteid, $parentid, $commentid, array &$comment) { if (array_key_exists('data', $comment)) { $version = 1; $data = $comment['data']; } else { - $version = 2; - $data = Json::encode($comment); + try { + $version = 2; + $data = Json::encode($comment); + } catch (Exception $e) { + return false; + } } list($createdKey, $iconKey) = $this->_getVersionedKeys($version); $meta = $comment['meta']; diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index be32c6ec..ac91db11 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -85,7 +85,7 @@ class Filesystem extends AbstractData * @param array $paste * @return bool */ - public function create($pasteid, array $paste) + public function create($pasteid, array &$paste) { $storagedir = $this->_dataid2path($pasteid); $file = $storagedir . $pasteid . '.php'; @@ -188,7 +188,7 @@ class Filesystem extends AbstractData * @param array $comment * @return bool */ - public function createComment($pasteid, $parentid, $commentid, array $comment) + public function createComment($pasteid, $parentid, $commentid, array &$comment) { $storagedir = $this->_dataid2discussionpath($pasteid); $file = $storagedir . $pasteid . '.' . $commentid . '.' . $parentid . '.php'; diff --git a/lib/Data/GoogleCloudStorage.php b/lib/Data/GoogleCloudStorage.php index c4354e83..e4866f46 100644 --- a/lib/Data/GoogleCloudStorage.php +++ b/lib/Data/GoogleCloudStorage.php @@ -105,7 +105,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']); @@ -136,7 +136,7 @@ class GoogleCloudStorage extends AbstractData /** * @inheritDoc */ - public function create($pasteid, array $paste) + public function create($pasteid, array &$paste) { if ($this->exists($pasteid)) { return false; @@ -201,7 +201,7 @@ class GoogleCloudStorage extends AbstractData /** * @inheritDoc */ - public function createComment($pasteid, $parentid, $commentid, array $comment) + public function createComment($pasteid, $parentid, $commentid, array &$comment) { if ($this->existsComment($pasteid, $parentid, $commentid)) { return false; diff --git a/lib/Data/S3Storage.php b/lib/Data/S3Storage.php index 70505aac..d221d845 100644 --- a/lib/Data/S3Storage.php +++ b/lib/Data/S3Storage.php @@ -165,7 +165,7 @@ class S3Storage 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']); @@ -191,7 +191,7 @@ class S3Storage extends AbstractData /** * @inheritDoc */ - public function create($pasteid, array $paste) + public function create($pasteid, array &$paste) { if ($this->exists($pasteid)) { return false; @@ -263,7 +263,7 @@ class S3Storage extends AbstractData /** * @inheritDoc */ - public function createComment($pasteid, $parentid, $commentid, array $comment) + public function createComment($pasteid, $parentid, $commentid, array &$comment) { if ($this->existsComment($pasteid, $parentid, $commentid)) { return false; diff --git a/tst/ControllerTest.php b/tst/ControllerTest.php index 98eb6458..89caa03e 100644 --- a/tst/ControllerTest.php +++ b/tst/ControllerTest.php @@ -273,7 +273,8 @@ class ControllerTest extends TestCase $options = parse_ini_file(CONF, true); $options['traffic']['limit'] = 0; Helper::createIniFile(CONF, $options); - $this->_data->create(Helper::getPasteId(), Helper::getPaste()); + $paste = Helper::getPaste(); + $this->_data->create(Helper::getPasteId(), $paste); $paste = Helper::getPasteJson(); $file = tempnam(sys_get_temp_dir(), 'FOO'); file_put_contents($file, $paste); @@ -523,7 +524,8 @@ class ControllerTest extends TestCase $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest'; $_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REMOTE_ADDR'] = '::1'; - $this->_data->create(Helper::getPasteId(), Helper::getPaste()); + $paste = Helper::getPaste(); + $this->_data->create(Helper::getPasteId(), $paste); ob_start(); new Controller; $content = ob_get_contents(); @@ -549,7 +551,8 @@ class ControllerTest extends TestCase $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest'; $_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REMOTE_ADDR'] = '::1'; - $this->_data->create(Helper::getPasteId(), Helper::getPaste()); + $paste = Helper::getPaste(); + $this->_data->create(Helper::getPasteId(), $paste); ob_start(); new Controller; $content = ob_get_contents(); @@ -618,8 +621,10 @@ class ControllerTest extends TestCase $options = parse_ini_file(CONF, true); $options['traffic']['limit'] = 0; Helper::createIniFile(CONF, $options); - $this->_data->create(Helper::getPasteId(), Helper::getPaste()); - $this->_data->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId(), Helper::getComment()); + $paste = Helper::getPaste(); + $this->_data->create(Helper::getPasteId(), $paste); + $comment = Helper::getComment(); + $this->_data->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId(), $comment); $this->assertTrue($this->_data->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId()), 'comment exists before posting data'); $comment = Helper::getCommentJson(); $file = tempnam(sys_get_temp_dir(), 'FOO'); @@ -804,7 +809,8 @@ class ControllerTest extends TestCase */ public function testDelete() { - $this->_data->create(Helper::getPasteId(), Helper::getPaste()); + $paste = Helper::getPaste(); + $this->_data->create(Helper::getPasteId(), $paste); $this->assertTrue($this->_data->exists(Helper::getPasteId()), 'paste exists before deleting data'); $paste = $this->_data->read(Helper::getPasteId()); $_GET['pasteid'] = Helper::getPasteId(); @@ -826,7 +832,8 @@ class ControllerTest extends TestCase */ public function testDeleteInvalidId() { - $this->_data->create(Helper::getPasteId(), Helper::getPaste()); + $paste = Helper::getPaste(); + $this->_data->create(Helper::getPasteId(), $paste); $_GET['pasteid'] = 'foo'; $_GET['deletetoken'] = 'bar'; ob_start(); @@ -864,7 +871,8 @@ class ControllerTest extends TestCase */ public function testDeleteInvalidToken() { - $this->_data->create(Helper::getPasteId(), Helper::getPaste()); + $paste = Helper::getPaste(); + $this->_data->create(Helper::getPasteId(), $paste); $_GET['pasteid'] = Helper::getPasteId(); $_GET['deletetoken'] = 'bar'; ob_start(); @@ -884,7 +892,8 @@ class ControllerTest extends TestCase */ public function testDeleteInvalidBurnAfterReading() { - $this->_data->create(Helper::getPasteId(), Helper::getPaste()); + $paste = Helper::getPaste(); + $this->_data->create(Helper::getPasteId(), $paste); $this->assertTrue($this->_data->exists(Helper::getPasteId()), 'paste exists before deleting data'); $file = tempnam(sys_get_temp_dir(), 'FOO'); file_put_contents($file, json_encode(array( diff --git a/tst/Data/DatabaseTest.php b/tst/Data/DatabaseTest.php index d1003c55..d76ea6ec 100644 --- a/tst/Data/DatabaseTest.php +++ b/tst/Data/DatabaseTest.php @@ -60,18 +60,22 @@ class DatabaseTest extends TestCase $this->assertEquals($paste, $this->_model->read(Helper::getPasteId())); // storing comments + $comment1 = Helper::getComment(1); + $comment2 = Helper::getComment(2); + $meta1 = $comment1['meta']; + $meta2 = $comment2['meta']; $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'v1 comment does not yet exist'); - $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), Helper::getComment(1)) !== false, 'store v1 comment'); + $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment1) !== false, 'store v1 comment'); $this->assertTrue($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'v1 comment exists after storing it'); $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId()), 'v2 comment does not yet exist'); - $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId(), Helper::getComment(2)) !== false, 'store v2 comment'); + $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId(), $comment2) !== false, 'store v2 comment'); $this->assertTrue($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getPasteId()), 'v2 comment exists after storing it'); - $comment1 = Helper::getComment(1); $comment1['id'] = Helper::getCommentId(); $comment1['parentid'] = Helper::getPasteId(); - $comment2 = Helper::getComment(2); + $comment1['meta'] = $meta1; $comment2['id'] = Helper::getPasteId(); $comment2['parentid'] = Helper::getPasteId(); + $comment2['meta'] = $meta2; $this->assertEquals( array( $comment1['meta']['postdate'] => $comment1, @@ -120,7 +124,8 @@ class DatabaseTest extends TestCase if (in_array($key, array('y', 'z'))) { $this->assertTrue($this->_model->create($ids[$key], $paste), "store $key paste"); } elseif ($key === 'x') { - $this->assertTrue($this->_model->create($ids[$key], Helper::getPaste()), "store $key paste"); + $data = Helper::getPaste(); + $this->assertTrue($this->_model->create($ids[$key], $data), "store $key paste"); } else { $this->assertTrue($this->_model->create($ids[$key], $expired), "store $key paste"); } @@ -137,6 +142,28 @@ class DatabaseTest extends TestCase } } + public function testErrorDetection() + { + $this->_model->delete(Helper::getPasteId()); + $paste = Helper::getPaste(2, array('expire' => "Invalid UTF-8 sequence: \xB1\x31")); + $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'); + } + + public function testCommentErrorDetection() + { + $this->_model->delete(Helper::getPasteId()); + $data = Helper::getPaste(); + $comment = Helper::getComment(2, array('nickname' => "Invalid UTF-8 sequence: \xB1\x31")); + $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste does not yet exist'); + $this->assertTrue($this->_model->create(Helper::getPasteId(), $data), 'store new paste'); + $this->assertTrue($this->_model->exists(Helper::getPasteId()), 'paste exists after storing it'); + $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does not yet exist'); + $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'); + } + public function testGetIbmInstance() { $this->expectException(PDOException::class); diff --git a/tst/Data/FilesystemTest.php b/tst/Data/FilesystemTest.php index 99966c4c..89f5395b 100644 --- a/tst/Data/FilesystemTest.php +++ b/tst/Data/FilesystemTest.php @@ -45,11 +45,11 @@ class FilesystemTest extends TestCase $this->assertEquals($paste, $this->_model->read(Helper::getPasteId())); // storing comments - $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does not yet exist'); - $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), Helper::getComment()), 'store comment'); - $this->assertTrue($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment exists after storing it'); - $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), Helper::getComment()), 'unable to store the same comment twice'); $comment = Helper::getComment(); + $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does not yet exist'); + $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'store comment'); + $this->assertTrue($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment exists after storing it'); + $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'unable to store the same comment twice'); $comment['id'] = Helper::getCommentId(); $comment['parentid'] = Helper::getPasteId(); $this->assertEquals( @@ -94,7 +94,8 @@ class FilesystemTest extends TestCase if (in_array($key, array('x', 'y', 'z'))) { $this->assertTrue($this->_model->create($ids[$key], $paste), "store $key paste"); } elseif ($key === 'x') { - $this->assertTrue($this->_model->create($ids[$key], Helper::getPaste()), "store $key paste"); + $data = Helper::getPaste(); + $this->assertTrue($this->_model->create($ids[$key], $data), "store $key paste"); } else { $this->assertTrue($this->_model->create($ids[$key], $expired), "store $key paste"); } @@ -124,9 +125,10 @@ class FilesystemTest extends TestCase public function testCommentErrorDetection() { $this->_model->delete(Helper::getPasteId()); + $data = Helper::getPaste(); $comment = Helper::getComment(1, array('nickname' => "Invalid UTF-8 sequence: \xB1\x31")); $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste does not yet exist'); - $this->assertTrue($this->_model->create(Helper::getPasteId(), Helper::getPaste()), 'store new paste'); + $this->assertTrue($this->_model->create(Helper::getPasteId(), $data), 'store new paste'); $this->assertTrue($this->_model->exists(Helper::getPasteId()), 'paste exists after storing it'); $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does not yet exist'); $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'unable to store broken comment'); diff --git a/tst/Data/GoogleCloudStorageTest.php b/tst/Data/GoogleCloudStorageTest.php index 6ada0c02..42a32c2f 100644 --- a/tst/Data/GoogleCloudStorageTest.php +++ b/tst/Data/GoogleCloudStorageTest.php @@ -58,11 +58,11 @@ class GoogleCloudStorageTest extends TestCase $this->assertEquals($paste, $this->_model->read(Helper::getPasteId())); // storing comments - $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does not yet exist'); - $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), Helper::getComment()), 'store comment'); - $this->assertTrue($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment exists after storing it'); - $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), Helper::getComment()), 'unable to store the same comment twice'); $comment = Helper::getComment(); + $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does not yet exist'); + $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'store comment'); + $this->assertTrue($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment exists after storing it'); + $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'unable to store the same comment twice'); $comment['id'] = Helper::getCommentId(); $comment['parentid'] = Helper::getPasteId(); $this->assertEquals( @@ -92,7 +92,8 @@ class GoogleCloudStorageTest extends TestCase if (in_array($key, array('x', 'y', 'z'))) { $this->assertTrue($this->_model->create($ids[$key], $paste), "store $key paste"); } elseif ($key === 'x') { - $this->assertTrue($this->_model->create($ids[$key], Helper::getPaste()), "store $key paste"); + $data = Helper::getPaste(); + $this->assertTrue($this->_model->create($ids[$key], $data), "store $key paste"); } else { $this->assertTrue($this->_model->create($ids[$key], $expired), "store $key paste"); } @@ -121,9 +122,10 @@ class GoogleCloudStorageTest extends TestCase public function testCommentErrorDetection() { $this->_model->delete(Helper::getPasteId()); + $data = Helper::getPaste(); $comment = Helper::getComment(1, array('nickname' => "Invalid UTF-8 sequence: \xB1\x31")); $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste does not yet exist'); - $this->assertTrue($this->_model->create(Helper::getPasteId(), Helper::getPaste()), 'store new paste'); + $this->assertTrue($this->_model->create(Helper::getPasteId(), $data), 'store new paste'); $this->assertTrue($this->_model->exists(Helper::getPasteId()), 'paste exists after storing it'); $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does not yet exist'); $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'unable to store broken comment'); diff --git a/tst/JsonApiTest.php b/tst/JsonApiTest.php index 43293200..3bbd2114 100644 --- a/tst/JsonApiTest.php +++ b/tst/JsonApiTest.php @@ -114,7 +114,8 @@ class JsonApiTest extends TestCase */ public function testDelete() { - $this->_model->create(Helper::getPasteId(), Helper::getPaste()); + $data = Helper::getPaste(); + $this->_model->create(Helper::getPasteId(), $data); $this->assertTrue($this->_model->exists(Helper::getPasteId()), 'paste exists before deleting data'); $paste = $this->_model->read(Helper::getPasteId()); $file = tempnam(sys_get_temp_dir(), 'FOO'); @@ -141,7 +142,8 @@ class JsonApiTest extends TestCase */ public function testDeleteWithPost() { - $this->_model->create(Helper::getPasteId(), Helper::getPaste()); + $data = Helper::getPaste(); + $this->_model->create(Helper::getPasteId(), $data); $this->assertTrue($this->_model->exists(Helper::getPasteId()), 'paste exists before deleting data'); $paste = $this->_model->read(Helper::getPasteId()); $file = tempnam(sys_get_temp_dir(), 'FOO'); @@ -166,7 +168,7 @@ class JsonApiTest extends TestCase */ public function testRead() { - $paste = Helper::getPaste(); + $paste = Helper::getPaste(); $this->_model->create(Helper::getPasteId(), $paste); $_SERVER['QUERY_STRING'] = Helper::getPasteId(); $_GET[Helper::getPasteId()] = ''; diff --git a/tst/MigrateTest.php b/tst/MigrateTest.php index d587d442..fd6cc9af 100644 --- a/tst/MigrateTest.php +++ b/tst/MigrateTest.php @@ -59,8 +59,10 @@ class MigrateTest extends TestCase $this->_model_2->delete(Helper::getPasteId()); // storing paste & comment - $this->_model_1->create(Helper::getPasteId(), Helper::getPaste()); - $this->_model_1->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), Helper::getComment()); + $data = Helper::getPaste(); + $this->_model_1->create(Helper::getPasteId(), $data); + $data = Helper::getComment(); + $this->_model_1->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $data); // migrate files to database $output = null;