From 348d36d6eeb05d2f5aaaeba9aa813703f9495acd Mon Sep 17 00:00:00 2001 From: El RIDO Date: Thu, 20 Nov 2025 09:10:01 +0100 Subject: [PATCH] prefer DirectoryIterator for readability, also test jbo translation, log deletion errors --- lib/Data/Database.php | 4 +++- lib/Data/Filesystem.php | 41 +++++++++++++++++++++-------------------- tst/I18nTest.php | 25 +++++++++++++------------ tst/ViewTest.php | 7 +++---- 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/lib/Data/Database.php b/lib/Data/Database.php index 108278a5..a574c13a 100644 --- a/lib/Data/Database.php +++ b/lib/Data/Database.php @@ -400,7 +400,9 @@ class Database extends AbstractData $fs = new Filesystem(array('dir' => 'data')); $value = $fs->getValue('salt'); $this->setValue($value, 'salt'); - unlink($file); + if (!unlink($file)) { + error_log('Error deleting migrated salt: ' . $file); + } return $value; } } diff --git a/lib/Data/Filesystem.php b/lib/Data/Filesystem.php index 9edb9900..8dbaa9d2 100644 --- a/lib/Data/Filesystem.php +++ b/lib/Data/Filesystem.php @@ -11,6 +11,7 @@ namespace PrivateBin\Data; +use DirectoryIterator; use GlobIterator; use PrivateBin\Exception\JsonException; use PrivateBin\Json; @@ -121,21 +122,24 @@ class Filesystem extends AbstractData $pastedir = $this->_dataid2path($pasteid); if (is_dir($pastedir)) { // Delete the paste itself. - if (is_file($pastedir . $pasteid . '.php')) { - unlink($pastedir . $pasteid . '.php'); + $pastefile = $pastedir . $pasteid . '.php'; + if (is_file($pastefile)) { + if (!unlink($pastefile)) { + error_log('Error deleting paste: ' . $pastefile); + } } // Delete discussion if it exists. $discdir = $this->_dataid2discussionpath($pasteid); if (is_dir($discdir)) { // Delete all files in discussion directory - $dir = dir($discdir); - while (false !== ($filename = $dir->read())) { - if (is_file($discdir . $filename)) { - unlink($discdir . $filename); + foreach (new DirectoryIterator($discdir) as $file) { + if ($file->isFile()) { + if (!unlink($file->getPathname())) { + error_log('Error deleting comment: ' . $file->getPathname()); + } } } - $dir->close(); rmdir($discdir); } } @@ -159,14 +163,11 @@ class Filesystem extends AbstractData // convert comments, too $discdir = $this->_dataid2discussionpath($pasteid); if (is_dir($discdir)) { - $dir = dir($discdir); - while (false !== ($filename = $dir->read())) { - if (substr($filename, -4) !== '.php' && strlen($filename) >= 16) { - $commentFilename = $discdir . $filename . '.php'; - $this->_prependRename($discdir . $filename, $commentFilename); + foreach (new DirectoryIterator($discdir) as $file) { + if ($file->getExtension() !== 'php' && strlen($file->getFilename()) >= 16) { + $this->_prependRename($file->getPathname(), $file->getPathname() . '.php'); } } - $dir->close(); } } return is_readable($pastePath); @@ -207,15 +208,14 @@ class Filesystem extends AbstractData $comments = array(); $discdir = $this->_dataid2discussionpath($pasteid); if (is_dir($discdir)) { - $dir = dir($discdir); - while (false !== ($filename = $dir->read())) { + foreach (new DirectoryIterator($discdir) as $file) { // Filename is in the form pasteid.commentid.parentid.php: // - pasteid is the paste this reply belongs to. // - commentid is the comment identifier itself. // - parentid is the comment this comment replies to (It can be pasteid) - if (is_file($discdir . $filename)) { - $comment = $this->_get($discdir . $filename); - $items = explode('.', $filename); + if ($file->isFile()) { + $comment = $this->_get($file->getPathname()); + $items = explode('.', $file->getBasename('.php')); // Add some meta information not contained in file. $comment['id'] = $items[1]; $comment['parentid'] = $items[2]; @@ -228,7 +228,6 @@ class Filesystem extends AbstractData $comments[$key] = $comment; } } - $dir->close(); // Sort comments by date, oldest first. ksort($comments); @@ -522,6 +521,8 @@ class Filesystem extends AbstractData file_put_contents($destFile, $handle, FILE_APPEND); fclose($handle); } - unlink($srcFile); + if (!unlink($srcFile)) { + error_log('Error deleting converted document: ' . $srcFile); + } } } diff --git a/tst/I18nTest.php b/tst/I18nTest.php index 9e196103..7118c9fc 100644 --- a/tst/I18nTest.php +++ b/tst/I18nTest.php @@ -2,6 +2,7 @@ use PHPUnit\Framework\TestCase; use PrivateBin\I18n; +use PrivateBin\Json; class I18nMock extends I18n { @@ -221,19 +222,19 @@ class I18nTest extends TestCase { $messageIds = array(); $languages = array(); - $dir = dir(PATH . 'i18n'); - while (false !== ($file = $dir->read())) { - if (strlen($file) === 7) { - $language = substr($file, 0, 2); - $languageMessageIds = array_keys( - json_decode( - file_get_contents(PATH . 'i18n' . DIRECTORY_SEPARATOR . $file), - true - ) - ); - $messageIds = array_unique(array_merge($messageIds, $languageMessageIds)); - $languages[$language] = $languageMessageIds; + foreach (new DirectoryIterator(PATH . 'i18n') as $file) { + $fileNameLength = strlen($file->getFilename()); + if ($fileNameLength === 7) { // xx.json + $language = substr($file->getFilename(), 0, 2); + } elseif ($fileNameLength === 8) { // jbo.json + $language = substr($file->getFilename(), 0, 3); + } else { + continue; } + $languageJson = file_get_contents($file->getPathname()); + $languageMessageIds = array_keys(Json::decode($languageJson)); + $messageIds = array_unique(array_merge($messageIds, $languageMessageIds)); + $languages[$language] = $languageMessageIds; } foreach ($messageIds as $messageId) { foreach (array_keys($languages) as $language) { diff --git a/tst/ViewTest.php b/tst/ViewTest.php index 64ec6503..20eaccbd 100644 --- a/tst/ViewTest.php +++ b/tst/ViewTest.php @@ -67,10 +67,9 @@ class ViewTest extends TestCase $page->assign('CSPHEADER', 'default-src \'none\''); $page->assign('SRI', array()); - $dir = dir(PATH . 'tpl'); - while (false !== ($file = $dir->read())) { - if (substr($file, -4) === '.php') { - $template = substr($file, 0, -4); + foreach (new DirectoryIterator(PATH . 'tpl') as $file) { + if ($file->getExtension() === 'php') { + $template = $file->getBasename('.php'); ob_start(); $page->draw($template); $this->_content[$template] = ob_get_contents();