From 3e6f1733f95c423fd51bb9113b31de1e01050f60 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Mon, 17 Nov 2025 17:28:13 +0100 Subject: [PATCH] refactored exceptions in controller - added missing exception doc blocks - introduced exception type that translates message during construction - catch explicit exception types where possible --- lib/Configuration.php | 11 +++++--- lib/Controller.php | 25 ++++++++++--------- lib/Model/AbstractModel.php | 26 ++++++++++++++----- lib/Model/Comment.php | 17 ++++++------- lib/Model/Paste.php | 28 ++++++++++----------- lib/Persistence/TrafficLimiter.php | 9 +++---- lib/TranslatedException.php | 36 +++++++++++++++++++++++++++ vendor/composer/autoload_classmap.php | 1 + vendor/composer/autoload_static.php | 1 + vendor/composer/installed.php | 4 +-- 10 files changed, 105 insertions(+), 53 deletions(-) create mode 100644 lib/TranslatedException.php diff --git a/lib/Configuration.php b/lib/Configuration.php index 2cccc342..c4651f8b 100644 --- a/lib/Configuration.php +++ b/lib/Configuration.php @@ -12,6 +12,7 @@ namespace PrivateBin; use Exception; +use PrivateBin\TranslatedException; /** * Configuration @@ -131,7 +132,7 @@ class Configuration /** * parse configuration file and ensure default configuration values are present * - * @throws Exception + * @throws TranslatedException */ public function __construct() { @@ -148,7 +149,9 @@ class Configuration $config = parse_ini_file($configFile, true); foreach (array('main', 'model', 'model_options') as $section) { if (!array_key_exists($section, $config)) { - throw new Exception(I18n::_('PrivateBin requires configuration section [%s] to be present in configuration file.', $section), 2); + $name = array_key_exists('main', $config) && array_key_exists('name', $config['main']) ? + $config['main']['name'] : self::getDefaults()['main']['name']; + throw new TranslatedException(array('%s requires configuration section [%s] to be present in configuration file.', I18n::_($name), $section), 2); } } break; @@ -304,13 +307,13 @@ class Configuration * get a section from the configuration, must exist * * @param string $section - * @throws Exception + * @throws TranslatedException * @return mixed */ public function getSection($section) { if (!array_key_exists($section, $this->_configuration)) { - throw new Exception(I18n::_('%s requires configuration section [%s] to be present in configuration file.', I18n::_($this->getKey('name')), $section), 3); + throw new TranslatedException(array('%s requires configuration section [%s] to be present in configuration file.', I18n::_($this->getKey('name')), $section), 3); } return $this->_configuration[$section]; } diff --git a/lib/Controller.php b/lib/Controller.php index bb161907..2e056db6 100644 --- a/lib/Controller.php +++ b/lib/Controller.php @@ -17,6 +17,7 @@ use PrivateBin\Persistence\TrafficLimiter; use PrivateBin\Proxy\AbstractProxy; use PrivateBin\Proxy\ShlinkProxy; use PrivateBin\Proxy\YourlsProxy; +use PrivateBin\TranslatedException; /** * Controller @@ -195,6 +196,7 @@ class Controller * Set default language * * @access private + * @throws Exception */ private function _setDefaultLanguage() { @@ -211,6 +213,7 @@ class Controller * Set default template * * @access private + * @throws Exception */ private function _setDefaultTemplate() { @@ -260,6 +263,7 @@ class Controller * pasteid (optional) = in discussions, which paste this comment belongs to. * * @access private + * @throws Exception * @return string */ private function _create() @@ -270,8 +274,7 @@ class Controller TrafficLimiter::setStore($this->_model->getStore()); try { TrafficLimiter::canPass(); - } catch (Exception $e) { - // traffic limiter exceptions come translated + } catch (TranslatedException $e) { $this->_json_error($e->getMessage()); return; } @@ -305,9 +308,8 @@ class Controller $comment = $paste->getComment($data['parentid']); $comment->setData($data); $comment->store(); - } catch (Exception $e) { - // comment exceptions need translation - $this->_json_error(I18n::_($e->getMessage())); + } catch (TranslatedException $e) { + $this->_json_error($e->getMessage()); return; } $this->_json_result($comment->getId()); @@ -319,7 +321,7 @@ class Controller else { try { $this->_model->purge(); - } catch (Exception $e) { + } catch (Exception $e) { // JSON error!!! error_log('Error purging documents: ' . $e->getMessage() . PHP_EOL . 'Use the administration scripts statistics to find ' . 'damaged paste IDs and either delete them or restore them ' . @@ -329,9 +331,8 @@ class Controller try { $paste->setData($data); $paste->store(); - } catch (Exception $e) { - // paste exceptions need translation - $this->_json_error(I18n::_($e->getMessage())); + } catch (TranslatedException $e) { + $this->_json_error($e->getMessage()); return; } $this->_json_result($paste->getId(), array('deletetoken' => $paste->getDeleteToken())); @@ -399,9 +400,8 @@ class Controller } else { $this->_json_error(I18n::_(self::GENERIC_ERROR)); } - } catch (Exception $e) { - // paste exceptions need translation - $this->_json_error(I18n::_($e->getMessage())); + } catch (TranslatedException $e) { + $this->_json_error($e->getMessage()); } } @@ -409,6 +409,7 @@ class Controller * Display frontend. * * @access private + * @throws Exception */ private function _view() { diff --git a/lib/Model/AbstractModel.php b/lib/Model/AbstractModel.php index d7b5102f..5ff4ab1b 100644 --- a/lib/Model/AbstractModel.php +++ b/lib/Model/AbstractModel.php @@ -11,9 +11,9 @@ namespace PrivateBin\Model; -use Exception; use PrivateBin\Configuration; use PrivateBin\Data\AbstractData; +use PrivateBin\TranslatedException; /** * AbstractModel @@ -22,6 +22,20 @@ use PrivateBin\Data\AbstractData; */ abstract class AbstractModel { + /** + * show the same error message if the data is invalid + * + * @const string + */ + const INVALID_DATA_ERROR = 'Invalid data.'; + + /** + * show the same error message if the document ID already exists + * + * @const string + */ + const COLLISION_ERROR = 'You are unlucky. Try again.'; + /** * Instance ID. * @@ -83,12 +97,12 @@ abstract class AbstractModel * * @access public * @param string $id - * @throws Exception + * @throws TranslatedException */ public function setId($id) { if (!self::isValidId($id)) { - throw new Exception('Invalid document ID.', 60); + throw new TranslatedException('Invalid document ID.', 60); } $this->_id = $id; } @@ -98,7 +112,7 @@ abstract class AbstractModel * * @access public * @param array $data - * @throws Exception + * @throws TranslatedException */ public function setData(array &$data) { @@ -125,7 +139,7 @@ abstract class AbstractModel * Store the instance's data. * * @access public - * @throws Exception + * @throws TranslatedException */ abstract public function store(); @@ -163,7 +177,7 @@ abstract class AbstractModel * * @access protected * @param array $data - * @throws Exception + * @throws TranslatedException */ protected function _validate(array &$data) { diff --git a/lib/Model/Comment.php b/lib/Model/Comment.php index 3c0b2b20..3a9d6336 100644 --- a/lib/Model/Comment.php +++ b/lib/Model/Comment.php @@ -11,10 +11,10 @@ namespace PrivateBin\Model; -use Exception; use Identicon\Identicon; use Jdenticon\Identicon as Jdenticon; use PrivateBin\Persistence\TrafficLimiter; +use PrivateBin\TranslatedException; use PrivateBin\Vizhash16x16; /** @@ -36,24 +36,24 @@ class Comment extends AbstractModel * Store the comment's data. * * @access public - * @throws Exception + * @throws TranslatedException */ public function store() { // Make sure paste exists. $pasteid = $this->getPaste()->getId(); if (!$this->getPaste()->exists()) { - throw new Exception('Invalid data.', 67); + throw new TranslatedException(self::INVALID_DATA_ERROR, 67); } // Make sure the discussion is opened in this paste and allowed in the configuration. if (!$this->getPaste()->isOpendiscussion() || !$this->_conf->getKey('discussion')) { - throw new Exception('Invalid data.', 68); + throw new TranslatedException(self::INVALID_DATA_ERROR, 68); } // Check for improbable collision. if ($this->exists()) { - throw new Exception('You are unlucky. Try again.', 69); + throw new TranslatedException(self::COLLISION_ERROR, 69); } $this->_data['meta']['created'] = time(); @@ -67,7 +67,7 @@ class Comment extends AbstractModel $this->_data ) === false ) { - throw new Exception('Error saving comment. Sorry.', 70); + throw new TranslatedException('Error saving comment. Sorry.', 70); } } @@ -91,7 +91,6 @@ class Comment extends AbstractModel * * @access public * @param Paste $paste - * @throws Exception */ public function setPaste(Paste &$paste) { @@ -115,12 +114,12 @@ class Comment extends AbstractModel * * @access public * @param string $id - * @throws Exception + * @throws TranslatedException */ public function setParentId($id) { if (!self::isValidId($id)) { - throw new Exception('Invalid document ID.', 65); + throw new TranslatedException('Invalid document ID.', 65); } $this->_data['parentid'] = $id; } diff --git a/lib/Model/Paste.php b/lib/Model/Paste.php index a42ab35e..0d52b2a3 100644 --- a/lib/Model/Paste.php +++ b/lib/Model/Paste.php @@ -11,9 +11,9 @@ namespace PrivateBin\Model; -use Exception; use PrivateBin\Controller; use PrivateBin\Persistence\ServerSalt; +use PrivateBin\TranslatedException; /** * Paste @@ -47,14 +47,14 @@ class Paste extends AbstractModel * Get paste data. * * @access public - * @throws Exception + * @throws TranslatedException * @return array */ public function get() { $data = $this->_store->read($this->getId()); if ($data === false) { - throw new Exception(Controller::GENERIC_ERROR, 64); + throw new TranslatedException(Controller::GENERIC_ERROR, 64); } // check if paste has expired and delete it if necessary. @@ -62,7 +62,7 @@ class Paste extends AbstractModel $now = time(); if ($data['meta']['expire_date'] < $now) { $this->delete(); - throw new Exception(Controller::GENERIC_ERROR, 63); + throw new TranslatedException(Controller::GENERIC_ERROR, 63); } // We kindly provide the remaining time before expiration (in seconds) $data['meta']['time_to_live'] = $data['meta']['expire_date'] - $now; @@ -93,13 +93,13 @@ class Paste extends AbstractModel * Store the paste's data. * * @access public - * @throws Exception + * @throws TranslatedException */ public function store() { // Check for improbable collision. if ($this->exists()) { - throw new Exception('You are unlucky. Try again.', 75); + throw new TranslatedException(self::COLLISION_ERROR, 75); } $this->_data['meta']['salt'] = ServerSalt::generate(); @@ -111,7 +111,7 @@ class Paste extends AbstractModel $this->_data ) === false ) { - throw new Exception('Error saving document. Sorry.', 76); + throw new TranslatedException('Error saving document. Sorry.', 76); } } @@ -119,7 +119,6 @@ class Paste extends AbstractModel * Delete the paste. * * @access public - * @throws Exception */ public function delete() { @@ -143,13 +142,13 @@ class Paste extends AbstractModel * @access public * @param string $parentId * @param string $commentId - * @throws Exception + * @throws TranslatedException * @return Comment */ public function getComment($parentId, $commentId = '') { if (!$this->exists()) { - throw new Exception('Invalid data.', 62); + throw new TranslatedException(self::INVALID_DATA_ERROR, 62); } $comment = new Comment($this->_conf, $this->_store); $comment->setPaste($this); @@ -201,7 +200,6 @@ class Paste extends AbstractModel * Check if paste has discussions enabled. * * @access public - * @throws Exception * @return bool */ public function isOpendiscussion() @@ -240,13 +238,13 @@ class Paste extends AbstractModel * * @access protected * @param array $data - * @throws Exception + * @throws TranslatedException */ protected function _validate(array &$data) { // reject invalid or disabled formatters if (!array_key_exists($data['adata'][self::ADATA_FORMATTER], $this->_conf->getSection('formatter_options'))) { - throw new Exception('Invalid data.', 75); + throw new TranslatedException(self::INVALID_DATA_ERROR, 75); } // discussion requested, but disabled in config or burn after reading requested as well, or invalid integer @@ -257,7 +255,7 @@ class Paste extends AbstractModel )) || ($data['adata'][self::ADATA_OPEN_DISCUSSION] !== 0 && $data['adata'][self::ADATA_OPEN_DISCUSSION] !== 1) ) { - throw new Exception('Invalid data.', 74); + throw new TranslatedException(self::INVALID_DATA_ERROR, 74); } // reject invalid burn after reading @@ -265,7 +263,7 @@ class Paste extends AbstractModel $data['adata'][self::ADATA_BURN_AFTER_READING] !== 0 && $data['adata'][self::ADATA_BURN_AFTER_READING] !== 1 ) { - throw new Exception('Invalid data.', 73); + throw new TranslatedException(self::INVALID_DATA_ERROR, 73); } } } diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index 60977f5d..c94bfdb2 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -12,11 +12,10 @@ namespace PrivateBin\Persistence; -use Exception; use IPLib\Factory; use IPLib\ParseStringFlag; use PrivateBin\Configuration; -use PrivateBin\I18n; +use PrivateBin\TranslatedException; /** * TrafficLimiter @@ -167,7 +166,7 @@ class TrafficLimiter extends AbstractPersistence * * @access public * @static - * @throws Exception + * @throws TranslatedException * @return true */ public static function canPass() @@ -181,7 +180,7 @@ class TrafficLimiter extends AbstractPersistence return true; } } - throw new Exception(I18n::_('Your IP is not authorized to create documents.')); + throw new TranslatedException('Your IP is not authorized to create documents.'); } // disable limits if set to less then 1 @@ -210,7 +209,7 @@ class TrafficLimiter extends AbstractPersistence } return true; } - throw new Exception(I18n::_( + throw new TranslatedException(array( 'Please wait %d seconds between each post.', self::$_limit )); diff --git a/lib/TranslatedException.php b/lib/TranslatedException.php new file mode 100644 index 00000000..bbadec43 --- /dev/null +++ b/lib/TranslatedException.php @@ -0,0 +1,36 @@ + $baseDir . '/lib/Proxy/YourlsProxy.php', 'PrivateBin\\Request' => $baseDir . '/lib/Request.php', 'PrivateBin\\TemplateSwitcher' => $baseDir . '/lib/TemplateSwitcher.php', + 'PrivateBin\\TranslatedException' => $baseDir . '/lib/TranslatedException.php', 'PrivateBin\\View' => $baseDir . '/lib/View.php', 'PrivateBin\\Vizhash16x16' => $baseDir . '/lib/Vizhash16x16.php', 'Stringable' => $vendorDir . '/symfony/polyfill-php80/Resources/stubs/Stringable.php', diff --git a/vendor/composer/autoload_static.php b/vendor/composer/autoload_static.php index e9194d01..aa7dd9e8 100644 --- a/vendor/composer/autoload_static.php +++ b/vendor/composer/autoload_static.php @@ -139,6 +139,7 @@ class ComposerStaticInitDontChange 'PrivateBin\\Proxy\\YourlsProxy' => __DIR__ . '/../..' . '/lib/Proxy/YourlsProxy.php', 'PrivateBin\\Request' => __DIR__ . '/../..' . '/lib/Request.php', 'PrivateBin\\TemplateSwitcher' => __DIR__ . '/../..' . '/lib/TemplateSwitcher.php', + 'PrivateBin\\TranslatedException' => __DIR__ . '/../..' . '/lib/TranslatedException.php', 'PrivateBin\\View' => __DIR__ . '/../..' . '/lib/View.php', 'PrivateBin\\Vizhash16x16' => __DIR__ . '/../..' . '/lib/Vizhash16x16.php', 'Stringable' => __DIR__ . '/..' . '/symfony/polyfill-php80/Resources/stubs/Stringable.php', diff --git a/vendor/composer/installed.php b/vendor/composer/installed.php index 56dccd55..c9befe93 100644 --- a/vendor/composer/installed.php +++ b/vendor/composer/installed.php @@ -3,7 +3,7 @@ 'name' => 'privatebin/privatebin', 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => '06496a1b0e975b79c5a7abc0bd54b492ca264640', + 'reference' => 'a051c4bd6b71fd56f0d7fc235e815dafc8eb54ea', 'type' => 'project', 'install_path' => __DIR__ . '/../../', 'aliases' => array(), @@ -31,7 +31,7 @@ 'privatebin/privatebin' => array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', - 'reference' => '06496a1b0e975b79c5a7abc0bd54b492ca264640', + 'reference' => 'a051c4bd6b71fd56f0d7fc235e815dafc8eb54ea', 'type' => 'project', 'install_path' => __DIR__ . '/../../', 'aliases' => array(),