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"