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); } /**