diff --git a/phpBB/config/default/container/services_avatar.yml b/phpBB/config/default/container/services_avatar.yml index 6cc38516ae..93a8cd0b62 100644 --- a/phpBB/config/default/container/services_avatar.yml +++ b/phpBB/config/default/container/services_avatar.yml @@ -61,11 +61,11 @@ services: - '@config' - '%core.root_path%' - '%core.php_ext%' - - '@filesystem' + - '@storage.avatar' - '@path_helper' - '@dispatcher' - '@files.factory' - - '@cache.driver' + - '@php_ini' calls: - [set_name, [avatar.driver.upload]] tags: diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index abf51d5f97..32b74687c9 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -1,4 +1,14 @@ services: + +# Storages + storage.avatar: + class: phpbb\storage\storage + arguments: + - '@storage.adapter.factory' + - 'avatar' + tags: + - { name: storage } + # Factory storage.adapter.factory: class: phpbb\storage\adapter_factory @@ -28,6 +38,8 @@ services: shared: false arguments: - '@filesystem' + - '@upload_imagesize' + - '@mimetype.guesser' - '%core.root_path%' tags: - { name: storage.adapter } diff --git a/phpBB/develop/adjust_avatars.php b/phpBB/develop/adjust_avatars.php index dc4ae88f37..a6b326109d 100644 --- a/phpBB/develop/adjust_avatars.php +++ b/phpBB/develop/adjust_avatars.php @@ -19,7 +19,7 @@ $auth->acl($user->data); $user->setup(); $echos = 0; - + if (!isset($config['avatar_salt'])) { $cache->purge(); @@ -30,6 +30,11 @@ if (!isset($config['avatar_salt'])) die('database not up to date'); } +if (!isset($config['storage\\avatar\\config\\path']) || $config['storage\\avatar\\config\\path'] !== 'phpbb\\storage\\provider\\local') +{ + die('use local provider'); +} + // let's start with the users using a group_avatar. $sql = 'SELECT group_id, group_avatar FROM ' . GROUPS_TABLE . ' @@ -46,16 +51,16 @@ while ($row = $db->sql_fetchrow($result)) { $new_avatar_name = adjust_avatar($row['group_avatar'], 'g' . $row['group_id']); $group_avatars[] = $new_avatar_name; - + // failure is probably due to the avatar name already being adjusted if ($new_avatar_name !== false) { $sql = 'UPDATE ' . USERS_TABLE . " SET user_avatar = '" . $db->sql_escape($new_avatar_name) . "' - WHERE user_avatar = '" . $db->sql_escape($row['group_avatar']) . "' + WHERE user_avatar = '" . $db->sql_escape($row['group_avatar']) . "' AND user_avatar_type = " . AVATAR_UPLOAD; $db->sql_query($sql); - + $sql = 'UPDATE ' . GROUPS_TABLE . " SET group_avatar = '" . $db->sql_escape($new_avatar_name) . "' WHERE group_id = {$row['group_id']}"; @@ -80,8 +85,8 @@ while ($row = $db->sql_fetchrow($result)) $db->sql_freeresult($result); $sql = 'SELECT user_id, username, user_avatar, user_avatar_type - FROM ' . USERS_TABLE . ' - WHERE user_avatar_type = ' . AVATAR_UPLOAD . ' + FROM ' . USERS_TABLE . ' + WHERE user_avatar_type = ' . AVATAR_UPLOAD . ' AND ' . $db->sql_in_set('user_avatar', $group_avatars, true, true); $result = $db->sql_query($sql); @@ -108,7 +113,7 @@ while ($row = $db->sql_fetchrow($result)) $db->sql_query($sql); echo '
Failed updating user ' . $row['user_id'] . "\n"; } - + if ($echos > 200) { echo '
' . "\n"; @@ -131,8 +136,8 @@ $db->sql_close(); function adjust_avatar($old_name, $midfix) { global $config, $phpbb_root_path; - - $avatar_path = $phpbb_root_path . $config['avatar_path']; + + $avatar_path = $phpbb_root_path . $config['storage\\avatar\\config\\path']; $extension = strtolower(substr(strrchr($old_name, '.'), 1)); $new_name = $config['avatar_salt'] . '_' . $midfix . '.' . $extension; diff --git a/phpBB/docs/coding-guidelines.html b/phpBB/docs/coding-guidelines.html index 3e6919c88a..97feea71ec 100644 --- a/phpBB/docs/coding-guidelines.html +++ b/phpBB/docs/coding-guidelines.html @@ -264,7 +264,6 @@ PHPBB_QA (Set board to QA-Mode, which means the updater also c
  • {T_SUPER_TEMPLATE_PATH} - styles/xxx/template
  • {T_IMAGES_PATH} - images/
  • {T_SMILIES_PATH} - $config['smilies_path']/
    • -
  • {T_AVATAR_PATH} - $config['avatar_path']/
  • {T_AVATAR_GALLERY_PATH} - $config['avatar_gallery_path']/
  • {T_ICONS_PATH} - $config['icons_path']/
  • {T_RANKS_PATH} - $config['ranks_path']/
    • diff --git a/phpBB/includes/acp/acp_main.php b/phpBB/includes/acp/acp_main.php index 4efa8c70b3..86ce84a4a1 100644 --- a/phpBB/includes/acp/acp_main.php +++ b/phpBB/includes/acp/acp_main.php @@ -502,26 +502,8 @@ class acp_main $upload_dir_size = get_formatted_filesize($config['upload_dir_size']); - $avatar_dir_size = 0; - - if ($avatar_dir = @opendir($phpbb_root_path . $config['avatar_path'])) - { - while (($file = readdir($avatar_dir)) !== false) - { - if ($file[0] != '.' && $file != 'CVS' && strpos($file, 'index.') === false) - { - $avatar_dir_size += filesize($phpbb_root_path . $config['avatar_path'] . '/' . $file); - } - } - closedir($avatar_dir); - - $avatar_dir_size = get_formatted_filesize($avatar_dir_size); - } - else - { - // Couldn't open Avatar dir. - $avatar_dir_size = $user->lang['NOT_AVAILABLE']; - } + // Couldn't open Avatar dir. + $avatar_dir_size = $user->lang['NOT_AVAILABLE']; if ($posts_per_day > $total_posts) { diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 5bf86276c8..9f7d4f02e2 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -4434,7 +4434,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id = 'T_SUPER_TEMPLATE_PATH' => "{$web_path}styles/" . rawurlencode($user->style['style_path']) . '/template', 'T_IMAGES_PATH' => "{$web_path}images/", 'T_SMILIES_PATH' => "{$web_path}{$config['smilies_path']}/", - 'T_AVATAR_PATH' => "{$web_path}{$config['avatar_path']}/", 'T_AVATAR_GALLERY_PATH' => "{$web_path}{$config['avatar_gallery_path']}/", 'T_ICONS_PATH' => "{$web_path}{$config['icons_path']}/", 'T_RANKS_PATH' => "{$web_path}{$config['ranks_path']}/", @@ -4452,7 +4451,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id = 'T_SUPER_TEMPLATE_NAME' => rawurlencode((isset($user->style['style_parent_tree']) && $user->style['style_parent_tree']) ? $user->style['style_parent_tree'] : $user->style['style_path']), 'T_IMAGES' => 'images', 'T_SMILIES' => $config['smilies_path'], - 'T_AVATAR' => $config['avatar_path'], 'T_AVATAR_GALLERY' => $config['avatar_gallery_path'], 'T_ICONS' => $config['icons_path'], 'T_RANKS' => $config['ranks_path'], diff --git a/phpBB/includes/functions_acp.php b/phpBB/includes/functions_acp.php index c79a84f8d4..b3bde79339 100644 --- a/phpBB/includes/functions_acp.php +++ b/phpBB/includes/functions_acp.php @@ -88,7 +88,6 @@ function adm_page_header($page_title) 'T_IMAGES_PATH' => "{$phpbb_root_path}images/", 'T_SMILIES_PATH' => "{$phpbb_root_path}{$config['smilies_path']}/", - 'T_AVATAR_PATH' => "{$phpbb_root_path}{$config['avatar_path']}/", 'T_AVATAR_GALLERY_PATH' => "{$phpbb_root_path}{$config['avatar_gallery_path']}/", 'T_ICONS_PATH' => "{$phpbb_root_path}{$config['icons_path']}/", 'T_RANKS_PATH' => "{$phpbb_root_path}{$config['ranks_path']}/", diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index cee7c39035..332a9c600f 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -25,30 +25,27 @@ if (!defined('IN_PHPBB')) */ function send_avatar_to_browser($file, $browser) { - global $config, $phpbb_root_path; + global $config, $phpbb_container; + + $storage = $phpbb_container->get('storage.avatar'); $prefix = $config['avatar_salt'] . '_'; - $image_dir = $config['avatar_path']; + $file_path = $prefix . $file; - // Adjust image_dir path (no trailing slash) - if (substr($image_dir, -1, 1) == '/' || substr($image_dir, -1, 1) == '\\') + if ($storage->exists($file_path) && !headers_sent()) { - $image_dir = substr($image_dir, 0, -1) . '/'; - } - $image_dir = str_replace(array('../', '..\\', './', '.\\'), '', $image_dir); + $file_info = $storage->file_info($file_path); - if ($image_dir && ($image_dir[0] == '/' || $image_dir[0] == '\\')) - { - $image_dir = ''; - } - $file_path = $phpbb_root_path . $image_dir . '/' . $prefix . $file; - - if ((@file_exists($file_path) && @is_readable($file_path)) && !headers_sent()) - { header('Cache-Control: public'); - $image_data = @getimagesize($file_path); - header('Content-Type: ' . image_type_to_mime_type($image_data[2])); + try + { + header('Content-Type: ' . $file_info->mimetype); + } + catch (\phpbb\storage\exception\exception $e) + { + // Just don't send this header + } if ((strpos(strtolower($browser), 'msie') !== false) && !phpbb_is_greater_ie_version($browser, 7)) { @@ -69,24 +66,26 @@ function send_avatar_to_browser($file, $browser) header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT'); } - $size = @filesize($file_path); - if ($size) + try { - header("Content-Length: $size"); + header('Content-Length: ' . $file_info->size); + } + catch (\phpbb\storage\exception\exception $e) + { + // Just don't send this header } - if (@readfile($file_path) == false) + try { - $fp = @fopen($file_path, 'rb'); - - if ($fp !== false) - { - while (!feof($fp)) - { - echo fread($fp, 8192); - } - fclose($fp); - } + $fp = $storage->read_stream($file_path); + $output = fopen('php://output', 'w+b'); + stream_copy_to_stream($fp, $output); + fclose($fp); + fclose($output); + } + catch (\Exception $e) + { + // Send nothing } flush(); diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 3a65a8820a..3042a5c290 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -2166,7 +2166,9 @@ function phpbb_style_is_active($style_id) */ function avatar_delete($mode, $row, $clean_db = false) { - global $phpbb_root_path, $config; + global $config, $phpbb_container; + + $storage = $phpbb_container->get('storage.avatar'); // Check if the users avatar is actually *not* a group avatar if ($mode == 'user') @@ -2183,11 +2185,16 @@ function avatar_delete($mode, $row, $clean_db = false) } $filename = get_avatar_filename($row[$mode . '_avatar']); - if (file_exists($phpbb_root_path . $config['avatar_path'] . '/' . $filename)) + try { - @unlink($phpbb_root_path . $config['avatar_path'] . '/' . $filename); + $storage->delete($filename); + return true; } + catch (\phpbb\storage\exception\exception $e) + { + // Fail is covered by return statement below + } return false; } @@ -2507,7 +2514,9 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow */ function group_correct_avatar($group_id, $old_entry) { - global $config, $db, $phpbb_root_path; + global $config, $db, $phpbb_container; + + $storage = $phpbb_container->get('storage.avatar'); $group_id = (int) $group_id; $ext = substr(strrchr($old_entry, '.'), 1); @@ -2515,14 +2524,19 @@ function group_correct_avatar($group_id, $old_entry) $new_filename = $config['avatar_salt'] . "_g$group_id.$ext"; $new_entry = 'g' . $group_id . '_' . substr(time(), -5) . ".$ext"; - $avatar_path = $phpbb_root_path . $config['avatar_path']; - if (@rename($avatar_path . '/'. $old_filename, $avatar_path . '/' . $new_filename)) + try { + $this->storage->rename($old_filename, $new_filename); + $sql = 'UPDATE ' . GROUPS_TABLE . ' SET group_avatar = \'' . $db->sql_escape($new_entry) . "' WHERE group_id = $group_id"; $db->sql_query($sql); } + catch (\phpbb\storage\exception\exception $e) + { + // If rename fail, dont execute the query + } } diff --git a/phpBB/install/convert/convertor.php b/phpBB/install/convert/convertor.php index 5118651b71..619fc73275 100644 --- a/phpBB/install/convert/convertor.php +++ b/phpBB/install/convert/convertor.php @@ -281,7 +281,7 @@ class convertor $bad_folders = array(); $local_paths = array( - 'avatar_path' => path($config['avatar_path']), + 'avatar_path' => path($config['storage\\avatar\\config\\path']), 'avatar_gallery_path' => path($config['avatar_gallery_path']), 'icons_path' => path($config['icons_path']), 'ranks_path' => path($config['ranks_path']), diff --git a/phpBB/install/schemas/schema_data.sql b/phpBB/install/schemas/schema_data.sql index ef3472a89a..fbe36bee05 100644 --- a/phpBB/install/schemas/schema_data.sql +++ b/phpBB/install/schemas/schema_data.sql @@ -55,7 +55,6 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_max_height' INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_max_width', '90'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_min_height', '20'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_min_width', '20'); -INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_path', 'images/avatars/upload'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_salt', 'phpbb_avatar'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('board_contact', 'contact@yourdomain.tld'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('board_contact_name', ''); @@ -288,6 +287,8 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_json INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_vendor_dir', 'vendor-ext/'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_enable_on_install', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_purge_on_remove', '1'); +INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\avatar\provider', 'phpbb\storage\provider\local'); +INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\avatar\config\path', 'images/avatars/upload'); INSERT INTO phpbb_config (config_name, config_value, is_dynamic) VALUES ('cache_last_gc', '0', 1); INSERT INTO phpbb_config (config_name, config_value, is_dynamic) VALUES ('cron_lock', '0', 1); diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index d765a27871..621164c765 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -13,15 +13,18 @@ namespace phpbb\avatar\driver; +use bantu\IniGetWrapper\IniGetWrapper; +use \phpbb\storage\exception\exception as storage_exception; + /** * Handles avatars uploaded to the board */ class upload extends \phpbb\avatar\driver\driver { /** - * @var \phpbb\filesystem\filesystem_interface + * @var \phpbb\storage\storage */ - protected $filesystem; + protected $storage; /** * @var \phpbb\event\dispatcher_interface @@ -33,28 +36,33 @@ class upload extends \phpbb\avatar\driver\driver */ protected $files_factory; + /** + * @var IniGetWrapper + */ + protected $php_ini; + /** * Construct a driver object * * @param \phpbb\config\config $config phpBB configuration * @param string $phpbb_root_path Path to the phpBB root * @param string $php_ext PHP file extension - * @param \phpbb\filesystem\filesystem_interface $filesystem phpBB filesystem helper + * @param \phpbb\storage\storage phpBB avatar storage * @param \phpbb\path_helper $path_helper phpBB path helper * @param \phpbb\event\dispatcher_interface $dispatcher phpBB Event dispatcher object * @param \phpbb\files\factory $files_factory File classes factory - * @param \phpbb\cache\driver\driver_interface $cache Cache driver + * @param IniGetWrapper $php_ini ini_get() wrapper */ - public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\filesystem\filesystem_interface $filesystem, \phpbb\path_helper $path_helper, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\files\factory $files_factory, \phpbb\cache\driver\driver_interface $cache = null) + public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\storage\storage $storage, \phpbb\path_helper $path_helper, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\files\factory $files_factory, IniGetWrapper $php_ini) { $this->config = $config; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; - $this->filesystem = $filesystem; + $this->storage = $storage; $this->path_helper = $path_helper; $this->dispatcher = $dispatcher; $this->files_factory = $files_factory; - $this->cache = $cache; + $this->php_ini = $php_ini; } /** @@ -116,7 +124,7 @@ class upload extends \phpbb\avatar\driver\driver if (!empty($upload_file['name'])) { - $file = $upload->handle_upload('files.types.form', 'avatar_upload_file'); + $file = $upload->handle_upload('files.types.form_storage', 'avatar_upload_file'); } else if (!empty($this->config['allow_avatar_remote_upload']) && !empty($url)) { @@ -156,7 +164,7 @@ class upload extends \phpbb\avatar\driver\driver return false; } - $file = $upload->handle_upload('files.types.remote', $url); + $file = $upload->handle_upload('files.types.remote_storage', $url); } else { @@ -169,26 +177,11 @@ class upload extends \phpbb\avatar\driver\driver // If there was an error during upload, then abort operation if (count($file->error)) { - $file->remove(); + $file->remove($this->storage); $error = $file->error; return false; } - // Calculate new destination - $destination = $this->config['avatar_path']; - - // Adjust destination path (no trailing slash) - if (substr($destination, -1, 1) == '/' || substr($destination, -1, 1) == '\\') - { - $destination = substr($destination, 0, -1); - } - - $destination = str_replace(array('../', '..\\', './', '.\\'), '', $destination); - if ($destination && ($destination[0] == '/' || $destination[0] == "\\")) - { - $destination = ''; - } - $filedata = array( 'filename' => $file->get('filename'), 'filesize' => $file->get('filesize'), @@ -203,7 +196,6 @@ class upload extends \phpbb\avatar\driver\driver * * @event core.avatar_driver_upload_move_file_before * @var array filedata Array containing uploaded file data - * @var string destination Destination directory where the file is going to be moved * @var string prefix Prefix for the avatar filename * @var array row Array with avatar row data * @var array error Array of errors, if filled in by this event file will not be moved @@ -212,7 +204,6 @@ class upload extends \phpbb\avatar\driver\driver */ $vars = array( 'filedata', - 'destination', 'prefix', 'row', 'error', @@ -224,14 +215,14 @@ class upload extends \phpbb\avatar\driver\driver if (!count($error)) { // Move file and overwrite any existing image - $file->move_file($destination, true); + $file->move_file($this->storage, true); } // If there was an error during move, then clean up leftovers $error = array_merge($error, $file->error); if (count($error)) { - $file->remove(); + $file->remove($this->storage); return false; } @@ -257,7 +248,6 @@ class upload extends \phpbb\avatar\driver\driver return array( 'allow_avatar_remote_upload'=> array('lang' => 'ALLOW_REMOTE_UPLOAD', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true), 'avatar_filesize' => array('lang' => 'MAX_FILESIZE', 'validate' => 'int:0', 'type' => 'number:0', 'explain' => true, 'append' => ' ' . $user->lang['BYTES']), - 'avatar_path' => array('lang' => 'AVATAR_STORAGE_PATH', 'validate' => 'rpath', 'type' => 'text:20:255', 'explain' => true), ); } @@ -268,37 +258,35 @@ class upload extends \phpbb\avatar\driver\driver { $error = array(); - $destination = $this->config['avatar_path']; $prefix = $this->config['avatar_salt'] . '_'; $ext = substr(strrchr($row['avatar'], '.'), 1); - $filename = $this->phpbb_root_path . $destination . '/' . $prefix . $row['id'] . '.' . $ext; + $filename = $prefix . $row['id'] . '.' . $ext; /** * Before deleting an existing avatar * * @event core.avatar_driver_upload_delete_before - * @var string destination Destination directory where the file is going to be deleted * @var string prefix Prefix for the avatar filename * @var array row Array with avatar row data * @var array error Array of errors, if filled in by this event file will not be deleted * @since 3.1.6-RC1 + * @changed 3.3.0-a1 Remove destination */ $vars = array( - 'destination', 'prefix', 'row', 'error', ); extract($this->dispatcher->trigger_event('core.avatar_driver_upload_delete_before', compact($vars))); - if (!count($error) && $this->filesystem->exists($filename)) + if (!count($error) && $this->storage->exists($filename)) { try { - $this->filesystem->remove($filename); + $this->storage->delete($filename); return true; } - catch (\phpbb\filesystem\exception\filesystem_exception $e) + catch (storage_exception $e) { // Fail is covered by return statement below } @@ -316,12 +304,12 @@ class upload extends \phpbb\avatar\driver\driver } /** - * Check if user is able to upload an avatar + * Check if user is able to upload an avatar to a temporary folder * * @return bool True if user can upload, false if not */ protected function can_upload() { - return ($this->filesystem->exists($this->phpbb_root_path . $this->config['avatar_path']) && $this->filesystem->is_writable($this->phpbb_root_path . $this->config['avatar_path']) && (@ini_get('file_uploads') || strtolower(@ini_get('file_uploads')) == 'on')); + return $this->php_ini->getBool('file_uploads'); } } diff --git a/phpBB/phpbb/db/migration/data/v330/storage_avatar.php b/phpBB/phpbb/db/migration/data/v330/storage_avatar.php new file mode 100644 index 0000000000..077904daa6 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v330/storage_avatar.php @@ -0,0 +1,26 @@ + +* @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\v330; + +class storage_avatar extends \phpbb\db\migration\migration +{ + public function update_data() + { + return array( + array('config.add', array('storage\\avatar\\provider', \phpbb\storage\provider\local::class)), + array('config.add', array('storage\\avatar\\config\\path', $this->config['avatar_path'])), + array('config.remove', array('avatar_path')), + ); + } +} diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index 2c3beb6e02..a915476191 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -25,21 +25,12 @@ class form extends base /** @var factory Files factory */ protected $factory; - /** @var language */ - protected $language; - - /** @var IniGetWrapper */ - protected $php_ini; - /** @var plupload */ protected $plupload; /** @var request_interface */ protected $request; - /** @var \phpbb\files\upload */ - protected $upload; - /** * Construct a form upload type * diff --git a/phpBB/phpbb/files/types/form_storage.php b/phpBB/phpbb/files/types/form_storage.php new file mode 100644 index 0000000000..29bef283bd --- /dev/null +++ b/phpBB/phpbb/files/types/form_storage.php @@ -0,0 +1,128 @@ + + * @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\files\types; + +use bantu\IniGetWrapper\IniGetWrapper; +use phpbb\files\factory; +use phpbb\files\filespec; +use phpbb\language\language; +use phpbb\plupload\plupload; +use phpbb\request\request_interface; + +class form_storage extends base +{ + /** @var factory Files factory */ + protected $factory; + + /** @var plupload */ + protected $plupload; + + /** @var request_interface */ + protected $request; + + /** + * Construct a form upload type + * + * @param factory $factory Files factory + * @param language $language Language class + * @param IniGetWrapper $php_ini ini_get() wrapper + * @param plupload $plupload Plupload + * @param request_interface $request Request object + */ + public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, plupload $plupload, request_interface $request) + { + $this->factory = $factory; + $this->language = $language; + $this->php_ini = $php_ini; + $this->plupload = $plupload; + $this->request = $request; + } + + /** + * {@inheritdoc} + */ + public function upload() + { + $args = func_get_args(); + return $this->form_upload($args[0]); + } + + /** + * Form upload method + * Upload file from users harddisk + * + * @param string $form_name Form name assigned to the file input field (if it is an array, the key has to be specified) + * + * @return filespec $file Object "filespec" is returned, all further operations can be done with this object + */ + protected function form_upload($form_name) + { + $upload = $this->request->file($form_name); + unset($upload['local_mode']); + + $result = $this->plupload->handle_upload($form_name); + if (is_array($result)) + { + $upload = array_merge($upload, $result); + } + + /** @var filespec $file */ + $file = $this->factory->get('filespec_storage') + ->set_upload_ary($upload) + ->set_upload_namespace($this->upload); + + if ($file->init_error()) + { + $file->error[] = ''; + return $file; + } + + // Error array filled? + if (isset($upload['error'])) + { + $error = $this->upload->assign_internal_error($upload['error']); + + if ($error !== false) + { + $file->error[] = $error; + return $file; + } + } + + // Check if empty file got uploaded (not catched by is_uploaded_file) + if (isset($upload['size']) && $upload['size'] == 0) + { + $file->error[] = $this->language->lang($this->upload->error_prefix . 'EMPTY_FILEUPLOAD'); + return $file; + } + + // PHP Upload file size check + $file = $this->check_upload_size($file); + if (sizeof($file->error)) + { + return $file; + } + + // Not correctly uploaded + if (!$file->is_uploaded()) + { + $file->error[] = $this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED'); + return $file; + } + + $this->upload->common_checks($file); + + return $file; + } +} diff --git a/phpBB/phpbb/files/types/local.php b/phpBB/phpbb/files/types/local.php index 4dfe4f7506..67948ea6df 100644 --- a/phpBB/phpbb/files/types/local.php +++ b/phpBB/phpbb/files/types/local.php @@ -24,18 +24,9 @@ class local extends base /** @var factory Files factory */ protected $factory; - /** @var language */ - protected $language; - - /** @var IniGetWrapper */ - protected $php_ini; - /** @var request_interface */ protected $request; - /** @var \phpbb\files\upload */ - protected $upload; - /** * Construct a form upload type * diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 1fdba0ca32..e64e360b6a 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -28,18 +28,9 @@ class remote extends base /** @var factory Files factory */ protected $factory; - /** @var language */ - protected $language; - - /** @var IniGetWrapper */ - protected $php_ini; - /** @var request_interface */ protected $request; - /** @var \phpbb\files\upload */ - protected $upload; - /** @var string phpBB root path */ protected $phpbb_root_path; diff --git a/phpBB/phpbb/files/types/remote_storage.php b/phpBB/phpbb/files/types/remote_storage.php new file mode 100644 index 0000000000..3e6953703e --- /dev/null +++ b/phpBB/phpbb/files/types/remote_storage.php @@ -0,0 +1,197 @@ + + * @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\files\types; + +use bantu\IniGetWrapper\IniGetWrapper; +use phpbb\config\config; +use phpbb\files\factory; +use phpbb\files\filespec; +use phpbb\language\language; +use phpbb\request\request_interface; + +class remote_storage extends base +{ + /** @var config phpBB config */ + protected $config; + + /** @var factory Files factory */ + protected $factory; + + /** @var request_interface */ + protected $request; + + /** @var string phpBB root path */ + protected $phpbb_root_path; + + /** + * Construct a form upload type + * + * @param config $config phpBB config + * @param factory $factory Files factory + * @param language $language Language class + * @param IniGetWrapper $php_ini ini_get() wrapper + * @param request_interface $request Request object + * @param string $phpbb_root_path phpBB root path + */ + public function __construct(config $config, factory $factory, language $language, IniGetWrapper $php_ini, request_interface $request, $phpbb_root_path) + { + $this->config = $config; + $this->factory = $factory; + $this->language = $language; + $this->php_ini = $php_ini; + $this->request = $request; + $this->phpbb_root_path = $phpbb_root_path; + } + + /** + * {@inheritdoc} + */ + public function upload() + { + $args = func_get_args(); + return $this->remote_upload($args[0]); + } + + /** + * Remote upload method + * Uploads file from given url + * + * @param string $upload_url URL pointing to file to upload, for example http://www.foobar.com/example.gif + * @return filespec $file Object "filespec" is returned, all further operations can be done with this object + */ + protected function remote_upload($upload_url) + { + $upload_ary = array(); + $upload_ary['local_mode'] = true; + + if (!preg_match('#^(https?://).*?\.(' . implode('|', $this->upload->allowed_extensions) . ')$#i', $upload_url, $match)) + { + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'URL_INVALID')); + } + + $url = parse_url($upload_url); + + $upload_ary['type'] = 'application/octet-stream'; + + $url['path'] = explode('.', $url['path']); + $ext = array_pop($url['path']); + + $url['path'] = implode('', $url['path']); + $upload_ary['name'] = utf8_basename($url['path']) . (($ext) ? '.' . $ext : ''); + + $remote_max_filesize = $this->get_max_file_size(); + + $guzzle_options = [ + 'timeout' => $this->upload->upload_timeout, + 'connect_timeout' => $this->upload->upload_timeout, + 'verify' => !empty($this->config['remote_upload_verify']) ? (bool) $this->config['remote_upload_verify'] : false, + ]; + $client = new \GuzzleHttp\Client($guzzle_options); + + try + { + $response = $client->get($upload_url, $guzzle_options); + } + catch (\GuzzleHttp\Exception\ClientException $clientException) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'URL_NOT_FOUND'); + } + catch (\GuzzleHttp\Exception\RequestException $requestException) + { + if (strpos($requestException->getMessage(), 'cURL error 28') !== false || preg_match('/408|504/', $requestException->getCode())) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'REMOTE_UPLOAD_TIMEOUT'); + } + else + { + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); + } + } + catch (\Exception $e) + { + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); + } + + $content_length = $response->getBody()->getSize(); + if ($remote_max_filesize && $content_length > $remote_max_filesize) + { + $max_filesize = get_formatted_filesize($remote_max_filesize, false); + + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); + } + + if ($content_length === 0) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'EMPTY_REMOTE_DATA'); + } + + $data = $response->getBody(); + + $filename = tempnam(sys_get_temp_dir(), unique_id() . '-'); + + if (!($fp = @fopen($filename, 'wb'))) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'NOT_UPLOADED'); + } + + $upload_ary['size'] = fwrite($fp, $data); + fclose($fp); + unset($data); + + $upload_ary['tmp_name'] = $filename; + + /** @var filespec $file */ + $file = $this->factory->get('filespec_storage') + ->set_upload_ary($upload_ary) + ->set_upload_namespace($this->upload); + $this->upload->common_checks($file); + + return $file; + } + + /** + * Get maximum file size for remote uploads + * + * @return int Maximum file size + */ + protected function get_max_file_size() + { + $max_file_size = $this->upload->max_filesize; + if (!$max_file_size) + { + $max_file_size = $this->php_ini->getString('upload_max_filesize'); + + if (!empty($max_file_size)) + { + $unit = strtolower(substr($max_file_size, -1, 1)); + $max_file_size = (int) $max_file_size; + + switch ($unit) + { + case 'g': + $max_file_size *= 1024; + // no break + case 'm': + $max_file_size *= 1024; + // no break + case 'k': + $max_file_size *= 1024; + // no break + } + } + } + + return $max_file_size; + } +} diff --git a/phpBB/phpbb/storage/adapter/local.php b/phpBB/phpbb/storage/adapter/local.php index de63d0f6a5..1ef2516f4f 100644 --- a/phpBB/phpbb/storage/adapter/local.php +++ b/phpBB/phpbb/storage/adapter/local.php @@ -18,6 +18,8 @@ use phpbb\storage\exception\exception; use phpbb\filesystem\exception\filesystem_exception; use phpbb\filesystem\filesystem; use phpbb\filesystem\helper as filesystem_helper; +use phpbb\mimetype\guesser; +use FastImageSize\FastImageSize; /** * @internal Experimental @@ -31,6 +33,20 @@ class local implements adapter_interface, stream_interface */ protected $filesystem; + /** + * FastImageSize + * + * @var \FastImageSize\FastImageSize + */ + protected $imagesize; + + /** + * Mimetype Guesser component + * + * @var \phpbb\mimetype\guesser + */ + protected $mimetype_guesser; + /** * @var string path */ @@ -44,9 +60,11 @@ class local implements adapter_interface, stream_interface /** * Constructor */ - public function __construct(filesystem $filesystem, $phpbb_root_path) + public function __construct(filesystem $filesystem, FastImageSize $imagesize, guesser $mimetype_guesser, $phpbb_root_path) { $this->filesystem = $filesystem; + $this->imagesize = $imagesize; + $this->mimetype_guesser = $mimetype_guesser; $this->phpbb_root_path = $phpbb_root_path; } @@ -235,4 +253,82 @@ class local implements adapter_interface, stream_interface throw new exception('STORAGE_CANNOT_COPY_RESOURCE'); } } + + /** + * Get file size. + * + * @param string $path The file + * + * @throws \phpbb\storage\exception\exception When cannot get size + * + * @return array Properties + */ + public function file_size($path) + { + $size = filesize($this->root_path . $path); + + if ($size === null) + { + throw new exception('STORAGE_CANNOT_GET_FILESIZE'); + } + + return ['size' => $size]; + } + + /** + * Get file mimetype. + * + * @param string $path The file + * + * @return array Properties + */ + public function file_mimetype($path) + { + return ['mimetype' => $this->mimetype_guesser->guess($this->root_path . $path)]; + } + + /** + * Get image dimensions. + * + * @param string $path The file + * + * @return array Properties + */ + protected function image_dimensions($path) + { + $size = $this->imagesize->getImageSize($this->root_path . $path); + + // For not supported types like swf + if ($size === false) + { + $imsize = getimagesize($this->root_path . $path); + $size = ['width' => $imsize[0], 'height' => $imsize[1]]; + } + + return ['image_width' => $size['width'], 'image_height' => $size['height']]; + } + + /** + * Get image width. + * + * @param string $path The file + * + * @return array Properties + */ + public function file_image_width($path) + { + return $this->image_dimensions($path); + } + + /** + * Get image height. + * + * @param string $path The file + * + * @return array Properties + */ + public function file_image_height($path) + { + return $this->image_dimensions($path); + } } diff --git a/phpBB/phpbb/storage/file_info.php b/phpBB/phpbb/storage/file_info.php new file mode 100644 index 0000000000..ae2bd5169d --- /dev/null +++ b/phpBB/phpbb/storage/file_info.php @@ -0,0 +1,85 @@ + + * @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\exception; +use phpbb\storage\adapter\adapter_interface; + +class file_info +{ + /** + * @var \phpbb\storage\adapter\adapter_interface + */ + protected $adapter; + + /** + * Path of the file + * + * @var string + */ + protected $path; + + /** + * Stores the properties of $path file, so dont have to be consulted multiple times. + * For example, when you need the width of an image, using getimagesize() you get + * both dimensions, so you store both here, and when you get the height, you dont have + * to call getimagesize() again. + * + * @var array + */ + protected $properties; + + /** + * Constructor + * + * @param \Symfony\Component\DependencyInjection\ContainerInterface $adapter + * @param string $path + */ + public function __construct(adapter_interface $adapter, $path) + { + $this->adapter = $adapter; + $this->path = $path; + $this->properties = []; + } + + /** + * Load propertys lazily. + * + * @param string name The property name. + * + * @return string Returns the property value + */ + public function get($name) + { + if (!isset($this->properties[$name])) + { + if (!method_exists($this->adapter, 'file_' . $name)) + { + throw new exception('STORAGE_METHOD_NOT_IMPLEMENTED'); + } + + $this->properties = array_merge($this->properties, call_user_func([$this->adapter, 'file_' . $name], $this->path)); + } + + return $this->properties[$name]; + } + + /** + * Alias of \phpbb\storage\file_info->get() + */ + public function __get($name) + { + return $this->get($name); + } +} diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index a2eb89ecb3..089ccce737 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -192,4 +192,18 @@ class storage $adapter->put_contents($path, stream_get_contents($resource)); } } + + /** + * Get file info. + * + * @param string $path The file + * + * @throws \phpbb\storage\exception\not_implemented When the adapter doesnt implement the method + * + * @return \phpbb\storage\file_info Returns file_info object + */ + public function file_info($path) + { + return new file_info($this->adapter, $path); + } } diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index 762fd776e3..2866a1673d 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -36,6 +36,13 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case ->will($this->returnArgument(0)); $filesystem = new \phpbb\filesystem\filesystem(); + $adapter = new \phpbb\storage\adapter\local($filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); + $adapter->configure(['path' => 'images/avatars/upload']); + $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); + $adapter_factory_mock->expects($this->any()) + ->method('get') + ->willReturn($adapter); + $storage = new \phpbb\storage\storage($adapter_factory_mock, ''); // Prepare dependencies for avatar manager and driver $this->config = new \phpbb\config\config(array()); @@ -82,6 +89,8 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case $files_factory = new \phpbb\files\factory($phpbb_container); + $php_ini = new \bantu\IniGetWrapper\IniGetWrapper; + foreach ($this->avatar_drivers() as $driver) { if ($driver !== 'upload') @@ -95,7 +104,7 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case { $cur_avatar = $this->getMockBuilder('\phpbb\avatar\driver\\' . $driver) ->setMethods(array('get_name')) - ->setConstructorArgs(array($this->config, $phpbb_root_path, $phpEx, $filesystem, $path_helper, $dispatcher, $files_factory, $cache)) + ->setConstructorArgs(array($this->config, $phpbb_root_path, $phpEx, $storage, $path_helper, $dispatcher, $files_factory, $php_ini)) ->getMock(); } $cur_avatar->expects($this->any()) diff --git a/tests/storage/adapter/local_test.php b/tests/storage/adapter/local_test.php index 6d3ba10bca..b478ce4009 100644 --- a/tests/storage/adapter/local_test.php +++ b/tests/storage/adapter/local_test.php @@ -24,7 +24,7 @@ $filesystem = new \phpbb\filesystem\filesystem(); $phpbb_root_path = getcwd() . DIRECTORY_SEPARATOR; - $this->adapter = new \phpbb\storage\adapter\local($filesystem, $phpbb_root_path); + $this->adapter = new \phpbb\storage\adapter\local($filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $this->adapter->configure(['path' => 'test_path']); $this->path = $phpbb_root_path . 'test_path/';