From c375f2c9e52ba0f817aae4357d89e9c58e69ca29 Mon Sep 17 00:00:00 2001 From: rubencm Date: Sun, 21 Mar 2021 21:46:56 +0100 Subject: [PATCH] [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', [] ], [