From dd069333244db94285b1619ba6be437484c19989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Fri, 29 Jun 2018 18:00:11 +0200 Subject: [PATCH 01/42] [ticket/14285] Move downloads to controller PHPBB3-14285 --- .../default/container/services_storage.yml | 19 ++ phpBB/config/default/routing/routing.yml | 3 + phpBB/config/default/routing/storage.yml | 9 + phpBB/download/file.php | 316 ++---------------- phpBB/includes/functions_download.php | 77 ----- phpBB/phpbb/storage/controller/attachment.php | 237 +++++++++++++ phpBB/phpbb/storage/controller/avatar.php | 74 ++++ phpBB/phpbb/storage/controller/controller.php | 111 ++++++ 8 files changed, 479 insertions(+), 367 deletions(-) create mode 100644 phpBB/config/default/routing/storage.yml create mode 100644 phpBB/phpbb/storage/controller/attachment.php create mode 100644 phpBB/phpbb/storage/controller/avatar.php create mode 100644 phpBB/phpbb/storage/controller/controller.php diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 92f31779e6..9577f52684 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -82,3 +82,22 @@ services: arguments: tags: - { name: storage.provider } + +# Controllers + storage.controller.avatar: + class: phpbb\storage\controller\avatar + arguments: + - '@config' + - '@storage.avatar' + + storage.controller.attachment: + class: phpbb\storage\controller\attachment + arguments: + - '@auth' + - '@cache' + - '@config' + - '@dbal.conn' + - '@dispatcher' + - '@request' + - '@storage.attachment' + - '@user' diff --git a/phpBB/config/default/routing/routing.yml b/phpBB/config/default/routing/routing.yml index a5e9265dc3..daf8ec7002 100644 --- a/phpBB/config/default/routing/routing.yml +++ b/phpBB/config/default/routing/routing.yml @@ -30,3 +30,6 @@ phpbb_report_routing: phpbb_ucp_routing: resource: ucp.yml prefix: /user + +phpbb_storage_routing: + resource: storage.yml diff --git a/phpBB/config/default/routing/storage.yml b/phpBB/config/default/routing/storage.yml new file mode 100644 index 0000000000..a793aa8a6f --- /dev/null +++ b/phpBB/config/default/routing/storage.yml @@ -0,0 +1,9 @@ +phpbb_storage_avatar: + path: /download/avatar/{file} + defaults: + _controller: storage.controller.avatar:handle + +phpbb_storage_attachment: + path: /download/attachment/{file} + defaults: + _controller: storage.controller.attachment:handle diff --git a/phpBB/download/file.php b/phpBB/download/file.php index 6b0b577489..bca13ba45d 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -11,310 +11,46 @@ * */ +use Symfony\Component\HttpFoundation\RedirectResponse; + /** * @ignore */ define('IN_PHPBB', true); $phpbb_root_path = (defined('PHPBB_ROOT_PATH')) ? PHPBB_ROOT_PATH : './../'; $phpEx = substr(strrchr(__FILE__, '.'), 1); +include($phpbb_root_path . 'common.' . $phpEx); -// Thank you sun. -if (isset($_SERVER['CONTENT_TYPE'])) -{ - if ($_SERVER['CONTENT_TYPE'] === 'application/x-java-archive') - { - exit; - } -} -else if (isset($_SERVER['HTTP_USER_AGENT']) && strpos($_SERVER['HTTP_USER_AGENT'], 'Java') !== false) -{ - exit; -} +// Start session management +$user->session_begin(); +$auth->acl($user->data); + +/** @var \phpbb\controller\helper $controller_helper */ +$controller_helper = $phpbb_container->get('controller.helper'); if (isset($_GET['avatar'])) { - require($phpbb_root_path . 'includes/startup.' . $phpEx); + $response = new RedirectResponse( + $controller_helper->route('phpbb_storage_avatar', array( + 'file' => $request->variable('avatar', ''), + )), + 301 + ); + $response->send(); - require($phpbb_root_path . 'phpbb/class_loader.' . $phpEx); - $phpbb_class_loader = new \phpbb\class_loader('phpbb\\', "{$phpbb_root_path}phpbb/", $phpEx); - $phpbb_class_loader->register(); - - $phpbb_config_php_file = new \phpbb\config_php_file($phpbb_root_path, $phpEx); - extract($phpbb_config_php_file->get_all()); - - if (!defined('PHPBB_ENVIRONMENT')) - { - @define('PHPBB_ENVIRONMENT', 'production'); - } - - if (!defined('PHPBB_INSTALLED') || empty($dbms) || empty($acm_type)) - { - exit; - } - - require($phpbb_root_path . 'includes/constants.' . $phpEx); - require($phpbb_root_path . 'includes/functions.' . $phpEx); - require($phpbb_root_path . 'includes/functions_download' . '.' . $phpEx); - require($phpbb_root_path . 'includes/utf/utf_tools.' . $phpEx); - - // Setup class loader first - $phpbb_class_loader_ext = new \phpbb\class_loader('\\', "{$phpbb_root_path}ext/", $phpEx); - $phpbb_class_loader_ext->register(); - - // Set up container - $phpbb_container_builder = new \phpbb\di\container_builder($phpbb_root_path, $phpEx); - $phpbb_container = $phpbb_container_builder->with_config($phpbb_config_php_file)->get_container(); - - $phpbb_class_loader->set_cache($phpbb_container->get('cache.driver')); - $phpbb_class_loader_ext->set_cache($phpbb_container->get('cache.driver')); - - // set up caching - /* @var $cache \phpbb\cache\service */ - $cache = $phpbb_container->get('cache'); - - /* @var $phpbb_dispatcher \phpbb\event\dispatcher */ - $phpbb_dispatcher = $phpbb_container->get('dispatcher'); - - /* @var $request \phpbb\request\request_interface */ - $request = $phpbb_container->get('request'); - - /* @var $db \phpbb\db\driver\driver_interface */ - $db = $phpbb_container->get('dbal.conn'); - - /* @var $phpbb_log \phpbb\log\log_interface */ - $phpbb_log = $phpbb_container->get('log'); - - unset($dbpasswd); - - /* @var $config \phpbb\config\config */ - $config = $phpbb_container->get('config'); - - // load extensions - /* @var $phpbb_extension_manager \phpbb\extension\manager */ - $phpbb_extension_manager = $phpbb_container->get('ext.manager'); - - // worst-case default - $browser = strtolower($request->header('User-Agent', 'msie 6.0')); - - /* @var $phpbb_avatar_manager \phpbb\avatar\manager */ - $phpbb_avatar_manager = $phpbb_container->get('avatar.manager'); - - if (@is_file($phpbb_root_path . $config['exts_composer_vendor_dir'] . '/autoload.php')) - { - require_once($phpbb_root_path . $config['exts_composer_vendor_dir'] . '/autoload.php'); - } - - $filename = $request->variable('avatar', ''); - $avatar_group = false; - $exit = false; - - if (isset($filename[0]) && $filename[0] === 'g') - { - $avatar_group = true; - $filename = substr($filename, 1); - } - - // '==' is not a bug - . as the first char is as bad as no dot at all - if (strpos($filename, '.') == false) - { - send_status_line(403, 'Forbidden'); - $exit = true; - } - - if (!$exit) - { - $ext = substr(strrchr($filename, '.'), 1); - $stamp = (int) substr(stristr($filename, '_'), 1); - $filename = (int) $filename; - $exit = set_modified_headers($stamp, $browser); - } - if (!$exit && !in_array($ext, array('png', 'gif', 'jpg', 'jpeg'))) - { - // no way such an avatar could exist. They are not following the rules, stop the show. - send_status_line(403, 'Forbidden'); - $exit = true; - } - - - if (!$exit) - { - if (!$filename) - { - // no way such an avatar could exist. They are not following the rules, stop the show. - send_status_line(403, 'Forbidden'); - } - else - { - send_avatar_to_browser(($avatar_group ? 'g' : '') . $filename . '.' . $ext, $browser); - } - } - file_gc(); + exit; } -// implicit else: we are not in avatar mode -include($phpbb_root_path . 'common.' . $phpEx); -require($phpbb_root_path . 'includes/functions_download' . '.' . $phpEx); - $attach_id = $request->variable('id', 0); $mode = $request->variable('mode', ''); $thumbnail = $request->variable('t', false); -// Start session management, do not update session page. -$user->session_begin(false); -$auth->acl($user->data); -$user->setup('viewtopic'); - -$phpbb_content_visibility = $phpbb_container->get('content.visibility'); - -if (!$config['allow_attachments'] && !$config['allow_pm_attach']) -{ - send_status_line(404, 'Not Found'); - trigger_error('ATTACHMENT_FUNCTIONALITY_DISABLED'); -} - -if (!$attach_id) -{ - send_status_line(404, 'Not Found'); - trigger_error('NO_ATTACHMENT_SELECTED'); -} - -$sql = 'SELECT attach_id, post_msg_id, topic_id, in_message, poster_id, is_orphan, physical_filename, real_filename, extension, mimetype, filesize, filetime - FROM ' . ATTACHMENTS_TABLE . " - WHERE attach_id = $attach_id"; -$result = $db->sql_query($sql); -$attachment = $db->sql_fetchrow($result); -$db->sql_freeresult($result); - -if (!$attachment) -{ - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); -} -else if (!download_allowed()) -{ - send_status_line(403, 'Forbidden'); - trigger_error($user->lang['LINKAGE_FORBIDDEN']); -} -else -{ - $attachment['physical_filename'] = utf8_basename($attachment['physical_filename']); - - if (!$attachment['in_message'] && !$config['allow_attachments'] || $attachment['in_message'] && !$config['allow_pm_attach']) - { - send_status_line(404, 'Not Found'); - trigger_error('ATTACHMENT_FUNCTIONALITY_DISABLED'); - } - - if ($attachment['is_orphan']) - { - // We allow admins having attachment permissions to see orphan attachments... - $own_attachment = ($auth->acl_get('a_attach') || $attachment['poster_id'] == $user->data['user_id']) ? true : false; - - if (!$own_attachment || ($attachment['in_message'] && !$auth->acl_get('u_pm_download')) || (!$attachment['in_message'] && !$auth->acl_get('u_download'))) - { - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); - } - - // Obtain all extensions... - $extensions = $cache->obtain_attach_extensions(true); - } - else - { - if (!$attachment['in_message']) - { - phpbb_download_handle_forum_auth($db, $auth, $attachment['topic_id']); - - $sql = 'SELECT forum_id, post_visibility - FROM ' . POSTS_TABLE . ' - WHERE post_id = ' . (int) $attachment['post_msg_id']; - $result = $db->sql_query($sql); - $post_row = $db->sql_fetchrow($result); - $db->sql_freeresult($result); - - if (!$post_row || !$phpbb_content_visibility->is_visible('post', $post_row['forum_id'], $post_row)) - { - // Attachment of a soft deleted post and the user is not allowed to see the post - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); - } - } - else - { - // Attachment is in a private message. - $post_row = array('forum_id' => false); - phpbb_download_handle_pm_auth($db, $auth, $user->data['user_id'], $attachment['post_msg_id']); - } - - $extensions = array(); - if (!extension_allowed($post_row['forum_id'], $attachment['extension'], $extensions)) - { - send_status_line(403, 'Forbidden'); - trigger_error(sprintf($user->lang['EXTENSION_DISABLED_AFTER_POSTING'], $attachment['extension'])); - } - } - - $display_cat = $extensions[$attachment['extension']]['display_cat']; - - if (($display_cat == ATTACHMENT_CATEGORY_IMAGE || $display_cat == ATTACHMENT_CATEGORY_THUMB) && !$user->optionget('viewimg')) - { - $display_cat = ATTACHMENT_CATEGORY_NONE; - } - - if ($thumbnail) - { - $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; - } - else if ($display_cat == ATTACHMENT_CATEGORY_NONE && !$attachment['is_orphan'] && !phpbb_http_byte_range($attachment['filesize'])) - { - // Update download count - phpbb_increment_downloads($db, $attachment['attach_id']); - } - - $redirect = ''; - - /** - * Event to modify data before sending file to browser - * - * @event core.download_file_send_to_browser_before - * @var int attach_id The attachment ID - * @var array attachment Array with attachment data - * @var int display_cat Attachment category - * @var array extensions Array with file extensions data - * @var string mode Download mode - * @var bool thumbnail Flag indicating if the file is a thumbnail - * @var string redirect Do a redirection instead of reading the file - * @since 3.1.6-RC1 - * @changed 3.1.7-RC1 Fixing wrong name of a variable (replacing "extension" by "extensions") - * @changed 3.3.0-a1 Add redirect variable - */ - $vars = array( - 'attach_id', - 'attachment', - 'display_cat', - 'extensions', - 'mode', - 'thumbnail', - 'redirect', - ); - extract($phpbb_dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); - - if ($display_cat == ATTACHMENT_CATEGORY_IMAGE && $mode === 'view' && (strpos($attachment['mimetype'], 'image') === 0) && (strpos(strtolower($user->browser), 'msie') !== false) && !phpbb_is_greater_ie_version($user->browser, 7)) - { - wrap_img_in_html(append_sid($phpbb_root_path . 'download/file.' . $phpEx, 'id=' . $attachment['attach_id']), $attachment['real_filename']); - file_gc(); - } - else - { - if (!empty($redirect)) - { - redirect($redirect, false, true); - } - else - { - send_file_to_browser($attachment, $display_cat); - } - - file_gc(); - } -} +$response = new RedirectResponse( + $controller_helper->route('phpbb_storage_attachment', array( + 'file' => $attach_id, + 'mode' => $mode, + 't' => $thumbnail, + )), + 301 +); +$response->send(); diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index 3dcfb4cc98..bda01c2944 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -19,83 +19,6 @@ if (!defined('IN_PHPBB')) exit; } -/** -* A simplified function to deliver avatars -* The argument needs to be checked before calling this function. -*/ -function send_avatar_to_browser($file, $browser) -{ - global $config, $phpbb_container; - - $storage = $phpbb_container->get('storage.avatar'); - - $prefix = $config['avatar_salt'] . '_'; - $file_path = $prefix . $file; - - if ($storage->exists($file_path) && !headers_sent()) - { - $file_info = $storage->file_info($file_path); - - header('Cache-Control: public'); - - 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)) - { - header('Content-Disposition: attachment; ' . header_filename($file)); - - if (strpos(strtolower($browser), 'msie 6.0') !== false) - { - header('Expires: ' . gmdate('D, d M Y H:i:s', time()) . ' GMT'); - } - else - { - header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT'); - } - } - else - { - header('Content-Disposition: inline; ' . header_filename($file)); - header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT'); - } - - try - { - header('Content-Length: ' . $file_info->size); - } - catch (\phpbb\storage\exception\exception $e) - { - // Just don't send this header - } - - try - { - $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(); - } - else - { - header('HTTP/1.0 404 Not Found'); - } -} - /** * Wraps an url into a simple html page. Used to display attachments in IE. * this is a workaround for now; might be moved to template system later diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php new file mode 100644 index 0000000000..ca03af096c --- /dev/null +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -0,0 +1,237 @@ + + * @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\controller; + +use phpbb\auth\auth; +use phpbb\cache\service; +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\event\dispatcher; +use phpbb\request\request; +use phpbb\storage\storage; +use phpbb\user; + +class attachment extends controller +{ + /** @var auth */ + protected $auth; + + /** @var service */ + protected $cache; + + /** @var config */ + protected $config; + + /** @var driver_interface */ + protected $db; + + /** @var dispatcher */ + protected $phpbb_dispatcher; + + /** @var request */ + protected $request; + + /** @var storage */ + protected $storage; + + /** @var user */ + protected $user; + + public function __construct(auth $auth, service $cache, config $config, driver_interface $db, dispatcher $phpbb_dispatcher, request $request, storage $storage, user $user) + { + $this->auth = $auth; + $this->cache = $cache; + $this->config = $config; + $this->db = $db; + $this->phpbb_dispatcher = $phpbb_dispatcher; + $this->request = $request; + $this->storage = $storage; + $this->user = $user; + } + + public function handle($file) + { + global $phpbb_root_path, $phpEx, $phpbb_container; + require($phpbb_root_path . 'includes/functions_download' . '.' . $phpEx); + + $attach_id = $file; + $mode = $this->request->variable('mode', ''); + $thumbnail = $this->request->variable('t', false); + + // Start session management, do not update session page. + $this->user->session_begin(false); + $this->auth->acl($this->user->data); + $this->user->setup('viewtopic'); + + $phpbb_content_visibility = $phpbb_container->get('content.visibility'); + + if (!$this->config['allow_attachments'] && !$this->config['allow_pm_attach']) + { + send_status_line(404, 'Not Found'); + trigger_error('ATTACHMENT_FUNCTIONALITY_DISABLED'); + } + + if (!$attach_id) + { + send_status_line(404, 'Not Found'); + trigger_error('NO_ATTACHMENT_SELECTED'); + } + + $sql = 'SELECT attach_id, post_msg_id, topic_id, in_message, poster_id, is_orphan, physical_filename, real_filename, extension, mimetype, filesize, filetime + FROM ' . ATTACHMENTS_TABLE . " + WHERE attach_id = $attach_id"; + $result = $this->db->sql_query($sql); + $attachment = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + if (!$attachment) + { + send_status_line(404, 'Not Found'); + trigger_error('ERROR_NO_ATTACHMENT'); + } + else if (!download_allowed()) + { + send_status_line(403, 'Forbidden'); + trigger_error($user->lang['LINKAGE_FORBIDDEN']); + } + else + { + $attachment['physical_filename'] = utf8_basename($attachment['physical_filename']); + + if (!$attachment['in_message'] && !$this->config['allow_attachments'] || $attachment['in_message'] && !$this->config['allow_pm_attach']) + { + send_status_line(404, 'Not Found'); + trigger_error('ATTACHMENT_FUNCTIONALITY_DISABLED'); + } + + if ($attachment['is_orphan']) + { + // We allow admins having attachment permissions to see orphan attachments... + $own_attachment = ($this->auth->acl_get('a_attach') || $attachment['poster_id'] == $this->user->data['user_id']) ? true : false; + + if (!$own_attachment || ($attachment['in_message'] && !$this->auth->acl_get('u_pm_download')) || (!$attachment['in_message'] && !$this->auth->acl_get('u_download'))) + { + send_status_line(404, 'Not Found'); + trigger_error('ERROR_NO_ATTACHMENT'); + } + + // Obtain all extensions... + $extensions = $this->cache->obtain_attach_extensions(true); + } + else + { + if (!$attachment['in_message']) + { + phpbb_download_handle_forum_auth($this->db, $this->auth, $attachment['topic_id']); + + $sql = 'SELECT forum_id, post_visibility + FROM ' . POSTS_TABLE . ' + WHERE post_id = ' . (int) $attachment['post_msg_id']; + $result = $this->db->sql_query($sql); + $post_row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + if (!$post_row || !$phpbb_content_visibility->is_visible('post', $post_row['forum_id'], $post_row)) + { + // Attachment of a soft deleted post and the user is not allowed to see the post + send_status_line(404, 'Not Found'); + trigger_error('ERROR_NO_ATTACHMENT'); + } + } + else + { + // Attachment is in a private message. + $post_row = array('forum_id' => false); + phpbb_download_handle_pm_auth($this->db, $this->auth, $this->user->data['user_id'], $attachment['post_msg_id']); + } + + $extensions = array(); + if (!extension_allowed($post_row['forum_id'], $attachment['extension'], $extensions)) + { + send_status_line(403, 'Forbidden'); + trigger_error(sprintf($this->user->lang['EXTENSION_DISABLED_AFTER_POSTING'], $attachment['extension'])); + } + } + + $display_cat = $extensions[$attachment['extension']]['display_cat']; + + if (($display_cat == ATTACHMENT_CATEGORY_IMAGE || $display_cat == ATTACHMENT_CATEGORY_THUMB) && !$this->user->optionget('viewimg')) + { + $display_cat = ATTACHMENT_CATEGORY_NONE; + } + + if ($display_cat == ATTACHMENT_CATEGORY_FLASH && !$this->user->optionget('viewflash')) + { + $display_cat = ATTACHMENT_CATEGORY_NONE; + } + + if ($thumbnail) + { + $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; + } + else if ($display_cat == ATTACHMENT_CATEGORY_NONE && !$attachment['is_orphan'] && !phpbb_http_byte_range($attachment['filesize'])) + { + // Update download count + phpbb_increment_downloads($this->db, $attachment['attach_id']); + } + + $redirect = ''; + + /** + * Event to modify data before sending file to browser + * + * @event core.download_file_send_to_browser_before + * @var int attach_id The attachment ID + * @var array attachment Array with attachment data + * @var int display_cat Attachment category + * @var array extensions Array with file extensions data + * @var string mode Download mode + * @var bool thumbnail Flag indicating if the file is a thumbnail + * @var string redirect Do a redirection instead of reading the file + * @since 3.1.6-RC1 + * @changed 3.1.7-RC1 Fixing wrong name of a variable (replacing "extension" by "extensions") + * @changed 3.3.0-a1 Add redirect variable + */ + $vars = array( + 'attach_id', + 'attachment', + 'display_cat', + 'extensions', + 'mode', + 'thumbnail', + 'redirect', + ); + extract($this->phpbb_dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); + + if ($display_cat == ATTACHMENT_CATEGORY_IMAGE && $mode === 'view' && (strpos($attachment['mimetype'], 'image') === 0) && (strpos(strtolower($user->browser), 'msie') !== false) && !phpbb_is_greater_ie_version($user->browser, 7)) + { + wrap_img_in_html(append_sid($phpbb_root_path . 'download/file.' . $phpEx, 'id=' . $attachment['attach_id']), $attachment['real_filename']); + file_gc(); + } + else + { + if (!empty($redirect)) + { + redirect($redirect, false, true); + } + else + { + send_file_to_browser($attachment, $display_cat); + } + + file_gc(); + } + } + } +} diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php new file mode 100644 index 0000000000..1285f110d9 --- /dev/null +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -0,0 +1,74 @@ + + * @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\controller; + +use phpbb\config\config; +use phpbb\storage\storage; + +class avatar extends controller +{ + /** @var config */ + protected $config; + + protected $allowed_extensions = ['png', 'gif', 'jpg', 'jpeg']; + + public function __construct(config $config, storage $storage) + { + $this->config = $config; + $this->storage = $storage; + } + + public function handle($file) + { + $file = $this->decode_avatar_filename($file); + + parent::handle($file); + } + + protected function is_allowed($file) + { + $ext = substr(strrchr($file, '.'), 1); + + // If filename have point and have an allowed extension + return strpos($file, '.') && in_array($ext, $this->allowed_extensions); + } + + protected function decode_avatar_filename($file) + { + $avatar_group = false; + + if (isset($file[0]) && $file[0] === 'g') + { + $avatar_group = true; + $file = substr($file, 1); + } + + $ext = substr(strrchr($file, '.'), 1); + $file = (int) $file; + + return $this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $file . '.' . $ext; + } + + protected function send($file) + { + if (!headers_sent()) + { + header('Content-Disposition: inline; ' . header_filename($file)); + + header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600*24*365) . ' GMT'); + } + + parent::send($file); + } +} diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php new file mode 100644 index 0000000000..e0da578b41 --- /dev/null +++ b/phpBB/phpbb/storage/controller/controller.php @@ -0,0 +1,111 @@ + + * @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\controller; + +use phpbb\storage\storage; + +class controller +{ + /** @var storage */ + protected $storage; + + public function __construct(storage $storage) + { + $this->storage = $storage; + } + + public function handle($file) + { + if (!function_exists('file_gc')) + { + global $phpbb_root_path, $phpEx; + require($phpbb_root_path . 'includes/functions_download' . '.' . $phpEx); + } + + if (!$this->is_allowed($file)) + { + send_status_line(403, 'Forbidden'); + file_gc(); + exit; + } + + if (!$this->file_exists($file)) + { + send_status_line(404, 'Not Found'); + file_gc(); + exit; + } + + $this->send($file); + } + + protected function is_allowed($file) + { + return true; + } + + protected function file_exists($file) + { + return $this->storage->exists($file); + } + + protected function send($file) + { + if (!function_exists('file_gc')) + { + global $phpbb_root_path, $phpEx; + require($phpbb_root_path . 'includes/functions_download' . '.' . $phpEx); + } + + if (!headers_sent()) + { + header('Cache-Control: public'); + + $file_info = $this->storage->file_info($file); + + try + { + header('Content-Type: ' . $file_info->mimetype); + } + catch (\phpbb\storage\exception\exception $e) + { + // Just don't send this header + } + + try + { + header('Content-Length: ' . $file_info->size); + } + catch (\phpbb\storage\exception\exception $e) + { + // Just don't send this header + } + + $fp = $this->storage->read_stream($file); + + // Close db connection + file_gc(false); + + $output = fopen('php://output', 'w+b'); + + stream_copy_to_stream($fp, $output); + + fclose($fp); + fclose($output); + + // ?? + flush(); + } + } +} From 8dcf8a4ddb9afaf767cbf0995d669b3434e860d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Fri, 29 Jun 2018 18:38:26 +0200 Subject: [PATCH 02/42] [ticket/14285] Remove support for old internet explorer versions PHPBB3-14285 --- phpBB/includes/functions_download.php | 32 +++++++++---------- phpBB/phpbb/storage/controller/attachment.php | 18 +++-------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index bda01c2944..31164221d1 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -23,6 +23,8 @@ if (!defined('IN_PHPBB')) * Wraps an url into a simple html page. Used to display attachments in IE. * this is a workaround for now; might be moved to template system later * direct any complaints to 1 Microsoft Way, Redmond +* +* @deprecated: 3.3.0-dev (To be removed: 4.0.0) */ function wrap_img_in_html($src, $title) { @@ -124,10 +126,7 @@ function send_file_to_browser($attachment, $category) // Send out the Headers. Do not set Content-Disposition to inline please, it is a security measure for users using the Internet Explorer. header('Content-Type: ' . $attachment['mimetype']); - if (phpbb_is_greater_ie_version($user->browser, 7)) - { - header('X-Content-Type-Options: nosniff'); - } + header('X-Content-Type-Options: nosniff'); if (empty($user->browser) || ((strpos(strtolower($user->browser), 'msie') !== false) && !phpbb_is_greater_ie_version($user->browser, 7))) { @@ -316,20 +315,17 @@ function set_modified_headers($stamp, $browser) // let's see if we have to send the file at all $last_load = $request->header('If-Modified-Since') ? strtotime(trim($request->header('If-Modified-Since'))) : false; - if (strpos(strtolower($browser), 'msie 6.0') === false && !phpbb_is_greater_ie_version($browser, 7)) + if ($last_load !== false && $last_load >= $stamp) { - if ($last_load !== false && $last_load >= $stamp) - { - send_status_line(304, 'Not Modified'); - // seems that we need those too ... browsers - header('Cache-Control: private'); - header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT'); - return true; - } - else - { - header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $stamp) . ' GMT'); - } + send_status_line(304, 'Not Modified'); + // seems that we need those too ... browsers + header('Cache-Control: private'); + header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT'); + return true; + } + else + { + header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $stamp) . ' GMT'); } return false; } @@ -646,6 +642,8 @@ function phpbb_download_check_pm_auth($db, $user_id, $msg_id) * @param int $version IE version to check against * * @return bool true if internet explorer version is greater than $version +* +* @deprecated: 3.3.0-dev (To be removed: 4.0.0) */ function phpbb_is_greater_ie_version($user_agent, $version) { diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index ca03af096c..7e93369754 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -214,24 +214,16 @@ class attachment extends controller ); extract($this->phpbb_dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); - if ($display_cat == ATTACHMENT_CATEGORY_IMAGE && $mode === 'view' && (strpos($attachment['mimetype'], 'image') === 0) && (strpos(strtolower($user->browser), 'msie') !== false) && !phpbb_is_greater_ie_version($user->browser, 7)) + if (!empty($redirect)) { - wrap_img_in_html(append_sid($phpbb_root_path . 'download/file.' . $phpEx, 'id=' . $attachment['attach_id']), $attachment['real_filename']); - file_gc(); + redirect($redirect, false, true); } else { - if (!empty($redirect)) - { - redirect($redirect, false, true); - } - else - { - send_file_to_browser($attachment, $display_cat); - } - - file_gc(); + send_file_to_browser($attachment, $display_cat); } + + file_gc(); } } } From 2f043cdb612f197160d08d84e58682dd010ab4ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 30 Jun 2018 03:56:31 +0200 Subject: [PATCH 03/42] [ticket/14285] Move functions from functions_download to controller PHPBB3-14285 --- .../default/container/services_storage.yml | 1 + phpBB/includes/functions_download.php | 550 +----------------- phpBB/phpbb/storage/controller/attachment.php | 423 +++++++++++++- phpBB/phpbb/storage/controller/avatar.php | 4 +- phpBB/phpbb/storage/controller/controller.php | 50 +- 5 files changed, 441 insertions(+), 587 deletions(-) diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 9577f52684..b3b118f6e2 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -96,6 +96,7 @@ services: - '@auth' - '@cache' - '@config' + - '@content.visibility' - '@dbal.conn' - '@dispatcher' - '@request' diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index 31164221d1..6ec95e966e 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -43,136 +43,6 @@ function wrap_img_in_html($src, $title) echo ''; } -/** -* Send file to browser -*/ -function send_file_to_browser($attachment, $category) -{ - global $user, $db, $phpbb_dispatcher, $request, $phpbb_container; - - $storage = $phpbb_container->get('storage.attachment'); - - $filename = $attachment['physical_filename']; - - if (!$storage->exists($filename)) - { - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); - } - - // Correct the mime type - we force application/octetstream for all files, except images - // Please do not change this, it is a security precaution - if ($category != ATTACHMENT_CATEGORY_IMAGE || strpos($attachment['mimetype'], 'image') !== 0) - { - $attachment['mimetype'] = (strpos(strtolower($user->browser), 'msie') !== false || strpos(strtolower($user->browser), 'opera') !== false) ? 'application/octetstream' : 'application/octet-stream'; - } - - if (@ob_get_length()) - { - @ob_end_clean(); - } - - // Now send the File Contents to the Browser - try - { - $file_info = $storage->file_info($filename); - $size = $file_info->size; - } - catch (\Exception $e) - { - $size = 0; - } - - /** - * Event to alter attachment before it is sent to browser. - * - * @event core.send_file_to_browser_before - * @var array attachment Attachment data - * @var int category Attachment category - * @var string filename Path to file, including filename - * @var int size File size - * @since 3.1.11-RC1 - */ - $vars = array( - 'attachment', - 'category', - 'filename', - 'size', - ); - extract($phpbb_dispatcher->trigger_event('core.send_file_to_browser_before', compact($vars))); - - // To correctly display further errors we need to make sure we are using the correct headers for both (unsetting content-length may not work) - - // Check if headers already sent or not able to get the file contents. - if (headers_sent()) - { - send_status_line(500, 'Internal Server Error'); - trigger_error('UNABLE_TO_DELIVER_FILE'); - } - - // Make sure the database record for the filesize is correct - if ($size > 0 && $size != $attachment['filesize'] && strpos($attachment['physical_filename'], 'thumb_') === false) - { - // Update database record - $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' - SET filesize = ' . (int) $size . ' - WHERE attach_id = ' . (int) $attachment['attach_id']; - $db->sql_query($sql); - } - - // Now the tricky part... let's dance - header('Cache-Control: private'); - - // Send out the Headers. Do not set Content-Disposition to inline please, it is a security measure for users using the Internet Explorer. - header('Content-Type: ' . $attachment['mimetype']); - - header('X-Content-Type-Options: nosniff'); - - if (empty($user->browser) || ((strpos(strtolower($user->browser), 'msie') !== false) && !phpbb_is_greater_ie_version($user->browser, 7))) - { - header('Content-Disposition: attachment; ' . header_filename(htmlspecialchars_decode($attachment['real_filename'], ENT_COMPAT))); - if (empty($user->browser) || (strpos(strtolower($user->browser), 'msie 6.0') !== false)) - { - header('Expires: ' . gmdate('D, d M Y H:i:s', time()) . ' GMT'); - } - } - else - { - header('Content-Disposition: ' . ((strpos($attachment['mimetype'], 'image') === 0) ? 'inline' : 'attachment') . '; ' . header_filename(htmlspecialchars_decode($attachment['real_filename'], ENT_COMPAT))); - if (phpbb_is_greater_ie_version($user->browser, 7) && (strpos($attachment['mimetype'], 'image') !== 0)) - { - header('X-Download-Options: noopen'); - } - } - - if (!set_modified_headers($attachment['filetime'], $user->browser)) - { - if ($size) - { - header("Content-Length: $size"); - } - - // Try to deliver in chunks - @set_time_limit(0); - - $fp = $storage->read_stream($filename); - - // Close the db connection before sending the file etc. - file_gc(false); - - if ($fp !== false) - { - $output = fopen('php://output', 'w+b'); - stream_copy_to_stream($fp, $output); - fclose($fp); - } - - flush(); - } - - exit; -} - /** * Get a browser friendly UTF-8 encoded filename */ @@ -193,149 +63,14 @@ function header_filename($file) return "filename*=UTF-8''" . rawurlencode($file); } -/** -* Check if downloading item is allowed -*/ -function download_allowed() -{ - global $config, $user, $db, $request; - - if (!$config['secure_downloads']) - { - return true; - } - - $url = htmlspecialchars_decode($request->header('Referer'), ENT_COMPAT); - - if (!$url) - { - return ($config['secure_allow_empty_referer']) ? true : false; - } - - // Split URL into domain and script part - $url = @parse_url($url); - - if ($url === false) - { - return ($config['secure_allow_empty_referer']) ? true : false; - } - - $hostname = $url['host']; - unset($url); - - $allowed = ($config['secure_allow_deny']) ? false : true; - $iplist = array(); - - if (($ip_ary = @gethostbynamel($hostname)) !== false) - { - foreach ($ip_ary as $ip) - { - if ($ip) - { - $iplist[] = $ip; - } - } - } - - // Check for own server... - $server_name = $user->host; - - // Forcing server vars is the only way to specify/override the protocol - if ($config['force_server_vars'] || !$server_name) - { - $server_name = $config['server_name']; - } - - if (preg_match('#^.*?' . preg_quote($server_name, '#') . '.*?$#i', $hostname)) - { - $allowed = true; - } - - // Get IP's and Hostnames - if (!$allowed) - { - $sql = 'SELECT site_ip, site_hostname, ip_exclude - FROM ' . SITELIST_TABLE; - $result = $db->sql_query($sql); - - while ($row = $db->sql_fetchrow($result)) - { - $site_ip = trim($row['site_ip']); - $site_hostname = trim($row['site_hostname']); - - if ($site_ip) - { - foreach ($iplist as $ip) - { - if (preg_match('#^' . str_replace('\*', '.*?', preg_quote($site_ip, '#')) . '$#i', $ip)) - { - if ($row['ip_exclude']) - { - $allowed = ($config['secure_allow_deny']) ? false : true; - break 2; - } - else - { - $allowed = ($config['secure_allow_deny']) ? true : false; - } - } - } - } - - if ($site_hostname) - { - if (preg_match('#^' . str_replace('\*', '.*?', preg_quote($site_hostname, '#')) . '$#i', $hostname)) - { - if ($row['ip_exclude']) - { - $allowed = ($config['secure_allow_deny']) ? false : true; - break; - } - else - { - $allowed = ($config['secure_allow_deny']) ? true : false; - } - } - } - } - $db->sql_freeresult($result); - } - - return $allowed; -} - -/** -* Check if the browser has the file already and set the appropriate headers- -* @returns false if a resend is in order. -*/ -function set_modified_headers($stamp, $browser) -{ - global $request; - - // let's see if we have to send the file at all - $last_load = $request->header('If-Modified-Since') ? strtotime(trim($request->header('If-Modified-Since'))) : false; - - if ($last_load !== false && $last_load >= $stamp) - { - send_status_line(304, 'Not Modified'); - // seems that we need those too ... browsers - header('Cache-Control: private'); - header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT'); - return true; - } - else - { - header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $stamp) . ' GMT'); - } - return false; -} - /** * Garbage Collection * * @param bool $exit Whether to die or not. * * @return null +* +* @deprecated: 3.3.0-dev (To be removed: 4.0.0) */ function file_gc($exit = true) { @@ -354,287 +89,6 @@ function file_gc($exit = true) } } -/** -* HTTP range support (RFC 2616 Section 14.35) -* -* Allows browsers to request partial file content -* in case a download has been interrupted. -* -* @param int $filesize the size of the file in bytes we are about to deliver -* -* @return mixed false if the whole file has to be delivered -* associative array on success -*/ -function phpbb_http_byte_range($filesize) -{ - // Only call find_range_request() once. - static $request_array; - - if (!$filesize) - { - return false; - } - - if (!isset($request_array)) - { - $request_array = phpbb_find_range_request(); - } - - return (empty($request_array)) ? false : phpbb_parse_range_request($request_array, $filesize); -} - -/** -* Searches for HTTP range request in request headers. -* -* @return mixed false if no request found -* array of strings containing the requested ranges otherwise -* e.g. array(0 => '0-0', 1 => '123-125') -*/ -function phpbb_find_range_request() -{ - global $request; - - $value = $request->header('Range'); - - // Make sure range request starts with "bytes=" - if (strpos($value, 'bytes=') === 0) - { - // Strip leading 'bytes=' - // Multiple ranges can be separated by a comma - return explode(',', substr($value, 6)); - } - - return false; -} - -/** -* Analyses a range request array. -* -* A range request can contain multiple ranges, -* we however only handle the first request and -* only support requests from a given byte to the end of the file. -* -* @param array $request_array array of strings containing the requested ranges -* @param int $filesize the full size of the file in bytes that has been requested -* -* @return mixed false if the whole file has to be delivered -* associative array on success -* byte_pos_start the first byte position, can be passed to fseek() -* byte_pos_end the last byte position -* bytes_requested the number of bytes requested -* bytes_total the full size of the file -*/ -function phpbb_parse_range_request($request_array, $filesize) -{ - $first_byte_pos = -1; - $last_byte_pos = -1; - - // Go through all ranges - foreach ($request_array as $range_string) - { - $range = explode('-', trim($range_string)); - - // "-" is invalid, "0-0" however is valid and means the very first byte. - if (count($range) != 2 || $range[0] === '' && $range[1] === '') - { - continue; - } - - // Substitute defaults - if ($range[0] === '') - { - $range[0] = 0; - } - - if ($range[1] === '') - { - $range[1] = $filesize - 1; - } - - if ($last_byte_pos >= 0 && $last_byte_pos + 1 != $range[0]) - { - // We only support contiguous ranges, no multipart stuff :( - return false; - } - - if ($range[1] && $range[1] < $range[0]) - { - // The requested range contains 0 bytes. - continue; - } - - // Return bytes from $range[0] to $range[1] - if ($first_byte_pos < 0) - { - $first_byte_pos = (int) $range[0]; - } - - $last_byte_pos = (int) $range[1]; - - if ($first_byte_pos >= $filesize) - { - // Requested range not satisfiable - return false; - } - - // Adjust last-byte-pos if it is absent or greater than the content. - if ($range[1] === '' || $last_byte_pos >= $filesize) - { - $last_byte_pos = $filesize - 1; - } - } - - if ($first_byte_pos < 0 || $last_byte_pos < 0) - { - return false; - } - - return array( - 'byte_pos_start' => $first_byte_pos, - 'byte_pos_end' => $last_byte_pos, - 'bytes_requested' => $last_byte_pos - $first_byte_pos + 1, - 'bytes_total' => $filesize, - ); -} - -/** -* Increments the download count of all provided attachments -* -* @param \phpbb\db\driver\driver_interface $db The database object -* @param array|int $ids The attach_id of each attachment -* -* @return null -*/ -function phpbb_increment_downloads($db, $ids) -{ - if (!is_array($ids)) - { - $ids = array($ids); - } - - $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' - SET download_count = download_count + 1 - WHERE ' . $db->sql_in_set('attach_id', $ids); - $db->sql_query($sql); -} - -/** -* Handles authentication when downloading attachments from a post or topic -* -* @param \phpbb\db\driver\driver_interface $db The database object -* @param \phpbb\auth\auth $auth The authentication object -* @param int $topic_id The id of the topic that we are downloading from -* -* @return null -*/ -function phpbb_download_handle_forum_auth($db, $auth, $topic_id) -{ - global $phpbb_container; - - $sql_array = array( - 'SELECT' => 't.topic_visibility, t.forum_id, f.forum_name, f.forum_password, f.parent_id', - 'FROM' => array( - TOPICS_TABLE => 't', - FORUMS_TABLE => 'f', - ), - 'WHERE' => 't.topic_id = ' . (int) $topic_id . ' - AND t.forum_id = f.forum_id', - ); - - $sql = $db->sql_build_query('SELECT', $sql_array); - $result = $db->sql_query($sql); - $row = $db->sql_fetchrow($result); - $db->sql_freeresult($result); - - $phpbb_content_visibility = $phpbb_container->get('content.visibility'); - - if ($row && !$phpbb_content_visibility->is_visible('topic', $row['forum_id'], $row)) - { - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); - } - else if ($row && $auth->acl_get('u_download') && $auth->acl_get('f_download', $row['forum_id'])) - { - if ($row['forum_password']) - { - // Do something else ... ? - login_forum_box($row); - } - } - else - { - send_status_line(403, 'Forbidden'); - trigger_error('SORRY_AUTH_VIEW_ATTACH'); - } -} - -/** -* Handles authentication when downloading attachments from PMs -* -* @param \phpbb\db\driver\driver_interface $db The database object -* @param \phpbb\auth\auth $auth The authentication object -* @param int $user_id The user id -* @param int $msg_id The id of the PM that we are downloading from -* -* @return null -*/ -function phpbb_download_handle_pm_auth($db, $auth, $user_id, $msg_id) -{ - global $phpbb_dispatcher; - - if (!$auth->acl_get('u_pm_download')) - { - send_status_line(403, 'Forbidden'); - trigger_error('SORRY_AUTH_VIEW_ATTACH'); - } - - $allowed = phpbb_download_check_pm_auth($db, $user_id, $msg_id); - - /** - * Event to modify PM attachments download auth - * - * @event core.modify_pm_attach_download_auth - * @var bool allowed Whether the user is allowed to download from that PM or not - * @var int msg_id The id of the PM to download from - * @var int user_id The user id for auth check - * @since 3.1.11-RC1 - */ - $vars = array('allowed', 'msg_id', 'user_id'); - extract($phpbb_dispatcher->trigger_event('core.modify_pm_attach_download_auth', compact($vars))); - - if (!$allowed) - { - send_status_line(403, 'Forbidden'); - trigger_error('ERROR_NO_ATTACHMENT'); - } -} - -/** -* Checks whether a user can download from a particular PM -* -* @param \phpbb\db\driver\driver_interface $db The database object -* @param int $user_id The user id -* @param int $msg_id The id of the PM that we are downloading from -* -* @return bool Whether the user is allowed to download from that PM or not -*/ -function phpbb_download_check_pm_auth($db, $user_id, $msg_id) -{ - // Check if the attachment is within the users scope... - $sql = 'SELECT msg_id - FROM ' . PRIVMSGS_TO_TABLE . ' - WHERE msg_id = ' . (int) $msg_id . ' - AND ( - user_id = ' . (int) $user_id . ' - OR author_id = ' . (int) $user_id . ' - )'; - $result = $db->sql_query_limit($sql, 1); - $allowed = (bool) $db->sql_fetchfield('msg_id'); - $db->sql_freeresult($result); - - return $allowed; -} - /** * Check if the browser is internet explorer version 7+ * diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 7e93369754..24f638289b 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -16,6 +16,7 @@ namespace phpbb\storage\controller; use phpbb\auth\auth; use phpbb\cache\service; use phpbb\config\config; +use phpbb\content_visibility; use phpbb\db\driver\driver_interface; use phpbb\event\dispatcher; use phpbb\request\request; @@ -27,17 +28,17 @@ class attachment extends controller /** @var auth */ protected $auth; - /** @var service */ - protected $cache; - /** @var config */ protected $config; + /** @var content_visibility */ + protected $content_visibility; + /** @var driver_interface */ protected $db; /** @var dispatcher */ - protected $phpbb_dispatcher; + protected $dispatcher; /** @var request */ protected $request; @@ -48,13 +49,14 @@ class attachment extends controller /** @var user */ protected $user; - public function __construct(auth $auth, service $cache, config $config, driver_interface $db, dispatcher $phpbb_dispatcher, request $request, storage $storage, user $user) + public function __construct(auth $auth, service $cache, config $config, $content_visibility, driver_interface $db, dispatcher $dispatcher, request $request, storage $storage, user $user) { $this->auth = $auth; $this->cache = $cache; $this->config = $config; + $this->content_visibility = $content_visibility; $this->db = $db; - $this->phpbb_dispatcher = $phpbb_dispatcher; + $this->dispatcher = $dispatcher; $this->request = $request; $this->storage = $storage; $this->user = $user; @@ -62,20 +64,16 @@ class attachment extends controller public function handle($file) { - global $phpbb_root_path, $phpEx, $phpbb_container; - require($phpbb_root_path . 'includes/functions_download' . '.' . $phpEx); - $attach_id = $file; $mode = $this->request->variable('mode', ''); $thumbnail = $this->request->variable('t', false); + global $phpbb_container; // Start session management, do not update session page. $this->user->session_begin(false); $this->auth->acl($this->user->data); $this->user->setup('viewtopic'); - $phpbb_content_visibility = $phpbb_container->get('content.visibility'); - if (!$this->config['allow_attachments'] && !$this->config['allow_pm_attach']) { send_status_line(404, 'Not Found'); @@ -100,10 +98,10 @@ class attachment extends controller send_status_line(404, 'Not Found'); trigger_error('ERROR_NO_ATTACHMENT'); } - else if (!download_allowed()) + else if (!$this->download_allowed()) { send_status_line(403, 'Forbidden'); - trigger_error($user->lang['LINKAGE_FORBIDDEN']); + trigger_error($this->user->lang['LINKAGE_FORBIDDEN']); } else { @@ -133,7 +131,7 @@ class attachment extends controller { if (!$attachment['in_message']) { - phpbb_download_handle_forum_auth($this->db, $this->auth, $attachment['topic_id']); + $this->phpbb_download_handle_forum_auth($attachment['topic_id']); $sql = 'SELECT forum_id, post_visibility FROM ' . POSTS_TABLE . ' @@ -142,7 +140,7 @@ class attachment extends controller $post_row = $this->db->sql_fetchrow($result); $this->db->sql_freeresult($result); - if (!$post_row || !$phpbb_content_visibility->is_visible('post', $post_row['forum_id'], $post_row)) + if (!$post_row || !$this->content_visibility->is_visible('post', $post_row['forum_id'], $post_row)) { // Attachment of a soft deleted post and the user is not allowed to see the post send_status_line(404, 'Not Found'); @@ -153,7 +151,7 @@ class attachment extends controller { // Attachment is in a private message. $post_row = array('forum_id' => false); - phpbb_download_handle_pm_auth($this->db, $this->auth, $this->user->data['user_id'], $attachment['post_msg_id']); + $this->phpbb_download_handle_pm_auth( $attachment['post_msg_id']); } $extensions = array(); @@ -180,10 +178,10 @@ class attachment extends controller { $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; } - else if ($display_cat == ATTACHMENT_CATEGORY_NONE && !$attachment['is_orphan'] && !phpbb_http_byte_range($attachment['filesize'])) + else if ($display_cat == ATTACHMENT_CATEGORY_NONE && !$attachment['is_orphan']) { // Update download count - phpbb_increment_downloads($this->db, $attachment['attach_id']); + $this->phpbb_increment_downloads($attachment['attach_id']); } $redirect = ''; @@ -212,7 +210,7 @@ class attachment extends controller 'thumbnail', 'redirect', ); - extract($this->phpbb_dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); + extract($this->dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); if (!empty($redirect)) { @@ -220,10 +218,393 @@ class attachment extends controller } else { - send_file_to_browser($attachment, $display_cat); + $this->send_file_to_browser($attachment, $display_cat); } - file_gc(); + $this->file_gc(); } } + + /** + * Send file to browser + */ + protected function send_file_to_browser($attachment, $category) + { + $filename = $attachment['physical_filename']; + + if (!$this->storage->exists($filename)) + { + send_status_line(404, 'Not Found'); + trigger_error('ERROR_NO_ATTACHMENT'); + } + + // Correct the mime type - we force application/octetstream for all files, except images + // Please do not change this, it is a security precaution + if ($category != ATTACHMENT_CATEGORY_IMAGE || strpos($attachment['mimetype'], 'image') !== 0) + { + $attachment['mimetype'] = (strpos(strtolower($this->user->browser), 'msie') !== false || strpos(strtolower($this->user->browser), 'opera') !== false) ? 'application/octetstream' : 'application/octet-stream'; + } + + if (@ob_get_length()) + { + @ob_end_clean(); + } + + // Now send the File Contents to the Browser + try + { + $file_info = $this->storage->file_info($filename); + $size = $file_info->size; + } + catch (\Exception $e) + { + $size = 0; + } + + /** + * Event to alter attachment before it is sent to browser. + * + * @event core.send_file_to_browser_before + * @var array attachment Attachment data + * @var int category Attachment category + * @var string filename Path to file, including filename + * @var int size File size + * @since 3.1.11-RC1 + */ + $vars = array( + 'attachment', + 'category', + 'filename', + 'size', + ); + extract($this->dispatcher->trigger_event('core.send_file_to_browser_before', compact($vars))); + + // To correctly display further errors we need to make sure we are using the correct headers for both (unsetting content-length may not work) + + // Check if headers already sent or not able to get the file contents. + if (headers_sent()) + { + send_status_line(500, 'Internal Server Error'); + trigger_error('UNABLE_TO_DELIVER_FILE'); + } + + // Make sure the database record for the filesize is correct + if ($size > 0 && $size != $attachment['filesize'] && strpos($attachment['physical_filename'], 'thumb_') === false) + { + // Update database record + $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' + SET filesize = ' . (int) $size . ' + WHERE attach_id = ' . (int) $attachment['attach_id']; + $this->db->sql_query($sql); + } + + // Now the tricky part... let's dance + header('Cache-Control: public'); + + // Send out the Headers. Do not set Content-Disposition to inline please, it is a security measure for users using the Internet Explorer. + header('Content-Type: ' . $attachment['mimetype']); + + header('X-Content-Type-Options: nosniff'); + + if ($category == ATTACHMENT_CATEGORY_FLASH && $this->request->variable('view', 0) === 1) + { + // We use content-disposition: inline for flash files and view=1 to let it correctly play with flash player 10 - any other disposition will fail to play inline + header('Content-Disposition: inline'); + } + else + { + header('Content-Disposition: ' . ((strpos($attachment['mimetype'], 'image') === 0) ? 'inline' : 'attachment') . '; ' . header_filename(htmlspecialchars_decode($attachment['real_filename']))); + + if (strpos($attachment['mimetype'], 'image') !== 0) + { + header('X-Download-Options: noopen'); + } + } + + if (!$this->set_modified_headers($attachment['filetime'], $this->user->browser)) + { + if ($size) + { + header("Content-Length: $size"); + } + + // Try to deliver in chunks + @set_time_limit(0); + + $fp = $this->storage->read_stream($filename); + + // Close the db connection before sending the file etc. + $this->file_gc(false); + + if ($fp !== false) + { + $output = fopen('php://output', 'w+b'); + stream_copy_to_stream($fp, $output); + fclose($fp); + } + + flush(); + } + + exit; + } + + /** + * Handles authentication when downloading attachments from a post or topic + * + * @param int $topic_id The id of the topic that we are downloading from + * + * @return null + */ + protected function phpbb_download_handle_forum_auth($topic_id) + { + $sql_array = array( + 'SELECT' => 't.topic_visibility, t.forum_id, f.forum_name, f.forum_password, f.parent_id', + 'FROM' => array( + TOPICS_TABLE => 't', + FORUMS_TABLE => 'f', + ), + 'WHERE' => 't.topic_id = ' . (int) $topic_id . ' + AND t.forum_id = f.forum_id', + ); + + $sql = $this->db->sql_build_query('SELECT', $sql_array); + $result = $this->db->sql_query($sql); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + if ($row && !$this->content_visibility->is_visible('topic', $row['forum_id'], $row)) + { + send_status_line(404, 'Not Found'); + trigger_error('ERROR_NO_ATTACHMENT'); + } + else if ($row && $this->auth->acl_get('u_download') && $this->auth->acl_get('f_download', $row['forum_id'])) + { + if ($row['forum_password']) + { + // Do something else ... ? + login_forum_box($row); + } + } + else + { + send_status_line(403, 'Forbidden'); + trigger_error('SORRY_AUTH_VIEW_ATTACH'); + } + } + + /** + * Handles authentication when downloading attachments from PMs + * + * @param int $msg_id The id of the PM that we are downloading from + * + * @return null + */ + protected function phpbb_download_handle_pm_auth($msg_id) + { + if (!$this->auth->acl_get('u_pm_download')) + { + send_status_line(403, 'Forbidden'); + trigger_error('SORRY_AUTH_VIEW_ATTACH'); + } + + $allowed = $this->phpbb_download_check_pm_auth($msg_id); + + /** + * Event to modify PM attachments download auth + * + * @event core.modify_pm_attach_download_auth + * @var bool allowed Whether the user is allowed to download from that PM or not + * @var int msg_id The id of the PM to download from + * @var int user_id The user id for auth check + * @since 3.1.11-RC1 + */ + $vars = array('allowed', 'msg_id', 'user_id'); + extract($this->dispatcher->trigger_event('core.modify_pm_attach_download_auth', compact($vars))); + + if (!$allowed) + { + send_status_line(403, 'Forbidden'); + trigger_error('ERROR_NO_ATTACHMENT'); + } + } + + /** + * Checks whether a user can download from a particular PM + * + * @param int $msg_id The id of the PM that we are downloading from + * + * @return bool Whether the user is allowed to download from that PM or not + */ + protected function phpbb_download_check_pm_auth($msg_id) + { + $user_id = $this->user->data['user_id']; + + // Check if the attachment is within the users scope... + $sql = 'SELECT msg_id + FROM ' . PRIVMSGS_TO_TABLE . ' + WHERE msg_id = ' . (int) $msg_id . ' + AND ( + user_id = ' . (int) $user_id . ' + OR author_id = ' . (int) $user_id . ' + )'; + $result = $this->db->sql_query_limit($sql, 1); + $allowed = (bool) $this->db->sql_fetchfield('msg_id'); + $this->db->sql_freeresult($result); + + return $allowed; + } + + /** + * Increments the download count of all provided attachments + * + * @param array|int $ids The attach_id of each attachment + * + * @return null + */ + protected function phpbb_increment_downloads($ids) + { + if (!is_array($ids)) + { + $ids = array($ids); + } + + $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' + SET download_count = download_count + 1 + WHERE ' . $this->db->sql_in_set('attach_id', $ids); + $this->db->sql_query($sql); + } + + /** + * Check if downloading item is allowed + */ + protected function download_allowed() + { + if (!$this->config['secure_downloads']) + { + return true; + } + + $url = htmlspecialchars_decode($this->request->header('Referer')); + + if (!$url) + { + return ($this->config['secure_allow_empty_referer']) ? true : false; + } + + // Split URL into domain and script part + $url = @parse_url($url); + + if ($url === false) + { + return ($this->config['secure_allow_empty_referer']) ? true : false; + } + + $hostname = $url['host']; + unset($url); + + $allowed = ($this->config['secure_allow_deny']) ? false : true; + $iplist = array(); + + if (($ip_ary = @gethostbynamel($hostname)) !== false) + { + foreach ($ip_ary as $ip) + { + if ($ip) + { + $iplist[] = $ip; + } + } + } + + // Check for own server... + $server_name = $this->user->host; + + // Forcing server vars is the only way to specify/override the protocol + if ($this->config['force_server_vars'] || !$server_name) + { + $server_name = $this->config['server_name']; + } + + if (preg_match('#^.*?' . preg_quote($server_name, '#') . '.*?$#i', $hostname)) + { + $allowed = true; + } + + // Get IP's and Hostnames + if (!$allowed) + { + $sql = 'SELECT site_ip, site_hostname, ip_exclude + FROM ' . SITELIST_TABLE; + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $site_ip = trim($row['site_ip']); + $site_hostname = trim($row['site_hostname']); + + if ($site_ip) + { + foreach ($iplist as $ip) + { + if (preg_match('#^' . str_replace('\*', '.*?', preg_quote($site_ip, '#')) . '$#i', $ip)) + { + if ($row['ip_exclude']) + { + $allowed = ($this->config['secure_allow_deny']) ? false : true; + break 2; + } + else + { + $allowed = ($this->config['secure_allow_deny']) ? true : false; + } + } + } + } + + if ($site_hostname) + { + if (preg_match('#^' . str_replace('\*', '.*?', preg_quote($site_hostname, '#')) . '$#i', $hostname)) + { + if ($row['ip_exclude']) + { + $allowed = ($this->config['secure_allow_deny']) ? false : true; + break; + } + else + { + $allowed = ($this->config['secure_allow_deny']) ? true : false; + } + } + } + } + $this->db->sql_freeresult($result); + } + + return $allowed; + } + + /** + * Check if the browser has the file already and set the appropriate headers- + * @returns false if a resend is in order. + */ + protected function set_modified_headers($stamp, $browser) + { + // let's see if we have to send the file at all + $last_load = $this->request->header('If-Modified-Since') ? strtotime(trim($this->request->header('If-Modified-Since'))) : false; + + if ($last_load !== false && $last_load >= $stamp) + { + send_status_line(304, 'Not Modified'); + // seems that we need those too ... browsers + header('Cache-Control: public'); + header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT'); + return true; + } + else + { + header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $stamp) . ' GMT'); + } + return false; + } } diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index 1285f110d9..f1344137e7 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -13,6 +13,7 @@ namespace phpbb\storage\controller; +use phpbb\cache\service; use phpbb\config\config; use phpbb\storage\storage; @@ -23,8 +24,9 @@ class avatar extends controller protected $allowed_extensions = ['png', 'gif', 'jpg', 'jpeg']; - public function __construct(config $config, storage $storage) + public function __construct(service $cache, config $config, storage $storage) { + $this->cache = $cache; $this->config = $config; $this->storage = $storage; } diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index e0da578b41..858173c069 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -13,37 +13,38 @@ namespace phpbb\storage\controller; + +use phpbb\cache\service; use phpbb\storage\storage; class controller { + + /** @var service */ + protected $cache; + /** @var storage */ protected $storage; - public function __construct(storage $storage) + public function __construct(service $cache, storage $storage) { + $this->cache = $cache; $this->storage = $storage; } public function handle($file) { - if (!function_exists('file_gc')) - { - global $phpbb_root_path, $phpEx; - require($phpbb_root_path . 'includes/functions_download' . '.' . $phpEx); - } - if (!$this->is_allowed($file)) { send_status_line(403, 'Forbidden'); - file_gc(); + $this->file_gc(); exit; } if (!$this->file_exists($file)) { send_status_line(404, 'Not Found'); - file_gc(); + $this->file_gc(); exit; } @@ -62,12 +63,6 @@ class controller protected function send($file) { - if (!function_exists('file_gc')) - { - global $phpbb_root_path, $phpEx; - require($phpbb_root_path . 'includes/functions_download' . '.' . $phpEx); - } - if (!headers_sent()) { header('Cache-Control: public'); @@ -95,7 +90,7 @@ class controller $fp = $this->storage->read_stream($file); // Close db connection - file_gc(false); + $this->file_gc(false); $output = fopen('php://output', 'w+b'); @@ -104,8 +99,29 @@ class controller fclose($fp); fclose($output); - // ?? flush(); } } + + /** + * Garbage Collection + * + * @param bool $exit Whether to die or not. + * + * @return null + */ + protected function file_gc($exit = true) + { + if (!empty($this->cache)) + { + $this->cache->unload(); + } + + $this->db->sql_close(); + + if ($exit) + { + exit; + } + } } From 0b136a623174531830da9687444ed2d7c760ff7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 30 Jun 2018 04:04:19 +0200 Subject: [PATCH 04/42] [ticket/14285] Remove support for old browsers PHPBB3-14285 --- phpBB/includes/functions_download.php | 20 ------------------- phpBB/phpbb/storage/controller/attachment.php | 2 +- phpBB/phpbb/storage/controller/avatar.php | 2 +- 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index 6ec95e966e..37da8e8887 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -43,26 +43,6 @@ function wrap_img_in_html($src, $title) echo ''; } -/** -* Get a browser friendly UTF-8 encoded filename -*/ -function header_filename($file) -{ - global $request; - - $user_agent = $request->header('User-Agent'); - - // There be dragons here. - // Not many follows the RFC... - if (strpos($user_agent, 'MSIE') !== false || strpos($user_agent, 'Konqueror') !== false) - { - return "filename=" . rawurlencode($file); - } - - // follow the RFC for extended filename for the rest - return "filename*=UTF-8''" . rawurlencode($file); -} - /** * Garbage Collection * diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 24f638289b..2535bcf4f6 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -313,7 +313,7 @@ class attachment extends controller } else { - header('Content-Disposition: ' . ((strpos($attachment['mimetype'], 'image') === 0) ? 'inline' : 'attachment') . '; ' . header_filename(htmlspecialchars_decode($attachment['real_filename']))); + header('Content-Disposition: ' . ((strpos($attachment['mimetype'], 'image') === 0) ? 'inline' : 'attachment') . "; filename*=UTF-8''" . rawurlencode(htmlspecialchars_decode($attachment['real_filename']))); if (strpos($attachment['mimetype'], 'image') !== 0) { diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index f1344137e7..6dc177cf06 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -66,7 +66,7 @@ class avatar extends controller { if (!headers_sent()) { - header('Content-Disposition: inline; ' . header_filename($file)); + header("Content-Disposition: inline; filename*=UTF-8''" . rawurlencode($file)); header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600*24*365) . ' GMT'); } From 5707d986827f187d8123128236e4178dcec88486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 30 Jun 2018 04:20:43 +0200 Subject: [PATCH 05/42] [ticket/14285] Fix undefined property PHPBB3-14285 --- phpBB/config/default/container/services_storage.yml | 2 ++ phpBB/phpbb/storage/controller/attachment.php | 3 --- phpBB/phpbb/storage/controller/avatar.php | 4 +++- phpBB/phpbb/storage/controller/controller.php | 8 ++++++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index b3b118f6e2..25b6e8c282 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -87,7 +87,9 @@ services: storage.controller.avatar: class: phpbb\storage\controller\avatar arguments: + - '@cache' - '@config' + - '@dbal.conn' - '@storage.avatar' storage.controller.attachment: diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 2535bcf4f6..45a4cc01c6 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -34,9 +34,6 @@ class attachment extends controller /** @var content_visibility */ protected $content_visibility; - /** @var driver_interface */ - protected $db; - /** @var dispatcher */ protected $dispatcher; diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index 6dc177cf06..add1047a58 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -15,6 +15,7 @@ namespace phpbb\storage\controller; use phpbb\cache\service; use phpbb\config\config; +use phpbb\db\driver\driver_interface; use phpbb\storage\storage; class avatar extends controller @@ -24,10 +25,11 @@ class avatar extends controller protected $allowed_extensions = ['png', 'gif', 'jpg', 'jpeg']; - public function __construct(service $cache, config $config, storage $storage) + public function __construct(service $cache, config $config, driver_interface $db, storage $storage) { $this->cache = $cache; $this->config = $config; + $this->db = $db; $this->storage = $storage; } diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 858173c069..0ca9db91c9 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -13,8 +13,8 @@ namespace phpbb\storage\controller; - use phpbb\cache\service; +use phpbb\db\driver\driver_interface; use phpbb\storage\storage; class controller @@ -23,12 +23,16 @@ class controller /** @var service */ protected $cache; + /** @var driver_interface */ + protected $db; + /** @var storage */ protected $storage; - public function __construct(service $cache, storage $storage) + public function __construct(service $cache, driver_interface $db, storage $storage) { $this->cache = $cache; + $this->db = $db; $this->storage = $storage; } From 7b678fdbfabc938233e6a2b7a4091a4603a7470c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Tue, 3 Jul 2018 04:14:25 +0200 Subject: [PATCH 06/42] [ticket/14285] Update controllers PHPBB3-14285 --- phpBB/config/default/routing/storage.yml | 2 + phpBB/phpbb/storage/controller/attachment.php | 228 ++++++++---------- phpBB/phpbb/storage/controller/controller.php | 18 +- 3 files changed, 113 insertions(+), 135 deletions(-) diff --git a/phpBB/config/default/routing/storage.yml b/phpBB/config/default/routing/storage.yml index a793aa8a6f..03ffb0ad16 100644 --- a/phpBB/config/default/routing/storage.yml +++ b/phpBB/config/default/routing/storage.yml @@ -7,3 +7,5 @@ phpbb_storage_attachment: path: /download/attachment/{file} defaults: _controller: storage.controller.attachment:handle + requirements: + file: \d+ diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 45a4cc01c6..f552f17728 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -19,9 +19,11 @@ use phpbb\config\config; use phpbb\content_visibility; use phpbb\db\driver\driver_interface; use phpbb\event\dispatcher; +use phpbb\exception\http_exception; use phpbb\request\request; use phpbb\storage\storage; use phpbb\user; +use Symfony\Component\HttpFoundation\RedirectResponse; class attachment extends controller { @@ -61,10 +63,9 @@ class attachment extends controller public function handle($file) { - $attach_id = $file; + $attach_id = (int) $file; $mode = $this->request->variable('mode', ''); $thumbnail = $this->request->variable('t', false); - global $phpbb_container; // Start session management, do not update session page. $this->user->session_begin(false); @@ -73,14 +74,12 @@ class attachment extends controller if (!$this->config['allow_attachments'] && !$this->config['allow_pm_attach']) { - send_status_line(404, 'Not Found'); - trigger_error('ATTACHMENT_FUNCTIONALITY_DISABLED'); + throw new http_exception(404, 'ATTACHMENT_FUNCTIONALITY_DISABLED'); } if (!$attach_id) { - send_status_line(404, 'Not Found'); - trigger_error('NO_ATTACHMENT_SELECTED'); + throw new http_exception(404, 'NO_ATTACHMENT_SELECTED'); } $sql = 'SELECT attach_id, post_msg_id, topic_id, in_message, poster_id, is_orphan, physical_filename, real_filename, extension, mimetype, filesize, filetime @@ -92,133 +91,124 @@ class attachment extends controller if (!$attachment) { - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); + throw new http_exception(404, 'ERROR_NO_ATTACHMENT'); } else if (!$this->download_allowed()) { - send_status_line(403, 'Forbidden'); - trigger_error($this->user->lang['LINKAGE_FORBIDDEN']); + throw new http_exception(403, 'LINKAGE_FORBIDDEN'); + } + + $attachment['physical_filename'] = utf8_basename($attachment['physical_filename']); + + if (!$attachment['in_message'] && !$this->config['allow_attachments'] || $attachment['in_message'] && !$this->config['allow_pm_attach']) + { + throw new http_exception(404, 'ATTACHMENT_FUNCTIONALITY_DISABLED'); + } + + if ($attachment['is_orphan']) + { + // We allow admins having attachment permissions to see orphan attachments... + $own_attachment = ($this->auth->acl_get('a_attach') || $attachment['poster_id'] == $this->user->data['user_id']) ? true : false; + + if (!$own_attachment || ($attachment['in_message'] && !$this->auth->acl_get('u_pm_download')) || (!$attachment['in_message'] && !$this->auth->acl_get('u_download'))) + { + throw new http_exception(404, 'ERROR_NO_ATTACHMENT'); + } + + // Obtain all extensions... + $extensions = $this->cache->obtain_attach_extensions(true); } else { - $attachment['physical_filename'] = utf8_basename($attachment['physical_filename']); - - if (!$attachment['in_message'] && !$this->config['allow_attachments'] || $attachment['in_message'] && !$this->config['allow_pm_attach']) + if (!$attachment['in_message']) { - send_status_line(404, 'Not Found'); - trigger_error('ATTACHMENT_FUNCTIONALITY_DISABLED'); - } + $this->phpbb_download_handle_forum_auth($attachment['topic_id']); - if ($attachment['is_orphan']) - { - // We allow admins having attachment permissions to see orphan attachments... - $own_attachment = ($this->auth->acl_get('a_attach') || $attachment['poster_id'] == $this->user->data['user_id']) ? true : false; + $sql = 'SELECT forum_id, post_visibility + FROM ' . POSTS_TABLE . ' + WHERE post_id = ' . (int) $attachment['post_msg_id']; + $result = $this->db->sql_query($sql); + $post_row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); - if (!$own_attachment || ($attachment['in_message'] && !$this->auth->acl_get('u_pm_download')) || (!$attachment['in_message'] && !$this->auth->acl_get('u_download'))) + if (!$post_row || !$this->content_visibility->is_visible('post', $post_row['forum_id'], $post_row)) { - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); + // Attachment of a soft deleted post and the user is not allowed to see the post + throw new http_exception(404, 'ERROR_NO_ATTACHMENT'); } - - // Obtain all extensions... - $extensions = $this->cache->obtain_attach_extensions(true); } else { - if (!$attachment['in_message']) - { - $this->phpbb_download_handle_forum_auth($attachment['topic_id']); - - $sql = 'SELECT forum_id, post_visibility - FROM ' . POSTS_TABLE . ' - WHERE post_id = ' . (int) $attachment['post_msg_id']; - $result = $this->db->sql_query($sql); - $post_row = $this->db->sql_fetchrow($result); - $this->db->sql_freeresult($result); - - if (!$post_row || !$this->content_visibility->is_visible('post', $post_row['forum_id'], $post_row)) - { - // Attachment of a soft deleted post and the user is not allowed to see the post - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); - } - } - else - { - // Attachment is in a private message. - $post_row = array('forum_id' => false); - $this->phpbb_download_handle_pm_auth( $attachment['post_msg_id']); - } - - $extensions = array(); - if (!extension_allowed($post_row['forum_id'], $attachment['extension'], $extensions)) - { - send_status_line(403, 'Forbidden'); - trigger_error(sprintf($this->user->lang['EXTENSION_DISABLED_AFTER_POSTING'], $attachment['extension'])); - } + // Attachment is in a private message. + $post_row = array('forum_id' => false); + $this->phpbb_download_handle_pm_auth( $attachment['post_msg_id']); } - $display_cat = $extensions[$attachment['extension']]['display_cat']; - - if (($display_cat == ATTACHMENT_CATEGORY_IMAGE || $display_cat == ATTACHMENT_CATEGORY_THUMB) && !$this->user->optionget('viewimg')) + $extensions = array(); + if (!extension_allowed($post_row['forum_id'], $attachment['extension'], $extensions)) { - $display_cat = ATTACHMENT_CATEGORY_NONE; + throw new http_exception(403, 'EXTENSION_DISABLED_AFTER_POSTING', [$attachment['extension']]); } + } - if ($display_cat == ATTACHMENT_CATEGORY_FLASH && !$this->user->optionget('viewflash')) - { - $display_cat = ATTACHMENT_CATEGORY_NONE; - } + $display_cat = $extensions[$attachment['extension']]['display_cat']; - if ($thumbnail) - { - $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; - } - else if ($display_cat == ATTACHMENT_CATEGORY_NONE && !$attachment['is_orphan']) - { - // Update download count - $this->phpbb_increment_downloads($attachment['attach_id']); - } + if (($display_cat == ATTACHMENT_CATEGORY_IMAGE || $display_cat == ATTACHMENT_CATEGORY_THUMB) && !$this->user->optionget('viewimg')) + { + $display_cat = ATTACHMENT_CATEGORY_NONE; + } - $redirect = ''; + if ($display_cat == ATTACHMENT_CATEGORY_FLASH && !$this->user->optionget('viewflash')) + { + $display_cat = ATTACHMENT_CATEGORY_NONE; + } - /** - * Event to modify data before sending file to browser - * - * @event core.download_file_send_to_browser_before - * @var int attach_id The attachment ID - * @var array attachment Array with attachment data - * @var int display_cat Attachment category - * @var array extensions Array with file extensions data - * @var string mode Download mode - * @var bool thumbnail Flag indicating if the file is a thumbnail - * @var string redirect Do a redirection instead of reading the file - * @since 3.1.6-RC1 - * @changed 3.1.7-RC1 Fixing wrong name of a variable (replacing "extension" by "extensions") - * @changed 3.3.0-a1 Add redirect variable - */ - $vars = array( - 'attach_id', - 'attachment', - 'display_cat', - 'extensions', - 'mode', - 'thumbnail', - 'redirect', - ); - extract($this->dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); + if ($thumbnail) + { + $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; + } + else if ($display_cat == ATTACHMENT_CATEGORY_NONE && !$attachment['is_orphan']) + { + // Update download count + $this->phpbb_increment_downloads($attachment['attach_id']); + } - if (!empty($redirect)) - { - redirect($redirect, false, true); - } - else - { - $this->send_file_to_browser($attachment, $display_cat); - } + $redirect = ''; - $this->file_gc(); + /** + * Event to modify data before sending file to browser + * + * @event core.download_file_send_to_browser_before + * @var int attach_id The attachment ID + * @var array attachment Array with attachment data + * @var int display_cat Attachment category + * @var array extensions Array with file extensions data + * @var string mode Download mode + * @var bool thumbnail Flag indicating if the file is a thumbnail + * @var string redirect Do a redirection instead of reading the file + * @since 3.1.6-RC1 + * @changed 3.1.7-RC1 Fixing wrong name of a variable (replacing "extension" by "extensions") + * @changed 3.3.0-a1 Add redirect variable + */ + $vars = array( + 'attach_id', + 'attachment', + 'display_cat', + 'extensions', + 'mode', + 'thumbnail', + 'redirect', + ); + extract($this->dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); + + if (!empty($redirect)) + { + $response = new RedirectResponse($redirect); + $response->send(); + } + else + { + $this->send_file_to_browser($attachment, $display_cat); } } @@ -231,8 +221,7 @@ class attachment extends controller if (!$this->storage->exists($filename)) { - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); + throw new http_exception(404, 'ERROR_NO_ATTACHMENT'); } // Correct the mime type - we force application/octetstream for all files, except images @@ -281,8 +270,7 @@ class attachment extends controller // Check if headers already sent or not able to get the file contents. if (headers_sent()) { - send_status_line(500, 'Internal Server Error'); - trigger_error('UNABLE_TO_DELIVER_FILE'); + throw new http_exception(500, 'UNABLE_TO_DELIVER_FILE'); } // Make sure the database record for the filesize is correct @@ -331,7 +319,7 @@ class attachment extends controller $fp = $this->storage->read_stream($filename); // Close the db connection before sending the file etc. - $this->file_gc(false); + $this->file_gc(); if ($fp !== false) { @@ -372,8 +360,7 @@ class attachment extends controller if ($row && !$this->content_visibility->is_visible('topic', $row['forum_id'], $row)) { - send_status_line(404, 'Not Found'); - trigger_error('ERROR_NO_ATTACHMENT'); + throw new http_exception(404, 'ERROR_NO_ATTACHMENT'); } else if ($row && $this->auth->acl_get('u_download') && $this->auth->acl_get('f_download', $row['forum_id'])) { @@ -385,8 +372,7 @@ class attachment extends controller } else { - send_status_line(403, 'Forbidden'); - trigger_error('SORRY_AUTH_VIEW_ATTACH'); + throw new http_exception(403, 'SORRY_AUTH_VIEW_ATTACH'); } } @@ -401,8 +387,7 @@ class attachment extends controller { if (!$this->auth->acl_get('u_pm_download')) { - send_status_line(403, 'Forbidden'); - trigger_error('SORRY_AUTH_VIEW_ATTACH'); + throw new http_exception(403, 'SORRY_AUTH_VIEW_ATTACH'); } $allowed = $this->phpbb_download_check_pm_auth($msg_id); @@ -421,8 +406,7 @@ class attachment extends controller if (!$allowed) { - send_status_line(403, 'Forbidden'); - trigger_error('ERROR_NO_ATTACHMENT'); + throw new http_exception(403, 'ERROR_NO_ATTACHMENT'); } } diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 0ca9db91c9..36a12fc99b 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -15,6 +15,7 @@ namespace phpbb\storage\controller; use phpbb\cache\service; use phpbb\db\driver\driver_interface; +use phpbb\exception\http_exception; use phpbb\storage\storage; class controller @@ -40,16 +41,12 @@ class controller { if (!$this->is_allowed($file)) { - send_status_line(403, 'Forbidden'); - $this->file_gc(); - exit; + throw new http_exception(403, 'Forbidden'); } if (!$this->file_exists($file)) { - send_status_line(404, 'Not Found'); - $this->file_gc(); - exit; + throw new http_exception(404, 'Not Found'); } $this->send($file); @@ -94,7 +91,7 @@ class controller $fp = $this->storage->read_stream($file); // Close db connection - $this->file_gc(false); + $this->file_gc(); $output = fopen('php://output', 'w+b'); @@ -114,7 +111,7 @@ class controller * * @return null */ - protected function file_gc($exit = true) + protected function file_gc() { if (!empty($this->cache)) { @@ -122,10 +119,5 @@ class controller } $this->db->sql_close(); - - if ($exit) - { - exit; - } } } From 05c44e74b3306989cc1e011efb9077630414deb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 4 Jul 2018 23:42:09 +0200 Subject: [PATCH 07/42] [ticket/14285] Use StreamedResponse PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 31 +++++----- phpBB/phpbb/storage/controller/avatar.php | 15 ++++- phpBB/phpbb/storage/controller/controller.php | 61 ++++++++++--------- 3 files changed, 60 insertions(+), 47 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index f552f17728..c543898356 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -24,6 +24,7 @@ use phpbb\request\request; use phpbb\storage\storage; use phpbb\user; use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\HttpFoundation\StreamedResponse; class attachment extends controller { @@ -59,6 +60,7 @@ class attachment extends controller $this->request = $request; $this->storage = $storage; $this->user = $user; + $this->response = new StreamedResponse(); } public function handle($file) @@ -203,13 +205,14 @@ class attachment extends controller if (!empty($redirect)) { - $response = new RedirectResponse($redirect); - $response->send(); + $this->response = new RedirectResponse($redirect); } else { $this->send_file_to_browser($attachment, $display_cat); } + + return $this->response->send(); } /** @@ -231,11 +234,6 @@ class attachment extends controller $attachment['mimetype'] = (strpos(strtolower($this->user->browser), 'msie') !== false || strpos(strtolower($this->user->browser), 'opera') !== false) ? 'application/octetstream' : 'application/octet-stream'; } - if (@ob_get_length()) - { - @ob_end_clean(); - } - // Now send the File Contents to the Browser try { @@ -284,10 +282,10 @@ class attachment extends controller } // Now the tricky part... let's dance - header('Cache-Control: public'); + $this->response->setPublic(); // Send out the Headers. Do not set Content-Disposition to inline please, it is a security measure for users using the Internet Explorer. - header('Content-Type: ' . $attachment['mimetype']); + $this->response->headers->set('Content-Type', $attachment['mimetype']); header('X-Content-Type-Options: nosniff'); @@ -310,7 +308,7 @@ class attachment extends controller { if ($size) { - header("Content-Length: $size"); + $this->response->headers->set('Content-Length', $size); } // Try to deliver in chunks @@ -324,14 +322,15 @@ class attachment extends controller if ($fp !== false) { $output = fopen('php://output', 'w+b'); - stream_copy_to_stream($fp, $output); - fclose($fp); + + $this->response->setCallback(function () use ($fp, $output) { + stream_copy_to_stream($fp, $output); + fclose($fp); + fclose($output); + flush(); + }); } - - flush(); } - - exit; } /** diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index add1047a58..8a817af3f4 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -17,6 +17,8 @@ use phpbb\cache\service; use phpbb\config\config; use phpbb\db\driver\driver_interface; use phpbb\storage\storage; +use Symfony\Component\HttpFoundation\ResponseHeaderBag; +use Symfony\Component\HttpFoundation\StreamedResponse; class avatar extends controller { @@ -31,13 +33,14 @@ class avatar extends controller $this->config = $config; $this->db = $db; $this->storage = $storage; + $this->response = new StreamedResponse(); } public function handle($file) { $file = $this->decode_avatar_filename($file); - parent::handle($file); + return parent::handle($file); } protected function is_allowed($file) @@ -68,9 +71,15 @@ class avatar extends controller { if (!headers_sent()) { - header("Content-Disposition: inline; filename*=UTF-8''" . rawurlencode($file)); + $disposition = $this->response->headers->makeDisposition( + ResponseHeaderBag::DISPOSITION_INLINE, + rawurlencode($file) + ); - header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 3600*24*365) . ' GMT'); + $this->response->headers->set('Content-Disposition', $disposition); + + $time = new \Datetime(); + $this->response->setExpires($time->modify('+1 year')); } parent::send($file); diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 36a12fc99b..c769d7b919 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -17,10 +17,10 @@ use phpbb\cache\service; use phpbb\db\driver\driver_interface; use phpbb\exception\http_exception; use phpbb\storage\storage; +use Symfony\Component\HttpFoundation\StreamedResponse; class controller { - /** @var service */ protected $cache; @@ -30,11 +30,15 @@ class controller /** @var storage */ protected $storage; + /** @var StreamedResponse */ + protected $response; + public function __construct(service $cache, driver_interface $db, storage $storage) { $this->cache = $cache; $this->db = $db; $this->storage = $storage; + $this->response = new StreamedResponse(); } public function handle($file) @@ -50,6 +54,8 @@ class controller } $this->send($file); + + return $this->response->send(); } protected function is_allowed($file) @@ -64,44 +70,43 @@ class controller protected function send($file) { - if (!headers_sent()) + $this->response->setPublic(); + + $file_info = $this->storage->file_info($file); + + try { - header('Cache-Control: public'); + $this->response->headers->set('Content-Type', $file_info->mimetype); + } + catch (\phpbb\storage\exception\exception $e) + { + // Just don't send this header + } - $file_info = $this->storage->file_info($file); + try + { + $this->response->headers->set('Content-Length', $file_info->size); + } + catch (\phpbb\storage\exception\exception $e) + { + // Just don't send this header + } - try - { - header('Content-Type: ' . $file_info->mimetype); - } - catch (\phpbb\storage\exception\exception $e) - { - // Just don't send this header - } + @set_time_limit(0); - try - { - header('Content-Length: ' . $file_info->size); - } - catch (\phpbb\storage\exception\exception $e) - { - // Just don't send this header - } + $fp = $this->storage->read_stream($file); - $fp = $this->storage->read_stream($file); + // Close db connection + $this->file_gc(); - // Close db connection - $this->file_gc(); - - $output = fopen('php://output', 'w+b'); + $output = fopen('php://output', 'w+b'); + $this->response->setCallback(function () use ($fp, $output) { stream_copy_to_stream($fp, $output); - fclose($fp); fclose($output); - flush(); - } + }); } /** From dc1d3683a8fb2151e8c80c3c6d3e9bfae9ada360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 11 Jul 2018 07:32:53 +0200 Subject: [PATCH 08/42] [ticket/14285] Update controllers PHPBB3-14285 --- .../default/container/services_storage.yml | 2 + phpBB/download/file.php | 4 +- phpBB/phpbb/storage/controller/attachment.php | 133 +++--------------- phpBB/phpbb/storage/controller/avatar.php | 30 ++-- phpBB/phpbb/storage/controller/controller.php | 48 +++++-- 5 files changed, 73 insertions(+), 144 deletions(-) diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 25b6e8c282..cd2e46731e 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -91,6 +91,7 @@ services: - '@config' - '@dbal.conn' - '@storage.avatar' + - '@symfony_request' storage.controller.attachment: class: phpbb\storage\controller\attachment @@ -103,4 +104,5 @@ services: - '@dispatcher' - '@request' - '@storage.attachment' + - '@symfony_request' - '@user' diff --git a/phpBB/download/file.php b/phpBB/download/file.php index bca13ba45d..2a4e3edac1 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -33,7 +33,7 @@ if (isset($_GET['avatar'])) $response = new RedirectResponse( $controller_helper->route('phpbb_storage_avatar', array( 'file' => $request->variable('avatar', ''), - )), + ), false), 301 ); $response->send(); @@ -50,7 +50,7 @@ $response = new RedirectResponse( 'file' => $attach_id, 'mode' => $mode, 't' => $thumbnail, - )), + ), false), 301 ); $response->send(); diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index c543898356..3906d9d0cc 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -23,7 +23,9 @@ use phpbb\exception\http_exception; use phpbb\request\request; use phpbb\storage\storage; use phpbb\user; +use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\HttpFoundation\ResponseHeaderBag; use Symfony\Component\HttpFoundation\StreamedResponse; class attachment extends controller @@ -43,24 +45,19 @@ class attachment extends controller /** @var request */ protected $request; - /** @var storage */ - protected $storage; - /** @var user */ protected $user; - public function __construct(auth $auth, service $cache, config $config, $content_visibility, driver_interface $db, dispatcher $dispatcher, request $request, storage $storage, user $user) + public function __construct(auth $auth, service $cache, config $config, $content_visibility, driver_interface $db, dispatcher $dispatcher, request $request, storage $storage, symfony_request $symfony_request, user $user) { + parent::__construct($cache, $db, $storage, $symfony_request); + $this->auth = $auth; - $this->cache = $cache; $this->config = $config; $this->content_visibility = $content_visibility; - $this->db = $db; $this->dispatcher = $dispatcher; $this->request = $request; - $this->storage = $storage; $this->user = $user; - $this->response = new StreamedResponse(); } public function handle($file) @@ -205,14 +202,16 @@ class attachment extends controller if (!empty($redirect)) { - $this->response = new RedirectResponse($redirect); - } - else - { - $this->send_file_to_browser($attachment, $display_cat); + return new RedirectResponse($redirect); } - return $this->response->send(); + $this->send_file_to_browser($attachment, $display_cat); + + $time = new \Datetime(); + $this->response->setExpires($time->modify('+1 year')); + + $file = $attachment['physical_filename']; + return parent::handle($file); } /** @@ -234,17 +233,6 @@ class attachment extends controller $attachment['mimetype'] = (strpos(strtolower($this->user->browser), 'msie') !== false || strpos(strtolower($this->user->browser), 'opera') !== false) ? 'application/octetstream' : 'application/octet-stream'; } - // Now send the File Contents to the Browser - try - { - $file_info = $this->storage->file_info($filename); - $size = $file_info->size; - } - catch (\Exception $e) - { - $size = 0; - } - /** * Event to alter attachment before it is sent to browser. * @@ -252,85 +240,34 @@ class attachment extends controller * @var array attachment Attachment data * @var int category Attachment category * @var string filename Path to file, including filename - * @var int size File size * @since 3.1.11-RC1 */ $vars = array( 'attachment', 'category', 'filename', - 'size', ); extract($this->dispatcher->trigger_event('core.send_file_to_browser_before', compact($vars))); - // To correctly display further errors we need to make sure we are using the correct headers for both (unsetting content-length may not work) - - // Check if headers already sent or not able to get the file contents. - if (headers_sent()) - { - throw new http_exception(500, 'UNABLE_TO_DELIVER_FILE'); - } - - // Make sure the database record for the filesize is correct - if ($size > 0 && $size != $attachment['filesize'] && strpos($attachment['physical_filename'], 'thumb_') === false) - { - // Update database record - $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' - SET filesize = ' . (int) $size . ' - WHERE attach_id = ' . (int) $attachment['attach_id']; - $this->db->sql_query($sql); - } - - // Now the tricky part... let's dance - $this->response->setPublic(); - // Send out the Headers. Do not set Content-Disposition to inline please, it is a security measure for users using the Internet Explorer. $this->response->headers->set('Content-Type', $attachment['mimetype']); - header('X-Content-Type-Options: nosniff'); - - if ($category == ATTACHMENT_CATEGORY_FLASH && $this->request->variable('view', 0) === 1) + if ($this->request->variable('view', 0) === 1 || strpos($attachment['mimetype'], 'image') !== false) { - // We use content-disposition: inline for flash files and view=1 to let it correctly play with flash player 10 - any other disposition will fail to play inline - header('Content-Disposition: inline'); + $disposition = $this->response->headers->makeDisposition( + ResponseHeaderBag::DISPOSITION_INLINE, + rawurlencode($filename) + ); } else { - header('Content-Disposition: ' . ((strpos($attachment['mimetype'], 'image') === 0) ? 'inline' : 'attachment') . "; filename*=UTF-8''" . rawurlencode(htmlspecialchars_decode($attachment['real_filename']))); - - if (strpos($attachment['mimetype'], 'image') !== 0) - { - header('X-Download-Options: noopen'); - } + $disposition = $this->response->headers->makeDisposition( + ResponseHeaderBag::DISPOSITION_ATTACHMENT, + rawurlencode($filename) + ); } - if (!$this->set_modified_headers($attachment['filetime'], $this->user->browser)) - { - if ($size) - { - $this->response->headers->set('Content-Length', $size); - } - - // Try to deliver in chunks - @set_time_limit(0); - - $fp = $this->storage->read_stream($filename); - - // Close the db connection before sending the file etc. - $this->file_gc(); - - if ($fp !== false) - { - $output = fopen('php://output', 'w+b'); - - $this->response->setCallback(function () use ($fp, $output) { - stream_copy_to_stream($fp, $output); - fclose($fp); - fclose($output); - flush(); - }); - } - } + $this->response->headers->set('Content-Disposition', $disposition); } /** @@ -563,28 +500,4 @@ class attachment extends controller return $allowed; } - - /** - * Check if the browser has the file already and set the appropriate headers- - * @returns false if a resend is in order. - */ - protected function set_modified_headers($stamp, $browser) - { - // let's see if we have to send the file at all - $last_load = $this->request->header('If-Modified-Since') ? strtotime(trim($this->request->header('If-Modified-Since'))) : false; - - if ($last_load !== false && $last_load >= $stamp) - { - send_status_line(304, 'Not Modified'); - // seems that we need those too ... browsers - header('Cache-Control: public'); - header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT'); - return true; - } - else - { - header('Last-Modified: ' . gmdate('D, d M Y H:i:s', $stamp) . ' GMT'); - } - return false; - } } diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index 8a817af3f4..28ead1b6e6 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -17,6 +17,7 @@ use phpbb\cache\service; use phpbb\config\config; use phpbb\db\driver\driver_interface; use phpbb\storage\storage; +use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\ResponseHeaderBag; use Symfony\Component\HttpFoundation\StreamedResponse; @@ -27,13 +28,11 @@ class avatar extends controller protected $allowed_extensions = ['png', 'gif', 'jpg', 'jpeg']; - public function __construct(service $cache, config $config, driver_interface $db, storage $storage) + public function __construct(service $cache, config $config, driver_interface $db, storage $storage, symfony_request $symfony_request) { - $this->cache = $cache; + parent::__construct($cache, $db, $storage, $symfony_request); + $this->config = $config; - $this->db = $db; - $this->storage = $storage; - $this->response = new StreamedResponse(); } public function handle($file) @@ -67,21 +66,18 @@ class avatar extends controller return $this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $file . '.' . $ext; } - protected function send($file) + protected function prepare($file) { - if (!headers_sent()) - { - $disposition = $this->response->headers->makeDisposition( - ResponseHeaderBag::DISPOSITION_INLINE, - rawurlencode($file) - ); + $disposition = $this->response->headers->makeDisposition( + ResponseHeaderBag::DISPOSITION_INLINE, + rawurlencode($file) + ); - $this->response->headers->set('Content-Disposition', $disposition); + $this->response->headers->set('Content-Disposition', $disposition); - $time = new \Datetime(); - $this->response->setExpires($time->modify('+1 year')); - } + $time = new \Datetime(); + $this->response->setExpires($time->modify('+1 year')); - parent::send($file); + parent::prepare($file); } } diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index c769d7b919..9cfcf77041 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -17,6 +17,7 @@ use phpbb\cache\service; use phpbb\db\driver\driver_interface; use phpbb\exception\http_exception; use phpbb\storage\storage; +use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\StreamedResponse; class controller @@ -33,11 +34,15 @@ class controller /** @var StreamedResponse */ protected $response; - public function __construct(service $cache, driver_interface $db, storage $storage) + /** @var symfony_request */ + protected $symfony_request; + + public function __construct(service $cache, driver_interface $db, storage $storage, symfony_request $symfony_request) { $this->cache = $cache; $this->db = $db; $this->storage = $storage; + $this->symfony_request = $symfony_request; $this->response = new StreamedResponse(); } @@ -53,7 +58,12 @@ class controller throw new http_exception(404, 'Not Found'); } - $this->send($file); + $this->prepare($file); + + if (headers_sent()) + { + throw new http_exception(500, 'Headers already sent'); + } return $this->response->send(); } @@ -68,28 +78,34 @@ class controller return $this->storage->exists($file); } - protected function send($file) + protected function prepare($file) { $this->response->setPublic(); $file_info = $this->storage->file_info($file); - try + if (!$this->response->headers->has('Content-Type')) { - $this->response->headers->set('Content-Type', $file_info->mimetype); - } - catch (\phpbb\storage\exception\exception $e) - { - // Just don't send this header + try + { + $this->response->headers->set('Content-Type', $file_info->mimetype); + } + catch (\phpbb\storage\exception\exception $e) + { + // Just don't send this header + } } - try + if (!$this->response->headers->has('Content-Length')) { - $this->response->headers->set('Content-Length', $file_info->size); - } - catch (\phpbb\storage\exception\exception $e) - { - // Just don't send this header + try + { + $this->response->headers->set('Content-Length', $file_info->size); + } + catch (\phpbb\storage\exception\exception $e) + { + // Just don't send this header + } } @set_time_limit(0); @@ -107,6 +123,8 @@ class controller fclose($output); flush(); }); + + $this->response->isNotModified($this->symfony_request); } /** From 821964dc7ab6e74d6caeed24ab44c79702db3384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 11 Jul 2018 07:52:09 +0200 Subject: [PATCH 09/42] [ticket/14285] Send content-type is application/octet-stream if uknown PHPBB3-14285 --- phpBB/phpbb/storage/controller/controller.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 9cfcf77041..63080645d3 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -88,12 +88,14 @@ class controller { try { - $this->response->headers->set('Content-Type', $file_info->mimetype); + $content_type = $file_info->mimetype; } catch (\phpbb\storage\exception\exception $e) { - // Just don't send this header + $content_type = 'application/octet-stream'; } + + $this->response->headers->set('Content-Type', $content_type); } if (!$this->response->headers->has('Content-Length')) From 9bb3398c1ed3705cc16caa6044dd1f30040be83c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 11 Jul 2018 08:11:25 +0200 Subject: [PATCH 10/42] [ticket/14285] Remove unused use PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 1 - phpBB/phpbb/storage/controller/avatar.php | 1 - 2 files changed, 2 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 3906d9d0cc..5322255f0a 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -26,7 +26,6 @@ use phpbb\user; use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\ResponseHeaderBag; -use Symfony\Component\HttpFoundation\StreamedResponse; class attachment extends controller { diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index 28ead1b6e6..7980b65266 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -19,7 +19,6 @@ use phpbb\db\driver\driver_interface; use phpbb\storage\storage; use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\ResponseHeaderBag; -use Symfony\Component\HttpFoundation\StreamedResponse; class avatar extends controller { From 0e92744fa4ea44b6d8e54081f6272dfc8cb982f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 11 Jul 2018 08:54:37 +0200 Subject: [PATCH 11/42] [ticket/14285] Remove flash support PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 5322255f0a..0105451f0c 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -156,11 +156,6 @@ class attachment extends controller $display_cat = ATTACHMENT_CATEGORY_NONE; } - if ($display_cat == ATTACHMENT_CATEGORY_FLASH && !$this->user->optionget('viewflash')) - { - $display_cat = ATTACHMENT_CATEGORY_NONE; - } - if ($thumbnail) { $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; From af222e73f36f12299e4a0befa41cf1107debc8dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 11 Jul 2018 08:58:53 +0200 Subject: [PATCH 12/42] [ticket/14285] Remove support for old browsers PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 0105451f0c..e86f233b97 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -186,7 +186,6 @@ class attachment extends controller $vars = array( 'attach_id', 'attachment', - 'display_cat', 'extensions', 'mode', 'thumbnail', @@ -199,7 +198,7 @@ class attachment extends controller return new RedirectResponse($redirect); } - $this->send_file_to_browser($attachment, $display_cat); + $this->send_file_to_browser($attachment); $time = new \Datetime(); $this->response->setExpires($time->modify('+1 year')); @@ -211,7 +210,7 @@ class attachment extends controller /** * Send file to browser */ - protected function send_file_to_browser($attachment, $category) + protected function send_file_to_browser($attachment) { $filename = $attachment['physical_filename']; @@ -220,13 +219,6 @@ class attachment extends controller throw new http_exception(404, 'ERROR_NO_ATTACHMENT'); } - // Correct the mime type - we force application/octetstream for all files, except images - // Please do not change this, it is a security precaution - if ($category != ATTACHMENT_CATEGORY_IMAGE || strpos($attachment['mimetype'], 'image') !== 0) - { - $attachment['mimetype'] = (strpos(strtolower($this->user->browser), 'msie') !== false || strpos(strtolower($this->user->browser), 'opera') !== false) ? 'application/octetstream' : 'application/octet-stream'; - } - /** * Event to alter attachment before it is sent to browser. * @@ -238,7 +230,6 @@ class attachment extends controller */ $vars = array( 'attachment', - 'category', 'filename', ); extract($this->dispatcher->trigger_event('core.send_file_to_browser_before', compact($vars))); From 469f139a2fc4835478ddb32cf329ad8f4d6ca3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 11 Jul 2018 09:16:49 +0200 Subject: [PATCH 13/42] [ticket/14285] Move code to increment download count PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index e86f233b97..8c30cae1fa 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -151,19 +151,17 @@ class attachment extends controller $display_cat = $extensions[$attachment['extension']]['display_cat']; - if (($display_cat == ATTACHMENT_CATEGORY_IMAGE || $display_cat == ATTACHMENT_CATEGORY_THUMB) && !$this->user->optionget('viewimg')) - { - $display_cat = ATTACHMENT_CATEGORY_NONE; - } - if ($thumbnail) { $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; } else if ($display_cat == ATTACHMENT_CATEGORY_NONE && !$attachment['is_orphan']) { - // Update download count - $this->phpbb_increment_downloads($attachment['attach_id']); + if (!(($display_cat == ATTACHMENT_CATEGORY_IMAGE || $display_cat == ATTACHMENT_CATEGORY_THUMB) && !$this->user->optionget('viewimg'))) + { + // Update download count + $this->phpbb_increment_downloads($attachment['attach_id']); + } } $redirect = ''; From d11b83a4c54a0822076d6124e9fc3186fb136bfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 11 Jul 2018 09:18:56 +0200 Subject: [PATCH 14/42] [ticket/14285] Remove useless variable of event PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 8c30cae1fa..cdba6b6d83 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -172,7 +172,6 @@ class attachment extends controller * @event core.download_file_send_to_browser_before * @var int attach_id The attachment ID * @var array attachment Array with attachment data - * @var int display_cat Attachment category * @var array extensions Array with file extensions data * @var string mode Download mode * @var bool thumbnail Flag indicating if the file is a thumbnail @@ -222,7 +221,6 @@ class attachment extends controller * * @event core.send_file_to_browser_before * @var array attachment Attachment data - * @var int category Attachment category * @var string filename Path to file, including filename * @since 3.1.11-RC1 */ From 4abdabe6f4bd032484db2bffcd3dd2fec791eca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Fri, 13 Jul 2018 08:17:33 +0200 Subject: [PATCH 15/42] [ticket/14285] Temporal fix for StreamedResponse PHPBB3-14285 --- phpBB/phpbb/storage/controller/controller.php | 6 ++-- phpBB/phpbb/storage/streamed_response.php | 30 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 phpBB/phpbb/storage/streamed_response.php diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 63080645d3..d36cd6e207 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -17,8 +17,8 @@ use phpbb\cache\service; use phpbb\db\driver\driver_interface; use phpbb\exception\http_exception; use phpbb\storage\storage; +use phpbb\storage\streamed_response; use Symfony\Component\HttpFoundation\Request as symfony_request; -use Symfony\Component\HttpFoundation\StreamedResponse; class controller { @@ -31,7 +31,7 @@ class controller /** @var storage */ protected $storage; - /** @var StreamedResponse */ + /** @var streamed_response */ protected $response; /** @var symfony_request */ @@ -43,7 +43,7 @@ class controller $this->db = $db; $this->storage = $storage; $this->symfony_request = $symfony_request; - $this->response = new StreamedResponse(); + $this->response = new streamed_response(); } public function handle($file) diff --git a/phpBB/phpbb/storage/streamed_response.php b/phpBB/phpbb/storage/streamed_response.php new file mode 100644 index 0000000000..426d86e34d --- /dev/null +++ b/phpBB/phpbb/storage/streamed_response.php @@ -0,0 +1,30 @@ + + * @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 Symfony\Component\HttpFoundation\StreamedResponse; + +// Temporal fix for: https://github.com/symfony/symfony/issues/27924 +class streamed_response extends StreamedResponse +{ + /** + * {@inheritdoc} + */ + public function setNotModified() + { + $this->setCallback(function () {}); + + return parent::setNotModified(); + } +} From 6232401dd73344f8d3e2794cd2bda95254fdc60a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 14 Jul 2018 12:06:01 +0200 Subject: [PATCH 16/42] [ticket/14285] Rename argument from file to id PHPBB3-14285 --- phpBB/config/default/routing/storage.yml | 4 ++-- phpBB/download/file.php | 2 +- phpBB/phpbb/storage/controller/attachment.php | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/phpBB/config/default/routing/storage.yml b/phpBB/config/default/routing/storage.yml index 03ffb0ad16..069180148d 100644 --- a/phpBB/config/default/routing/storage.yml +++ b/phpBB/config/default/routing/storage.yml @@ -4,8 +4,8 @@ phpbb_storage_avatar: _controller: storage.controller.avatar:handle phpbb_storage_attachment: - path: /download/attachment/{file} + path: /download/attachment/{id} defaults: _controller: storage.controller.attachment:handle requirements: - file: \d+ + id: \d+ diff --git a/phpBB/download/file.php b/phpBB/download/file.php index 2a4e3edac1..da4e432f0e 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -47,7 +47,7 @@ $thumbnail = $request->variable('t', false); $response = new RedirectResponse( $controller_helper->route('phpbb_storage_attachment', array( - 'file' => $attach_id, + 'id' => $attach_id, 'mode' => $mode, 't' => $thumbnail, ), false), diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index cdba6b6d83..a6943b9b78 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -59,9 +59,9 @@ class attachment extends controller $this->user = $user; } - public function handle($file) + public function handle($id) { - $attach_id = (int) $file; + $attach_id = (int) $id; $mode = $this->request->variable('mode', ''); $thumbnail = $this->request->variable('t', false); From d0fa8014c78d21e7de67050e623f315c4ebfb3e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sat, 14 Jul 2018 12:29:13 +0200 Subject: [PATCH 17/42] [ticket/14285] Update attachment controller PHPBB3-14285 --- phpBB/download/file.php | 1 - phpBB/phpbb/storage/controller/attachment.php | 45 ++++++++----------- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/phpBB/download/file.php b/phpBB/download/file.php index da4e432f0e..38802193cc 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -48,7 +48,6 @@ $thumbnail = $request->variable('t', false); $response = new RedirectResponse( $controller_helper->route('phpbb_storage_attachment', array( 'id' => $attach_id, - 'mode' => $mode, 't' => $thumbnail, ), false), 301 diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index a6943b9b78..17e6b408c9 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -62,7 +62,6 @@ class attachment extends controller public function handle($id) { $attach_id = (int) $id; - $mode = $this->request->variable('mode', ''); $thumbnail = $this->request->variable('t', false); // Start session management, do not update session page. @@ -173,45 +172,31 @@ class attachment extends controller * @var int attach_id The attachment ID * @var array attachment Array with attachment data * @var array extensions Array with file extensions data - * @var string mode Download mode * @var bool thumbnail Flag indicating if the file is a thumbnail * @var string redirect Do a redirection instead of reading the file * @since 3.1.6-RC1 * @changed 3.1.7-RC1 Fixing wrong name of a variable (replacing "extension" by "extensions") * @changed 3.3.0-a1 Add redirect variable + * @changed 3.3.0-a1 Remove display_cat variable + * @changed 3.3.0-a1 Remove mode variable */ $vars = array( 'attach_id', 'attachment', 'extensions', - 'mode', 'thumbnail', 'redirect', ); extract($this->dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); + // If the redirect variable have been overwritten, do redirect there if (!empty($redirect)) { return new RedirectResponse($redirect); } - $this->send_file_to_browser($attachment); - - $time = new \Datetime(); - $this->response->setExpires($time->modify('+1 year')); - - $file = $attachment['physical_filename']; - return parent::handle($file); - } - - /** - * Send file to browser - */ - protected function send_file_to_browser($attachment) - { - $filename = $attachment['physical_filename']; - - if (!$this->storage->exists($filename)) + // Check if the file exists in the storage table too + if (!$this->storage->exists($attachment['physical_filename'])) { throw new http_exception(404, 'ERROR_NO_ATTACHMENT'); } @@ -221,34 +206,42 @@ class attachment extends controller * * @event core.send_file_to_browser_before * @var array attachment Attachment data - * @var string filename Path to file, including filename * @since 3.1.11-RC1 + * @changed 3.3.0-a1 Removed category variable + * @changed 3.3.0-a1 Removed size variable + * @changed 3.3.0-a1 Removed filename variable */ $vars = array( 'attachment', - 'filename', ); extract($this->dispatcher->trigger_event('core.send_file_to_browser_before', compact($vars))); - // Send out the Headers. Do not set Content-Disposition to inline please, it is a security measure for users using the Internet Explorer. + // Content-type header $this->response->headers->set('Content-Type', $attachment['mimetype']); - if ($this->request->variable('view', 0) === 1 || strpos($attachment['mimetype'], 'image') !== false) + // Display images in browser and force download for other file types + if (strpos($attachment['mimetype'], 'image') !== false) { $disposition = $this->response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_INLINE, - rawurlencode($filename) + rawurlencode($attachment['physical_filename']) ); } else { $disposition = $this->response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_ATTACHMENT, - rawurlencode($filename) + rawurlencode($attachment['physical_filename']) ); } $this->response->headers->set('Content-Disposition', $disposition); + + // Set expires header for browser cache + $time = new \Datetime(); + $this->response->setExpires($time->modify('+1 year')); + + return parent::handle($attachment['physical_filename']); } /** From 6f69803a5366bbb1f19d5eddce2700abc5789148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Tue, 17 Jul 2018 12:18:14 +0200 Subject: [PATCH 18/42] [ticket/14285] Fix return PHPBB3-14285 --- phpBB/phpbb/storage/controller/controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index d36cd6e207..97ecd92730 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -65,7 +65,7 @@ class controller throw new http_exception(500, 'Headers already sent'); } - return $this->response->send(); + return $this->response; } protected function is_allowed($file) From d105d222869cd2e00bc7f7568e21c86a530b603a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Tue, 17 Jul 2018 12:48:27 +0200 Subject: [PATCH 19/42] [ticket/14285] Add comments PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 22 +++++++++- phpBB/phpbb/storage/controller/avatar.php | 29 +++++++++++++ phpBB/phpbb/storage/controller/controller.php | 43 +++++++++++++++++-- 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 17e6b408c9..13bb834380 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -27,6 +27,9 @@ use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\ResponseHeaderBag; +/** + * Controller for /download/attachment/{id} routes + */ class attachment extends controller { /** @var auth */ @@ -47,7 +50,21 @@ class attachment extends controller /** @var user */ protected $user; - public function __construct(auth $auth, service $cache, config $config, $content_visibility, driver_interface $db, dispatcher $dispatcher, request $request, storage $storage, symfony_request $symfony_request, user $user) + /** + * Constructor + * + * @param auth $auth + * @param service $cache + * @param config $config + * @param content_visibility $content_visibility + * @param driver_interface $db + * @param dispatcher_interface $dispatcher + * @param request $request + * @param storage $storage + * @param symfony_request $symfony_request + * @param user $user + */ + public function __construct(auth $auth, service $cache, config $config, content_visibility $content_visibility, driver_interface $db, dispatcher $dispatcher, request $request, storage $storage, symfony_request $symfony_request, user $user) { parent::__construct($cache, $db, $storage, $symfony_request); @@ -59,6 +76,9 @@ class attachment extends controller $this->user = $user; } + /** + * {@inheritdoc} + */ public function handle($id) { $attach_id = (int) $id; diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index 7980b65266..a6ec3339e1 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -20,13 +20,26 @@ use phpbb\storage\storage; use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\ResponseHeaderBag; +/** + * Controller for /download/avatar/{file} routes + */ class avatar extends controller { /** @var config */ protected $config; + /** @var array */ protected $allowed_extensions = ['png', 'gif', 'jpg', 'jpeg']; + /** + * Constructor + * + * @param service $cache + * @param config $config + * @param driver_interface $db + * @param storage $storage + * @param symfony_request $symfony_request + */ public function __construct(service $cache, config $config, driver_interface $db, storage $storage, symfony_request $symfony_request) { parent::__construct($cache, $db, $storage, $symfony_request); @@ -34,6 +47,9 @@ class avatar extends controller $this->config = $config; } + /** + * {@inheritdoc} + */ public function handle($file) { $file = $this->decode_avatar_filename($file); @@ -41,6 +57,9 @@ class avatar extends controller return parent::handle($file); } + /** + * {@inheritdoc} + */ protected function is_allowed($file) { $ext = substr(strrchr($file, '.'), 1); @@ -49,6 +68,13 @@ class avatar extends controller return strpos($file, '.') && in_array($ext, $this->allowed_extensions); } + /** + * Decode avatar filename + * + * @param string $file Filename + * + * @return string Filename in filesystem + */ protected function decode_avatar_filename($file) { $avatar_group = false; @@ -65,6 +91,9 @@ class avatar extends controller return $this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $file . '.' . $ext; } + /** + * {@inheritdoc} + */ protected function prepare($file) { $disposition = $this->response->headers->makeDisposition( diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 97ecd92730..a93c4f9567 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -20,6 +20,9 @@ use phpbb\storage\storage; use phpbb\storage\streamed_response; use Symfony\Component\HttpFoundation\Request as symfony_request; +/** + * Generic controller for storage + */ class controller { /** @var service */ @@ -37,6 +40,14 @@ class controller /** @var symfony_request */ protected $symfony_request; + /** + * Constructor + * + * @param service $cache + * @param driver_interfacd $db + * @param storage $storage + * @param symfony_request $symfony_request + */ public function __construct(service $cache, driver_interface $db, storage $storage, symfony_request $symfony_request) { $this->cache = $cache; @@ -46,6 +57,15 @@ class controller $this->response = new streamed_response(); } + /** + * Handler + * + * @param string $file File path + * + * @throws \phpbb\exception\http_exception when can't access $file + * + * @return \Symfony\Component\HttpFoundation\StreamedResponse a Symfony response object + */ public function handle($file) { if (!$this->is_allowed($file)) @@ -68,16 +88,35 @@ class controller return $this->response; } + /** + * If the user is allowed to download the file + * + * @param string $file File path + * + * @return bool + */ protected function is_allowed($file) { return true; } + /** + * Check if file exists + * + * @param string $file File path + * + * @return bool + */ protected function file_exists($file) { return $this->storage->exists($file); } + /** + * Prepare response + * + * @param string $file File path + */ protected function prepare($file) { $this->response->setPublic(); @@ -132,9 +171,7 @@ class controller /** * Garbage Collection * - * @param bool $exit Whether to die or not. - * - * @return null + * @param bool $exit Whether to die or not */ protected function file_gc() { From 8725d0b23e3570929c47f276e1f71a100e2ab829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Tue, 17 Jul 2018 22:33:03 +0200 Subject: [PATCH 20/42] [ticket/14285] Terminate after send file PHPBB3-14285 --- phpBB/phpbb/storage/controller/controller.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index a93c4f9567..7bf596b194 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -85,7 +85,9 @@ class controller throw new http_exception(500, 'Headers already sent'); } - return $this->response; + $this->response->send(); + + exit; } /** From 034314c0a172979ce1ff266ab9ebed45d433ba9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Mon, 30 Jul 2018 13:45:12 +0200 Subject: [PATCH 21/42] [ticket/14285] Add comment PHPBB3-14285 --- phpBB/phpbb/storage/streamed_response.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpBB/phpbb/storage/streamed_response.php b/phpBB/phpbb/storage/streamed_response.php index 426d86e34d..2f62ced9eb 100644 --- a/phpBB/phpbb/storage/streamed_response.php +++ b/phpBB/phpbb/storage/streamed_response.php @@ -16,6 +16,7 @@ namespace phpbb\storage; use Symfony\Component\HttpFoundation\StreamedResponse; // Temporal fix for: https://github.com/symfony/symfony/issues/27924 +// Fixed 23/7/2018 in symfony v3.4.13 class streamed_response extends StreamedResponse { /** From b2cea59c3e195d877183189ac955cf44c2322b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Thu, 2 Aug 2018 18:08:29 +0200 Subject: [PATCH 22/42] [ticket/14285] Add comment PHPBB3-14285 --- phpBB/phpbb/storage/controller/controller.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 7bf596b194..e3b4cd4d18 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -87,6 +87,8 @@ class controller $this->response->send(); + // Terminate script to avoid the execution of terminate events + // This avoid possible errors with db connection closed exit; } From 5c2a8bbfdf3c32085980e1849ac8a259ec80a6b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Wed, 8 Aug 2018 16:51:50 +0200 Subject: [PATCH 23/42] [ticket/14285] Remove streamed_response PHPBB3-14285 --- phpBB/phpbb/storage/controller/controller.php | 6 ++-- phpBB/phpbb/storage/streamed_response.php | 31 ------------------- 2 files changed, 3 insertions(+), 34 deletions(-) delete mode 100644 phpBB/phpbb/storage/streamed_response.php diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index e3b4cd4d18..4f1c407533 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -17,8 +17,8 @@ use phpbb\cache\service; use phpbb\db\driver\driver_interface; use phpbb\exception\http_exception; use phpbb\storage\storage; -use phpbb\storage\streamed_response; use Symfony\Component\HttpFoundation\Request as symfony_request; +use Symfony\Component\HttpFoundation\StreamedResponse; /** * Generic controller for storage @@ -34,7 +34,7 @@ class controller /** @var storage */ protected $storage; - /** @var streamed_response */ + /** @var StreamedResponse */ protected $response; /** @var symfony_request */ @@ -54,7 +54,7 @@ class controller $this->db = $db; $this->storage = $storage; $this->symfony_request = $symfony_request; - $this->response = new streamed_response(); + $this->response = new StreamedResponse(); } /** diff --git a/phpBB/phpbb/storage/streamed_response.php b/phpBB/phpbb/storage/streamed_response.php deleted file mode 100644 index 2f62ced9eb..0000000000 --- a/phpBB/phpbb/storage/streamed_response.php +++ /dev/null @@ -1,31 +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 Symfony\Component\HttpFoundation\StreamedResponse; - -// Temporal fix for: https://github.com/symfony/symfony/issues/27924 -// Fixed 23/7/2018 in symfony v3.4.13 -class streamed_response extends StreamedResponse -{ - /** - * {@inheritdoc} - */ - public function setNotModified() - { - $this->setCallback(function () {}); - - return parent::setNotModified(); - } -} From 26b7180874cd6e7adfb6604648fae308a40fb38a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Mon, 13 Aug 2018 16:01:40 +0200 Subject: [PATCH 24/42] [ticket/14285] Move exit inside callback so the controller can return a response PHPBB3-14285 --- phpBB/phpbb/storage/controller/controller.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 4f1c407533..f2bc16ad0a 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -85,11 +85,7 @@ class controller throw new http_exception(500, 'Headers already sent'); } - $this->response->send(); - - // Terminate script to avoid the execution of terminate events - // This avoid possible errors with db connection closed - exit; + return $this->response; } /** @@ -167,6 +163,10 @@ class controller fclose($fp); fclose($output); flush(); + + // Terminate script to avoid the execution of terminate events + // This avoid possible errors with db connection closed + exit; }); $this->response->isNotModified($this->symfony_request); From b1b29cd692c4e58c4279f7b65b81d1ad723ddf60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Mon, 20 Aug 2018 17:14:07 +0200 Subject: [PATCH 25/42] [ticket/14285] Fix filenames when downloading from controller PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 118 +++++++++--------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 13bb834380..66c891bfc7 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -186,20 +186,20 @@ class attachment extends controller $redirect = ''; /** - * Event to modify data before sending file to browser - * - * @event core.download_file_send_to_browser_before - * @var int attach_id The attachment ID - * @var array attachment Array with attachment data - * @var array extensions Array with file extensions data - * @var bool thumbnail Flag indicating if the file is a thumbnail - * @var string redirect Do a redirection instead of reading the file - * @since 3.1.6-RC1 - * @changed 3.1.7-RC1 Fixing wrong name of a variable (replacing "extension" by "extensions") - * @changed 3.3.0-a1 Add redirect variable - * @changed 3.3.0-a1 Remove display_cat variable - * @changed 3.3.0-a1 Remove mode variable - */ + * Event to modify data before sending file to browser + * + * @event core.download_file_send_to_browser_before + * @var int attach_id The attachment ID + * @var array attachment Array with attachment data + * @var array extensions Array with file extensions data + * @var bool thumbnail Flag indicating if the file is a thumbnail + * @var string redirect Do a redirection instead of reading the file + * @since 3.1.6-RC1 + * @changed 3.1.7-RC1 Fixing wrong name of a variable (replacing "extension" by "extensions") + * @changed 3.3.0-a1 Add redirect variable + * @changed 3.3.0-a1 Remove display_cat variable + * @changed 3.3.0-a1 Remove mode variable + */ $vars = array( 'attach_id', 'attachment', @@ -222,15 +222,15 @@ class attachment extends controller } /** - * Event to alter attachment before it is sent to browser. - * - * @event core.send_file_to_browser_before - * @var array attachment Attachment data - * @since 3.1.11-RC1 - * @changed 3.3.0-a1 Removed category variable - * @changed 3.3.0-a1 Removed size variable - * @changed 3.3.0-a1 Removed filename variable - */ + * Event to alter attachment before it is sent to browser. + * + * @event core.send_file_to_browser_before + * @var array attachment Attachment data + * @since 3.1.11-RC1 + * @changed 3.3.0-a1 Removed category variable + * @changed 3.3.0-a1 Removed size variable + * @changed 3.3.0-a1 Removed filename variable + */ $vars = array( 'attachment', ); @@ -244,14 +244,14 @@ class attachment extends controller { $disposition = $this->response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_INLINE, - rawurlencode($attachment['physical_filename']) + rawurlencode(htmlspecialchars_decode($attachment['real_filename'])) ); } else { $disposition = $this->response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_ATTACHMENT, - rawurlencode($attachment['physical_filename']) + rawurlencode(htmlspecialchars_decode($attachment['real_filename'])) ); } @@ -265,12 +265,12 @@ class attachment extends controller } /** - * Handles authentication when downloading attachments from a post or topic - * - * @param int $topic_id The id of the topic that we are downloading from - * - * @return null - */ + * Handles authentication when downloading attachments from a post or topic + * + * @param int $topic_id The id of the topic that we are downloading from + * + * @return null + */ protected function phpbb_download_handle_forum_auth($topic_id) { $sql_array = array( @@ -307,12 +307,12 @@ class attachment extends controller } /** - * Handles authentication when downloading attachments from PMs - * - * @param int $msg_id The id of the PM that we are downloading from - * - * @return null - */ + * Handles authentication when downloading attachments from PMs + * + * @param int $msg_id The id of the PM that we are downloading from + * + * @return null + */ protected function phpbb_download_handle_pm_auth($msg_id) { if (!$this->auth->acl_get('u_pm_download')) @@ -323,14 +323,14 @@ class attachment extends controller $allowed = $this->phpbb_download_check_pm_auth($msg_id); /** - * Event to modify PM attachments download auth - * - * @event core.modify_pm_attach_download_auth - * @var bool allowed Whether the user is allowed to download from that PM or not - * @var int msg_id The id of the PM to download from - * @var int user_id The user id for auth check - * @since 3.1.11-RC1 - */ + * Event to modify PM attachments download auth + * + * @event core.modify_pm_attach_download_auth + * @var bool allowed Whether the user is allowed to download from that PM or not + * @var int msg_id The id of the PM to download from + * @var int user_id The user id for auth check + * @since 3.1.11-RC1 + */ $vars = array('allowed', 'msg_id', 'user_id'); extract($this->dispatcher->trigger_event('core.modify_pm_attach_download_auth', compact($vars))); @@ -341,12 +341,12 @@ class attachment extends controller } /** - * Checks whether a user can download from a particular PM - * - * @param int $msg_id The id of the PM that we are downloading from - * - * @return bool Whether the user is allowed to download from that PM or not - */ + * Checks whether a user can download from a particular PM + * + * @param int $msg_id The id of the PM that we are downloading from + * + * @return bool Whether the user is allowed to download from that PM or not + */ protected function phpbb_download_check_pm_auth($msg_id) { $user_id = $this->user->data['user_id']; @@ -367,12 +367,12 @@ class attachment extends controller } /** - * Increments the download count of all provided attachments - * - * @param array|int $ids The attach_id of each attachment - * - * @return null - */ + * Increments the download count of all provided attachments + * + * @param array|int $ids The attach_id of each attachment + * + * @return null + */ protected function phpbb_increment_downloads($ids) { if (!is_array($ids)) @@ -387,8 +387,8 @@ class attachment extends controller } /** - * Check if downloading item is allowed - */ + * Check if downloading item is allowed + */ protected function download_allowed() { if (!$this->config['secure_downloads']) From ff7c44165d8cca8e2ae7e707852e1ea18f4968fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sun, 23 Sep 2018 23:46:59 +0200 Subject: [PATCH 26/42] [ticket/14285] Use request object instead of globals PHPBB3-14285 --- phpBB/download/file.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/download/file.php b/phpBB/download/file.php index 38802193cc..e1eddf5bba 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -28,7 +28,7 @@ $auth->acl($user->data); /** @var \phpbb\controller\helper $controller_helper */ $controller_helper = $phpbb_container->get('controller.helper'); -if (isset($_GET['avatar'])) +if ($request->is_set('avatar')) { $response = new RedirectResponse( $controller_helper->route('phpbb_storage_avatar', array( From 71403a216e60dd676a9f7dc8f10341ffea5c1e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= Date: Sun, 23 Sep 2018 23:47:53 +0200 Subject: [PATCH 27/42] [ticket/14285] Remove session management stuff since is already on app.php PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 66c891bfc7..f20d120bd8 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -84,10 +84,7 @@ class attachment extends controller $attach_id = (int) $id; $thumbnail = $this->request->variable('t', false); - // Start session management, do not update session page. - $this->user->session_begin(false); - $this->auth->acl($this->user->data); - $this->user->setup('viewtopic'); + $this->user->add_lang('viewtopic'); if (!$this->config['allow_attachments'] && !$this->config['allow_pm_attach']) { From 6f731f0a3e872e51198615237732b97b7fce54e3 Mon Sep 17 00:00:00 2001 From: rubencm Date: Wed, 6 Feb 2019 05:19:29 +0000 Subject: [PATCH 28/42] [ticket/14285] Apply suggestions PHPBB3-14285 --- .../default/container/services_storage.yml | 1 + phpBB/phpbb/storage/controller/attachment.php | 24 +++++++++++++------ phpBB/phpbb/storage/controller/avatar.php | 4 ++-- phpBB/phpbb/storage/controller/controller.php | 8 +++---- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index cd2e46731e..c9caab3f84 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -102,6 +102,7 @@ services: - '@content.visibility' - '@dbal.conn' - '@dispatcher' + - '@language' - '@request' - '@storage.attachment' - '@symfony_request' diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index f20d120bd8..cbf798e953 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -18,8 +18,9 @@ use phpbb\cache\service; use phpbb\config\config; use phpbb\content_visibility; use phpbb\db\driver\driver_interface; -use phpbb\event\dispatcher; +use phpbb\event\dispatcher_interface; use phpbb\exception\http_exception; +use phpbb\language\language; use phpbb\request\request; use phpbb\storage\storage; use phpbb\user; @@ -41,9 +42,12 @@ class attachment extends controller /** @var content_visibility */ protected $content_visibility; - /** @var dispatcher */ + /** @var dispatcher_interface */ protected $dispatcher; + /** @var \phpbb\language\language */ + protected $language; + /** @var request */ protected $request; @@ -59,12 +63,13 @@ class attachment extends controller * @param content_visibility $content_visibility * @param driver_interface $db * @param dispatcher_interface $dispatcher + * @param language $language * @param request $request * @param storage $storage * @param symfony_request $symfony_request * @param user $user */ - public function __construct(auth $auth, service $cache, config $config, content_visibility $content_visibility, driver_interface $db, dispatcher $dispatcher, request $request, storage $storage, symfony_request $symfony_request, user $user) + public function __construct(auth $auth, service $cache, config $config, content_visibility $content_visibility, driver_interface $db, dispatcher_interface $dispatcher, language $language, request $request, storage $storage, symfony_request $symfony_request, user $user) { parent::__construct($cache, $db, $storage, $symfony_request); @@ -72,6 +77,7 @@ class attachment extends controller $this->config = $config; $this->content_visibility = $content_visibility; $this->dispatcher = $dispatcher; + $this->language = $language; $this->request = $request; $this->user = $user; } @@ -84,7 +90,7 @@ class attachment extends controller $attach_id = (int) $id; $thumbnail = $this->request->variable('t', false); - $this->user->add_lang('viewtopic'); + $this->language->add_lang('viewtopic'); if (!$this->config['allow_attachments'] && !$this->config['allow_pm_attach']) { @@ -96,7 +102,9 @@ class attachment extends controller throw new http_exception(404, 'NO_ATTACHMENT_SELECTED'); } - $sql = 'SELECT attach_id, post_msg_id, topic_id, in_message, poster_id, is_orphan, physical_filename, real_filename, extension, mimetype, filesize, filetime + $sql = 'SELECT attach_id, post_msg_id, topic_id, in_message, poster_id, + is_orphan, physical_filename, real_filename, extension, mimetype, + filesize, filetime FROM ' . ATTACHMENTS_TABLE . " WHERE attach_id = $attach_id"; $result = $this->db->sql_query($sql); @@ -114,7 +122,8 @@ class attachment extends controller $attachment['physical_filename'] = utf8_basename($attachment['physical_filename']); - if (!$attachment['in_message'] && !$this->config['allow_attachments'] || $attachment['in_message'] && !$this->config['allow_pm_attach']) + if (!$attachment['in_message'] && !$this->config['allow_attachments'] || + $attachment['in_message'] && !$this->config['allow_pm_attach']) { throw new http_exception(404, 'ATTACHMENT_FUNCTIONALITY_DISABLED'); } @@ -124,7 +133,8 @@ class attachment extends controller // We allow admins having attachment permissions to see orphan attachments... $own_attachment = ($this->auth->acl_get('a_attach') || $attachment['poster_id'] == $this->user->data['user_id']) ? true : false; - if (!$own_attachment || ($attachment['in_message'] && !$this->auth->acl_get('u_pm_download')) || (!$attachment['in_message'] && !$this->auth->acl_get('u_download'))) + if (!$own_attachment || ($attachment['in_message'] && !$this->auth->acl_get('u_pm_download')) || + (!$attachment['in_message'] && !$this->auth->acl_get('u_download'))) { throw new http_exception(404, 'ERROR_NO_ATTACHMENT'); } diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index a6ec3339e1..c657773374 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -52,7 +52,7 @@ class avatar extends controller */ public function handle($file) { - $file = $this->decode_avatar_filename($file); + $file = $this->decode_filename($file); return parent::handle($file); } @@ -75,7 +75,7 @@ class avatar extends controller * * @return string Filename in filesystem */ - protected function decode_avatar_filename($file) + protected function decode_filename($file) { $avatar_group = false; diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index f2bc16ad0a..5c199ff2cd 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -44,7 +44,7 @@ class controller * Constructor * * @param service $cache - * @param driver_interfacd $db + * @param driver_interface $db * @param storage $storage * @param symfony_request $symfony_request */ @@ -127,7 +127,7 @@ class controller { try { - $content_type = $file_info->mimetype; + $content_type = $file_info->get('mimetype'); } catch (\phpbb\storage\exception\exception $e) { @@ -141,7 +141,7 @@ class controller { try { - $this->response->headers->set('Content-Length', $file_info->size); + $this->response->headers->set('Content-Length', $file_info->get('size')); } catch (\phpbb\storage\exception\exception $e) { @@ -174,8 +174,6 @@ class controller /** * Garbage Collection - * - * @param bool $exit Whether to die or not */ protected function file_gc() { From b1495bd54aee2a865ec4f39076f01a3d9b4ff337 Mon Sep 17 00:00:00 2001 From: rubencm Date: Tue, 28 May 2019 02:41:08 +0000 Subject: [PATCH 29/42] [ticket/14285] Use private cache-control for atttachments PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index cbf798e953..c2b027d147 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -271,6 +271,16 @@ class attachment extends controller return parent::handle($attachment['physical_filename']); } + /** + * {@inheritdoc} + */ + protected function prepare($file) + { + parent::prepare($file); + + $this->response->setPivate(); + } + /** * Handles authentication when downloading attachments from a post or topic * From 6e345c37a9997e0f6fda8fbf5ac4a6ef26343ae4 Mon Sep 17 00:00:00 2001 From: rubencm Date: Fri, 21 Jun 2019 23:59:34 +0000 Subject: [PATCH 30/42] [ticket/14285] Don't set cache-control by default PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 8 ++++++-- phpBB/phpbb/storage/controller/avatar.php | 2 ++ phpBB/phpbb/storage/controller/controller.php | 2 -- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index c2b027d147..a4bbe49825 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -243,6 +243,9 @@ class attachment extends controller ); extract($this->dispatcher->trigger_event('core.send_file_to_browser_before', compact($vars))); + // TODO: The next lines should go better in prepare, also the mimetype is handled by the storage table + // so probably can be removed + // Content-type header $this->response->headers->set('Content-Type', $attachment['mimetype']); @@ -276,9 +279,9 @@ class attachment extends controller */ protected function prepare($file) { - parent::prepare($file); + $this->response->setPivate(); // But default should be private, but make sure of it - $this->response->setPivate(); + parent::prepare($file); } /** @@ -405,6 +408,7 @@ class attachment extends controller /** * Check if downloading item is allowed + * FIXME (See: https://tracker.phpbb.com/browse/PHPBB3-15264 and http://area51.phpbb.com/phpBB/viewtopic.php?f=81&t=51921) */ protected function download_allowed() { diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index c657773374..c89dca95a9 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -96,6 +96,8 @@ class avatar extends controller */ protected function prepare($file) { + $this->response->setPublic(); + $disposition = $this->response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_INLINE, rawurlencode($file) diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 5c199ff2cd..050d5a542b 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -119,8 +119,6 @@ class controller */ protected function prepare($file) { - $this->response->setPublic(); - $file_info = $this->storage->file_info($file); if (!$this->response->headers->has('Content-Type')) From 278d4d83188fcedcda57ed42c72a81830e9ae1ba Mon Sep 17 00:00:00 2001 From: rubencm Date: Sat, 22 Jun 2019 01:29:10 +0000 Subject: [PATCH 31/42] [ticket/14285] Add filename fallback PHPBB3-14285 --- phpBB/phpbb/storage/controller/attachment.php | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index a4bbe49825..8805e699a8 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -254,14 +254,16 @@ class attachment extends controller { $disposition = $this->response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_INLINE, - rawurlencode(htmlspecialchars_decode($attachment['real_filename'])) + $attachment['real_filename'], + $this->filenameFallback($attachment['real_filename']) ); } else { $disposition = $this->response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_ATTACHMENT, - rawurlencode(htmlspecialchars_decode($attachment['real_filename'])) + $attachment['real_filename'], + $this->filenameFallback($attachment['real_filename']) ); } @@ -274,12 +276,22 @@ class attachment extends controller return parent::handle($attachment['physical_filename']); } + /** + * Remove non valid characters https://github.com/symfony/http-foundation/commit/c7df9082ee7205548a97031683bc6550b5dc9551 + */ + protected function filenameFallback($filename) + { + $filename = preg_replace(['/[^\x20-\x7e]/', '/%/', '/\//', '/\\\/'], '', $filename); + + return (!empty($filename)) ?: 'File'; + } + /** * {@inheritdoc} */ protected function prepare($file) { - $this->response->setPivate(); // But default should be private, but make sure of it + $this->response->setPrivate(); // But default should be private, but make sure of it parent::prepare($file); } From e6c6d01e0a6e7f4ed692f66bcb3be1e46576e2c9 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sat, 22 Jun 2019 01:36:45 +0000 Subject: [PATCH 32/42] [ticket/14284] Update routes PHPBB3-14285 --- tests/functional/download_test.php | 10 +++++----- tests/functional/feed_test.php | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/functional/download_test.php b/tests/functional/download_test.php index 7eed292052..d96092efb2 100644 --- a/tests/functional/download_test.php +++ b/tests/functional/download_test.php @@ -83,7 +83,7 @@ class phpbb_functional_download_test extends phpbb_functional_test_case )); // Download attachment as guest - $crawler = self::request('GET', "download/file.php?id={$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); + $crawler = self::request('GET', "download/attachment/{$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); self::assert_response_status_code(200); $content = self::$client->getResponse()->getContent(); $finfo = new finfo(FILEINFO_MIME_TYPE); @@ -141,7 +141,7 @@ class phpbb_functional_download_test extends phpbb_functional_test_case $this->add_lang('viewtopic'); // No download attachment as guest - $crawler = self::request('GET', "download/file.php?id={$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); + $crawler = self::request('GET', "download/attachment/{$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); self::assert_response_html(404); $this->assertContainsLang('ERROR_NO_ATTACHMENT', $crawler->filter('#message')->text()); @@ -149,7 +149,7 @@ class phpbb_functional_download_test extends phpbb_functional_test_case $this->login(); // Download attachment as admin - $crawler = self::request('GET', "download/file.php?id={$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); + $crawler = self::request('GET', "download/attachment/{$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); self::assert_response_status_code(200); $content = self::$client->getResponse()->getContent(); $finfo = new finfo(FILEINFO_MIME_TYPE); @@ -208,7 +208,7 @@ class phpbb_functional_download_test extends phpbb_functional_test_case $this->add_lang('viewtopic'); // No download attachment as guest - $crawler = self::request('GET', "download/file.php?id={$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); + $crawler = self::request('GET', "download/attachment/{$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); self::assert_response_html(404); $this->assertContainsLang('ERROR_NO_ATTACHMENT', $crawler->filter('#message')->text()); @@ -216,7 +216,7 @@ class phpbb_functional_download_test extends phpbb_functional_test_case $this->login(); // Download attachment as admin - $crawler = self::request('GET', "download/file.php?id={$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); + $crawler = self::request('GET', "download/attachment/{$this->data['attachments'][$this->data['posts']['Re: Download Topic #1-#2']]}", array(), false); self::assert_response_status_code(200); $content = self::$client->getResponse()->getContent(); $finfo = new finfo(FILEINFO_MIME_TYPE); diff --git a/tests/functional/feed_test.php b/tests/functional/feed_test.php index 159703744f..3651134c61 100644 --- a/tests/functional/feed_test.php +++ b/tests/functional/feed_test.php @@ -1417,7 +1417,7 @@ class phpbb_functional_feed_test extends phpbb_functional_test_case $content = $crawler->filterXPath("//entry[{$entry_id}]/content")->text(); foreach ($attachments as $i => $attachment) { - $url = self::$root_url . "download/file.php?id={$attachment['id']}"; + $url = self::$root_url . "download/attachment/{$attachment['id']}"; $string = "Attachment #{$i}"; if ($attachment['displayed']) From 925502a92fef1e53dcf862ec11ac04b609f1f725 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sat, 22 Jun 2019 04:56:02 +0000 Subject: [PATCH 33/42] [ticket/14285] Move deprecated functions to functions_compatibility.php PHPBB3-14285 --- phpBB/includes/functions_compatibility.php | 106 ++++++++++++++++----- phpBB/includes/functions_download.php | 93 ------------------ 2 files changed, 83 insertions(+), 116 deletions(-) delete mode 100644 phpBB/includes/functions_download.php diff --git a/phpBB/includes/functions_compatibility.php b/phpBB/includes/functions_compatibility.php index 2a53b2ec3a..7c12914b3a 100644 --- a/phpBB/includes/functions_compatibility.php +++ b/phpBB/includes/functions_compatibility.php @@ -894,17 +894,14 @@ function parse_cfg_file($filename, $lines = false) { $parsed_items = array(); - if ($lines === false) - { + if ($lines === false) { $lines = file($filename); } - foreach ($lines as $line) - { + foreach ($lines as $line) { $line = trim($line); - if (!$line || $line[0] == '#' || ($delim_pos = strpos($line, '=')) === false) - { + if (!$line || $line[0] == '#' || ($delim_pos = strpos($line, '=')) === false) { continue; } @@ -912,34 +909,97 @@ function parse_cfg_file($filename, $lines = false) $key = htmlspecialchars(strtolower(trim(substr($line, 0, $delim_pos))), ENT_COMPAT); $value = trim(substr($line, $delim_pos + 1)); - if (in_array($value, array('off', 'false', '0'))) - { + if (in_array($value, array('off', 'false', '0'))) { $value = false; - } - else if (in_array($value, array('on', 'true', '1'))) - { + } else if (in_array($value, array('on', 'true', '1'))) { $value = true; - } - else if (!trim($value)) - { + } else if (!trim($value)) { $value = ''; - } - else if (($value[0] == "'" && $value[strlen($value) - 1] == "'") || ($value[0] == '"' && $value[strlen($value) - 1] == '"')) - { - $value = htmlspecialchars(substr($value, 1, strlen($value)-2), ENT_COMPAT); - } - else - { + } else if (($value[0] == "'" && $value[strlen($value) - 1] == "'") || ($value[0] == '"' && $value[strlen($value) - 1] == '"')) { + $value = htmlspecialchars(substr($value, 1, strlen($value) - 2), ENT_COMPAT); + } else { $value = htmlspecialchars($value, ENT_COMPAT); } $parsed_items[$key] = $value; } - if (isset($parsed_items['parent']) && isset($parsed_items['name']) && $parsed_items['parent'] == $parsed_items['name']) - { + if (isset($parsed_items['parent']) && isset($parsed_items['name']) && $parsed_items['parent'] == $parsed_items['name']) { unset($parsed_items['parent']); } return $parsed_items; } + +/** +* Wraps an url into a simple html page. Used to display attachments in IE. +* this is a workaround for now; might be moved to template system later +* direct any complaints to 1 Microsoft Way, Redmond +* +* @deprecated: 3.3.0-dev (To be removed: 4.0.0) +*/ +function wrap_img_in_html($src, $title) +{ + echo ''; + echo ''; + echo ''; + echo ''; + echo ''; + echo '' . $title . ''; + echo ''; + echo ''; + echo '
'; + echo '' . $title . ''; + echo '
'; + echo ''; + echo ''; +} + +/** +* Garbage Collection +* +* @param bool $exit Whether to die or not. +* +* @return null +* +* @deprecated: 3.3.0-dev (To be removed: 4.0.0) +*/ +function file_gc($exit = true) +{ + global $cache, $db; + + if (!empty($cache)) + { + $cache->unload(); + } + + $db->sql_close(); + + if ($exit) + { + exit; + } +} + +/** +* Check if the browser is internet explorer version 7+ +* +* @param string $user_agent User agent HTTP header +* @param int $version IE version to check against +* +* @return bool true if internet explorer version is greater than $version +* +* @deprecated: 3.3.0-dev (To be removed: 4.0.0) +*/ +function phpbb_is_greater_ie_version($user_agent, $version) +{ + if (preg_match('/msie (\d+)/', strtolower($user_agent), $matches)) + { + $ie_version = (int) $matches[1]; + return ($ie_version > $version); + } + else + { + return false; + } +} diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php deleted file mode 100644 index 37da8e8887..0000000000 --- a/phpBB/includes/functions_download.php +++ /dev/null @@ -1,93 +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. -* -*/ - -/** -* @ignore -*/ -if (!defined('IN_PHPBB')) -{ - exit; -} - -/** -* Wraps an url into a simple html page. Used to display attachments in IE. -* this is a workaround for now; might be moved to template system later -* direct any complaints to 1 Microsoft Way, Redmond -* -* @deprecated: 3.3.0-dev (To be removed: 4.0.0) -*/ -function wrap_img_in_html($src, $title) -{ - echo ''; - echo ''; - echo ''; - echo ''; - echo ''; - echo '' . $title . ''; - echo ''; - echo ''; - echo '
'; - echo '' . $title . ''; - echo '
'; - echo ''; - echo ''; -} - -/** -* Garbage Collection -* -* @param bool $exit Whether to die or not. -* -* @return null -* -* @deprecated: 3.3.0-dev (To be removed: 4.0.0) -*/ -function file_gc($exit = true) -{ - global $cache, $db; - - if (!empty($cache)) - { - $cache->unload(); - } - - $db->sql_close(); - - if ($exit) - { - exit; - } -} - -/** -* Check if the browser is internet explorer version 7+ -* -* @param string $user_agent User agent HTTP header -* @param int $version IE version to check against -* -* @return bool true if internet explorer version is greater than $version -* -* @deprecated: 3.3.0-dev (To be removed: 4.0.0) -*/ -function phpbb_is_greater_ie_version($user_agent, $version) -{ - if (preg_match('/msie (\d+)/', strtolower($user_agent), $matches)) - { - $ie_version = (int) $matches[1]; - return ($ie_version > $version); - } - else - { - return false; - } -} From f66c1d9e1ebcaefc5a5cf34ebd4d421e690c0450 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sun, 21 Mar 2021 20:02:45 +0100 Subject: [PATCH 34/42] [ticket/14285] Fix tests PHPBB3-14285 --- phpBB/includes/functions_compatibility.php | 22 ++- phpBB/phpbb/storage/controller/attachment.php | 6 +- phpBB/phpbb/storage/controller/controller.php | 4 +- tests/download/http_byte_range_test.php | 116 --------------- tests/download/http_user_agent_test.php | 134 ------------------ 5 files changed, 21 insertions(+), 261 deletions(-) delete mode 100644 tests/download/http_byte_range_test.php delete mode 100644 tests/download/http_user_agent_test.php diff --git a/phpBB/includes/functions_compatibility.php b/phpBB/includes/functions_compatibility.php index 7c12914b3a..b6b8f775ab 100644 --- a/phpBB/includes/functions_compatibility.php +++ b/phpBB/includes/functions_compatibility.php @@ -901,7 +901,8 @@ function parse_cfg_file($filename, $lines = false) foreach ($lines as $line) { $line = trim($line); - if (!$line || $line[0] == '#' || ($delim_pos = strpos($line, '=')) === false) { + if (!$line || $line[0] == '#' || ($delim_pos = strpos($line, '=')) === false) + { continue; } @@ -909,15 +910,24 @@ function parse_cfg_file($filename, $lines = false) $key = htmlspecialchars(strtolower(trim(substr($line, 0, $delim_pos))), ENT_COMPAT); $value = trim(substr($line, $delim_pos + 1)); - if (in_array($value, array('off', 'false', '0'))) { + if (in_array($value, array('off', 'false', '0'))) + { $value = false; - } else if (in_array($value, array('on', 'true', '1'))) { + } + else if (in_array($value, array('on', 'true', '1'))) + { $value = true; - } else if (!trim($value)) { + } + else if (!trim($value)) + { $value = ''; - } else if (($value[0] == "'" && $value[strlen($value) - 1] == "'") || ($value[0] == '"' && $value[strlen($value) - 1] == '"')) { + } + else if (($value[0] == "'" && $value[strlen($value) - 1] == "'") || ($value[0] == '"' && $value[strlen($value) - 1] == '"')) + { $value = htmlspecialchars(substr($value, 1, strlen($value) - 2), ENT_COMPAT); - } else { + } + else + { $value = htmlspecialchars($value, ENT_COMPAT); } diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 8805e699a8..44550c19ed 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -45,7 +45,7 @@ class attachment extends controller /** @var dispatcher_interface */ protected $dispatcher; - /** @var \phpbb\language\language */ + /** @var language */ protected $language; /** @var request */ @@ -122,8 +122,8 @@ class attachment extends controller $attachment['physical_filename'] = utf8_basename($attachment['physical_filename']); - if (!$attachment['in_message'] && !$this->config['allow_attachments'] || - $attachment['in_message'] && !$this->config['allow_pm_attach']) + if ((!$attachment['in_message'] && !$this->config['allow_attachments']) || + ($attachment['in_message'] && !$this->config['allow_pm_attach'])) { throw new http_exception(404, 'ATTACHMENT_FUNCTIONALITY_DISABLED'); } diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 050d5a542b..22c786793a 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -62,9 +62,9 @@ class controller * * @param string $file File path * - * @throws \phpbb\exception\http_exception when can't access $file + * @throws http_exception when can't access $file * - * @return \Symfony\Component\HttpFoundation\StreamedResponse a Symfony response object + * @return StreamedResponse a Symfony response object */ public function handle($file) { diff --git a/tests/download/http_byte_range_test.php b/tests/download/http_byte_range_test.php deleted file mode 100644 index b138c4da30..0000000000 --- a/tests/download/http_byte_range_test.php +++ /dev/null @@ -1,116 +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. -* -*/ - -require_once __DIR__ . '/../../phpBB/includes/functions_download.php'; - -class phpbb_download_http_byte_range_test extends phpbb_test_case -{ - public function test_find_range_request() - { - // Missing 'bytes=' prefix - $GLOBALS['request'] = new phpbb_mock_request(); - $GLOBALS['request']->set_header('Range', 'bztes='); - $this->assertEquals(false, phpbb_find_range_request()); - unset($GLOBALS['request']); - - $GLOBALS['request'] = new phpbb_mock_request(); - $_ENV['HTTP_RANGE'] = 'bztes='; - $this->assertEquals(false, phpbb_find_range_request()); - unset($_ENV['HTTP_RANGE']); - - $GLOBALS['request'] = new phpbb_mock_request(); - $GLOBALS['request']->set_header('Range', 'bytes=0-0,123-125'); - $this->assertEquals(array('0-0', '123-125'), phpbb_find_range_request()); - unset($GLOBALS['request']); - } - - /** - * @dataProvider parse_range_request_data() - */ - public function test_parse_range_request($request_array, $filesize, $expected) - { - $this->assertEquals($expected, phpbb_parse_range_request($request_array, $filesize)); - } - - public function parse_range_request_data() - { - return array( - // Valid request - array( - array('3-4'), - 10, - array( - 'byte_pos_start' => 3, - 'byte_pos_end' => 4, - 'bytes_requested' => 2, - 'bytes_total' => 10, - ), - ), - - // Get the beginning - array( - array('-5'), - 10, - array( - 'byte_pos_start' => 0, - 'byte_pos_end' => 5, - 'bytes_requested' => 6, - 'bytes_total' => 10, - ), - ), - - // Get the end - array( - array('5-'), - 10, - array( - 'byte_pos_start' => 5, - 'byte_pos_end' => 9, - 'bytes_requested' => 5, - 'bytes_total' => 10, - ), - ), - - // Overlong request - array( - array('3-20'), - 10, - array( - 'byte_pos_start' => 3, - 'byte_pos_end' => 9, - 'bytes_requested' => 7, - 'bytes_total' => 10, - ), - ), - - // Multiple, contiguous range - array( - array('10-20', '21-30'), - 125, - array( - 'byte_pos_start' => 10, - 'byte_pos_end' => 30, - 'bytes_requested' => 21, - 'bytes_total' => 125, - ) - ), - - // We don't do multiple, non-contiguous range - array( - array('0-0', '120-125'), - 125, - false, - ), - ); - } -} diff --git a/tests/download/http_user_agent_test.php b/tests/download/http_user_agent_test.php deleted file mode 100644 index 74a626432f..0000000000 --- a/tests/download/http_user_agent_test.php +++ /dev/null @@ -1,134 +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. -* -*/ - -require_once __DIR__ . '/../../phpBB/includes/functions_download.php'; - -class phpbb_download_http_user_agent_test extends phpbb_test_case -{ - public function user_agents_check_greater_ie_version() - { - return array( - // user agent - // IE version - // expected - array( - 'Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; Trident/5.0)', - 7, - true, - ), - array( - 'Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; WOW64; Trident/6.0)', - 7, - true, - ), - array( - 'Mozilla/5.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; GTB7.4; InfoPath.2; SV1; .NET CLR 3.3.69573; WOW64; en-US)', - 7, - true, - ), - array( - 'Mozilla/5.0 (Windows; U; MSIE 7.0; Windows NT 6.0; en-US)', - 7, - false, - ), - array( - 'Mozilla/4.0 (compatible; MSIE 6.1; Windows XP; .NET CLR 1.1.4322; .NET CLR 2.0.50727)', - 7, - false, - ), - array( - 'Mozilla/4.0 (compatible; MSIE 6.01; Windows NT 6.0)', - 7, - false, - ), - array( - 'Mozilla/5.0 (Windows; U; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 2.0.50727)', - 7, - false, - ), - array( - 'Mozilla/5.0 (Windows NT 6.2; Win64; x64;) Gecko/20100101 Firefox/20.0', - 7, - false, - ), - array( - 'Mozilla/5.0 (Windows NT 6.2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/28.0.1464.0 Safari/537.36', - 7, - false, - ), - array( - 'Googlebot-Image/1.0', - 7, - false, - ), - array( - 'Googlebot/2.1 ( http://www.google.com/bot.html)', - 7, - false, - ), - array( - 'Lynx/2.8.3dev.9 libwww-FM/2.14 SSL-MM/1.4.1 OpenSSL/0.9.6', - 7, - false, - ), - array( - 'Links (0.9x; Linux 2.4.7-10 i686)', - 7, - false, - ), - array( - 'Opera/9.60 (Windows NT 5.1; U; de) Presto/2.1.1', - 7, - false, - ), - array( - 'Mozilla/4.0 (compatible; MSIE 5.0; Windows NT;)', - 7, - false, - ), - array( - 'Mozilla/4.0 (compatible; MSIE 5.0; Windows NT 4.0) Opera 6.01 [en]', - 7, - false, - ), - array( - 'Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; en) Opera 9.24', - 7, - false, - ), - array( - 'Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; Trident/5.0)', - 8, - true, - ), - array( - 'Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; WOW64; Trident/6.0)', - 9, - true, - ), - array( - 'Mozilla/5.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; GTB7.4; InfoPath.2; SV1; .NET CLR 3.3.69573; WOW64; en-US)', - 10, - false, - ), - ); - } - - /** - * @dataProvider user_agents_check_greater_ie_version - */ - public function test_is_greater_ie_version($user_agent, $version, $expected) - { - $this->assertEquals($expected, phpbb_is_greater_ie_version($user_agent, $version)); - } -} From c375f2c9e52ba0f817aae4357d89e9c58e69ca29 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sun, 21 Mar 2021 21:46:56 +0100 Subject: [PATCH 35/42] [ticket/14285] Use route service for download routes PHPBB3-14285 --- .../default/container/services_avatar.yml | 1 + phpBB/includes/acp/acp_attachments.php | 36 ++++++++++----- phpBB/includes/acp/acp_users.php | 11 +++-- phpBB/includes/functions_compatibility.php | 11 +++-- phpBB/includes/functions_content.php | 11 ++--- phpBB/includes/functions_posting.php | 4 +- phpBB/includes/message_parser.php | 5 ++- phpBB/includes/ucp/ucp_attachments.php | 8 +++- phpBB/phpbb/avatar/driver/upload.php | 45 ++++++++++++------- phpBB/phpbb/feed/helper.php | 4 +- tests/avatar/manager_test.php | 4 +- tests/template/extension_test.php | 15 +++++-- 12 files changed, 106 insertions(+), 49 deletions(-) diff --git a/phpBB/config/default/container/services_avatar.yml b/phpBB/config/default/container/services_avatar.yml index 1cbc44115d..23b7034f0a 100644 --- a/phpBB/config/default/container/services_avatar.yml +++ b/phpBB/config/default/container/services_avatar.yml @@ -60,6 +60,7 @@ services: class: phpbb\avatar\driver\upload arguments: - '@config' + - '@controller.helper' - '%core.root_path%' - '%core.php_ext%' - '@storage.avatar' diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index 91396b80fe..ef7cd2f5cc 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -14,6 +14,16 @@ /** * @ignore */ + +use phpbb\attachment\manager; +use phpbb\config\config; +use phpbb\controller\helper; +use phpbb\db\driver\driver_interface; +use phpbb\filesystem\filesystem_interface; +use phpbb\language\language; +use phpbb\template\template; +use phpbb\user; + if (!defined('IN_PHPBB')) { exit; @@ -21,30 +31,33 @@ if (!defined('IN_PHPBB')) class acp_attachments { - /** @var \phpbb\db\driver\driver_interface */ + /** @var driver_interface */ protected $db; - /** @var \phpbb\config\config */ + /** @var config */ protected $config; - /** @var \phpbb\language\language */ + /** @var language */ protected $language; /** @var ContainerBuilder */ protected $phpbb_container; - /** @var \phpbb\template\template */ + /** @var template */ protected $template; - /** @var \phpbb\user */ + /** @var user */ protected $user; - /** @var \phpbb\filesystem\filesystem_interface */ + /** @var filesystem_interface */ protected $filesystem; - /** @var \phpbb\attachment\manager */ + /** @var manager */ protected $attachment_manager; + /** @var helper */ + protected $controller_helper; + public $id; public $u_action; protected $new_config; @@ -63,6 +76,7 @@ class acp_attachments $this->phpbb_container = $phpbb_container; $this->filesystem = $phpbb_filesystem; $this->attachment_manager = $phpbb_container->get('attachment.manager'); + $this->controller_helper = $phpbb_container->get('controller.helper'); $user->add_lang(array('posting', 'viewtopic', 'acp/attachments')); @@ -1082,8 +1096,8 @@ class acp_attachments 'PHYSICAL_FILENAME' => utf8_basename($row['physical_filename']), 'ATTACH_ID' => $row['attach_id'], 'POST_IDS' => (!empty($post_ids[$row['attach_id']])) ? $post_ids[$row['attach_id']] : '', - 'U_FILE' => append_sid($phpbb_root_path . 'download/file.' . $phpEx, 'mode=view&id=' . $row['attach_id'])) - ); + 'U_FILE' => $this->controller_helper->route('phpbb_storage_attachment', ['id' => (int) $row['attach_id']]) + )); } $db->sql_freeresult($result); @@ -1270,8 +1284,8 @@ class acp_attachments 'S_IN_MESSAGE' => (bool) $row['in_message'], 'U_VIEW_TOPIC' => append_sid("{$phpbb_root_path}viewtopic.$phpEx", "t={$row['topic_id']}&p={$row['post_msg_id']}") . "#p{$row['post_msg_id']}", - 'U_FILE' => append_sid($phpbb_root_path . 'download/file.' . $phpEx, 'mode=view&id=' . $row['attach_id'])) - ); + 'U_FILE' => $this->controller_helper->route('phpbb_storage_attachment', ['id' => $row['attach_id']]) + )); } break; diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index 94a9e50a7b..6b73bad2fe 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -14,6 +14,9 @@ /** * @ignore */ + +use phpbb\controller\helper; + if (!defined('IN_PHPBB')) { exit; @@ -36,6 +39,9 @@ class acp_users global $phpbb_dispatcher, $request; global $phpbb_container, $phpbb_log; + /** @var helper $controller_helper */ + $controller_helper = $phpbb_container->get('controller.helper'); + $user->add_lang(array('posting', 'ucp', 'acp/users')); $this->tpl_name = 'acp_users'; @@ -2123,9 +2129,6 @@ class acp_users $decoded_message = generate_text_for_edit($signature, $bbcode_uid, $bbcode_flags); } - /** @var \phpbb\controller\helper $controller_helper */ - $controller_helper = $phpbb_container->get('controller.helper'); - $template->assign_vars(array( 'S_SIGNATURE' => true, @@ -2295,7 +2298,7 @@ class acp_users 'S_IN_MESSAGE' => $row['in_message'], - 'U_DOWNLOAD' => append_sid("{$phpbb_root_path}download/file.$phpEx", 'mode=view&id=' . $row['attach_id']), + 'U_DOWNLOAD' => $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $row['attach_id']]), 'U_VIEW_TOPIC' => $view_topic) ); } diff --git a/phpBB/includes/functions_compatibility.php b/phpBB/includes/functions_compatibility.php index b6b8f775ab..6e1dbee589 100644 --- a/phpBB/includes/functions_compatibility.php +++ b/phpBB/includes/functions_compatibility.php @@ -894,11 +894,13 @@ function parse_cfg_file($filename, $lines = false) { $parsed_items = array(); - if ($lines === false) { + if ($lines === false) + { $lines = file($filename); } - foreach ($lines as $line) { + foreach ($lines as $line) + { $line = trim($line); if (!$line || $line[0] == '#' || ($delim_pos = strpos($line, '=')) === false) @@ -927,14 +929,15 @@ function parse_cfg_file($filename, $lines = false) $value = htmlspecialchars(substr($value, 1, strlen($value) - 2), ENT_COMPAT); } else - { + { $value = htmlspecialchars($value, ENT_COMPAT); } $parsed_items[$key] = $value; } - if (isset($parsed_items['parent']) && isset($parsed_items['name']) && $parsed_items['parent'] == $parsed_items['name']) { + if (isset($parsed_items['parent']) && isset($parsed_items['name']) && $parsed_items['parent'] == $parsed_items['name']) + { unset($parsed_items['parent']); } diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index 3de8334674..521bafef33 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -1124,6 +1124,9 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a $storage_attachment = $phpbb_container->get('storage.attachment'); + /** @var \phpbb\controller\helper */ + $controller_helper = $phpbb_container->get('controller.helper'); + // $compiled_attachments = array(); @@ -1283,15 +1286,14 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a $display_cat = ATTACHMENT_CATEGORY_NONE; } - $download_link = append_sid("{$phpbb_root_path}download/file.$phpEx", 'id=' . $attachment['attach_id']); + $download_link = $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $attachment['attach_id']]); $l_downloaded_viewed = 'VIEWED_COUNTS'; switch ($display_cat) { // Images case ATTACHMENT_CATEGORY_IMAGE: - $inline_link = append_sid("{$phpbb_root_path}download/file.$phpEx", 'id=' . $attachment['attach_id']); - $download_link .= '&mode=view'; + $inline_link = $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $attachment['attach_id']]); $block_array += array( 'S_IMAGE' => true, @@ -1303,8 +1305,7 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a // Images, but display Thumbnail case ATTACHMENT_CATEGORY_THUMB: - $thumbnail_link = append_sid("{$phpbb_root_path}download/file.$phpEx", 'id=' . $attachment['attach_id'] . '&t=1'); - $download_link .= '&mode=view'; + $thumbnail_link = $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $attachment['attach_id'], 't' => 1]); $block_array += array( 'S_THUMBNAIL' => true, diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index fade2a54a7..7c95cc12e8 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -779,7 +779,7 @@ function posting_gen_inline_attachments(&$attachment_data) */ function posting_gen_attachment_entry($attachment_data, &$filename_data, $show_attach_box = true) { - global $template, $config, $phpbb_root_path, $phpEx, $user, $phpbb_dispatcher; + global $template, $config, $phpbb_root_path, $phpEx, $user, $phpbb_dispatcher, $phpbb_container; // Some default template variables $template->assign_vars(array( @@ -807,7 +807,7 @@ function posting_gen_attachment_entry($attachment_data, &$filename_data, $show_a $hidden .= ''; } - $download_link = append_sid("{$phpbb_root_path}download/file.$phpEx", 'mode=view&id=' . (int) $attach_row['attach_id'], true, ($attach_row['is_orphan']) ? $user->session_id : false); + $download_link = $phpbb_container->get('controller.helper')->route('phpbb_storage_attachment', ['id' => (int) $attach_row['attach_id']]); $attachrow_template_vars[(int) $attach_row['attach_id']] = array( 'FILENAME' => utf8_basename($attach_row['real_filename']), diff --git a/phpBB/includes/message_parser.php b/phpBB/includes/message_parser.php index c463179227..ca85962f6b 100644 --- a/phpBB/includes/message_parser.php +++ b/phpBB/includes/message_parser.php @@ -1073,6 +1073,7 @@ class bbcode_firstpass extends bbcode if ($pos_domain !== false && $pos_path >= $pos_domain && $pos_ext >= $pos_path) { // Ok, actually we allow linking to some files (this may be able to be extended in some way later...) + // @deprecated if (strpos($url, '/' . $check_path . '/download/file.' . $phpEx) !== 0) { return false; @@ -1534,6 +1535,8 @@ class parse_message extends bbcode_firstpass global $config, $auth, $user, $phpbb_root_path, $phpEx, $db, $request; global $phpbb_container, $phpbb_dispatcher; + $controller_helper = $phpbb_container->get('controller.helper'); + $error = array(); $num_attachments = count($this->attachment_data); @@ -1776,7 +1779,7 @@ class parse_message extends bbcode_firstpass if (isset($this->plupload) && $this->plupload->is_active()) { - $download_url = append_sid("{$phpbb_root_path}download/file.{$phpEx}", 'mode=view&id=' . $new_entry['attach_id']); + $download_url = $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $new_entry['attach_id']]); // Send the client the attachment data to maintain state $json_response->send(array('data' => $this->attachment_data, 'download_url' => $download_url)); diff --git a/phpBB/includes/ucp/ucp_attachments.php b/phpBB/includes/ucp/ucp_attachments.php index 7808fed325..b48e74128d 100644 --- a/phpBB/includes/ucp/ucp_attachments.php +++ b/phpBB/includes/ucp/ucp_attachments.php @@ -14,6 +14,9 @@ /** * @ignore */ + +use phpbb\controller\helper; + if (!defined('IN_PHPBB')) { exit; @@ -31,6 +34,9 @@ class ucp_attachments { global $template, $user, $db, $config, $phpEx, $phpbb_root_path, $phpbb_container, $request, $auth; + /** @var helper $controller_helper */ + $controller_helper = $phpbb_container->get('controller.helper'); + $start = $request->variable('start', 0); $sort_key = $request->variable('sk', 'a'); $sort_dir = $request->variable('sd', 'a'); @@ -179,7 +185,7 @@ class ucp_attachments 'S_IN_MESSAGE' => $row['in_message'], 'S_LOCKED' => !$row['in_message'] && !$auth->acl_get('m_edit', $row['forum_id']) && ($row['forum_status'] == ITEM_LOCKED || $row['topic_status'] == ITEM_LOCKED || $row['post_edit_locked']), - 'U_VIEW_ATTACHMENT' => append_sid("{$phpbb_root_path}download/file.$phpEx", 'id=' . $row['attach_id']), + 'U_VIEW_ATTACHMENT' => $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $row['attach_id']]), 'U_VIEW_TOPIC' => $view_topic) ); diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index c0f15536ae..0e0fe9e8a5 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -14,7 +14,13 @@ namespace phpbb\avatar\driver; use bantu\IniGetWrapper\IniGetWrapper; +use phpbb\config\config; +use phpbb\controller\helper; +use phpbb\event\dispatcher_interface; +use phpbb\files\factory; +use phpbb\path_helper; use phpbb\storage\exception\exception as storage_exception; +use phpbb\storage\storage; /** * Handles avatars uploaded to the board @@ -22,17 +28,22 @@ use phpbb\storage\exception\exception as storage_exception; class upload extends \phpbb\avatar\driver\driver { /** - * @var \phpbb\storage\storage + * @var helper + */ + private $controller_helper; + + /** + * @var storage */ protected $storage; /** - * @var \phpbb\event\dispatcher_interface + * @var dispatcher_interface */ protected $dispatcher; /** - * @var \phpbb\files\factory + * @var factory */ protected $files_factory; @@ -42,20 +53,22 @@ class upload extends \phpbb\avatar\driver\driver 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\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 IniGetWrapper $php_ini ini_get() wrapper - */ - 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) + * Construct a driver object + * + * @param config $config phpBB configuration + * @param helper $controller_helper + * @param string $phpbb_root_path Path to the phpBB root + * @param string $php_ext PHP file extension + * @param storage $storage phpBB avatar storage + * @param path_helper $path_helper phpBB path helper + * @param dispatcher_interface $dispatcher phpBB Event dispatcher object + * @param factory $files_factory File classes factory + * @param IniGetWrapper $php_ini ini_get() wrapper + */ + public function __construct(config $config, helper $controller_helper, string $phpbb_root_path, string $php_ext, storage $storage, path_helper $path_helper, dispatcher_interface $dispatcher, factory $files_factory, IniGetWrapper $php_ini) { $this->config = $config; + $this->controller_helper = $controller_helper; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; $this->storage = $storage; @@ -73,7 +86,7 @@ class upload extends \phpbb\avatar\driver\driver $root_path = (defined('PHPBB_USE_BOARD_URL_PATH') && PHPBB_USE_BOARD_URL_PATH) ? generate_board_url() . '/' : $this->path_helper->get_web_root_path(); return array( - 'src' => $root_path . 'download/file.' . $this->php_ext . '?avatar=' . $row['avatar'], + 'src' => $root_path . $this->controller_helper->route('phpbb_storage_avatar', ['file' => $row['avatar']]), 'width' => $row['avatar_width'], 'height' => $row['avatar_height'], ); diff --git a/phpBB/phpbb/feed/helper.php b/phpBB/phpbb/feed/helper.php index 6d185271cc..4227e0c15d 100644 --- a/phpBB/phpbb/feed/helper.php +++ b/phpBB/phpbb/feed/helper.php @@ -167,7 +167,9 @@ class helper $content .= implode('
', $post_attachments); // Convert attachments' relative path to absolute path - $content = str_replace($this->path_helper->get_web_root_path() . 'download/file.' . $this->path_helper->get_php_ext(), $this->get_board_url() . '/download/file.' . $this->path_helper->get_php_ext(), $content); + $pattern = '#(/app.php)/?download/attachment/#'; + $replacement = $this->get_board_url() . '\1/download/attachment/'; + $content = preg_replace($pattern, $replacement, $content); } // Remove Comments from inline attachments [ia] diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index a91b02e8a5..f33f29abdf 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -56,6 +56,8 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case $dispatcher = new phpbb_mock_event_dispatcher(); + $controller_helper = $this->createMock('\phpbb\controller\helper'); + // $this->avatar_foobar will be needed later on $this->avatar_foobar = $this->getMockBuilder('\phpbb\avatar\driver\foobar') ->setMethods(array('get_name')) @@ -93,7 +95,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, $storage, $path_helper, $dispatcher, $files_factory, $php_ini)) + ->setConstructorArgs(array($this->config, $controller_helper, $phpbb_root_path, $phpEx, $storage, $path_helper, $dispatcher, $files_factory, $php_ini)) ->getMock(); } $cur_avatar->expects($this->any()) diff --git a/tests/template/extension_test.php b/tests/template/extension_test.php index b53efea0bb..d6e181259f 100644 --- a/tests/template/extension_test.php +++ b/tests/template/extension_test.php @@ -11,6 +11,8 @@ * */ +use phpbb\controller\helper; + require_once __DIR__ . '/template_test_case.php'; class phpbb_template_extension_test extends phpbb_template_template_test_case @@ -66,10 +68,17 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case ->disableOriginalConstructor() ->getMock(); + $controller_helper = $this->createMock(helper::class); + $controller_helper + ->method('route') + ->willReturnCallback(function($route, $params) { + return 'download/avatar/' . $params['file']; + }); + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); $phpbb_container = new phpbb_mock_container_builder(); $files = new phpbb\files\factory($phpbb_container); - $upload_avatar_driver = new phpbb\avatar\driver\upload($config, $phpbb_root_path, $phpEx, $storage, $phpbb_path_helper, $phpbb_dispatcher, $files, new \bantu\IniGetWrapper\IniGetWrapper()); + $upload_avatar_driver = new phpbb\avatar\driver\upload($config, $controller_helper, $phpbb_root_path, $phpEx, $storage, $phpbb_path_helper, $phpbb_dispatcher, $files, new \bantu\IniGetWrapper\IniGetWrapper()); $upload_avatar_driver->set_name('avatar.driver.upload'); $phpbb_container->set('avatar.manager', new \phpbb\avatar\manager($config, $phpbb_dispatcher, [ $upload_avatar_driver, @@ -141,7 +150,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case ], [], [], - 'foo', + 'foo', [] ], [ @@ -159,7 +168,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case ], [], [], - 'foo', + 'foo', [] ], [ From f2914b286937c2810623661c39a3249988745335 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sun, 21 Mar 2021 23:20:51 +0100 Subject: [PATCH 36/42] [ticket/14285] Fix regular expression PHPBB3-14285 --- phpBB/phpbb/feed/helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/feed/helper.php b/phpBB/phpbb/feed/helper.php index 4227e0c15d..8261027eb0 100644 --- a/phpBB/phpbb/feed/helper.php +++ b/phpBB/phpbb/feed/helper.php @@ -167,7 +167,7 @@ class helper $content .= implode('
', $post_attachments); // Convert attachments' relative path to absolute path - $pattern = '#(/app.php)/?download/attachment/#'; + $pattern = '#(/app.php)?/download/attachment/#'; $replacement = $this->get_board_url() . '\1/download/attachment/'; $content = preg_replace($pattern, $replacement, $content); } From 31ca404ff7ba5e8734172b9f23a60cb86f1d91d3 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sun, 21 Mar 2021 23:58:48 +0100 Subject: [PATCH 37/42] [ticket/14285] Fix tests PHPBB3-14285 --- tests/functional/feed_test.php | 2 +- tests/test_framework/phpbb_functional_test_case.php | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/functional/feed_test.php b/tests/functional/feed_test.php index 3651134c61..4c1f05542d 100644 --- a/tests/functional/feed_test.php +++ b/tests/functional/feed_test.php @@ -1417,7 +1417,7 @@ class phpbb_functional_feed_test extends phpbb_functional_test_case $content = $crawler->filterXPath("//entry[{$entry_id}]/content")->text(); foreach ($attachments as $i => $attachment) { - $url = self::$root_url . "download/attachment/{$attachment['id']}"; + $url = self::$root_url . "app.php/download/attachment/{$attachment['id']}"; $string = "Attachment #{$i}"; if ($attachment['displayed']) diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 2dfbfcfd5a..5df9d49be5 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -47,7 +47,6 @@ class phpbb_functional_test_case extends phpbb_test_case parent::setUpBeforeClass(); self::$config = phpbb_test_case_helpers::get_test_config(); - self::$root_url = self::$config['phpbb_functional_url']; // Important: this is used both for installation and by // test cases for querying the tables. @@ -60,6 +59,8 @@ class phpbb_functional_test_case extends phpbb_test_case self::markTestSkipped('phpbb_functional_url was not set in test_config and wasn\'t set as PHPBB_FUNCTIONAL_URL environment variable either.'); } + self::$root_url = self::$config['phpbb_functional_url']; + if (!self::$already_installed) { self::install_board(); From 1e624bef1535e21ac50b81dd6e639039e44e71cd Mon Sep 17 00:00:00 2001 From: rubencm Date: Tue, 25 May 2021 07:28:05 +0200 Subject: [PATCH 38/42] [ticket/14285] Add prefix in routing PHPBB3-14285 --- phpBB/config/default/routing/routing.yml | 1 + phpBB/config/default/routing/storage.yml | 4 +- phpBB/includes/functions_compatibility.php | 49 ------------------- phpBB/phpbb/storage/controller/attachment.php | 21 ++++---- 4 files changed, 15 insertions(+), 60 deletions(-) diff --git a/phpBB/config/default/routing/routing.yml b/phpBB/config/default/routing/routing.yml index daf8ec7002..9c258ba26b 100644 --- a/phpBB/config/default/routing/routing.yml +++ b/phpBB/config/default/routing/routing.yml @@ -33,3 +33,4 @@ phpbb_ucp_routing: phpbb_storage_routing: resource: storage.yml + prefix: /download diff --git a/phpBB/config/default/routing/storage.yml b/phpBB/config/default/routing/storage.yml index 069180148d..e61c58c356 100644 --- a/phpBB/config/default/routing/storage.yml +++ b/phpBB/config/default/routing/storage.yml @@ -1,10 +1,10 @@ phpbb_storage_avatar: - path: /download/avatar/{file} + path: /avatar/{file} defaults: _controller: storage.controller.avatar:handle phpbb_storage_attachment: - path: /download/attachment/{id} + path: /attachment/{id} defaults: _controller: storage.controller.attachment:handle requirements: diff --git a/phpBB/includes/functions_compatibility.php b/phpBB/includes/functions_compatibility.php index 6e1dbee589..888ecca729 100644 --- a/phpBB/includes/functions_compatibility.php +++ b/phpBB/includes/functions_compatibility.php @@ -967,52 +967,3 @@ function wrap_img_in_html($src, $title) echo ''; echo ''; } - -/** -* Garbage Collection -* -* @param bool $exit Whether to die or not. -* -* @return null -* -* @deprecated: 3.3.0-dev (To be removed: 4.0.0) -*/ -function file_gc($exit = true) -{ - global $cache, $db; - - if (!empty($cache)) - { - $cache->unload(); - } - - $db->sql_close(); - - if ($exit) - { - exit; - } -} - -/** -* Check if the browser is internet explorer version 7+ -* -* @param string $user_agent User agent HTTP header -* @param int $version IE version to check against -* -* @return bool true if internet explorer version is greater than $version -* -* @deprecated: 3.3.0-dev (To be removed: 4.0.0) -*/ -function phpbb_is_greater_ie_version($user_agent, $version) -{ - if (preg_match('/msie (\d+)/', strtolower($user_agent), $matches)) - { - $ie_version = (int) $matches[1]; - return ($ie_version > $version); - } - else - { - return false; - } -} diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index 44550c19ed..e2e6f19967 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -131,7 +131,7 @@ class attachment extends controller if ($attachment['is_orphan']) { // We allow admins having attachment permissions to see orphan attachments... - $own_attachment = ($this->auth->acl_get('a_attach') || $attachment['poster_id'] == $this->user->data['user_id']) ? true : false; + $own_attachment = $this->auth->acl_get('a_attach') || $attachment['poster_id'] == $this->user->data['user_id']; if (!$own_attachment || ($attachment['in_message'] && !$this->auth->acl_get('u_pm_download')) || (!$attachment['in_message'] && !$this->auth->acl_get('u_download'))) @@ -301,9 +301,11 @@ class attachment extends controller * * @param int $topic_id The id of the topic that we are downloading from * - * @return null + * @return void + * @throws http_exception If attachment is not found + * If user don't have permission to download the attachment */ - protected function phpbb_download_handle_forum_auth($topic_id) + protected function phpbb_download_handle_forum_auth($topic_id): void { $sql_array = array( 'SELECT' => 't.topic_visibility, t.forum_id, f.forum_name, f.forum_password, f.parent_id', @@ -343,9 +345,10 @@ class attachment extends controller * * @param int $msg_id The id of the PM that we are downloading from * - * @return null + * @return void + * @throws http_exception If attachment is not found */ - protected function phpbb_download_handle_pm_auth($msg_id) + protected function phpbb_download_handle_pm_auth(int $msg_id): void { if (!$this->auth->acl_get('u_pm_download')) { @@ -379,7 +382,7 @@ class attachment extends controller * * @return bool Whether the user is allowed to download from that PM or not */ - protected function phpbb_download_check_pm_auth($msg_id) + protected function phpbb_download_check_pm_auth(int $msg_id): bool { $user_id = $this->user->data['user_id']; @@ -403,9 +406,9 @@ class attachment extends controller * * @param array|int $ids The attach_id of each attachment * - * @return null + * @return void */ - protected function phpbb_increment_downloads($ids) + protected function phpbb_increment_downloads($ids): void { if (!is_array($ids)) { @@ -422,7 +425,7 @@ class attachment extends controller * Check if downloading item is allowed * FIXME (See: https://tracker.phpbb.com/browse/PHPBB3-15264 and http://area51.phpbb.com/phpBB/viewtopic.php?f=81&t=51921) */ - protected function download_allowed() + protected function download_allowed(): bool { if (!$this->config['secure_downloads']) { From a67f2cf0864643be7468080252cd8ef141cba560 Mon Sep 17 00:00:00 2001 From: rubencm Date: Thu, 27 May 2021 00:53:39 +0200 Subject: [PATCH 39/42] [ticket/14285] Remove no longer needed variable PHPBB3-14285 --- phpBB/phpbb/avatar/driver/upload.php | 2 +- tests/template/extension_test.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index 0e0fe9e8a5..6ef3c50623 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -86,7 +86,7 @@ class upload extends \phpbb\avatar\driver\driver $root_path = (defined('PHPBB_USE_BOARD_URL_PATH') && PHPBB_USE_BOARD_URL_PATH) ? generate_board_url() . '/' : $this->path_helper->get_web_root_path(); return array( - 'src' => $root_path . $this->controller_helper->route('phpbb_storage_avatar', ['file' => $row['avatar']]), + 'src' => $this->controller_helper->route('phpbb_storage_avatar', ['file' => $row['avatar']]), 'width' => $row['avatar_width'], 'height' => $row['avatar_height'], ); diff --git a/tests/template/extension_test.php b/tests/template/extension_test.php index d6e181259f..156c7e2563 100644 --- a/tests/template/extension_test.php +++ b/tests/template/extension_test.php @@ -150,7 +150,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case ], [], [], - 'foo', + 'foo', [] ], [ @@ -168,7 +168,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case ], [], [], - 'foo', + 'foo', [] ], [ From 8518b9864e92655ac1f8f23700eb2459e0d49457 Mon Sep 17 00:00:00 2001 From: rubencm Date: Thu, 27 May 2021 07:15:18 +0200 Subject: [PATCH 40/42] [ticket/14285] Move response variable and small improvements PHPBB3-14285 --- phpBB/config/default/routing/storage.yml | 2 +- phpBB/download/file.php | 2 +- phpBB/includes/acp/acp_attachments.php | 4 +- phpBB/includes/acp/acp_users.php | 2 +- phpBB/includes/functions_content.php | 6 +- phpBB/includes/functions_posting.php | 2 +- phpBB/includes/message_parser.php | 2 +- phpBB/includes/ucp/ucp_attachments.php | 2 +- phpBB/phpbb/storage/controller/attachment.php | 37 +++++----- phpBB/phpbb/storage/controller/avatar.php | 24 ++++--- phpBB/phpbb/storage/controller/controller.php | 67 ++++++++++--------- vagrant/after.sh | 2 +- 12 files changed, 78 insertions(+), 74 deletions(-) diff --git a/phpBB/config/default/routing/storage.yml b/phpBB/config/default/routing/storage.yml index e61c58c356..ac135c5e6b 100644 --- a/phpBB/config/default/routing/storage.yml +++ b/phpBB/config/default/routing/storage.yml @@ -4,7 +4,7 @@ phpbb_storage_avatar: _controller: storage.controller.avatar:handle phpbb_storage_attachment: - path: /attachment/{id} + path: /attachment/{file} defaults: _controller: storage.controller.attachment:handle requirements: diff --git a/phpBB/download/file.php b/phpBB/download/file.php index e1eddf5bba..c71cc2274c 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -47,7 +47,7 @@ $thumbnail = $request->variable('t', false); $response = new RedirectResponse( $controller_helper->route('phpbb_storage_attachment', array( - 'id' => $attach_id, + 'file' => $attach_id, 't' => $thumbnail, ), false), 301 diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index ef7cd2f5cc..92363b0ff9 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -1096,7 +1096,7 @@ class acp_attachments 'PHYSICAL_FILENAME' => utf8_basename($row['physical_filename']), 'ATTACH_ID' => $row['attach_id'], 'POST_IDS' => (!empty($post_ids[$row['attach_id']])) ? $post_ids[$row['attach_id']] : '', - 'U_FILE' => $this->controller_helper->route('phpbb_storage_attachment', ['id' => (int) $row['attach_id']]) + 'U_FILE' => $this->controller_helper->route('phpbb_storage_attachment', ['file' => (int) $row['attach_id']]) )); } $db->sql_freeresult($result); @@ -1284,7 +1284,7 @@ class acp_attachments 'S_IN_MESSAGE' => (bool) $row['in_message'], 'U_VIEW_TOPIC' => append_sid("{$phpbb_root_path}viewtopic.$phpEx", "t={$row['topic_id']}&p={$row['post_msg_id']}") . "#p{$row['post_msg_id']}", - 'U_FILE' => $this->controller_helper->route('phpbb_storage_attachment', ['id' => $row['attach_id']]) + 'U_FILE' => $this->controller_helper->route('phpbb_storage_attachment', ['file' => $row['attach_id']]) )); } diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index 6b73bad2fe..9e5a31ab5d 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -2298,7 +2298,7 @@ class acp_users 'S_IN_MESSAGE' => $row['in_message'], - 'U_DOWNLOAD' => $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $row['attach_id']]), + 'U_DOWNLOAD' => $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $row['attach_id']]), 'U_VIEW_TOPIC' => $view_topic) ); } diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index 521bafef33..e6dde54737 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -1286,14 +1286,14 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a $display_cat = ATTACHMENT_CATEGORY_NONE; } - $download_link = $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $attachment['attach_id']]); + $download_link = $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $attachment['attach_id']]); $l_downloaded_viewed = 'VIEWED_COUNTS'; switch ($display_cat) { // Images case ATTACHMENT_CATEGORY_IMAGE: - $inline_link = $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $attachment['attach_id']]); + $inline_link = $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $attachment['attach_id']]); $block_array += array( 'S_IMAGE' => true, @@ -1305,7 +1305,7 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a // Images, but display Thumbnail case ATTACHMENT_CATEGORY_THUMB: - $thumbnail_link = $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $attachment['attach_id'], 't' => 1]); + $thumbnail_link = $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $attachment['attach_id'], 't' => 1]); $block_array += array( 'S_THUMBNAIL' => true, diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index 7c95cc12e8..7feb8d7b7d 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -807,7 +807,7 @@ function posting_gen_attachment_entry($attachment_data, &$filename_data, $show_a $hidden .= ''; } - $download_link = $phpbb_container->get('controller.helper')->route('phpbb_storage_attachment', ['id' => (int) $attach_row['attach_id']]); + $download_link = $phpbb_container->get('controller.helper')->route('phpbb_storage_attachment', ['file' => (int) $attach_row['attach_id']]); $attachrow_template_vars[(int) $attach_row['attach_id']] = array( 'FILENAME' => utf8_basename($attach_row['real_filename']), diff --git a/phpBB/includes/message_parser.php b/phpBB/includes/message_parser.php index ca85962f6b..056f2c0bfa 100644 --- a/phpBB/includes/message_parser.php +++ b/phpBB/includes/message_parser.php @@ -1779,7 +1779,7 @@ class parse_message extends bbcode_firstpass if (isset($this->plupload) && $this->plupload->is_active()) { - $download_url = $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $new_entry['attach_id']]); + $download_url = $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $new_entry['attach_id']]); // Send the client the attachment data to maintain state $json_response->send(array('data' => $this->attachment_data, 'download_url' => $download_url)); diff --git a/phpBB/includes/ucp/ucp_attachments.php b/phpBB/includes/ucp/ucp_attachments.php index b48e74128d..ab91c4f9f8 100644 --- a/phpBB/includes/ucp/ucp_attachments.php +++ b/phpBB/includes/ucp/ucp_attachments.php @@ -185,7 +185,7 @@ class ucp_attachments 'S_IN_MESSAGE' => $row['in_message'], 'S_LOCKED' => !$row['in_message'] && !$auth->acl_get('m_edit', $row['forum_id']) && ($row['forum_status'] == ITEM_LOCKED || $row['topic_status'] == ITEM_LOCKED || $row['post_edit_locked']), - 'U_VIEW_ATTACHMENT' => $controller_helper->route('phpbb_storage_attachment', ['id' => (int) $row['attach_id']]), + 'U_VIEW_ATTACHMENT' => $controller_helper->route('phpbb_storage_attachment', ['file' => (int) $row['attach_id']]), 'U_VIEW_TOPIC' => $view_topic) ); diff --git a/phpBB/phpbb/storage/controller/attachment.php b/phpBB/phpbb/storage/controller/attachment.php index e2e6f19967..5768fc7d82 100644 --- a/phpBB/phpbb/storage/controller/attachment.php +++ b/phpBB/phpbb/storage/controller/attachment.php @@ -26,7 +26,9 @@ use phpbb\storage\storage; use phpbb\user; use Symfony\Component\HttpFoundation\Request as symfony_request; use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\ResponseHeaderBag; +use Symfony\Component\HttpFoundation\StreamedResponse; /** * Controller for /download/attachment/{id} routes @@ -85,9 +87,9 @@ class attachment extends controller /** * {@inheritdoc} */ - public function handle($id) + public function handle(string $file): Response { - $attach_id = (int) $id; + $attach_id = (int) $file; $thumbnail = $this->request->variable('t', false); $this->language->add_lang('viewtopic'); @@ -246,13 +248,15 @@ class attachment extends controller // TODO: The next lines should go better in prepare, also the mimetype is handled by the storage table // so probably can be removed + $response = new StreamedResponse(); + // Content-type header - $this->response->headers->set('Content-Type', $attachment['mimetype']); + $response->headers->set('Content-Type', $attachment['mimetype']); // Display images in browser and force download for other file types if (strpos($attachment['mimetype'], 'image') !== false) { - $disposition = $this->response->headers->makeDisposition( + $disposition = $response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_INLINE, $attachment['real_filename'], $this->filenameFallback($attachment['real_filename']) @@ -260,18 +264,18 @@ class attachment extends controller } else { - $disposition = $this->response->headers->makeDisposition( + $disposition = $response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_ATTACHMENT, $attachment['real_filename'], $this->filenameFallback($attachment['real_filename']) ); } - $this->response->headers->set('Content-Disposition', $disposition); + $response->headers->set('Content-Disposition', $disposition); // Set expires header for browser cache $time = new \Datetime(); - $this->response->setExpires($time->modify('+1 year')); + $response->setExpires($time->modify('+1 year')); return parent::handle($attachment['physical_filename']); } @@ -289,11 +293,11 @@ class attachment extends controller /** * {@inheritdoc} */ - protected function prepare($file) + protected function prepare(StreamedResponse $response, string $file): void { - $this->response->setPrivate(); // But default should be private, but make sure of it + $response->setPrivate(); // By default should be private, but make sure of it - parent::prepare($file); + parent::prepare($response, $file); } /** @@ -305,7 +309,7 @@ class attachment extends controller * @throws http_exception If attachment is not found * If user don't have permission to download the attachment */ - protected function phpbb_download_handle_forum_auth($topic_id): void + protected function phpbb_download_handle_forum_auth(int $topic_id): void { $sql_array = array( 'SELECT' => 't.topic_visibility, t.forum_id, f.forum_name, f.forum_password, f.parent_id', @@ -404,20 +408,15 @@ class attachment extends controller /** * Increments the download count of all provided attachments * - * @param array|int $ids The attach_id of each attachment + * @param int $id The attach_id of the attachment * * @return void */ - protected function phpbb_increment_downloads($ids): void + protected function phpbb_increment_downloads(int $id): void { - if (!is_array($ids)) - { - $ids = array($ids); - } - $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' SET download_count = download_count + 1 - WHERE ' . $this->db->sql_in_set('attach_id', $ids); + WHERE attach_id = ' . $id; $this->db->sql_query($sql); } diff --git a/phpBB/phpbb/storage/controller/avatar.php b/phpBB/phpbb/storage/controller/avatar.php index c89dca95a9..aaf347fd79 100644 --- a/phpBB/phpbb/storage/controller/avatar.php +++ b/phpBB/phpbb/storage/controller/avatar.php @@ -18,7 +18,9 @@ use phpbb\config\config; use phpbb\db\driver\driver_interface; use phpbb\storage\storage; use Symfony\Component\HttpFoundation\Request as symfony_request; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\ResponseHeaderBag; +use Symfony\Component\HttpFoundation\StreamedResponse; /** * Controller for /download/avatar/{file} routes @@ -50,7 +52,7 @@ class avatar extends controller /** * {@inheritdoc} */ - public function handle($file) + public function handle(string $file): Response { $file = $this->decode_filename($file); @@ -60,22 +62,22 @@ class avatar extends controller /** * {@inheritdoc} */ - protected function is_allowed($file) + protected function is_allowed(string $file): bool { $ext = substr(strrchr($file, '.'), 1); // If filename have point and have an allowed extension - return strpos($file, '.') && in_array($ext, $this->allowed_extensions); + return strpos($file, '.') && in_array($ext, $this->allowed_extensions, true); } /** * Decode avatar filename * - * @param string $file Filename + * @param string $file Filename * * @return string Filename in filesystem */ - protected function decode_filename($file) + protected function decode_filename(string $file): string { $avatar_group = false; @@ -94,20 +96,20 @@ class avatar extends controller /** * {@inheritdoc} */ - protected function prepare($file) + protected function prepare(StreamedResponse $response, string $file): void { - $this->response->setPublic(); + $response->setPublic(); - $disposition = $this->response->headers->makeDisposition( + $disposition = $response->headers->makeDisposition( ResponseHeaderBag::DISPOSITION_INLINE, rawurlencode($file) ); - $this->response->headers->set('Content-Disposition', $disposition); + $response->headers->set('Content-Disposition', $disposition); $time = new \Datetime(); - $this->response->setExpires($time->modify('+1 year')); + $response->setExpires($time->modify('+1 year')); - parent::prepare($file); + parent::prepare($response, $file); } } diff --git a/phpBB/phpbb/storage/controller/controller.php b/phpBB/phpbb/storage/controller/controller.php index 22c786793a..54f95aa224 100644 --- a/phpBB/phpbb/storage/controller/controller.php +++ b/phpBB/phpbb/storage/controller/controller.php @@ -16,8 +16,10 @@ namespace phpbb\storage\controller; use phpbb\cache\service; use phpbb\db\driver\driver_interface; use phpbb\exception\http_exception; +use phpbb\storage\exception\exception; use phpbb\storage\storage; use Symfony\Component\HttpFoundation\Request as symfony_request; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\StreamedResponse; /** @@ -34,9 +36,6 @@ class controller /** @var storage */ protected $storage; - /** @var StreamedResponse */ - protected $response; - /** @var symfony_request */ protected $symfony_request; @@ -54,48 +53,50 @@ class controller $this->db = $db; $this->storage = $storage; $this->symfony_request = $symfony_request; - $this->response = new StreamedResponse(); } /** * Handler * - * @param string $file File path + * @param string $file File path + * + * @return Response a Symfony response object * * @throws http_exception when can't access $file - * - * @return StreamedResponse a Symfony response object + * @throws exception when there is an error reading the file */ - public function handle($file) + public function handle(string $file): Response { - if (!$this->is_allowed($file)) + $response = new StreamedResponse(); + + if (!static::is_allowed($file)) { throw new http_exception(403, 'Forbidden'); } - if (!$this->file_exists($file)) + if (!static::file_exists($file)) { throw new http_exception(404, 'Not Found'); } - $this->prepare($file); + static::prepare($response, $file); if (headers_sent()) { throw new http_exception(500, 'Headers already sent'); } - return $this->response; + return $response; } /** * If the user is allowed to download the file * - * @param string $file File path + * @param string $file File path * * @return bool */ - protected function is_allowed($file) + protected function is_allowed(string $file): bool { return true; } @@ -103,11 +104,11 @@ class controller /** * Check if file exists * - * @param string $file File path + * @param string $file File path * * @return bool */ - protected function file_exists($file) + protected function file_exists(string $file): bool { return $this->storage->exists($file); } @@ -115,33 +116,39 @@ class controller /** * Prepare response * - * @param string $file File path + * @param StreamedResponse $response + * @param string $file File path + * + * @return void + * @throws exception when there is an error reading the file */ - protected function prepare($file) + protected function prepare(StreamedResponse $response, string $file): void { $file_info = $this->storage->file_info($file); - if (!$this->response->headers->has('Content-Type')) + // Add Content-Type header + if (!$response->headers->has('Content-Type')) { try { $content_type = $file_info->get('mimetype'); } - catch (\phpbb\storage\exception\exception $e) + catch (exception $e) { $content_type = 'application/octet-stream'; } - $this->response->headers->set('Content-Type', $content_type); + $response->headers->set('Content-Type', $content_type); } - if (!$this->response->headers->has('Content-Length')) + // Add Content-Length header if we have the file size + if (!$response->headers->has('Content-Length')) { try { - $this->response->headers->set('Content-Length', $file_info->get('size')); + $response->headers->set('Content-Length', $file_info->get('size')); } - catch (\phpbb\storage\exception\exception $e) + catch (exception $e) { // Just don't send this header } @@ -156,7 +163,7 @@ class controller $output = fopen('php://output', 'w+b'); - $this->response->setCallback(function () use ($fp, $output) { + $response->setCallback(function () use ($fp, $output) { stream_copy_to_stream($fp, $output); fclose($fp); fclose($output); @@ -167,19 +174,15 @@ class controller exit; }); - $this->response->isNotModified($this->symfony_request); + $response->isNotModified($this->symfony_request); } /** * Garbage Collection */ - protected function file_gc() + protected function file_gc(): void { - if (!empty($this->cache)) - { - $this->cache->unload(); - } - + $this->cache->unload(); // Equivalent to $this->cache->get_driver()->unload(); $this->db->sql_close(); } } diff --git a/vagrant/after.sh b/vagrant/after.sh index 5980e5c334..345cd02c80 100755 --- a/vagrant/after.sh +++ b/vagrant/after.sh @@ -1,6 +1,6 @@ #!/bin/sh -PHP_VERSION="7.2" +PHP_VERSION="7.3" PHPBB_PATH="/home/vagrant/phpbb" PHPBB_CONFIG="${PHPBB_PATH}/phpBB/config.php" PHPBB_INSTALL="${PHPBB_PATH}/vagrant/phpbb-install-config.yml" From 551004a2094660797cf6846b8fff2e20d2cdcaa4 Mon Sep 17 00:00:00 2001 From: rubencm Date: Thu, 27 May 2021 09:00:59 +0200 Subject: [PATCH 41/42] [ticket/14285] Fix code sniffer PHPBB3-14285 --- build/code_sniffer/phpbb/Sniffs/Namespaces/UnusedUseSniff.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build/code_sniffer/phpbb/Sniffs/Namespaces/UnusedUseSniff.php b/build/code_sniffer/phpbb/Sniffs/Namespaces/UnusedUseSniff.php index b0c5487f5f..1da728d5b7 100644 --- a/build/code_sniffer/phpbb/Sniffs/Namespaces/UnusedUseSniff.php +++ b/build/code_sniffer/phpbb/Sniffs/Namespaces/UnusedUseSniff.php @@ -192,6 +192,9 @@ class phpbb_Sniffs_Namespaces_UnusedUseSniff implements Sniff { $ok = $this->check($phpcsFile, $param['type_hint'], $class_name_full, $class_name_short, $function_declaration) || $ok; } + + $method_properties = $phpcsFile->getMethodProperties($function_declaration); + $ok = $this->check($phpcsFile, $method_properties['return_type'], $class_name_full, $class_name_short, $function_declaration) || $ok; } // Checks in catch blocks From fca636ed58a0c78293a486148f902f15c02e4e7f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 27 May 2021 22:23:36 +0200 Subject: [PATCH 42/42] [ticket/14285] Remove no longer needed root_path PHPBB3-14285 --- phpBB/phpbb/avatar/driver/upload.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index 6ef3c50623..8a168fc356 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -83,8 +83,6 @@ class upload extends \phpbb\avatar\driver\driver */ public function get_data($row) { - $root_path = (defined('PHPBB_USE_BOARD_URL_PATH') && PHPBB_USE_BOARD_URL_PATH) ? generate_board_url() . '/' : $this->path_helper->get_web_root_path(); - return array( 'src' => $this->controller_helper->route('phpbb_storage_avatar', ['file' => $row['avatar']]), 'width' => $row['avatar_width'],