From e9c445925b6fd42936dde13357d8e33329748bf0 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sun, 24 Nov 2024 00:15:10 +0100 Subject: [PATCH 01/14] [ticket/17361] Rewrite storage PHPBB-17361 --- phpBB/adm/style/acp_storage.html | 2 +- .../default/container/services_storage.yml | 20 +- phpBB/includes/acp/acp_database.php | 2 +- phpBB/includes/acp/acp_main.php | 2 +- phpBB/includes/acp/acp_storage.php | 18 +- phpBB/includes/functions_user.php | 3 +- phpBB/phpbb/attachment/upload.php | 2 +- .../db/migration/data/v400/storage_track.php | 23 +- .../data/v400/storage_track_index.php | 48 +++ phpBB/phpbb/files/filespec_storage.php | 2 +- .../storage/adapter/adapter_interface.php | 63 +-- phpBB/phpbb/storage/adapter/local.php | 225 ++-------- phpBB/phpbb/storage/adapter_factory.php | 9 +- phpBB/phpbb/storage/controller/attachment.php | 14 +- phpBB/phpbb/storage/controller/avatar.php | 2 +- phpBB/phpbb/storage/controller/controller.php | 4 +- .../storage/exception/storage_exception.php | 2 +- phpBB/phpbb/storage/file_tracker.php | 198 +++++++++ phpBB/phpbb/storage/helper.php | 6 +- phpBB/phpbb/storage/provider/local.php | 6 +- .../storage/provider/provider_interface.php | 6 +- phpBB/phpbb/storage/storage.php | 383 +++--------------- phpBB/phpbb/storage/stream_interface.php | 41 -- tests/storage/adapter/local_test.php | 88 +--- 24 files changed, 407 insertions(+), 762 deletions(-) create mode 100644 phpBB/phpbb/db/migration/data/v400/storage_track_index.php create mode 100644 phpBB/phpbb/storage/file_tracker.php delete mode 100644 phpBB/phpbb/storage/stream_interface.php diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index 86c9298828..f064438aca 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -82,7 +82,7 @@ {% set buttons = [] %} {% for button in options.buttons %} - {% set new_button = button | merge({"name": input_name, "checked": button.value == input_value}) %} + {% set new_button = button | merge({"name": input_name, "label": lang(button.label), "checked": button.value == input_value}) %} {% set buttons = buttons | merge([new_button]) %} {% endfor %} diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 4ff6f96ae3..1109aa626b 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -4,33 +4,27 @@ services: storage.attachment: class: phpbb\storage\storage arguments: - - '@dbal.conn' - - '@cache.driver' - '@storage.adapter.factory' + - '@storage.file_tracker' - 'attachment' - - '%tables.storage%' tags: - { name: storage } storage.avatar: class: phpbb\storage\storage arguments: - - '@dbal.conn' - - '@cache.driver' - '@storage.adapter.factory' + - '@storage.file_tracker' - 'avatar' - - '%tables.storage%' tags: - { name: storage } storage.backup: class: phpbb\storage\storage arguments: - - '@dbal.conn' - - '@cache.driver' - '@storage.adapter.factory' + - '@storage.file_tracker' - 'backup' - - '%tables.storage%' tags: - { name: storage } @@ -124,3 +118,11 @@ services: - '@storage.state_helper' - '@storage.provider_collection' - '@storage.adapter_collection' + + storage.file_tracker: + class: phpbb\storage\file_tracker + arguments: + - '@dbal.conn' + - '@cache.driver' + - '%tables.storage%' + diff --git a/phpBB/includes/acp/acp_database.php b/phpBB/includes/acp/acp_database.php index 85943dc5e4..2b95ff3ad5 100644 --- a/phpBB/includes/acp/acp_database.php +++ b/phpBB/includes/acp/acp_database.php @@ -279,7 +279,7 @@ class acp_database try { - $stream = $storage->read_stream($backup_info['file_name']); + $stream = $storage->read($backup_info['file_name']); $fp = fopen($temp_file_name, 'w+b'); stream_copy_to_stream($stream, $fp); diff --git a/phpBB/includes/acp/acp_main.php b/phpBB/includes/acp/acp_main.php index 3692dd40bd..8b8a08be80 100644 --- a/phpBB/includes/acp/acp_main.php +++ b/phpBB/includes/acp/acp_main.php @@ -496,7 +496,7 @@ class acp_main $upload_dir_size = get_formatted_filesize($config['upload_dir_size']); $storage_avatar = $phpbb_container->get('storage.avatar'); - $avatar_dir_size = get_formatted_filesize($storage_avatar->get_size()); + $avatar_dir_size = get_formatted_filesize($storage_avatar->total_size()); if ($posts_per_day > $total_posts) { diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 2adf833562..35c07074b6 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -431,8 +431,8 @@ class acp_storage $storage_stats[] = [ 'name' => $this->lang->lang('STORAGE_' . strtoupper($storage->get_name()) . '_TITLE'), - 'files' => $storage->get_num_files(), - 'size' => get_formatted_filesize($storage->get_size()), + 'files' => $storage->total_files(), + 'size' => get_formatted_filesize($storage->total_size()), 'free_space' => $free_space, ]; } @@ -619,10 +619,18 @@ class acp_storage */ protected function validate_path(string $storage_name, array &$messages) : void { - $current_provider = $this->storage_helper->get_current_provider($storage_name); - $options = $this->storage_helper->get_provider_options($current_provider); + if ($this->request->is_set_post('submit')) + { + $provider = $this->request->variable([$storage_name, 'provider'], ''); + } + else + { + $provider = $this->storage_helper->get_current_provider($storage_name); + } - if ($this->provider_collection->get_by_class($current_provider)->get_name() == 'local' && isset($options['path'])) + $options = $this->storage_helper->get_provider_options($provider); + + if ($this->provider_collection->get_by_class($provider)->get_name() === 'local' && isset($options['path'])) { $path = $this->request->is_set_post('submit') ? $this->request->variable([$storage_name, 'path'], '') : $this->storage_helper->get_current_definition($storage_name, 'path'); diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 737332f4fa..78512d1971 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -2124,7 +2124,8 @@ function group_correct_avatar($group_id, $old_entry) try { - $storage->rename($old_filename, $new_filename); + $storage->write($new_filename, $storage->read($old_filename)); + $storage->delete($old_filename); $sql = 'UPDATE ' . GROUPS_TABLE . ' SET group_avatar = \'' . $db->sql_escape($new_entry) . "' diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index c0b0c490c4..ba9ffa2526 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -245,7 +245,7 @@ class upload // Move the thumbnail from temp folder to the storage $fp = fopen($destination, 'rb'); - $this->storage->write_stream($destination_name, $fp); + $this->storage->write($destination_name, $fp); if (is_resource($fp)) { diff --git a/phpBB/phpbb/db/migration/data/v400/storage_track.php b/phpBB/phpbb/db/migration/data/v400/storage_track.php index 7cf05503d6..d9890fd80f 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_track.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_track.php @@ -15,7 +15,7 @@ namespace phpbb\db\migration\data\v400; use phpbb\db\migration\container_aware_migration; use phpbb\storage\exception\storage_exception; -use phpbb\storage\storage; +use phpbb\storage\file_tracker; class storage_track extends container_aware_migration { @@ -70,8 +70,8 @@ class storage_track extends container_aware_migration public function track_avatars() { - /** @var storage $storage */ - $storage = $this->container->get('storage.avatar'); + /** @var file_tracker $file_tracker */ + $file_tracker = $this->container->get('storage.file_tracker'); $sql = 'SELECT user_avatar FROM ' . USERS_TABLE . " @@ -95,7 +95,8 @@ class storage_track extends container_aware_migration try { - $storage->track_file($this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $filename . '.' . $ext); + $filename = $this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $filename . '.' . $ext; + $file_tracker->track_file('avatar', $filename, filesize($this->phpbb_root_path . $this->config['storage\\avatar\\config\\path'] . '/' . $filename)); } catch (storage_exception $e) { @@ -107,8 +108,8 @@ class storage_track extends container_aware_migration public function track_attachments() { - /** @var storage $storage */ - $storage = $this->container->get('storage.attachment'); + /** @var file_tracker $file_tracker */ + $file_tracker = $this->container->get('storage.file_tracker'); $sql = 'SELECT physical_filename, thumbnail FROM ' . ATTACHMENTS_TABLE; @@ -119,7 +120,7 @@ class storage_track extends container_aware_migration { try { - $storage->track_file($row['physical_filename']); + $file_tracker->track_file('attachment', $row['physical_filename'], filesize($this->phpbb_root_path . $this->config['storage\\attachment\\config\\path'] . '/' . $row['physical_filename'])); } catch (storage_exception $e) { @@ -130,7 +131,7 @@ class storage_track extends container_aware_migration { try { - $storage->track_file('thumb_' . $row['physical_filename']); + $file_tracker->track_file('attachment', 'thumb_' . $row['physical_filename'], filesize($this->phpbb_root_path . $this->config['storage\\attachment\\config\\path'] . '/thumb_' . $row['physical_filename'])); } catch (storage_exception $e) { @@ -143,8 +144,8 @@ class storage_track extends container_aware_migration public function track_backups() { - /** @var storage $storage */ - $storage = $this->container->get('storage.backup'); + /** @var file_tracker $file_tracker */ + $file_tracker = $this->container->get('storage.file_tracker'); $sql = 'SELECT filename FROM ' . BACKUPS_TABLE; @@ -155,7 +156,7 @@ class storage_track extends container_aware_migration { try { - $storage->track_file($row['filename']); + $file_tracker->track_file('backup', $row['filename'], filesize($this->phpbb_root_path . $this->config['storage\\backup\\config\\path'] . '/' . $row['filename'])); } catch (storage_exception $e) { diff --git a/phpBB/phpbb/db/migration/data/v400/storage_track_index.php b/phpBB/phpbb/db/migration/data/v400/storage_track_index.php new file mode 100644 index 0000000000..63f57f5e00 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v400/storage_track_index.php @@ -0,0 +1,48 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\db\migration\data\v400; + +use phpbb\db\migration\container_aware_migration; + +class storage_track_index extends container_aware_migration +{ + public static function depends_on() + { + return [ + '\phpbb\db\migration\data\v400\storage_track', + ]; + } + + public function update_schema() + { + return [ + 'add_unique_index' => [ + $this->table_prefix . 'storage' => [ + 'uidx_storage' => ['file_path', 'storage'], + ], + ] + ]; + } + + public function revert_schema() + { + return [ + 'drop_keys' => [ + $this->table_prefix . 'storage' => [ + 'uidx_storage', + ], + ], + ]; + } +} diff --git a/phpBB/phpbb/files/filespec_storage.php b/phpBB/phpbb/files/filespec_storage.php index 83678718b5..5ded1e380b 100644 --- a/phpBB/phpbb/files/filespec_storage.php +++ b/phpBB/phpbb/files/filespec_storage.php @@ -446,7 +446,7 @@ class filespec_storage { $fp = fopen($this->filename, 'rb'); - $storage->write_stream($this->destination_file, $fp); + $storage->write($this->destination_file, $fp); if (is_resource($fp)) { diff --git a/phpBB/phpbb/storage/adapter/adapter_interface.php b/phpBB/phpbb/storage/adapter/adapter_interface.php index d7fcd77bcb..db7a5475d4 100644 --- a/phpBB/phpbb/storage/adapter/adapter_interface.php +++ b/phpBB/phpbb/storage/adapter/adapter_interface.php @@ -25,32 +25,26 @@ interface adapter_interface public function configure(array $options): void; /** - * Dumps content into a file + * Reads a file as a stream * - * @param string $path - * @param string $content - * @throws storage_exception When the file cannot be written + * @param string $path File to read + * + * @return resource Returns a file pointer + * @throws storage_exception When unable to open file */ - public function put_contents(string $path, string $content): void; + public function read(string $path); /** - * Read the contents of a file + * Writes a new file using a stream * - * @param string $path The file to read + * @param string $path The target file + * @param resource $resource The resource * - * @return string Returns file contents - * @throws storage_exception When cannot read file contents + * @return int Returns the number of bytes written + * @throws storage_exception When target file exists + * When target file cannot be created */ - public function get_contents(string $path): string; - - /** - * Checks the existence of files or directories - * - * @param string $path file/directory to check - * - * @return bool Returns true if the file/directory exist, false otherwise. - */ - public function exists(string $path): bool; + public function write(string $path, $resource): int; /** * Removes files or directories @@ -61,37 +55,6 @@ interface adapter_interface */ public function delete(string $path): void; - /** - * Rename a file or a directory - * - * @param string $path_orig The original file/direcotry - * @param string $path_dest The target file/directory - * - * @throws storage_exception When file/directory cannot be renamed - */ - public function rename(string $path_orig, string $path_dest): void; - - /** - * Copies a file - * - * @param string $path_orig The original filename - * @param string $path_dest The target filename - * - * @throws storage_exception When the file cannot be copied - */ - public function copy(string $path_orig, string $path_dest): void; - - /** - * Get file size in bytes - * - * @param string $path The file - * - * @return int Size in bytes. - * - * @throws storage_exception When unable to retrieve file size - */ - public function file_size(string $path): int; - /** * Get space available in bytes * diff --git a/phpBB/phpbb/storage/adapter/local.php b/phpBB/phpbb/storage/adapter/local.php index 72d92096d5..3099df6241 100644 --- a/phpBB/phpbb/storage/adapter/local.php +++ b/phpBB/phpbb/storage/adapter/local.php @@ -13,7 +13,6 @@ namespace phpbb\storage\adapter; -use phpbb\storage\stream_interface; use phpbb\storage\exception\storage_exception; use phpbb\filesystem\exception\filesystem_exception; use phpbb\filesystem\filesystem; @@ -22,7 +21,7 @@ use phpbb\filesystem\helper as filesystem_helper; /** * Experimental */ -class local implements adapter_interface, stream_interface +class local implements adapter_interface { /** * Filesystem component @@ -47,16 +46,6 @@ class local implements adapter_interface, stream_interface */ protected $root_path; - /** - * Relative path from $phpbb_root_path to the storage folder - * Always finish with slash (/) character - * Example: - * - images/avatars/upload/ - * - * @var string path - */ - protected $path; - /** * Constructor * @@ -71,176 +60,20 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} + * + * */ public function configure(array $options): void { - $this->path = $options['path']; - - if (substr($this->path, -1, 1) !== '/') - { - $this->path = $this->path . '/'; - } - $this->root_path = filesystem_helper::realpath($this->phpbb_root_path . $options['path']) . DIRECTORY_SEPARATOR; } /** * {@inheritdoc} */ - public function put_contents(string $path, string $content): void + public function read(string $path) { - $this->ensure_directory_exists($path); - - try - { - $this->filesystem->dump_file($this->root_path . $this->get_path($path) . $this->get_filename($path), $content); - } - catch (filesystem_exception $e) - { - throw new storage_exception('STORAGE_CANNOT_WRITE_FILE', $path, array(), $e); - } - } - - /** - * {@inheritdoc} - */ - public function get_contents(string $path): string - { - $content = @file_get_contents($this->root_path . $this->get_path($path) . $this->get_filename($path)); - - if ($content === false) - { - throw new storage_exception('STORAGE_CANNOT_READ_FILE', $path); - } - - return $content; - } - - /** - * {@inheritdoc} - */ - public function exists(string $path): bool - { - return $this->filesystem->exists($this->root_path . $this->get_path($path) . $this->get_filename($path)); - } - - /** - * {@inheritdoc} - */ - public function delete(string $path): void - { - try - { - $this->filesystem->remove($this->root_path . $this->get_path($path) . $this->get_filename($path)); - } - catch (filesystem_exception $e) - { - throw new storage_exception('STORAGE_CANNOT_DELETE', $path, array(), $e); - } - } - - /** - * {@inheritdoc} - */ - public function rename(string $path_orig, string $path_dest): void - { - $this->ensure_directory_exists($path_dest); - - try - { - $this->filesystem->rename($this->root_path . $this->get_path($path_orig) . $this->get_filename($path_orig), $this->root_path . $this->get_path($path_dest) . $this->get_filename($path_dest), false); - } - catch (filesystem_exception $e) - { - throw new storage_exception('STORAGE_CANNOT_RENAME', $path_orig, array(), $e); - } - } - - /** - * {@inheritdoc} - */ - public function copy(string $path_orig, string $path_dest): void - { - $this->ensure_directory_exists($path_dest); - - try - { - $this->filesystem->copy($this->root_path . $this->get_path($path_orig) . $this->get_filename($path_orig), $this->root_path . $this->get_path($path_dest) . $this->get_filename($path_dest), false); - } - catch (filesystem_exception $e) - { - throw new storage_exception('STORAGE_CANNOT_COPY', $path_orig, array(), $e); - } - } - - /** - * Creates a directory recursively. - * - * @param string $path The directory path - * - * @throws storage_exception On any directory creation failure - */ - protected function create_dir(string $path): void - { - try - { - $this->filesystem->mkdir($this->root_path . $path); - } - catch (filesystem_exception $e) - { - throw new storage_exception('STORAGE_CANNOT_CREATE_DIR', $path, array(), $e); - } - } - - /** - * Ensures that the directory of a file exists. - * - * @param string $path The file path - * - * @throws storage_exception On any directory creation failure - */ - protected function ensure_directory_exists(string $path): void - { - $path = dirname($this->root_path . $this->get_path($path) . $this->get_filename($path)); - $path = filesystem_helper::make_path_relative($path, $this->root_path); - - if (!$this->exists($path)) - { - $this->create_dir($path); - } - } - - /** - * Get the path to the file - * - * @param string $path The file path - * @return string - */ - protected function get_path(string $path): string - { - $dirname = dirname($path); - $dirname = ($dirname != '.') ? $dirname . DIRECTORY_SEPARATOR : ''; - - return $dirname; - } - - /** - * To be used in other PR - * - * @param string $path The file path - * @return string - */ - protected function get_filename(string $path): string - { - return basename($path); - } - - /** - * {@inheritdoc} - */ - public function read_stream(string $path) - { - $stream = @fopen($this->root_path . $this->get_path($path) . $this->get_filename($path), 'rb'); + $stream = @fopen($this->root_path . $path, 'rb'); if (!$stream) { @@ -253,60 +86,52 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function write_stream(string $path, $resource): void + public function write(string $path, $resource): int { - $this->ensure_directory_exists($path); - - $stream = @fopen($this->root_path . $this->get_path($path) . $this->get_filename($path), 'w+b'); + $stream = @fopen($this->root_path . $path, 'w+b'); if (!$stream) { throw new storage_exception('STORAGE_CANNOT_CREATE_FILE', $path); } - if (stream_copy_to_stream($resource, $stream) === false) + if (($size = stream_copy_to_stream($resource, $stream)) === false) { fclose($stream); throw new storage_exception('STORAGE_CANNOT_COPY_RESOURCE'); } fclose($stream); - } - - /** - * {@inheritdoc} - */ - public function file_size(string $path): int - { - $size = @filesize($this->root_path . $this->get_path($path) . $this->get_filename($path)); - - if ($size === null) - { - throw new storage_exception('STORAGE_CANNOT_GET_FILESIZE'); - } return $size; } + /** + * {@inheritdoc} + */ + public function delete(string $path): void + { + try + { + $this->filesystem->remove($this->root_path . $path); + } + catch (filesystem_exception $e) + { + throw new storage_exception('STORAGE_CANNOT_DELETE', $path, array(), $e); + } + } + /** * {@inheritdoc} */ public function free_space(): float { - if (function_exists('disk_free_space')) - { - $free_space = @disk_free_space($this->root_path); - - if ($free_space === false) - { - throw new storage_exception('STORAGE_CANNOT_GET_FREE_SPACE'); - } - } - else + if (!function_exists('disk_free_space') || ($free_space = @disk_free_space($this->root_path)) === false) { throw new storage_exception('STORAGE_CANNOT_GET_FREE_SPACE'); } return $free_space; } + } diff --git a/phpBB/phpbb/storage/adapter_factory.php b/phpBB/phpbb/storage/adapter_factory.php index 61ea879482..3d55fc7a41 100644 --- a/phpBB/phpbb/storage/adapter_factory.php +++ b/phpBB/phpbb/storage/adapter_factory.php @@ -67,20 +67,20 @@ class adapter_factory $options[$definition] = $this->config['storage\\' . $storage_name . '\\config\\' . $definition]; } - return $this->get_with_options($storage_name, $options); + return $this->get_with_options($storage_name, $provider_class, $options); } /** - * Obtains a configured adapters for a given storage with custom options + * Obtains a configured adapters with custom options * * @param string $storage_name + * @param string $provider_class * @param array $options * * @return mixed */ - public function get_with_options(string $storage_name, array $options): mixed + public function get_with_options(string $storage_name, string $provider_class, array $options): mixed { - $provider_class = $this->config['storage\\' . $storage_name . '\\provider']; $provider = $this->providers->get_by_class($provider_class); if (!$provider->is_available()) @@ -89,6 +89,7 @@ class adapter_factory } $adapter = $this->adapters->get_by_class($provider->get_adapter_class()); + $options['storage'] = $storage_name; $adapter->configure($options); return $adapter; diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 9d84fceb45..7aed1ccdb3 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -293,11 +293,11 @@ class attachment extends controller /** * Remove non valid characters https://github.com/symfony/http-foundation/commit/c7df9082ee7205548a97031683bc6550b5dc9551 */ - protected function filenameFallback($filename) + protected function filenameFallback($filename): string { - $filename = preg_replace(['/[^\x20-\x7e]/', '/%/', '/\//', '/\\\\/'], '', $filename); + $filename = (string) preg_replace(['/[^\x20-\x7e]/', '/%/', '/\//', '/\\\\/'], '', $filename); - return (!empty($filename)) ?: 'File'; + return !empty($filename) ? $filename : 'File'; } /** @@ -305,7 +305,7 @@ class attachment extends controller */ protected function prepare(StreamedResponse $response, string $file): void { - $response->setPrivate(); // By default should be private, but make sure of it + $response->setPrivate(); // By default, should be private, but make sure of it parent::prepare($response, $file); } @@ -445,7 +445,7 @@ class attachment extends controller if (!$url) { - return ($this->config['secure_allow_empty_referer']) ? true : false; + return (bool) $this->config['secure_allow_empty_referer']; } // Split URL into domain and script part @@ -453,13 +453,13 @@ class attachment extends controller if ($url === false) { - return ($this->config['secure_allow_empty_referer']) ? true : false; + return (bool) $this->config['secure_allow_empty_referer']; } $hostname = $url['host']; unset($url); - $allowed = ($this->config['secure_allow_deny']) ? false : true; + $allowed = !$this->config['secure_allow_deny']; $iplist = array(); if (($ip_ary = @gethostbynamel($hostname)) !== false) diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index 720745e2cc..ba8a8a4e95 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -90,7 +90,7 @@ class avatar extends controller } $ext = substr(strrchr($file, '.'), 1); - $file = (int) $file; + $file = (int) $file; // This removes the timestamp leaving only the user id return $this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $file . '.' . $ext; } diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 94af901eed..155d3df7a6 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -159,7 +159,7 @@ class controller @set_time_limit(0); - $fp = $this->storage->read_stream($file); + $fp = $this->storage->read($file); // Close db connection $this->file_gc(); @@ -173,7 +173,7 @@ class controller flush(); // Terminate script to avoid the execution of terminate events - // This avoid possible errors with db connection closed + // This avoids possible errors with db connection closed exit; }); diff --git a/phpBB/phpbb/storage/exception/storage_exception.php b/phpBB/phpbb/storage/exception/storage_exception.php index 08c0cfa4ef..205cf82561 100644 --- a/phpBB/phpbb/storage/exception/storage_exception.php +++ b/phpBB/phpbb/storage/exception/storage_exception.php @@ -36,7 +36,7 @@ class storage_exception extends runtime_exception * * @return string */ - public function get_filename() + public function get_filename(): string { $parameters = $this->get_parameters(); return $parameters['filename']; diff --git a/phpBB/phpbb/storage/file_tracker.php b/phpBB/phpbb/storage/file_tracker.php new file mode 100644 index 0000000000..c858b733ac --- /dev/null +++ b/phpBB/phpbb/storage/file_tracker.php @@ -0,0 +1,198 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\storage; + +use phpbb\cache\driver\driver_interface as cache; +use phpbb\db\driver\driver_interface as db; +use phpbb\storage\exception\storage_exception; + +class file_tracker +{ + + /** + * @var db + */ + protected $db; + + /** + * Cache driver + * @var cache + */ + protected $cache; + + /** + * @var string + */ + protected $storage_table; + + /** + * Constructor + * + * @param db $db + * @param cache $cache + * @param string $storage_table + */ + public function __construct(db $db, cache $cache, string $storage_table) + { + $this->db = $db; + $this->cache = $cache; + $this->storage_table = $storage_table; + } + + /** + * Track file in database + * + * @param string $storage Storage name + * @param string $path The target file + * @param int $size Size in bytes + */ + public function track_file(string $storage, string $path, int $size): void + { + $sql_ary = array( + 'file_path' => $path, + 'storage' => $storage, + 'filesize' => $size, + ); + + $sql = 'INSERT INTO ' . $this->storage_table . $this->db->sql_build_array('INSERT', $sql_ary); + $this->db->sql_query($sql); + + $this->cache->destroy('_storage_' . $storage . '_totalsize'); + $this->cache->destroy('_storage_' . $storage . '_numfiles'); + } + + /** + * Untrack file + * + * @param string $storage Storage name + * @param string $path The target file + */ + public function untrack_file(string $storage, $path): void + { + $sql_ary = array( + 'file_path' => $path, + 'storage' => $storage, + ); + + $sql = 'DELETE FROM ' . $this->storage_table . ' + WHERE ' . $this->db->sql_build_array('DELETE', $sql_ary); + $this->db->sql_query($sql); + + $this->cache->destroy('_storage_' . $storage . '_totalsize'); + $this->cache->destroy('_storage_' . $storage . '_numfiles'); + } + + /** + * Check if a file is tracked + * + * @param string $storage Storage name + * @param string $path The file + * + * @return bool True if file is tracked + */ + public function is_tracked(string $storage, string $path): bool + { + $sql_ary = array( + 'file_path' => $path, + 'storage' => $storage, + ); + + $sql = 'SELECT file_id FROM ' . $this->storage_table . ' + WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); + $result = $this->db->sql_query($sql); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + return $row !== false; + } + + /** + * Get file size in bytes + * + * @param string $path The file + * + * @return int Size in bytes. + * + * @throws storage_exception When unable to retrieve file size + */ + public function file_size(string $storage, string $path): int + { + $sql_ary = array( + 'file_path' => $path, + 'storage' => $storage, + ); + + $sql = 'SELECT filesize FROM ' . $this->storage_table . ' + WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); + $result = $this->db->sql_query($sql); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + return (int) $row['filesize']; + } + + /** + * Get number of tracked storage files for a storage + * + * @param string $storage Storage name + * + * @return int Number of files + */ + public function total_files(string $storage): int + { + $number_files = $this->cache->get('_storage_' . $storage. '_numfiles'); + + if ($number_files === false) + { + $sql = 'SELECT COUNT(file_id) AS numfiles + FROM ' . $this->storage_table . " + WHERE storage = '" . $this->db->sql_escape($storage) . "'"; + $result = $this->db->sql_query($sql); + + $number_files = $this->db->sql_fetchfield('numfiles'); + $this->cache->put('_storage_' . $storage . '_numfiles', $number_files); + + $this->db->sql_freeresult($result); + } + + return (int) $number_files; + } + + /** + * Get total storage size + * + * @param string $storage Storage name + * + * @return float Size in bytes + */ + public function total_size(string $storage): float + { + $total_size = $this->cache->get('_storage_' . $storage . '_totalsize'); + + if ($total_size === false) + { + $sql = 'SELECT SUM(filesize) AS totalsize + FROM ' . $this->storage_table . " + WHERE storage = '" . $this->db->sql_escape($storage) . "'"; + $result = $this->db->sql_query($sql); + + $total_size = $this->db->sql_fetchfield('totalsize'); + $this->cache->put('_storage_' . $storage . '_totalsize', $total_size); + + $this->db->sql_freeresult($result); + } + + return (float) $total_size; + } +} diff --git a/phpBB/phpbb/storage/helper.php b/phpBB/phpbb/storage/helper.php index 66921b60a7..66db2f4e8d 100644 --- a/phpBB/phpbb/storage/helper.php +++ b/phpBB/phpbb/storage/helper.php @@ -129,7 +129,7 @@ class helper $options[$definition] = $this->state_helper->new_definition_value($storage_name, $definition); } - $adapters[$storage_name] = $this->adapter_factory->get_with_options($storage_name, $options); + $adapters[$storage_name] = $this->adapter_factory->get_with_options($storage_name, $provider_class, $options); } return $adapters[$storage_name]; @@ -193,8 +193,8 @@ class helper $current_adapter = $this->get_current_adapter($storage_name); $new_adapter = $this->get_new_adapter($storage_name); - $stream = $current_adapter->read_stream($file); - $new_adapter->write_stream($file, $stream); + $stream = $current_adapter->read($file); + $new_adapter->write($file, $stream); if (is_resource($stream)) { diff --git a/phpBB/phpbb/storage/provider/local.php b/phpBB/phpbb/storage/provider/local.php index c6338e0c31..67a1789434 100644 --- a/phpBB/phpbb/storage/provider/local.php +++ b/phpBB/phpbb/storage/provider/local.php @@ -18,7 +18,7 @@ class local implements provider_interface /** * {@inheritdoc} */ - public function get_name() + public function get_name(): string { return 'local'; } @@ -34,7 +34,7 @@ class local implements provider_interface /** * {@inheritdoc} */ - public function get_options() + public function get_options(): array { return [ 'path' => [ @@ -47,7 +47,7 @@ class local implements provider_interface /** * {@inheritdoc} */ - public function is_available() + public function is_available(): bool { return true; } diff --git a/phpBB/phpbb/storage/provider/provider_interface.php b/phpBB/phpbb/storage/provider/provider_interface.php index 484b660aa8..aa01a51e37 100644 --- a/phpBB/phpbb/storage/provider/provider_interface.php +++ b/phpBB/phpbb/storage/provider/provider_interface.php @@ -20,7 +20,7 @@ interface provider_interface * * @return string */ - public function get_name(); + public function get_name(): string; /** * Gets adapter class @@ -34,12 +34,12 @@ interface provider_interface * * @return array Configuration keys */ - public function get_options(); + public function get_options(): array; /** * Return true if the adapter is available * * @return bool */ - public function is_available(); + public function is_available(): bool; } diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index fa1ca0d204..b07ea62441 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -13,8 +13,6 @@ namespace phpbb\storage; -use phpbb\cache\driver\driver_interface as cache; -use phpbb\db\driver\driver_interface as db; use phpbb\storage\adapter\adapter_interface; use phpbb\storage\exception\storage_exception; @@ -28,48 +26,33 @@ class storage */ protected $adapter; - /** - * @var db - */ - protected $db; - - /** - * Cache driver - * @var cache - */ - protected $cache; - /** * @var adapter_factory */ protected $factory; + /** + * @var file_tracker + */ + protected $file_tracker; + /** * @var string */ protected $storage_name; - /** - * @var string - */ - protected $storage_table; - /** * Constructor * - * @param db $db - * @param cache $cache * @param adapter_factory $factory + * @param file_tracker $file_tracker * @param string $storage_name - * @param string $storage_table */ - public function __construct(db $db, cache $cache, adapter_factory $factory, string $storage_name, string $storage_table) + public function __construct(adapter_factory $factory, file_tracker $file_tracker, string $storage_name) { - $this->db = $db; - $this->cache = $cache; $this->factory = $factory; + $this->file_tracker = $file_tracker; $this->storage_name = $storage_name; - $this->storage_table = $storage_table; } /** @@ -98,57 +81,48 @@ class storage } /** - * Dumps content into a file + * Reads a file as a stream * - * @param string $path The file to be written to. - * @param string $content The data to write into the file. - * - * @throws storage_exception When the file already exists - * When the file cannot be written - */ - public function put_contents(string $path, string $content): void - { - if ($this->exists($path)) - { - throw new storage_exception('STORAGE_FILE_EXISTS', $path); - } - - $this->get_adapter()->put_contents($path, $content); - $this->track_file($path); - } - - /** - * Read the contents of a file - * - * @param string $path The file to read - * - * @return string Returns file contents + * @param string $path File to read * + * @return resource Returns a file pointer * @throws storage_exception When the file doesn't exist - * When cannot read file contents + * When unable to open file * */ - public function get_contents(string $path): string + public function read(string $path) { if (!$this->exists($path)) { throw new storage_exception('STORAGE_FILE_NO_EXIST', $path); } - return $this->get_adapter()->get_contents($path); + return $this->get_adapter()->read($path); } /** - * Checks the existence of files or directories + * Writes a new file using a stream * - * @param string $path file/directory to check - * @param bool $full_check check in the filesystem too + * @param string $path The target file + * @param resource $resource The resource * - * @return bool Returns true if the file/directory exist, false otherwise + * @throws storage_exception When the file exist + * When target file cannot be created */ - public function exists(string $path, bool $full_check = false): bool + public function write(string $path, $resource): void { - return ($this->is_tracked($path) && (!$full_check || $this->get_adapter()->exists($path))); + if ($this->exists($path)) + { + throw new storage_exception('STORAGE_FILE_EXISTS', $path); + } + + if (!is_resource($resource)) + { + throw new storage_exception('STORAGE_INVALID_RESOURCE'); + } + + $size = $this->get_adapter()->write($path, $resource); + $this->file_tracker->track_file($this->storage_name, $path, $size); } /** @@ -167,231 +141,19 @@ class storage } $this->get_adapter()->delete($path); - $this->untrack_file($path); + $this->file_tracker->untrack_file($this->get_name(), $path); } /** - * Rename a file or a directory + * Checks the existence of files or directories * - * @param string $path_orig The original file/direcotry - * @param string $path_dest The target file/directory + * @param string $path file/directory to check * - * @throws storage_exception When the file doesn't exist - * When target exists - * When file/directory cannot be renamed + * @return bool Returns true if the file/directory exist, false otherwise */ - public function rename(string $path_orig, string $path_dest): void + public function exists(string $path): bool { - if (!$this->exists($path_orig)) - { - throw new storage_exception('STORAGE_FILE_NO_EXIST', $path_orig); - } - - if ($this->exists($path_dest)) - { - throw new storage_exception('STORAGE_FILE_EXISTS', $path_dest); - } - - $this->get_adapter()->rename($path_orig, $path_dest); - $this->track_rename($path_orig, $path_dest); - } - - /** - * Copies a file - * - * @param string $path_orig The original filename - * @param string $path_dest The target filename - * - * @throws storage_exception When the file doesn't exist - * When target exists - * When the file cannot be copied - */ - public function copy(string $path_orig, string $path_dest): void - { - if (!$this->exists($path_orig)) - { - throw new storage_exception('STORAGE_FILE_NO_EXIST', $path_orig); - } - - if ($this->exists($path_dest)) - { - throw new storage_exception('STORAGE_FILE_EXISTS', $path_dest); - } - - $this->get_adapter()->copy($path_orig, $path_dest); - $this->track_file($path_dest); - } - - /** - * Reads a file as a stream - * - * @param string $path File to read - * - * @return resource Returns a file pointer - * @throws storage_exception When the file doesn't exist - * When unable to open file - * - */ - public function read_stream(string $path) - { - if (!$this->exists($path)) - { - throw new storage_exception('STORAGE_FILE_NO_EXIST', $path); - } - - $stream = null; - $adapter = $this->get_adapter(); - - if ($adapter instanceof stream_interface) - { - $stream = $adapter->read_stream($path); - } - else - { - // Simulate the stream - $stream = fopen('php://temp', 'w+b'); - fwrite($stream, $adapter->get_contents($path)); - rewind($stream); - } - - return $stream; - } - - /** - * Writes a new file using a stream - * - * @param string $path The target file - * @param resource $resource The resource - * - * @throws storage_exception When the file exist - * When target file cannot be created - */ - public function write_stream(string $path, $resource): void - { - if ($this->exists($path)) - { - throw new storage_exception('STORAGE_FILE_EXISTS', $path); - } - - if (!is_resource($resource)) - { - throw new storage_exception('STORAGE_INVALID_RESOURCE'); - } - - $adapter = $this->get_adapter(); - - if ($adapter instanceof stream_interface) - { - $adapter->write_stream($path, $resource); - $this->track_file($path); - } - else - { - // Simulate the stream - $adapter->put_contents($path, stream_get_contents($resource)); - } - } - - /** - * Track file in database - * - * @param string $path The target file - * @param bool $update Update file size when already tracked - */ - public function track_file(string $path, bool $update = false): void - { - if (!$this->get_adapter()->exists($path)) - { - throw new storage_exception('STORAGE_FILE_NO_EXIST', $path); - } - - $sql_ary = array( - 'file_path' => $path, - 'storage' => $this->get_name(), - ); - - // Get file, if exist update filesize, if not add new record - $sql = 'SELECT * FROM ' . $this->storage_table . ' - WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); - $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - if (!$row) - { - $sql_ary['filesize'] = $this->get_adapter()->file_size($path); - - $sql = 'INSERT INTO ' . $this->storage_table . $this->db->sql_build_array('INSERT', $sql_ary); - $this->db->sql_query($sql); - } - else if ($update) - { - $sql = 'UPDATE ' . $this->storage_table . ' - SET filesize = ' . $this->get_adapter()->file_size($path) . ' - WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); - $this->db->sql_query($sql); - } - - $this->cache->destroy('_storage_' . $this->get_name() . '_totalsize'); - $this->cache->destroy('_storage_' . $this->get_name() . '_numfiles'); - } - - /** - * Untrack file - * - * @param string $path The target file - */ - public function untrack_file($path) - { - $sql_ary = array( - 'file_path' => $path, - 'storage' => $this->get_name(), - ); - - $sql = 'DELETE FROM ' . $this->storage_table . ' - WHERE ' . $this->db->sql_build_array('DELETE', $sql_ary); - $this->db->sql_query($sql); - - $this->cache->destroy('_storage_' . $this->get_name() . '_totalsize'); - $this->cache->destroy('_storage_' . $this->get_name() . '_numfiles'); - } - - /** - * Check if a file is tracked - * - * @param string $path The file - * - * @return bool True if file is tracked - */ - public function is_tracked(string $path): bool - { - $sql_ary = array( - 'file_path' => $path, - 'storage' => $this->get_name(), - ); - - $sql = 'SELECT file_id FROM ' . $this->storage_table . ' - WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); - $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - return $row !== false; - } - - /** - * Rename tracked file - * - * @param string $path_orig The original file/direcotry - * @param string $path_dest The target file/directory - */ - protected function track_rename(string $path_orig, string $path_dest): void - { - $sql = 'UPDATE ' . $this->storage_table . " - SET file_path = '" . $this->db->sql_escape($path_dest) . "' - WHERE file_path = '" . $this->db->sql_escape($path_orig) . "' - AND storage = '" . $this->db->sql_escape($this->get_name()) . "'"; - $this->db->sql_query($sql); + return $this->file_tracker->is_tracked($this->get_name(), $path); } /** @@ -400,73 +162,30 @@ class storage * @param string $path The file * * @return int Size in bytes. - * - * @throws storage_exception When unable to retrieve file size */ public function file_size(string $path): int { - $sql_ary = array( - 'file_path' => $path, - 'storage' => $this->get_name(), - ); + return $this->file_tracker->file_size($this->get_name(), $path); + } - $sql = 'SELECT filesize FROM ' . $this->storage_table . ' - WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); - $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - return $row !== false && !empty($row['filesize']) ? $row['filesize'] : $this->get_adapter()->file_size($path); + /** + * Return the number of files stored in this storage + * + * @return int Number of files. + */ + public function total_files(): int + { + return $this->file_tracker->total_files($this->get_name()); } /** * Get total storage size * - * @return int Size in bytes + * @return float Size in bytes */ - public function get_size(): int + public function total_size(): float { - $total_size = $this->cache->get('_storage_' . $this->get_name() . '_totalsize'); - - if ($total_size === false) - { - $sql = 'SELECT SUM(filesize) AS totalsize - FROM ' . $this->storage_table . " - WHERE storage = '" . $this->db->sql_escape($this->get_name()) . "'"; - $result = $this->db->sql_query($sql); - - $total_size = (int) $this->db->sql_fetchfield('totalsize'); - $this->cache->put('_storage_' . $this->get_name() . '_totalsize', $total_size); - - $this->db->sql_freeresult($result); - } - - return (int) $total_size; - } - - /** - * Get number of storage files - * - * @return int Number of files - */ - public function get_num_files(): int - { - $number_files = $this->cache->get('_storage_' . $this->get_name() . '_numfiles'); - - if ($number_files === false) - { - $sql = 'SELECT COUNT(file_id) AS numfiles - FROM ' . $this->storage_table . " - WHERE storage = '" . $this->db->sql_escape($this->get_name()) . "'"; - $result = $this->db->sql_query($sql); - - $number_files = (int) $this->db->sql_fetchfield('numfiles'); - $this->cache->put('_storage_' . $this->get_name() . '_numfiles', $number_files); - - $this->db->sql_freeresult($result); - } - - return (int) $number_files; + return $this->file_tracker->total_size($this->get_name()); } /** diff --git a/phpBB/phpbb/storage/stream_interface.php b/phpBB/phpbb/storage/stream_interface.php deleted file mode 100644 index 424ffcb95c..0000000000 --- a/phpBB/phpbb/storage/stream_interface.php +++ /dev/null @@ -1,41 +0,0 @@ - - * @license GNU General Public License, version 2 (GPL-2.0) - * - * For full copyright and license information, please see - * the docs/CREDITS.txt file. - * - */ - -namespace phpbb\storage; - -use phpbb\storage\exception\storage_exception; - -interface stream_interface -{ - /** - * Reads a file as a stream - * - * @param string $path File to read - * - * @return resource Returns a file pointer - * @throws storage_exception When unable to open file - */ - public function read_stream(string $path); - - /** - * Writes a new file using a stream - * - * @param string $path The target file - * @param resource $resource The resource - * - * @return void - * @throws storage_exception When target file exists - * When target file cannot be created - */ - public function write_stream(string $path, $resource): void; -} diff --git a/tests/storage/adapter/local_test.php b/tests/storage/adapter/local_test.php index 2fee54b2f5..01a7884596 100644 --- a/tests/storage/adapter/local_test.php +++ b/tests/storage/adapter/local_test.php @@ -22,51 +22,6 @@ class phpbb_storage_adapter_local_test extends phpbb_local_test_case $this->adapter->configure(['path' => 'test_path']); } - public function test_put_contents(): void - { - // When - $this->adapter->put_contents('file.txt', 'abc'); - - // Then - $this->assertFileExists($this->path . 'file.txt'); - $this->assertFileContains($this->path . 'file.txt', 'abc'); - - // Clean test - unlink($this->path . 'file.txt'); - } - - public function test_get_contents(): void - { - // Given - file_put_contents($this->path . 'file.txt', 'abc'); - - // When - $content = $this->adapter->get_contents('file.txt'); - - // Then - $this->assertEquals('abc', $content); - - // Clean test - unlink($this->path . 'file.txt'); - } - - public function test_exists(): void - { - // Given - touch($this->path . 'file.txt'); - - // When - $existent_file = $this->adapter->exists('file.txt'); - $non_existent_file = $this->adapter->exists('noexist.txt'); - - // Then - $this->assertTrue($existent_file); - $this->assertFalse($non_existent_file); - - // Clean test - unlink($this->path . 'file.txt'); - } - public function test_delete_file(): void { // Given @@ -80,48 +35,13 @@ class phpbb_storage_adapter_local_test extends phpbb_local_test_case $this->assertFileDoesNotExist($this->path . 'file.txt'); } - public function test_rename(): void - { - // Given - touch($this->path . 'file.txt'); - $this->assertFileExists($this->path . 'file.txt'); - $this->assertFileDoesNotExist($this->path . 'file2.txt'); - - // When - $this->adapter->rename('file.txt', 'file2.txt'); - - // Then - $this->assertFileDoesNotExist($this->path . 'file.txt'); - $this->assertFileExists($this->path . 'file2.txt'); - - // Clean test - unlink($this->path . 'file2.txt'); - } - - public function test_copy(): void + public function test_read() { // Given file_put_contents($this->path . 'file.txt', 'abc'); // When - $this->adapter->copy('file.txt', 'file2.txt'); - - // Then - $this->assertFileContains($this->path . 'file.txt', 'abc'); - $this->assertFileContains($this->path . 'file2.txt', 'abc'); - - // Clean test - unlink($this->path . 'file.txt'); - unlink($this->path . 'file2.txt'); - } - - public function test_read_stream() - { - // Given - file_put_contents($this->path . 'file.txt', 'abc'); - - // When - $stream = $this->adapter->read_stream('file.txt'); + $stream = $this->adapter->read('file.txt'); // Then $this->assertIsResource($stream); @@ -132,14 +52,14 @@ class phpbb_storage_adapter_local_test extends phpbb_local_test_case unlink($this->path . 'file.txt'); } - public function test_write_stream() + public function test_write() { // Given file_put_contents($this->path . 'file.txt', 'abc'); $stream = fopen($this->path . 'file.txt', 'rb'); // When - $this->adapter->write_stream('file2.txt', $stream); + $this->adapter->write('file2.txt', $stream); fclose($stream); // Then From 6d9a8ae8afce9b45bebd55abb58d3c175b4fe06c Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Thu, 28 Nov 2024 21:21:15 +0100 Subject: [PATCH 02/14] [ticket/17361] Remove fclose after write PHPBB-17361 --- phpBB/phpbb/attachment/upload.php | 5 ----- phpBB/phpbb/files/filespec_storage.php | 5 ----- phpBB/phpbb/storage/helper.php | 5 ----- 3 files changed, 15 deletions(-) diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index ba9ffa2526..f1ae0b8533 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -246,11 +246,6 @@ class upload $fp = fopen($destination, 'rb'); $this->storage->write($destination_name, $fp); - - if (is_resource($fp)) - { - fclose($fp); - } } else { diff --git a/phpBB/phpbb/files/filespec_storage.php b/phpBB/phpbb/files/filespec_storage.php index 5ded1e380b..f223976b9e 100644 --- a/phpBB/phpbb/files/filespec_storage.php +++ b/phpBB/phpbb/files/filespec_storage.php @@ -447,11 +447,6 @@ class filespec_storage $fp = fopen($this->filename, 'rb'); $storage->write($this->destination_file, $fp); - - if (is_resource($fp)) - { - fclose($fp); - } } catch (\phpbb\storage\exception\storage_exception $e) { diff --git a/phpBB/phpbb/storage/helper.php b/phpBB/phpbb/storage/helper.php index 66db2f4e8d..429e9b0a7c 100644 --- a/phpBB/phpbb/storage/helper.php +++ b/phpBB/phpbb/storage/helper.php @@ -195,11 +195,6 @@ class helper $stream = $current_adapter->read($file); $new_adapter->write($file, $stream); - - if (is_resource($stream)) - { - fclose($stream); - } } From a8399c14319378bc0a008a87821dcb11abf04088 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Thu, 28 Nov 2024 22:19:04 +0100 Subject: [PATCH 03/14] [ticket/17361] Use storage in commands create and delete thumbnail PHPBB-17361 --- .../default/container/services_console.yml | 8 ++- .../console/command/thumbnail/delete.php | 61 +++++++++++-------- .../console/command/thumbnail/generate.php | 59 +++++++++++------- .../console/command/thumbnail/recreate.php | 21 ++++++- 4 files changed, 98 insertions(+), 51 deletions(-) diff --git a/phpBB/config/default/container/services_console.yml b/phpBB/config/default/container/services_console.yml index 6c85e22f9a..e1c965ce17 100644 --- a/phpBB/config/default/container/services_console.yml +++ b/phpBB/config/default/container/services_console.yml @@ -284,20 +284,21 @@ services: console.command.thumbnail.delete: class: phpbb\console\command\thumbnail\delete arguments: - - '@config' - '@user' - '@dbal.conn' - - '%core.root_path%' + - '@language' + - '@storage.attachment' tags: - { name: console.command } console.command.thumbnail.generate: class: phpbb\console\command\thumbnail\generate arguments: - - '@config' - '@user' - '@dbal.conn' - '@cache' + - '@language' + - '@storage.attachment' - '%core.root_path%' - '%core.php_ext%' tags: @@ -307,6 +308,7 @@ services: class: phpbb\console\command\thumbnail\recreate arguments: - '@user' + - '@language' tags: - { name: console.command } diff --git a/phpBB/phpbb/console/command/thumbnail/delete.php b/phpBB/phpbb/console/command/thumbnail/delete.php index 21fb843e7d..8b767cbbd8 100644 --- a/phpBB/phpbb/console/command/thumbnail/delete.php +++ b/phpBB/phpbb/console/command/thumbnail/delete.php @@ -12,6 +12,11 @@ */ namespace phpbb\console\command\thumbnail; +use phpbb\db\driver\driver_interface; +use phpbb\language\language; +use phpbb\storage\exception\storage_exception; +use phpbb\storage\storage; +use phpbb\user; use Symfony\Component\Console\Command\Command as symfony_command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -20,15 +25,20 @@ use Symfony\Component\Console\Style\SymfonyStyle; class delete extends \phpbb\console\command\command { /** - * @var \phpbb\config\config - */ - protected $config; - - /** - * @var \phpbb\db\driver\driver_interface + * @var driver_interface */ protected $db; + /** + * @var language + */ + protected $language; + + /** + * @var storage + */ + protected $storage; + /** * phpBB root path * @var string @@ -38,16 +48,16 @@ class delete extends \phpbb\console\command\command /** * Constructor * - * @param \phpbb\config\config $config The config - * @param \phpbb\user $user The user object (used to get language information) - * @param \phpbb\db\driver\driver_interface $db Database connection - * @param string $phpbb_root_path Root path + * @param user $user The user object (used to get language information) + * @param driver_interface $db Database connection + * @param language $language Language + * @param storage $storage Storage */ - public function __construct(\phpbb\config\config $config, \phpbb\user $user, \phpbb\db\driver\driver_interface $db, $phpbb_root_path) + public function __construct(user $user, driver_interface $db, language $language, storage $storage) { - $this->config = $config; $this->db = $db; - $this->phpbb_root_path = $phpbb_root_path; + $this->language = $language; + $this->storage = $storage; parent::__construct($user); } @@ -61,7 +71,7 @@ class delete extends \phpbb\console\command\command { $this ->setName('thumbnail:delete') - ->setDescription($this->user->lang('CLI_DESCRIPTION_THUMBNAIL_DELETE')) + ->setDescription($this->language->lang('CLI_DESCRIPTION_THUMBNAIL_DELETE')) ; } @@ -79,7 +89,7 @@ class delete extends \phpbb\console\command\command { $io = new SymfonyStyle($input, $output); - $io->section($this->user->lang('CLI_THUMBNAIL_DELETING')); + $io->section($this->language->lang('CLI_THUMBNAIL_DELETING')); $sql = 'SELECT COUNT(*) AS nb_missing_thumbnails FROM ' . ATTACHMENTS_TABLE . ' @@ -90,7 +100,7 @@ class delete extends \phpbb\console\command\command if ($nb_missing_thumbnails === 0) { - $io->warning($this->user->lang('CLI_THUMBNAIL_NOTHING_TO_DELETE')); + $io->warning($this->language->lang('CLI_THUMBNAIL_NOTHING_TO_DELETE')); return symfony_command::SUCCESS; } @@ -101,7 +111,7 @@ class delete extends \phpbb\console\command\command $progress = $this->create_progress_bar($nb_missing_thumbnails, $io, $output); - $progress->setMessage($this->user->lang('CLI_THUMBNAIL_DELETING')); + $progress->setMessage($this->language->lang('CLI_THUMBNAIL_DELETING')); $progress->start(); @@ -109,10 +119,10 @@ class delete extends \phpbb\console\command\command $return = symfony_command::SUCCESS; while ($row = $this->db->sql_fetchrow($result)) { - $thumbnail_path = $this->phpbb_root_path . $this->config['upload_path'] . '/thumb_' . $row['physical_filename']; - - if (@unlink($thumbnail_path)) + try { + $this->storage->delete('thumb_' . $row['physical_filename']); + $thumbnail_deleted[] = $row['attach_id']; if (count($thumbnail_deleted) === 250) @@ -121,12 +131,11 @@ class delete extends \phpbb\console\command\command $thumbnail_deleted = array(); } - $progress->setMessage($this->user->lang('CLI_THUMBNAIL_DELETED', $row['real_filename'], $row['physical_filename'])); - } - else - { + $progress->setMessage($this->language->lang('CLI_THUMBNAIL_DELETED', $row['real_filename'], $row['physical_filename'])); + } catch (storage_exception $e) { $return = symfony_command::FAILURE; - $progress->setMessage('' . $this->user->lang('CLI_THUMBNAIL_SKIPPED', $row['real_filename'], $row['physical_filename']) . ''); + $progress->setMessage('' . $this->language->lang('CLI_THUMBNAIL_SKIPPED', $row['real_filename'], $row['physical_filename']) . ''); + } $progress->advance(); @@ -141,7 +150,7 @@ class delete extends \phpbb\console\command\command $progress->finish(); $io->newLine(2); - $io->success($this->user->lang('CLI_THUMBNAIL_DELETING_DONE')); + $io->success($this->language->lang('CLI_THUMBNAIL_DELETING_DONE')); return $return; } diff --git a/phpBB/phpbb/console/command/thumbnail/generate.php b/phpBB/phpbb/console/command/thumbnail/generate.php index 9be4ce5a2e..8c8c46dd6d 100644 --- a/phpBB/phpbb/console/command/thumbnail/generate.php +++ b/phpBB/phpbb/console/command/thumbnail/generate.php @@ -13,6 +13,11 @@ namespace phpbb\console\command\thumbnail; +use phpbb\cache\service; +use phpbb\db\driver\driver_interface; +use phpbb\language\language; +use phpbb\storage\storage; +use phpbb\user; use Symfony\Component\Console\Command\Command as symfony_command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -20,21 +25,27 @@ use Symfony\Component\Console\Style\SymfonyStyle; class generate extends \phpbb\console\command\command { - /** - * @var \phpbb\config\config - */ - protected $config; /** - * @var \phpbb\db\driver\driver_interface + * @var driver_interface */ protected $db; /** - * @var \phpbb\cache\service + * @var service */ protected $cache; + /** + * @var language + */ + protected $language; + + /** + * @var storage + */ + protected $storage; + /** * phpBB root path * @var string @@ -51,18 +62,20 @@ class generate extends \phpbb\console\command\command /** * Constructor * - * @param \phpbb\config\config $config The config - * @param \phpbb\user $user The user object (used to get language information) - * @param \phpbb\db\driver\driver_interface $db Database connection - * @param \phpbb\cache\service $cache The cache service + * @param user $user The user object (used to get language information) + * @param driver_interface $db Database connection + * @param service $cache The cache service + * @param language $language Language + * @param storage $storage Storage * @param string $phpbb_root_path Root path * @param string $php_ext PHP extension */ - public function __construct(\phpbb\config\config $config, \phpbb\user $user, \phpbb\db\driver\driver_interface $db, \phpbb\cache\service $cache, $phpbb_root_path, $php_ext) + public function __construct(user $user, driver_interface $db, service $cache, language $language, storage $storage, string $phpbb_root_path, string $php_ext) { - $this->config = $config; $this->db = $db; $this->cache = $cache; + $this->language = $language; + $this->storage = $storage; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; @@ -78,7 +91,7 @@ class generate extends \phpbb\console\command\command { $this ->setName('thumbnail:generate') - ->setDescription($this->user->lang('CLI_DESCRIPTION_THUMBNAIL_GENERATE')) + ->setDescription($this->language->lang('CLI_DESCRIPTION_THUMBNAIL_GENERATE')) ; } @@ -96,7 +109,7 @@ class generate extends \phpbb\console\command\command { $io = new SymfonyStyle($input, $output); - $io->section($this->user->lang('CLI_THUMBNAIL_GENERATING')); + $io->section($this->language->lang('CLI_THUMBNAIL_GENERATING')); $sql = 'SELECT COUNT(*) AS nb_missing_thumbnails FROM ' . ATTACHMENTS_TABLE . ' @@ -107,7 +120,7 @@ class generate extends \phpbb\console\command\command if ($nb_missing_thumbnails === 0) { - $io->warning($this->user->lang('CLI_THUMBNAIL_NOTHING_TO_GENERATE')); + $io->warning($this->language->lang('CLI_THUMBNAIL_NOTHING_TO_GENERATE')); return symfony_command::SUCCESS; } @@ -125,7 +138,7 @@ class generate extends \phpbb\console\command\command $progress = $this->create_progress_bar($nb_missing_thumbnails, $io, $output); - $progress->setMessage($this->user->lang('CLI_THUMBNAIL_GENERATING')); + $progress->setMessage($this->language->lang('CLI_THUMBNAIL_GENERATING')); $progress->start(); @@ -134,11 +147,15 @@ class generate extends \phpbb\console\command\command { if (isset($extensions[$row['extension']]['display_cat']) && $extensions[$row['extension']]['display_cat'] == \phpbb\attachment\attachment_category::IMAGE) { - $source = $this->phpbb_root_path . $this->config['upload_path'] . '/' . $row['physical_filename']; - $destination = $this->phpbb_root_path . $this->config['upload_path'] . '/thumb_' . $row['physical_filename']; + $source = tempnam(sys_get_temp_dir(), 'thumbnail_source'); + $destination = tempnam(sys_get_temp_dir(), 'thumbnail_destination'); + + file_put_contents($source, $this->storage->read($row['physical_filename'])); if (create_thumbnail($source, $destination, $row['mimetype'])) { + $this->storage->write('thumb_' . $row['physical_filename'], fopen($destination, 'rb')); + $thumbnail_created[] = (int) $row['attach_id']; if (count($thumbnail_created) === 250) @@ -147,11 +164,11 @@ class generate extends \phpbb\console\command\command $thumbnail_created = array(); } - $progress->setMessage($this->user->lang('CLI_THUMBNAIL_GENERATED', $row['real_filename'], $row['physical_filename'])); + $progress->setMessage($this->language->lang('CLI_THUMBNAIL_GENERATED', $row['real_filename'], $row['physical_filename'])); } else { - $progress->setMessage('' . $this->user->lang('CLI_THUMBNAIL_SKIPPED', $row['real_filename'], $row['physical_filename']) . ''); + $progress->setMessage('' . $this->language->lang('CLI_THUMBNAIL_SKIPPED', $row['real_filename'], $row['physical_filename']) . ''); } } @@ -167,7 +184,7 @@ class generate extends \phpbb\console\command\command $progress->finish(); $io->newLine(2); - $io->success($this->user->lang('CLI_THUMBNAIL_GENERATING_DONE')); + $io->success($this->language->lang('CLI_THUMBNAIL_GENERATING_DONE')); return symfony_command::SUCCESS; } diff --git a/phpBB/phpbb/console/command/thumbnail/recreate.php b/phpBB/phpbb/console/command/thumbnail/recreate.php index cffa2c29dc..a0a0524b89 100644 --- a/phpBB/phpbb/console/command/thumbnail/recreate.php +++ b/phpBB/phpbb/console/command/thumbnail/recreate.php @@ -12,6 +12,8 @@ */ namespace phpbb\console\command\thumbnail; +use phpbb\language\language; +use phpbb\user; use Symfony\Component\Console\Command\Command as symfony_command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\ArrayInput; @@ -19,6 +21,23 @@ use Symfony\Component\Console\Output\OutputInterface; class recreate extends \phpbb\console\command\command { + /** + * @var language + */ + protected $language; + + /** + * Constructor + * + * @param language $language Language + */ + public function __construct(user $user, language $language) + { + $this->language = $language; + + parent::__construct($user); + } + /** * Sets the command name and description * @@ -28,7 +47,7 @@ class recreate extends \phpbb\console\command\command { $this ->setName('thumbnail:recreate') - ->setDescription($this->user->lang('CLI_DESCRIPTION_THUMBNAIL_RECREATE')) + ->setDescription($this->language->lang('CLI_DESCRIPTION_THUMBNAIL_RECREATE')) ; } From 4becf438e12f43ca3bd07fb7cd2cb6767fcbc231 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Thu, 28 Nov 2024 22:24:28 +0100 Subject: [PATCH 04/14] [ticket/17361] Remove temp files PHPBB-17361 --- phpBB/phpbb/console/command/thumbnail/generate.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpBB/phpbb/console/command/thumbnail/generate.php b/phpBB/phpbb/console/command/thumbnail/generate.php index 8c8c46dd6d..dfc6480510 100644 --- a/phpBB/phpbb/console/command/thumbnail/generate.php +++ b/phpBB/phpbb/console/command/thumbnail/generate.php @@ -151,10 +151,12 @@ class generate extends \phpbb\console\command\command $destination = tempnam(sys_get_temp_dir(), 'thumbnail_destination'); file_put_contents($source, $this->storage->read($row['physical_filename'])); + @unlink($source); if (create_thumbnail($source, $destination, $row['mimetype'])) { $this->storage->write('thumb_' . $row['physical_filename'], fopen($destination, 'rb')); + @unlink($destination); $thumbnail_created[] = (int) $row['attach_id']; From 9bf9538dc642aeb2ae076bdbe8aad125bc566458 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Thu, 28 Nov 2024 22:30:37 +0100 Subject: [PATCH 05/14] [ticket/17361] Update thumbnail_test test PHPBB-17361 --- .../console/command/thumbnail/generate.php | 3 ++- tests/console/thumbnail_test.php | 27 +++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/phpBB/phpbb/console/command/thumbnail/generate.php b/phpBB/phpbb/console/command/thumbnail/generate.php index dfc6480510..52ae19a9de 100644 --- a/phpBB/phpbb/console/command/thumbnail/generate.php +++ b/phpBB/phpbb/console/command/thumbnail/generate.php @@ -151,11 +151,12 @@ class generate extends \phpbb\console\command\command $destination = tempnam(sys_get_temp_dir(), 'thumbnail_destination'); file_put_contents($source, $this->storage->read($row['physical_filename'])); - @unlink($source); if (create_thumbnail($source, $destination, $row['mimetype'])) { $this->storage->write('thumb_' . $row['physical_filename'], fopen($destination, 'rb')); + + @unlink($source); @unlink($destination); $thumbnail_created[] = (int) $row['attach_id']; diff --git a/tests/console/thumbnail_test.php b/tests/console/thumbnail_test.php index 3a08e4b1ac..ebe0dff62e 100644 --- a/tests/console/thumbnail_test.php +++ b/tests/console/thumbnail_test.php @@ -24,6 +24,7 @@ class phpbb_console_command_thumbnail_test extends phpbb_database_test_case protected $config; protected $cache; protected $user; + protected $storage; protected $phpEx; protected $phpbb_root_path; protected $application; @@ -46,11 +47,10 @@ class phpbb_console_command_thumbnail_test extends phpbb_database_test_case $config = $this->config = new \phpbb\config\config(array( 'img_min_thumb_filesize' => 2, - 'img_max_thumb_width' => 2, - 'upload_path' => 'files', + 'img_max_thumb_width' => 2 )); - $this->db = $this->db = $this->new_dbal(); + $this->db = $this->new_dbal(); $this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); $this->user = new \phpbb\user($this->language, '\phpbb\datetime'); $this->phpbb_root_path = $phpbb_root_path; @@ -62,13 +62,24 @@ class phpbb_console_command_thumbnail_test extends phpbb_database_test_case 'txt' => array('display_cat' => attachment_category::NONE), ))); - $this->application = new Application(); - $this->application->add(new generate($config, $this->user, $this->db, $this->cache, $this->phpbb_root_path, $this->phpEx)); - $this->application->add(new delete($config, $this->user, $this->db, $this->phpbb_root_path)); - $this->application->add(new recreate($this->user)); - $phpbb_filesystem = new \phpbb\filesystem\filesystem(); + $this->storage = $this->createMock('\phpbb\storage\storage'); + $this->storage->method('write')->willReturnCallback(function ($path, $data) use ($phpbb_root_path) { + file_put_contents($phpbb_root_path . 'files/' . $path, $data); + }); + $this->storage->method('read')->willReturnCallback(function ($path) use ($phpbb_root_path) { + return fopen($phpbb_root_path . 'files/' . $path, 'rb'); + }); + $this->storage->method('delete')->willReturnCallback(function ($path) use ($phpbb_root_path) { + unlink($phpbb_root_path . 'files/' . $path); + }); + + $this->application = new Application(); + $this->application->add(new generate($this->user, $this->db, $this->cache, $this->language, $this->storage, $this->phpbb_root_path, $this->phpEx)); + $this->application->add(new delete($this->user, $this->db, $this->language, $this->storage)); + $this->application->add(new recreate($this->user, $this->language)); + copy(__DIR__ . '/fixtures/png.png', $this->phpbb_root_path . 'files/test_png_1'); copy(__DIR__ . '/fixtures/png.png', $this->phpbb_root_path . 'files/test_png_2'); copy(__DIR__ . '/fixtures/png.png', $this->phpbb_root_path . 'files/thumb_test_png_2'); From be3966c0cbd3558670a75a4062e6c74ac5a1cb1e Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Fri, 29 Nov 2024 21:43:58 +0100 Subject: [PATCH 06/14] [ticket/17361] Remove temp files always PHPBB-17361 --- phpBB/phpbb/console/command/thumbnail/delete.php | 4 +++- phpBB/phpbb/console/command/thumbnail/generate.php | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/console/command/thumbnail/delete.php b/phpBB/phpbb/console/command/thumbnail/delete.php index 8b767cbbd8..114969c0d0 100644 --- a/phpBB/phpbb/console/command/thumbnail/delete.php +++ b/phpBB/phpbb/console/command/thumbnail/delete.php @@ -132,7 +132,9 @@ class delete extends \phpbb\console\command\command } $progress->setMessage($this->language->lang('CLI_THUMBNAIL_DELETED', $row['real_filename'], $row['physical_filename'])); - } catch (storage_exception $e) { + } + catch (storage_exception $e) + { $return = symfony_command::FAILURE; $progress->setMessage('' . $this->language->lang('CLI_THUMBNAIL_SKIPPED', $row['real_filename'], $row['physical_filename']) . ''); diff --git a/phpBB/phpbb/console/command/thumbnail/generate.php b/phpBB/phpbb/console/command/thumbnail/generate.php index 52ae19a9de..c0bda918f4 100644 --- a/phpBB/phpbb/console/command/thumbnail/generate.php +++ b/phpBB/phpbb/console/command/thumbnail/generate.php @@ -156,9 +156,6 @@ class generate extends \phpbb\console\command\command { $this->storage->write('thumb_' . $row['physical_filename'], fopen($destination, 'rb')); - @unlink($source); - @unlink($destination); - $thumbnail_created[] = (int) $row['attach_id']; if (count($thumbnail_created) === 250) @@ -173,6 +170,9 @@ class generate extends \phpbb\console\command\command { $progress->setMessage('' . $this->language->lang('CLI_THUMBNAIL_SKIPPED', $row['real_filename'], $row['physical_filename']) . ''); } + + @unlink($source); + @unlink($destination); } $progress->advance(); From a44295a1ba21fa3141dedf151db405ecd9448232 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 30 Nov 2024 01:25:17 +0100 Subject: [PATCH 07/14] [ticket/17361] Move language keys to storage adapters PHPBB-17361 --- phpBB/adm/style/acp_storage.html | 39 +++++------ .../default/container/services_storage.yml | 1 + phpBB/language/en/acp/storage.php | 1 + phpBB/phpbb/storage/provider/local.php | 30 +++++++- .../storage/provider/provider_interface.php | 69 ++++++++++++++++++- phpBB/phpbb/template/twig/extension/forms.php | 2 +- 6 files changed, 118 insertions(+), 24 deletions(-) diff --git a/phpBB/adm/style/acp_storage.html b/phpBB/adm/style/acp_storage.html index f064438aca..f3b93020f8 100644 --- a/phpBB/adm/style/acp_storage.html +++ b/phpBB/adm/style/acp_storage.html @@ -47,7 +47,7 @@ {% for provider in PROVIDERS %} {% if provider.is_available %} {% endif %} {% endfor %} @@ -59,43 +59,42 @@ {% for provider in PROVIDERS %} {% if provider.is_available %}
- {{ lang('STORAGE_' ~ storage.get_name | upper ~ '_TITLE') }} - {{ lang('STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_NAME') }} + {{ lang('STORAGE_' ~ storage.get_name | upper ~ '_TITLE') }} - {{ provider.get_title }} {% for name, options in provider.get_options %}
- {% set title = 'STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_OPTION_' ~ name | upper %} - {% set description = 'STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_OPTION_' ~ name | upper ~ '_EXPLAIN' %} - - {% if lang_defined(description) %} -
{{ lang(description) }} + + {% if options.description %} +
{{ options.description }} {% endif %}
{% set input_name = storage.get_name ~ '[' ~ name ~ ']' %} {% set input_value = attribute(config, 'storage\\' ~ storage.get_name ~ '\\config\\' ~ name) %} + {% set form_macro = options.form_macro %} - {% if options.tag == 'input' %} - {{ FormsInput(options | merge({"name": input_name, "value": input_value})) }} - {% elseif options.tag == 'textarea' %} - {{ FormsTextarea(options | merge({"name": input_name, "content": input_value})) }} - {% elseif options.tag == 'radio' %} + {% if form_macro.tag == 'input' %} + {{ FormsInput(form_macro | merge({"name": input_name, "value": input_value})) }} + {% elseif form_macro.tag == 'textarea' %} + {{ FormsTextarea(form_macro | merge({"name": input_name, "content": input_value})) }} + {% elseif form_macro.tag == 'radio' %} {% set buttons = [] %} - {% for button in options.buttons %} - {% set new_button = button | merge({"name": input_name, "label": lang(button.label), "checked": button.value == input_value}) %} + {% for button in form_macro.buttons %} + {% set new_button = button | merge({"name": input_name, "checked": button.value == input_value}) %} {% set buttons = buttons | merge([new_button]) %} {% endfor %} - {{ FormsRadioButtons(options | merge({"buttons": buttons})) }} - {% elseif options.tag == 'select' %} + {{ FormsRadioButtons(form_macro | merge({"buttons": buttons})) }} + {% elseif form_macro.tag == 'select' %} {% set select_options = [] %} - {% for option in options.options %} + {% for option in form_macro.options %} {% set new_option = option | merge({"selected": option.value == input_value}) %} {% set select_options = select_options | merge([new_option]) %} {% endfor %} - {{ FormsSelect(options | merge({"name": input_name, "options": select_options})) }} + {{ FormsSelect(form_macro | merge({"name": input_name, "options": select_options})) }} {% endif %}
@@ -109,9 +108,9 @@
- + - +
diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 1109aa626b..8db8ceaede 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -72,6 +72,7 @@ services: storage.provider.local: class: phpbb\storage\provider\local arguments: + - '@language' tags: - { name: storage.provider } diff --git a/phpBB/language/en/acp/storage.php b/phpBB/language/en/acp/storage.php index 9f928ee58e..575805e650 100644 --- a/phpBB/language/en/acp/storage.php +++ b/phpBB/language/en/acp/storage.php @@ -65,6 +65,7 @@ $lang = array_merge($lang, [ // Local adapter 'STORAGE_ADAPTER_LOCAL_NAME' => 'Local', 'STORAGE_ADAPTER_LOCAL_OPTION_PATH' => 'Path', + 'STORAGE_ADAPTER_LOCAL_OPTION_PATH_EXPLAIN' => 'Storage path for files.
For example: files, images/avatars/upload or store.', // Form validation 'STORAGE_UPDATE_SUCCESSFUL' => 'All storage types were successfully updated.', diff --git a/phpBB/phpbb/storage/provider/local.php b/phpBB/phpbb/storage/provider/local.php index 67a1789434..11bca055ce 100644 --- a/phpBB/phpbb/storage/provider/local.php +++ b/phpBB/phpbb/storage/provider/local.php @@ -13,8 +13,25 @@ namespace phpbb\storage\provider; +use phpbb\language\language; + class local implements provider_interface { + /** + * @var language + */ + protected $language; + + /** + * Constructor + * + * @param language $language + */ + public function __construct(language $language) + { + $this->language = $language; + } + /** * {@inheritdoc} */ @@ -23,6 +40,11 @@ class local implements provider_interface return 'local'; } + public function get_title(): string + { + return $this->language->lang('STORAGE_ADAPTER_LOCAL_NAME'); + } + /** * {@inheritdoc} */ @@ -38,8 +60,12 @@ class local implements provider_interface { return [ 'path' => [ - 'tag' => 'input', - 'type' => 'text', + 'title' => $this->language->lang('STORAGE_ADAPTER_LOCAL_OPTION_PATH'), + 'description' => $this->language->lang('STORAGE_ADAPTER_LOCAL_OPTION_PATH_EXPLAIN'), + 'form_macro' => [ + 'tag' => 'input', + 'type' => 'text', + ], ], ]; } diff --git a/phpBB/phpbb/storage/provider/provider_interface.php b/phpBB/phpbb/storage/provider/provider_interface.php index aa01a51e37..7baac79f72 100644 --- a/phpBB/phpbb/storage/provider/provider_interface.php +++ b/phpBB/phpbb/storage/provider/provider_interface.php @@ -22,6 +22,13 @@ interface provider_interface */ public function get_name(): string; + /** + * Gets adapter title for acp + * + * @return string + */ + public function get_title(): string; + /** * Gets adapter class * @@ -32,7 +39,67 @@ interface provider_interface /** * Gets adapter options * - * @return array Configuration keys + * Example: + * public function get_options() + * { + * return [ + * 'text-test' => [ + * 'title' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_TEXT_TEST'), + * 'description' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_TEXT_TEST_EXPLAIN'), + * 'form_macro' => [ + * 'tag' => 'input', + * 'type' => 'text', + * ], + * ], + * 'password-test' => [ + * 'title' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_PASSWORD_TEST'), + * 'description' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_PASSWORD_TEST_EXPLAIN'), + * 'form_macro' => [ + * 'tag' => 'input', + * 'type' => 'password', + * ], + * ], + * 'radio-test' => [ + * 'title' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_RADIO_TEST'), + * 'description' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_RADIO_TEST_EXPLAIN'), + * 'form_macro' => [ + * 'tag' => 'radio', + * 'buttons' => [ + * [ + * 'type' => 'radio', + * 'value' => '1', + * 'label' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_RADIO_TEST_LABEL_ONE'), + * ], + * [ + * 'type' => 'radio', + * 'value' => '2', + * 'label' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_RADIO_TEST_LABEL_TWO'), + * ], + * ], + * ], + * ], + * 'select-test' => [ + * 'title' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_SELECT_TEST'), + * 'description' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_SELECT_TEST_EXPLAIN'), + * 'form_macro' => [ + * 'tag' => 'select', + * 'options' => [ + * ['value' => 'one', 'label' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_SELECT_TEST_LABEL_ONE')], + * ['value' => 'two', 'label' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_SELECT_TEST_LABEL_TWO')], + * ], + * ], + * ], + * 'textarea-test' => [ + * 'title' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_TEXTAREA_TEST'), + * 'description' => $this->language->lang('STORAGE_ADAPTER_DEMO_OPTION_TEXTAREA_TEST_EXPLAIN'), + * 'form_macro' => [ + * 'tag' => 'textarea', + * ] + * ], + * ]; + * } + * + * @return array Configuration keys */ public function get_options(): array; diff --git a/phpBB/phpbb/template/twig/extension/forms.php b/phpBB/phpbb/template/twig/extension/forms.php index b146aacfb9..bf1bed006e 100644 --- a/phpBB/phpbb/template/twig/extension/forms.php +++ b/phpBB/phpbb/template/twig/extension/forms.php @@ -206,7 +206,7 @@ class forms extends AbstractExtension { return $environment->render('macros/forms/textarea.twig', [ 'CLASS' => (string) ($form_data['class'] ?? ''), - 'ID' => (string) $form_data['id'], + 'ID' => (string) ($form_data['id'] ?? ''), 'DATA' => $form_data['data'] ?? [], 'NAME' => (string) $form_data['name'], 'ROWS' => (int) ($form_data['rows'] ?? ''), From 1f7ae9e2b0d9d487e1be6c9136d301c07b519b48 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 30 Nov 2024 03:22:46 +0100 Subject: [PATCH 08/14] [ticket/17361] Fix migrations PHPBB-17361 --- phpBB/includes/acp/acp_database.php | 7 +- .../data/v400/storage_attachment.php | 9 +++ .../db/migration/data/v400/storage_avatar.php | 9 +++ .../db/migration/data/v400/storage_backup.php | 17 +++++ .../data/v400/storage_backup_data.php | 65 +++++++++++++++++++ .../db/migration/data/v400/storage_track.php | 1 + phpBB/phpbb/storage/adapter_factory.php | 2 +- 7 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 phpBB/phpbb/db/migration/data/v400/storage_backup_data.php diff --git a/phpBB/includes/acp/acp_database.php b/phpBB/includes/acp/acp_database.php index 2b95ff3ad5..7743ce5dbc 100644 --- a/phpBB/includes/acp/acp_database.php +++ b/phpBB/includes/acp/acp_database.php @@ -162,12 +162,7 @@ class acp_database throw new \phpbb\exception\runtime_exception('CANNOT_OPEN_FILE'); } - $storage->write_stream($file, $fp); - - if (is_resource($fp)) - { - fclose($fp); - } + $storage->write($file, $fp); // Remove file from tmp @unlink($temp_dir . '/' . $file); diff --git a/phpBB/phpbb/db/migration/data/v400/storage_attachment.php b/phpBB/phpbb/db/migration/data/v400/storage_attachment.php index d9f60ccdac..9e1919f3f3 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_attachment.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_attachment.php @@ -38,4 +38,13 @@ class storage_attachment extends migration ['config.remove', ['upload_path']], ]; } + + public function revert_data() + { + return [ + ['config.remove', ['storage\\attachment\\provider']], + ['config.remove', ['storage\\attachment\\config\\path']], + ['config.add', ['upload_path', 'files']], + ]; + } } diff --git a/phpBB/phpbb/db/migration/data/v400/storage_avatar.php b/phpBB/phpbb/db/migration/data/v400/storage_avatar.php index 5afed251d7..f9de7cd8a5 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_avatar.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_avatar.php @@ -38,4 +38,13 @@ class storage_avatar extends migration ['config.remove', ['avatar_path']], ]; } + + public function revert_data() + { + return [ + ['config.remove', ['storage\\avatar\\provider']], + ['config.remove', ['storage\\avatar\\config\\path']], + ['config.add', ['avatar_path', 'images/avatars/upload']], + ]; + } } diff --git a/phpBB/phpbb/db/migration/data/v400/storage_backup.php b/phpBB/phpbb/db/migration/data/v400/storage_backup.php index f764f5636e..d9f19c42f0 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_backup.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_backup.php @@ -45,6 +45,15 @@ class storage_backup extends migration ]; } + public function revert_schema() + { + return [ + 'drop_tables' => [ + $this->table_prefix . 'backups', + ], + ]; + } + public function update_data() { return [ @@ -52,4 +61,12 @@ class storage_backup extends migration ['config.add', ['storage\\backup\\config\\path', 'store']], ]; } + + public function revert_data() + { + return [ + ['config.remove', ['storage\\backup\\provider']], + ['config.remove', ['storage\\backup\\config\\path']], + ]; + } } diff --git a/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php b/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php new file mode 100644 index 0000000000..507d4a09d9 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php @@ -0,0 +1,65 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace phpbb\db\migration\data\v400; + +use phpbb\db\migration\migration; + +class storage_backup_data extends migration +{ + public static function depends_on() + { + return [ + '\phpbb\db\migration\data\v400\storage_backup', + ]; + } + + public function update_data() + { + return [ + ['custom', [[$this, 'update_backup_data']]], + ]; + } + + public function update_backup_data() + { + $methods = ['sql', 'sql.gz', 'sql.bz2']; + + $dir = $this->phpbb_root_path . 'store/'; + $dh = @opendir($dir); + + if ($dh) + { + while (($file = readdir($dh)) !== false) + { + echo "FILE $file\n"; + if (preg_match('#^backup_(\d{10,})_(?:[a-z\d]{16}|[a-z\d]{32})\.(sql(?:\.(?:gz|bz2))?)$#i', $file, $matches)) + { + if (in_array($matches[2], $methods)) + { + $insert_ary = [ + 'filename' => $file, + ]; + + $sql = 'INSERT INTO ' . $this->table_prefix . 'backups ' . $this->db->sql_build_array('INSERT', $insert_ary); + + $this->db->sql_query($sql); + } + } + + } + + closedir($dh); + } + } +} diff --git a/phpBB/phpbb/db/migration/data/v400/storage_track.php b/phpBB/phpbb/db/migration/data/v400/storage_track.php index d9890fd80f..ebda4c8678 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_track.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_track.php @@ -30,6 +30,7 @@ class storage_track extends container_aware_migration '\phpbb\db\migration\data\v400\storage_attachment', '\phpbb\db\migration\data\v400\storage_avatar', '\phpbb\db\migration\data\v400\storage_backup', + '\phpbb\db\migration\data\v400\storage_backup_data', ]; } diff --git a/phpBB/phpbb/storage/adapter_factory.php b/phpBB/phpbb/storage/adapter_factory.php index 3d55fc7a41..1cf4ae12cd 100644 --- a/phpBB/phpbb/storage/adapter_factory.php +++ b/phpBB/phpbb/storage/adapter_factory.php @@ -89,7 +89,7 @@ class adapter_factory } $adapter = $this->adapters->get_by_class($provider->get_adapter_class()); - $options['storage'] = $storage_name; + $options['storage'] = $storage_name; // Inject storage name into options so it can be used by extensiosn $adapter->configure($options); return $adapter; From d6953a0422dd0ff6e967caa45db2df9b3b1a4421 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 30 Nov 2024 14:45:19 +0100 Subject: [PATCH 09/14] [ticket/17361] Use new adapter options format in acp_storage PHPBB-17361 --- phpBB/includes/acp/acp_storage.php | 22 +++++++++---------- .../data/v400/storage_backup_data.php | 1 - phpBB/phpbb/language/language.php | 4 ++-- phpBB/phpbb/storage/adapter_factory.php | 2 +- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/phpBB/includes/acp/acp_storage.php b/phpBB/includes/acp/acp_storage.php index 35c07074b6..a35f6d4f0c 100644 --- a/phpBB/includes/acp/acp_storage.php +++ b/phpBB/includes/acp/acp_storage.php @@ -378,7 +378,6 @@ class acp_storage foreach ($this->storage_collection as $storage) { $storage_name = $storage->get_name(); - $options = $this->storage_helper->get_provider_options($this->storage_helper->get_current_provider($storage_name)); $modified = false; @@ -389,6 +388,8 @@ class acp_storage } else { + $options = $this->storage_helper->get_provider_options($this->storage_helper->get_current_provider($storage_name)); + // Check if options have been modified foreach (array_keys($options) as $definition) { @@ -535,30 +536,29 @@ class acp_storage $this->validate_path($storage_name, $messages); // Check options - $new_options = $this->storage_helper->get_provider_options($this->request->variable([$storage_name, 'provider'], '')); + $new_provider = $this->provider_collection->get_by_class($this->request->variable([$storage_name, 'provider'], '')); - foreach ($new_options as $definition_key => $definition_value) + foreach ($new_provider->get_options() as $definition_key => $definition_value) { - $provider = $this->provider_collection->get_by_class($this->request->variable([$storage_name, 'provider'], '')); - $definition_title = $this->lang->lang('STORAGE_ADAPTER_' . strtoupper($provider->get_name()) . '_OPTION_' . strtoupper($definition_key)); + $definition_title = $definition_value['title']; $value = $this->request->variable([$storage_name, $definition_key], ''); - switch ($definition_value['tag']) + switch ($definition_value['form_macro']['tag']) { case 'text': - if ($definition_value['type'] == 'email' && filter_var($value, FILTER_VALIDATE_EMAIL)) + if ($definition_value['form_macro']['type'] === 'email' && filter_var($value, FILTER_VALIDATE_EMAIL)) { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT', $definition_title, $storage_title); } - $maxlength = $definition_value['max'] ?? 255; + $maxlength = $definition_value['form_macro']['max'] ?? 255; if (strlen($value) > $maxlength) { $messages[] = $this->lang->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG', $definition_title, $storage_title); } - if ($provider->get_name() == 'local' && $definition_key == 'path') + if ($new_provider->get_name() === 'local' && $definition_key === 'path') { $path = $value; @@ -575,7 +575,7 @@ class acp_storage case 'radio': $found = false; - foreach ($definition_value['buttons'] as $button) + foreach ($definition_value['form_macro']['buttons'] as $button) { if ($button['value'] == $value) { @@ -592,7 +592,7 @@ class acp_storage case 'select': $found = false; - foreach ($definition_value['options'] as $option) + foreach ($definition_value['form_macro']['options'] as $option) { if ($option['value'] == $value) { diff --git a/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php b/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php index 507d4a09d9..772a03b945 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php @@ -42,7 +42,6 @@ class storage_backup_data extends migration { while (($file = readdir($dh)) !== false) { - echo "FILE $file\n"; if (preg_match('#^backup_(\d{10,})_(?:[a-z\d]{16}|[a-z\d]{32})\.(sql(?:\.(?:gz|bz2))?)$#i', $file, $matches)) { if (in_array($matches[2], $methods)) diff --git a/phpBB/phpbb/language/language.php b/phpBB/phpbb/language/language.php index 1c51049de5..4336c8f9f2 100644 --- a/phpBB/phpbb/language/language.php +++ b/phpBB/phpbb/language/language.php @@ -230,10 +230,10 @@ class language * Params are the language key and the parameters to be substituted. * This function/functionality is inspired by SHS` and Ashe. * - * Example call: $user->lang('NUM_POSTS_IN_QUEUE', 1); + * Example call: $language->lang('NUM_POSTS_IN_QUEUE', 1); * * If the first parameter is an array, the elements are used as keys and subkeys to get the language entry: - * Example: $user->lang(array('datetime', 'AGO'), 1) uses $user->lang['datetime']['AGO'] as language entry. + * Example: $language->lang(array('datetime', 'AGO'), 1) uses $language->lang['datetime']['AGO'] as language entry. * * @return string Return localized string or the language key if the translation is not available */ diff --git a/phpBB/phpbb/storage/adapter_factory.php b/phpBB/phpbb/storage/adapter_factory.php index 1cf4ae12cd..e80d25cfbc 100644 --- a/phpBB/phpbb/storage/adapter_factory.php +++ b/phpBB/phpbb/storage/adapter_factory.php @@ -89,7 +89,7 @@ class adapter_factory } $adapter = $this->adapters->get_by_class($provider->get_adapter_class()); - $options['storage'] = $storage_name; // Inject storage name into options so it can be used by extensiosn + $options['storage'] = $storage_name; // Inject storage name into options so it can be used by extensions $adapter->configure($options); return $adapter; From b33f17e7ce630f5860f26f4f7c1c5824dd9472ec Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Tue, 24 Dec 2024 19:30:05 +0100 Subject: [PATCH 10/14] [ticket/17361] Use filesystem classes for tempnam and temp dir. Code reviews. PHPBB-17361 --- .../default/container/services_console.yml | 1 + .../console/command/thumbnail/generate.php | 24 ++++++-- .../console/command/thumbnail/recreate.php | 1 + .../data/v400/storage_backup_data.php | 8 +-- phpBB/phpbb/filesystem/temp.php | 55 +++++++++++++------ phpBB/phpbb/storage/file_tracker.php | 27 ++++----- tests/console/thumbnail_test.php | 8 ++- 7 files changed, 86 insertions(+), 38 deletions(-) diff --git a/phpBB/config/default/container/services_console.yml b/phpBB/config/default/container/services_console.yml index e1c965ce17..3cfacb080b 100644 --- a/phpBB/config/default/container/services_console.yml +++ b/phpBB/config/default/container/services_console.yml @@ -299,6 +299,7 @@ services: - '@cache' - '@language' - '@storage.attachment' + - '@filesystem.temp' - '%core.root_path%' - '%core.php_ext%' tags: diff --git a/phpBB/phpbb/console/command/thumbnail/generate.php b/phpBB/phpbb/console/command/thumbnail/generate.php index c0bda918f4..38e3646648 100644 --- a/phpBB/phpbb/console/command/thumbnail/generate.php +++ b/phpBB/phpbb/console/command/thumbnail/generate.php @@ -13,8 +13,10 @@ namespace phpbb\console\command\thumbnail; +use phpbb\attachment\attachment_category; use phpbb\cache\service; use phpbb\db\driver\driver_interface; +use phpbb\filesystem\temp; use phpbb\language\language; use phpbb\storage\storage; use phpbb\user; @@ -22,6 +24,7 @@ use Symfony\Component\Console\Command\Command as symfony_command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; +use Symfony\Component\Filesystem\Filesystem as symfony_filesystem; class generate extends \phpbb\console\command\command { @@ -41,11 +44,21 @@ class generate extends \phpbb\console\command\command */ protected $language; + /** + * @var symfony_filesystem + */ + protected $symfony_filesystem; + /** * @var storage */ protected $storage; + /** + * @var temp + */ + protected $temp; + /** * phpBB root path * @var string @@ -67,15 +80,18 @@ class generate extends \phpbb\console\command\command * @param service $cache The cache service * @param language $language Language * @param storage $storage Storage + * @param temp $temp Temp * @param string $phpbb_root_path Root path * @param string $php_ext PHP extension */ - public function __construct(user $user, driver_interface $db, service $cache, language $language, storage $storage, string $phpbb_root_path, string $php_ext) + public function __construct(user $user, driver_interface $db, service $cache, language $language, storage $storage, temp $temp, string $phpbb_root_path, string $php_ext) { $this->db = $db; $this->cache = $cache; + $this->symfony_filesystem = new symfony_filesystem(); $this->language = $language; $this->storage = $storage; + $this->temp = $temp; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; @@ -145,10 +161,10 @@ class generate extends \phpbb\console\command\command $thumbnail_created = array(); while ($row = $this->db->sql_fetchrow($result)) { - if (isset($extensions[$row['extension']]['display_cat']) && $extensions[$row['extension']]['display_cat'] == \phpbb\attachment\attachment_category::IMAGE) + if (isset($extensions[$row['extension']]['display_cat']) && $extensions[$row['extension']]['display_cat'] == attachment_category::IMAGE) { - $source = tempnam(sys_get_temp_dir(), 'thumbnail_source'); - $destination = tempnam(sys_get_temp_dir(), 'thumbnail_destination'); + $source = $this->symfony_filesystem->tempnam($this->temp->get_dir(), 'thumbnail_source'); + $destination = $this->symfony_filesystem->tempnam($this->temp->get_dir(), 'thumbnail_destination'); file_put_contents($source, $this->storage->read($row['physical_filename'])); diff --git a/phpBB/phpbb/console/command/thumbnail/recreate.php b/phpBB/phpbb/console/command/thumbnail/recreate.php index a0a0524b89..4b774c251d 100644 --- a/phpBB/phpbb/console/command/thumbnail/recreate.php +++ b/phpBB/phpbb/console/command/thumbnail/recreate.php @@ -29,6 +29,7 @@ class recreate extends \phpbb\console\command\command /** * Constructor * + * @param user $user User * @param language $language Language */ public function __construct(user $user, language $language) diff --git a/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php b/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php index 772a03b945..190445a04f 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_backup_data.php @@ -36,11 +36,11 @@ class storage_backup_data extends migration $methods = ['sql', 'sql.gz', 'sql.bz2']; $dir = $this->phpbb_root_path . 'store/'; - $dh = @opendir($dir); + $handle = @opendir($dir); - if ($dh) + if ($handle) { - while (($file = readdir($dh)) !== false) + while (($file = readdir($handle)) !== false) { if (preg_match('#^backup_(\d{10,})_(?:[a-z\d]{16}|[a-z\d]{32})\.(sql(?:\.(?:gz|bz2))?)$#i', $file, $matches)) { @@ -58,7 +58,7 @@ class storage_backup_data extends migration } - closedir($dh); + closedir($handle); } } } diff --git a/phpBB/phpbb/filesystem/temp.php b/phpBB/phpbb/filesystem/temp.php index 649221d802..4c8a71d600 100644 --- a/phpBB/phpbb/filesystem/temp.php +++ b/phpBB/phpbb/filesystem/temp.php @@ -13,8 +13,20 @@ namespace phpbb\filesystem; +use phpbb\filesystem\exception\filesystem_exception; + class temp { + /** + * @var filesystem + */ + protected $filesystem; + + /** + * @var string + */ + protected $cache_temp_dir; + /** * @var string Temporary directory path */ @@ -23,23 +35,10 @@ class temp /** * Constructor */ - public function __construct($filesystem, $cache_temp_dir) + public function __construct(filesystem $filesystem, string $cache_temp_dir) { - $tmp_dir = (function_exists('sys_get_temp_dir')) ? sys_get_temp_dir() : ''; - - // Prevent trying to write to system temp dir in case of open_basedir - // restrictions being in effect - if (empty($tmp_dir) || !@file_exists($tmp_dir) || !@is_writable($tmp_dir)) - { - $tmp_dir = $cache_temp_dir; - - if (!is_dir($tmp_dir)) - { - $filesystem->mkdir($tmp_dir, 0777); - } - } - - $this->temp_dir = helper::realpath($tmp_dir); + $this->filesystem = $filesystem; + $this->cache_temp_dir = $cache_temp_dir; } /** @@ -49,6 +48,30 @@ class temp */ public function get_dir() { + if ($this->temp_dir === null) + { + $tmp_dir = (function_exists('sys_get_temp_dir')) ? sys_get_temp_dir() : ''; + + // Prevent trying to write to system temp dir in case of open_basedir + // restrictions being in effect + if (empty($tmp_dir) || !@file_exists($tmp_dir) || !@is_writable($tmp_dir)) + { + $tmp_dir = $this->cache_temp_dir; + + if (!is_dir($tmp_dir)) + { + $this->filesystem->mkdir($tmp_dir, 0777); + } + } + + $this->temp_dir = helper::realpath($tmp_dir); + + if ($this->temp_dir === false) + { + throw new filesystem_exception('FILESYSTEM_CANNOT_CREATE_DIRECTORY', $tmp_dir); + } + } + return $this->temp_dir; } } diff --git a/phpBB/phpbb/storage/file_tracker.php b/phpBB/phpbb/storage/file_tracker.php index c858b733ac..ab6f1c8a11 100644 --- a/phpBB/phpbb/storage/file_tracker.php +++ b/phpBB/phpbb/storage/file_tracker.php @@ -19,7 +19,6 @@ use phpbb\storage\exception\storage_exception; class file_tracker { - /** * @var db */ @@ -59,11 +58,11 @@ class file_tracker */ public function track_file(string $storage, string $path, int $size): void { - $sql_ary = array( + $sql_ary = [ 'file_path' => $path, 'storage' => $storage, 'filesize' => $size, - ); + ]; $sql = 'INSERT INTO ' . $this->storage_table . $this->db->sql_build_array('INSERT', $sql_ary); $this->db->sql_query($sql); @@ -80,10 +79,10 @@ class file_tracker */ public function untrack_file(string $storage, $path): void { - $sql_ary = array( + $sql_ary = [ 'file_path' => $path, 'storage' => $storage, - ); + ]; $sql = 'DELETE FROM ' . $this->storage_table . ' WHERE ' . $this->db->sql_build_array('DELETE', $sql_ary); @@ -103,13 +102,14 @@ class file_tracker */ public function is_tracked(string $storage, string $path): bool { - $sql_ary = array( + $sql_ary = [ 'file_path' => $path, 'storage' => $storage, - ); + ]; - $sql = 'SELECT file_id FROM ' . $this->storage_table . ' - WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); + $sql = 'SELECT file_id + FROM ' . $this->storage_table . ' + WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); $result = $this->db->sql_query($sql); $row = $this->db->sql_fetchrow($result); $this->db->sql_freeresult($result); @@ -128,13 +128,14 @@ class file_tracker */ public function file_size(string $storage, string $path): int { - $sql_ary = array( + $sql_ary = [ 'file_path' => $path, 'storage' => $storage, - ); + ]; - $sql = 'SELECT filesize FROM ' . $this->storage_table . ' - WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); + $sql = 'SELECT filesize + FROM ' . $this->storage_table . ' + WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); $result = $this->db->sql_query($sql); $row = $this->db->sql_fetchrow($result); $this->db->sql_freeresult($result); diff --git a/tests/console/thumbnail_test.php b/tests/console/thumbnail_test.php index ebe0dff62e..3a006d27d7 100644 --- a/tests/console/thumbnail_test.php +++ b/tests/console/thumbnail_test.php @@ -25,6 +25,7 @@ class phpbb_console_command_thumbnail_test extends phpbb_database_test_case protected $cache; protected $user; protected $storage; + protected $temp; protected $phpEx; protected $phpbb_root_path; protected $application; @@ -75,8 +76,13 @@ class phpbb_console_command_thumbnail_test extends phpbb_database_test_case unlink($phpbb_root_path . 'files/' . $path); }); + $this->temp = $this->createMock('\phpbb\filesystem\temp'); + $this->temp->method('get_dir')->willReturnCallback(function () { + return sys_get_temp_dir(); + }); + $this->application = new Application(); - $this->application->add(new generate($this->user, $this->db, $this->cache, $this->language, $this->storage, $this->phpbb_root_path, $this->phpEx)); + $this->application->add(new generate($this->user, $this->db, $this->cache, $this->language, $this->storage, $this->temp, $this->phpbb_root_path, $this->phpEx)); $this->application->add(new delete($this->user, $this->db, $this->language, $this->storage)); $this->application->add(new recreate($this->user, $this->language)); From 90bd9d006e768fb96e3f093481c1af9b1e3aa0e6 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 18 Jan 2025 23:08:52 +0100 Subject: [PATCH 11/14] [ticket/17361] Add method to track multiple files in a single query PHPBB-17361 --- .../db/migration/data/v400/storage_track.php | 85 ++++++++++++------- phpBB/phpbb/storage/file_tracker.php | 27 +++++- 2 files changed, 76 insertions(+), 36 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v400/storage_track.php b/phpBB/phpbb/db/migration/data/v400/storage_track.php index ebda4c8678..4d50ac0393 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_track.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_track.php @@ -19,6 +19,8 @@ use phpbb\storage\file_tracker; class storage_track extends container_aware_migration { + private const BATCH_SIZE = 100; + public function effectively_installed() { return $this->db_tools->sql_table_exists($this->tables['storage']); @@ -77,9 +79,9 @@ class storage_track extends container_aware_migration $sql = 'SELECT user_avatar FROM ' . USERS_TABLE . " WHERE user_avatar_type = 'avatar.driver.upload'"; - $result = $this->db->sql_query($sql); + $files = []; while ($row = $this->db->sql_fetchrow($result)) { $avatar_group = false; @@ -94,16 +96,24 @@ class storage_track extends container_aware_migration $ext = substr(strrchr($filename, '.'), 1); $filename = (int) $filename; - try + $filename = $this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $filename . '.' . $ext; + $files[] = [ + 'file_path' => $filename, + 'filesize' => filesize($this->phpbb_root_path . $this->config['storage\\avatar\\config\\path'] . '/' . $filename), + ]; + + if (count($files) >= self::BATCH_SIZE) { - $filename = $this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $filename . '.' . $ext; - $file_tracker->track_file('avatar', $filename, filesize($this->phpbb_root_path . $this->config['storage\\avatar\\config\\path'] . '/' . $filename)); - } - catch (storage_exception $e) - { - // If file doesn't exist, don't track it + $file_tracker->track_files('avatar', $files); + $files = []; } } + + if (!empty($files)) + { + $file_tracker->track_files('avatar', $files); + } + $this->db->sql_freeresult($result); } @@ -114,32 +124,36 @@ class storage_track extends container_aware_migration $sql = 'SELECT physical_filename, thumbnail FROM ' . ATTACHMENTS_TABLE; - $result = $this->db->sql_query($sql); + $files = []; while ($row = $this->db->sql_fetchrow($result)) { - try - { - $file_tracker->track_file('attachment', $row['physical_filename'], filesize($this->phpbb_root_path . $this->config['storage\\attachment\\config\\path'] . '/' . $row['physical_filename'])); - } - catch (storage_exception $e) - { - // If file doesn't exist, don't track it - } + $files[] = [ + 'file_path' => $row['physical_filename'], + 'filesize' => filesize($this->phpbb_root_path . $this->config['storage\\attachment\\config\\path'] . '/' . $row['physical_filename']), + ]; if ($row['thumbnail'] == 1) { - try - { - $file_tracker->track_file('attachment', 'thumb_' . $row['physical_filename'], filesize($this->phpbb_root_path . $this->config['storage\\attachment\\config\\path'] . '/thumb_' . $row['physical_filename'])); - } - catch (storage_exception $e) - { - // If file doesn't exist, don't track it - } + $files[] = [ + 'file_path' => 'thumb_' . $row['physical_filename'], + 'filesize' => filesize($this->phpbb_root_path . $this->config['storage\\attachment\\config\\path'] . '/thumb_' . $row['physical_filename']), + ]; + } + + if (count($files) >= self::BATCH_SIZE) + { + $file_tracker->track_files('attachment', $files); + $files = []; } } + + if (!empty($files)) + { + $file_tracker->track_files('attachment', $files); + } + $this->db->sql_freeresult($result); } @@ -150,21 +164,28 @@ class storage_track extends container_aware_migration $sql = 'SELECT filename FROM ' . BACKUPS_TABLE; - $result = $this->db->sql_query($sql); + $files = []; while ($row = $this->db->sql_fetchrow($result)) { - try + $files[] = [ + 'file_path' => $row['filename'], + 'filesize' => filesize($this->phpbb_root_path . $this->config['storage\\backup\\config\\path'] . '/' . $row['filename']), + ]; + + if (count($files) >= self::BATCH_SIZE) { - $file_tracker->track_file('backup', $row['filename'], filesize($this->phpbb_root_path . $this->config['storage\\backup\\config\\path'] . '/' . $row['filename'])); - } - catch (storage_exception $e) - { - // If file doesn't exist, don't track it + $file_tracker->track_files('backup', $files); + $files = []; } } + if (!empty($files)) + { + $file_tracker->track_files('backup', $files); + } + $this->db->sql_freeresult($result); } } diff --git a/phpBB/phpbb/storage/file_tracker.php b/phpBB/phpbb/storage/file_tracker.php index ab6f1c8a11..aa05756996 100644 --- a/phpBB/phpbb/storage/file_tracker.php +++ b/phpBB/phpbb/storage/file_tracker.php @@ -58,14 +58,33 @@ class file_tracker */ public function track_file(string $storage, string $path, int $size): void { - $sql_ary = [ + $file = [ 'file_path' => $path, - 'storage' => $storage, 'filesize' => $size, ]; - $sql = 'INSERT INTO ' . $this->storage_table . $this->db->sql_build_array('INSERT', $sql_ary); - $this->db->sql_query($sql); + $this->track_files($storage, [$file]); + } + + /** + * Track files in database + * + * @param string $storage Storage name + * @param array $files Array of files to track + */ + public function track_files(string $storage, array $files): void + { + $sql_ary = []; + foreach ($files as $file) + { + $sql_ary[] = [ + 'storage' => $storage, + 'file_path' => $file['file_path'], + 'filesize' => $file['filesize'], + ]; + } + + $this->db->sql_multi_insert($this->storage_table, $sql_ary); $this->cache->destroy('_storage_' . $storage . '_totalsize'); $this->cache->destroy('_storage_' . $storage . '_numfiles'); From 598d1c7f49586110ad2b70c61b178c41f078b094 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 18 Jan 2025 23:15:18 +0100 Subject: [PATCH 12/14] [ticket/17361] Change order of arguments PHPBB-17361 --- .../default/container/services_storage.yml | 2 +- phpBB/phpbb/storage/file_tracker.php | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 8db8ceaede..98a44fae1f 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -123,7 +123,7 @@ services: storage.file_tracker: class: phpbb\storage\file_tracker arguments: - - '@dbal.conn' - '@cache.driver' + - '@dbal.conn' - '%tables.storage%' diff --git a/phpBB/phpbb/storage/file_tracker.php b/phpBB/phpbb/storage/file_tracker.php index aa05756996..6a34fde239 100644 --- a/phpBB/phpbb/storage/file_tracker.php +++ b/phpBB/phpbb/storage/file_tracker.php @@ -19,17 +19,17 @@ use phpbb\storage\exception\storage_exception; class file_tracker { - /** - * @var db - */ - protected $db; - /** * Cache driver * @var cache */ protected $cache; + /** + * @var db + */ + protected $db; + /** * @var string */ @@ -38,14 +38,14 @@ class file_tracker /** * Constructor * - * @param db $db - * @param cache $cache - * @param string $storage_table + * @param cache $cache + * @param db $db + * @param string $storage_table */ - public function __construct(db $db, cache $cache, string $storage_table) + public function __construct(cache $cache,db $db, string $storage_table) { - $this->db = $db; $this->cache = $cache; + $this->db = $db; $this->storage_table = $storage_table; } From 921505ca3bf190d18d5dd412a2c7908548ec7b65 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sat, 18 Jan 2025 23:24:34 +0100 Subject: [PATCH 13/14] [ticket/17361] Fix code style PHPBB-17361 --- phpBB/phpbb/db/migration/data/v400/storage_track.php | 1 - phpBB/phpbb/storage/file_tracker.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v400/storage_track.php b/phpBB/phpbb/db/migration/data/v400/storage_track.php index 4d50ac0393..54827d7d29 100644 --- a/phpBB/phpbb/db/migration/data/v400/storage_track.php +++ b/phpBB/phpbb/db/migration/data/v400/storage_track.php @@ -14,7 +14,6 @@ namespace phpbb\db\migration\data\v400; use phpbb\db\migration\container_aware_migration; -use phpbb\storage\exception\storage_exception; use phpbb\storage\file_tracker; class storage_track extends container_aware_migration diff --git a/phpBB/phpbb/storage/file_tracker.php b/phpBB/phpbb/storage/file_tracker.php index 6a34fde239..991eef2d7e 100644 --- a/phpBB/phpbb/storage/file_tracker.php +++ b/phpBB/phpbb/storage/file_tracker.php @@ -42,7 +42,7 @@ class file_tracker * @param db $db * @param string $storage_table */ - public function __construct(cache $cache,db $db, string $storage_table) + public function __construct(cache $cache, db $db, string $storage_table) { $this->cache = $cache; $this->db = $db; From c9ff5a09f61ebf1bc3cd56c094d7a08f0b117566 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sun, 19 Jan 2025 21:09:36 +0100 Subject: [PATCH 14/14] [ticket/17361] Fix file not deleted when there is an error PHPBB-17361 --- phpBB/phpbb/files/filespec_storage.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpBB/phpbb/files/filespec_storage.php b/phpBB/phpbb/files/filespec_storage.php index f223976b9e..fcda005d7b 100644 --- a/phpBB/phpbb/files/filespec_storage.php +++ b/phpBB/phpbb/files/filespec_storage.php @@ -447,6 +447,7 @@ class filespec_storage $fp = fopen($this->filename, 'rb'); $storage->write($this->destination_file, $fp); + $this->file_moved = true; } catch (\phpbb\storage\exception\storage_exception $e) {