From e749e060421f7b6b0250cfd455f6fc0361eeb9d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Wed, 9 Aug 2017 12:21:19 +0200 Subject: [PATCH 01/10] [ticket/15286] Use storage in attachments PHPBB3-15286 --- .../default/container/services_attachment.yml | 1 + .../default/container/services_storage.yml | 8 ++ phpBB/docs/lighttpd.sample.conf | 27 ++-- phpBB/docs/nginx.sample.conf | 8 -- phpBB/download/file.php | 2 +- phpBB/includes/acp/acp_attachments.php | 48 ------- phpBB/includes/functions_download.php | 56 +++----- phpBB/install/schemas/schema_data.sql | 3 +- phpBB/phpbb/attachment/upload.php | 51 ++++--- .../data/v330/storage_attachment.php | 26 ++++ phpBB/phpbb/files/filespec_storage.php | 3 - phpBB/phpbb/files/types/local_storage.php | 136 ++++++++++++++++++ 12 files changed, 233 insertions(+), 136 deletions(-) create mode 100644 phpBB/phpbb/db/migration/data/v330/storage_attachment.php create mode 100644 phpBB/phpbb/files/types/local_storage.php diff --git a/phpBB/config/default/container/services_attachment.yml b/phpBB/config/default/container/services_attachment.yml index c56ced21f4..9253726496 100644 --- a/phpBB/config/default/container/services_attachment.yml +++ b/phpBB/config/default/container/services_attachment.yml @@ -36,5 +36,6 @@ services: - '@mimetype.guesser' - '@dispatcher' - '@plupload' + - '@storage.attachment' - '@user' - '%core.root_path%' diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml index 3593b8264a..f114dc1dfe 100644 --- a/phpBB/config/default/container/services_storage.yml +++ b/phpBB/config/default/container/services_storage.yml @@ -1,6 +1,14 @@ services: # Storages + storage.attachment: + class: phpbb\storage\storage + arguments: + - '@storage.adapter.factory' + - 'attachment' + tags: + - { name: storage } + storage.avatar: class: phpbb\storage\storage arguments: diff --git a/phpBB/docs/lighttpd.sample.conf b/phpBB/docs/lighttpd.sample.conf index f5b509e002..e783c809fc 100644 --- a/phpBB/docs/lighttpd.sample.conf +++ b/phpBB/docs/lighttpd.sample.conf @@ -3,17 +3,8 @@ # from your system's lighttpd.conf. # Tested with lighttpd 1.4.35 -# If you want to use the X-Sendfile feature, -# uncomment the 'allow-x-send-file' for the fastcgi -# server below and add the following to your config.php -# -# define('PHPBB_ENABLE_X_SENDFILE', true); -# -# See http://blog.lighttpd.net/articles/2006/07/02/x-sendfile -# for the details on X-Sendfile. - # Load moules -server.modules += ( +server.modules += ( "mod_access", "mod_fastcgi", "mod_rewrite", @@ -32,11 +23,11 @@ $HTTP["host"] == "www.myforums.com" { server.name = "www.myforums.com" server.document-root = "/path/to/phpbb" server.dir-listing = "disable" - + index-file.names = ( "index.php", "index.htm", "index.html" ) accesslog.filename = "/var/log/lighttpd/access-www.myforums.com.log" - - # Deny access to internal phpbb files. + + # Deny access to internal phpbb files. $HTTP["url"] =~ "^/(config\.php|common\.php|cache|files|images/avatars/upload|includes|phpbb|store|vendor)" { url.access-deny = ( "" ) } @@ -45,12 +36,12 @@ $HTTP["host"] == "www.myforums.com" { $HTTP["url"] =~ "/\.svn|/\.git" { url.access-deny = ( "" ) } - + # Deny access to apache configuration files. $HTTP["url"] =~ "/\.htaccess|/\.htpasswd|/\.htgroups" { url.access-deny = ( "" ) } - + # The following 3 lines will rewrite URLs passed through the front controller # to not require app.php in the actual URL. In other words, a controller is # by default accessed at /app.php/my/controller, but can also be accessed at @@ -58,14 +49,14 @@ $HTTP["host"] == "www.myforums.com" { url.rewrite-if-not-file = ( "^/(.*)$" => "/app.php/$1" ) - - fastcgi.server = ( ".php" => + + fastcgi.server = ( ".php" => (( "bin-path" => "/usr/bin/php-cgi", "socket" => "/tmp/php.socket", "max-procs" => 4, "idle-timeout" => 30, - "bin-environment" => ( + "bin-environment" => ( "PHP_FCGI_CHILDREN" => "10", "PHP_FCGI_MAX_REQUESTS" => "10000" ), diff --git a/phpBB/docs/nginx.sample.conf b/phpBB/docs/nginx.sample.conf index 55c01a1fc9..4e40400b36 100644 --- a/phpBB/docs/nginx.sample.conf +++ b/phpBB/docs/nginx.sample.conf @@ -3,14 +3,6 @@ # from your system's nginx.conf. # Tested with nginx 0.8.35. -# If you want to use the X-Accel-Redirect feature, -# add the following to your config.php. -# -# define('PHPBB_ENABLE_X_ACCEL_REDIRECT', true); -# -# See http://wiki.nginx.org/XSendfile for the details -# on X-Accel-Redirect. - http { # Compression - requires gzip and gzip static modules. gzip on; diff --git a/phpBB/download/file.php b/phpBB/download/file.php index c7540c5380..a885c886cd 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -324,7 +324,7 @@ else } else { - send_file_to_browser($attachment, $config['upload_path'], $display_cat); + send_file_to_browser($attachment, $display_cat); file_gc(); } } diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index dc4eb66cf8..c735c6265b 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -147,7 +147,6 @@ class acp_attachments 'allow_attachments' => array('lang' => 'ALLOW_ATTACHMENTS', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => false), 'allow_pm_attach' => array('lang' => 'ALLOW_PM_ATTACHMENTS', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => false), - 'upload_path' => array('lang' => 'UPLOAD_DIR', 'validate' => 'wpath', 'type' => 'text:25:100', 'explain' => true), 'display_order' => array('lang' => 'DISPLAY_ORDER', 'validate' => 'bool', 'type' => 'custom', 'method' => 'display_order', 'explain' => true), 'attachment_quota' => array('lang' => 'ATTACH_QUOTA', 'validate' => 'string', 'type' => 'custom', 'method' => 'max_filesize', 'explain' => true), 'max_filesize' => array('lang' => 'ATTACH_MAX_FILESIZE', 'validate' => 'string', 'type' => 'custom', 'method' => 'max_filesize', 'explain' => true), @@ -223,9 +222,6 @@ class acp_attachments { $phpbb_log->add('admin', $user->data['user_id'], $user->ip, 'LOG_CONFIG_ATTACH'); - // Check Settings - $this->test_upload($error, $this->new_config['upload_path'], false); - if (!count($error)) { trigger_error($user->lang['CONFIG_UPDATED'] . adm_back_link($this->u_action)); @@ -1536,50 +1532,6 @@ class acp_attachments return $imagick; } - /** - * Test Settings - */ - function test_upload(&$error, $upload_dir, $create_directory = false) - { - global $user, $phpbb_root_path; - - // Does the target directory exist, is it a directory and writable. - if ($create_directory) - { - if (!file_exists($phpbb_root_path . $upload_dir)) - { - @mkdir($phpbb_root_path . $upload_dir, 0777); - - try - { - $this->filesystem->phpbb_chmod($phpbb_root_path . $upload_dir, CHMOD_READ | CHMOD_WRITE); - } - catch (\phpbb\filesystem\exception\filesystem_exception $e) - { - // Do nothing - } - } - } - - if (!file_exists($phpbb_root_path . $upload_dir)) - { - $error[] = sprintf($user->lang['NO_UPLOAD_DIR'], $upload_dir); - return; - } - - if (!is_dir($phpbb_root_path . $upload_dir)) - { - $error[] = sprintf($user->lang['UPLOAD_NOT_DIR'], $upload_dir); - return; - } - - if (!$this->filesystem->is_writable($phpbb_root_path . $upload_dir)) - { - $error[] = sprintf($user->lang['NO_WRITE_UPLOAD'], $upload_dir); - return; - } - } - /** * Perform operations on sites for external linking */ diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index b6414c40dc..b6b55ab59d 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -121,13 +121,15 @@ function wrap_img_in_html($src, $title) /** * Send file to browser */ -function send_file_to_browser($attachment, $upload_dir, $category) +function send_file_to_browser($attachment, $category) { - global $user, $db, $phpbb_dispatcher, $phpbb_root_path, $request; + global $user, $db, $phpbb_dispatcher, $request, $phpbb_container; - $filename = $phpbb_root_path . $upload_dir . '/' . $attachment['physical_filename']; + $storage = $phpbb_container->get('storage.attachment'); - if (!@file_exists($filename)) + $filename = $attachment['physical_filename']; + + if (!$storage->exists($filename)) { send_status_line(404, 'Not Found'); trigger_error('ERROR_NO_ATTACHMENT'); @@ -146,14 +148,22 @@ function send_file_to_browser($attachment, $upload_dir, $category) } // Now send the File Contents to the Browser - $size = @filesize($filename); + 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 string upload_dir Relative path of upload directory * @var int category Attachment category * @var string filename Path to file, including filename * @var int size File size @@ -161,7 +171,6 @@ function send_file_to_browser($attachment, $upload_dir, $category) */ $vars = array( 'attachment', - 'upload_dir', 'category', 'filename', 'size', @@ -171,15 +180,8 @@ function send_file_to_browser($attachment, $upload_dir, $category) // 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() || !@file_exists($filename) || !@is_readable($filename)) + if (headers_sent()) { - // PHP track_errors setting On? - if (!empty($php_errormsg)) - { - send_status_line(500, 'Internal Server Error'); - trigger_error($user->lang['UNABLE_TO_DELIVER_FILE'] . '<br />' . sprintf($user->lang['TRACKED_PHP_ERROR'], $php_errormsg)); - } - send_status_line(500, 'Internal Server Error'); trigger_error('UNABLE_TO_DELIVER_FILE'); } @@ -235,24 +237,6 @@ function send_file_to_browser($attachment, $upload_dir, $category) if (!set_modified_headers($attachment['filetime'], $user->browser)) { - // We make sure those have to be enabled manually by defining a constant - // because of the potential disclosure of full attachment path - // in case support for features is absent in the webserver software. - if (defined('PHPBB_ENABLE_X_ACCEL_REDIRECT') && PHPBB_ENABLE_X_ACCEL_REDIRECT) - { - // X-Accel-Redirect - http://wiki.nginx.org/XSendfile - header('X-Accel-Redirect: ' . $user->page['root_script_path'] . $upload_dir . '/' . $attachment['physical_filename']); - exit; - } - else if (defined('PHPBB_ENABLE_X_SENDFILE') && PHPBB_ENABLE_X_SENDFILE && !phpbb_http_byte_range($size)) - { - // X-Sendfile - http://blog.lighttpd.net/articles/2006/07/02/x-sendfile - // Lighttpd's X-Sendfile does not support range requests as of 1.4.26 - // and always requires an absolute path. - header('X-Sendfile: ' . dirname(__FILE__) . "/../$upload_dir/{$attachment['physical_filename']}"); - exit; - } - if ($size) { header("Content-Length: $size"); @@ -261,7 +245,7 @@ function send_file_to_browser($attachment, $upload_dir, $category) // Try to deliver in chunks @set_time_limit(0); - $fp = @fopen($filename, 'rb'); + $fp = $storage->read_stream($filename); if ($fp !== false) { @@ -291,10 +275,6 @@ function send_file_to_browser($attachment, $upload_dir, $category) } fclose($fp); } - else - { - @readfile($filename); - } flush(); } diff --git a/phpBB/install/schemas/schema_data.sql b/phpBB/install/schemas/schema_data.sql index 58a7ff4f63..c802480eea 100644 --- a/phpBB/install/schemas/schema_data.sql +++ b/phpBB/install/schemas/schema_data.sql @@ -277,7 +277,6 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('teampage_forums', INSERT INTO phpbb_config (config_name, config_value) VALUES ('topics_per_page', '25'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('tpl_allow_php', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('upload_icons_path', 'images/upload_icons'); -INSERT INTO phpbb_config (config_name, config_value) VALUES ('upload_path', 'files'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('use_system_cron', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('version', '3.3.0-a1-dev'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('warnings_expire_days', '90'); @@ -288,6 +287,8 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_json INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_vendor_dir', 'vendor-ext/'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_enable_on_install', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_purge_on_remove', '1'); +INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\attachment\provider', 'phpbb\storage\provider\local'); +INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\attachment\config\path', 'files'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\avatar\provider', 'phpbb\storage\provider\local'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\avatar\config\path', 'images/avatars/upload'); diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index b9d32058db..70ce439381 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -20,6 +20,7 @@ use \phpbb\event\dispatcher; use \phpbb\language\language; use \phpbb\mimetype\guesser; use \phpbb\plupload\plupload; +use \phpbb\storage\storage; use \phpbb\user; /** @@ -51,6 +52,9 @@ class upload /** @var plupload Plupload */ protected $plupload; + /** @var storage */ + protected $storage; + /** @var user */ protected $user; @@ -79,7 +83,7 @@ class upload * @param user $user * @param $phpbb_root_path */ - public function __construct(auth $auth, service $cache, config $config, \phpbb\files\upload $files_upload, language $language, guesser $mimetype_guesser, dispatcher $phpbb_dispatcher, plupload $plupload, user $user, $phpbb_root_path) + public function __construct(auth $auth, service $cache, config $config, \phpbb\files\upload $files_upload, language $language, guesser $mimetype_guesser, dispatcher $phpbb_dispatcher, plupload $plupload, storage $storage, user $user, $phpbb_root_path) { $this->auth = $auth; $this->cache = $cache; @@ -89,6 +93,7 @@ class upload $this->mimetype_guesser = $mimetype_guesser; $this->phpbb_dispatcher = $phpbb_dispatcher; $this->plupload = $plupload; + $this->storage = $storage; $this->user = $user; $this->phpbb_root_path = $phpbb_root_path; } @@ -118,7 +123,7 @@ class upload return $this->file_data; } - $this->file = ($local) ? $this->files_upload->handle_upload('files.types.local', $local_storage, $local_filedata) : $this->files_upload->handle_upload('files.types.form', $form_name); + $this->file = ($local) ? $this->files_upload->handle_upload('files.types.local_storage', $local_storage, $local_filedata) : $this->files_upload->handle_upload('files.types.form_storage', $form_name); if ($this->file->init_error()) { @@ -152,25 +157,12 @@ class upload $this->file->clean_filename('unique', $this->user->data['user_id'] . '_'); - // Are we uploading an image *and* this image being within the image category? - // Only then perform additional image checks. - $this->file->move_file($this->config['upload_path'], false, !$is_image); - // Do we have to create a thumbnail? $this->file_data['thumbnail'] = ($is_image && $this->config['img_create_thumbnail']) ? 1 : 0; // Make sure the image category only holds valid images... $this->check_image($is_image); - if (count($this->file->error)) - { - $this->file->remove(); - $this->file_data['error'] = array_merge($this->file_data['error'], $this->file->error); - $this->file_data['post_attach'] = false; - - return $this->file_data; - } - $this->fill_file_data(); $filedata = $this->file_data; @@ -200,6 +192,19 @@ class upload // Create Thumbnail $this->create_thumbnail(); + // Are we uploading an image *and* this image being within the image category? + // Only then perform additional image checks. + $this->file->move_file($this->storage, false, !$is_image); + + if (count($this->file->error)) + { + $this->file->remove($this->storage); + $this->file_data['error'] = array_merge($this->file_data['error'], $this->file->error); + $this->file_data['post_attach'] = false; + + return $this->file_data; + } + return $this->file_data; } @@ -212,10 +217,18 @@ class upload { if ($this->file_data['thumbnail']) { - $source = $this->file->get('destination_file'); - $destination = $this->file->get('destination_path') . '/thumb_' . $this->file->get('realname'); + $source = $this->file->get('filename'); + $destination_name = 'thumb_' . $this->file->get('realname'); + $destination = sys_get_temp_dir() . '/' . $destination_name; - if (!create_thumbnail($source, $destination, $this->file->get('mimetype'))) + if (create_thumbnail($source, $destination, $this->file->get('mimetype'))) + { + // Move the thumbnail from temp folder to the storage + $fp = fopen($destination, 'rb'); + $this->storage->write_stream($destination_name, $fp); + fclose($fp); + } + else { $this->file_data['thumbnail'] = 0; } @@ -253,7 +266,7 @@ class upload // Make sure the image category only holds valid images... if ($is_image && !$this->file->is_image()) { - $this->file->remove(); + $this->file->remove($this->storage); if ($this->plupload && $this->plupload->is_active()) { diff --git a/phpBB/phpbb/db/migration/data/v330/storage_attachment.php b/phpBB/phpbb/db/migration/data/v330/storage_attachment.php new file mode 100644 index 0000000000..626bafed65 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v330/storage_attachment.php @@ -0,0 +1,26 @@ +<?php +/** +* +* This file is part of the phpBB Forum Software package. +* +* @copyright (c) phpBB Limited <https://www.phpbb.com> +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace phpbb\db\migration\data\v330; + +class storage_attachment extends \phpbb\db\migration\migration +{ + public function update_data() + { + return array( + array('config.add', array('storage\\attachment\\provider', \phpbb\storage\provider\local::class)), + array('config.add', array('storage\\attachment\\config\\path', $this->config['upload_path'])), + array('config.remove', array('upload_path')), + ); + } +} diff --git a/phpBB/phpbb/files/filespec_storage.php b/phpBB/phpbb/files/filespec_storage.php index b370b66a4e..8b6194fe99 100644 --- a/phpBB/phpbb/files/filespec_storage.php +++ b/phpBB/phpbb/files/filespec_storage.php @@ -51,9 +51,6 @@ class filespec_storage /** @var string Destination file name */ protected $destination_file = ''; - /** @var string Destination file path */ - protected $destination_path = ''; - /** @var bool Whether file was moved */ protected $file_moved = false; diff --git a/phpBB/phpbb/files/types/local_storage.php b/phpBB/phpbb/files/types/local_storage.php new file mode 100644 index 0000000000..7485bf5362 --- /dev/null +++ b/phpBB/phpbb/files/types/local_storage.php @@ -0,0 +1,136 @@ +<?php +/** + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited <https://www.phpbb.com> + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files\types; + +use bantu\IniGetWrapper\IniGetWrapper; +use phpbb\files\factory; +use phpbb\files\filespec; +use phpbb\language\language; +use phpbb\request\request_interface; + +class local extends base +{ + /** @var factory Files factory */ + protected $factory; + + /** @var language */ + protected $language; + + /** @var IniGetWrapper */ + protected $php_ini; + + /** @var request_interface */ + protected $request; + + /** @var \phpbb\files\upload */ + protected $upload; + + /** + * Construct a form upload type + * + * @param factory $factory Files factory + * @param language $language Language class + * @param IniGetWrapper $php_ini ini_get() wrapper + * @param request_interface $request Request object + */ + public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, request_interface $request) + { + $this->factory = $factory; + $this->language = $language; + $this->php_ini = $php_ini; + $this->request = $request; + } + + /** + * {@inheritdoc} + */ + public function upload() + { + $args = func_get_args(); + return $this->local_upload($args[0], isset($args[1]) ? $args[1] : false); + } + + /** + * Move file from another location to phpBB + * + * @param string $source_file Filename of source file + * @param array|bool $filedata Array with filedata or false + * + * @return filespec Object "filespec" is returned, all further operations can be done with this object + */ + protected function local_upload($source_file, $filedata = false) + { + $upload = $this->get_upload_ary($source_file, $filedata); + + /** @var filespec $file */ + $file = $this->factory->get('filespec_storage') + ->set_upload_ary($upload) + ->set_upload_namespace($this->upload); + + if ($file->init_error()) + { + $file->error[] = ''; + return $file; + } + + // PHP Upload file size check + $file = $this->check_upload_size($file); + if (count($file->error)) + { + return $file; + } + + // Not correctly uploaded + if (!$file->is_uploaded()) + { + $file->error[] = $this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED'); + return $file; + } + + $this->upload->common_checks($file); + $this->request->overwrite('local', $upload, request_interface::FILES); + + return $file; + } + + /** + * Retrieve upload array + * + * @param string $source_file Source file name + * @param array $filedata File data array + * + * @return array Upload array + */ + protected function get_upload_ary($source_file, $filedata) + { + $upload = array(); + + $upload['local_mode'] = true; + $upload['tmp_name'] = $source_file; + + if ($filedata === false) + { + $upload['name'] = utf8_basename($source_file); + $upload['size'] = 0; + } + else + { + $upload['name'] = $filedata['realname']; + $upload['size'] = $filedata['size']; + $upload['type'] = $filedata['type']; + } + + return $upload; + } +} From e564ca6e60d856e51e8432c6f7164ce1d3bcd35d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Wed, 9 Aug 2017 14:22:03 +0200 Subject: [PATCH 02/10] [ticket/15286] Fix tests PHPBB3-15286 --- phpBB/includes/functions_download.php | 1 - phpBB/phpbb/attachment/upload.php | 2 +- phpBB/phpbb/files/types/local_storage.php | 2 +- tests/attachment/upload_test.php | 43 ++++++++++++++--------- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index b6b55ab59d..b74a386e3e 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -158,7 +158,6 @@ function send_file_to_browser($attachment, $category) $size = 0; } - /** * Event to alter attachment before it is sent to browser. * diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index 70ce439381..00f944dd5b 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -293,7 +293,7 @@ class upload $this->file_data['error'][] = $this->language->lang('ATTACH_QUOTA_REACHED'); $this->file_data['post_attach'] = false; - $this->file->remove(); + $this->file->remove($this->storage); return false; } diff --git a/phpBB/phpbb/files/types/local_storage.php b/phpBB/phpbb/files/types/local_storage.php index 7485bf5362..c3990fe389 100644 --- a/phpBB/phpbb/files/types/local_storage.php +++ b/phpBB/phpbb/files/types/local_storage.php @@ -19,7 +19,7 @@ use phpbb\files\filespec; use phpbb\language\language; use phpbb\request\request_interface; -class local extends base +class local_storage extends base { /** @var factory Files factory */ protected $factory; diff --git a/tests/attachment/upload_test.php b/tests/attachment/upload_test.php index 6aaae6ad61..f728830a26 100644 --- a/tests/attachment/upload_test.php +++ b/tests/attachment/upload_test.php @@ -39,6 +39,9 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case /** @var \phpbb\plupload\plupload */ protected $plupload; + /** @var \phpbb\storage\storage */ + protected $storage; + /** @var \phpbb\user */ protected $user; @@ -75,7 +78,6 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->auth = new \phpbb\auth\auth(); $this->config = new \phpbb\config\config(array( - 'upload_path' => '', 'img_create_thumbnail' => true, )); $config = $this->config; @@ -96,27 +98,34 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $guessers[3]->set_priority(-2); $this->mimetype_guesser = new \phpbb\mimetype\guesser($guessers); $this->plupload = new \phpbb\plupload\plupload($phpbb_root_path, $this->config, $this->request, new \phpbb\user($this->language, '\phpbb\datetime'), $this->php_ini, $this->mimetype_guesser); + + $local_adapter = new \phpbb\storage\adapter\local($this->filesystem, $phpbb_root_path); + $local_adapter->configure(['path' => 'files']); + $adapter_factory_mock = $this->getMockBuilder('\phpbb\storage\adapter_factory') + ->disableOriginalConstructor() + ->getMock(); + $adapter_factory_mock->expects($this->any()) + ->method('get') + ->willReturn($local_adapter); + $this->storage = new \phpbb\storage\storage($adapter_factory_mock, 'attachment'); + $factory_mock = $this->getMockBuilder('\phpbb\files\factory') ->disableOriginalConstructor() ->getMock(); $factory_mock->expects($this->any()) ->method('get') - ->willReturn(new \phpbb\files\filespec( - $this->filesystem, + ->willReturn(new \phpbb\files\filespec_storage( $this->language, $this->php_ini, new \FastImageSize\FastImageSize(), - $this->phpbb_root_path, $this->mimetype_guesser )); $this->container = new phpbb_mock_container_builder($phpbb_root_path, $phpEx); - $this->container->set('files.filespec', new \phpbb\files\filespec( - $this->filesystem, + $this->container->set('files.filespec_storage', new \phpbb\files\filespec_storage( $this->language, $this->php_ini, new \FastImageSize\FastImageSize(), - $phpbb_root_path, new \phpbb\mimetype\guesser(array( 'mimetype.extension_guesser' => new \phpbb\mimetype\extension_guesser(), )))); @@ -127,7 +136,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->plupload, $this->request )); - $this->container->set('files.types.local', new \phpbb\files\types\local( + $this->container->set('files.types.local_storage', new \phpbb\files\types\local_storage( $factory_mock, $this->language, $this->php_ini, @@ -138,7 +147,6 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->phpbb_dispatcher = new phpbb_mock_event_dispatcher(); $this->user = new \phpbb\user($this->language, '\phpbb\datetime'); - $this->upload = new \phpbb\attachment\upload( $this->auth, $this->cache, @@ -148,6 +156,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->mimetype_guesser, $this->phpbb_dispatcher, $this->plupload, + $this->storage, $this->user, $this->phpbb_root_path ); @@ -205,7 +214,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case public function test_init_error() { - $filespec = $this->getMockBuilder('\phpbb\files\filespec') + $filespec = $this->getMockBuilder('\phpbb\files\filespec_storage') ->disableOriginalConstructor() ->getMock(); $filespec->expects($this->any()) @@ -217,14 +226,14 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $filespec->expects($this->any()) ->method('set_upload_ary') ->willReturnSelf(); - $this->container->set('files.filespec', $filespec); + $this->container->set('files.filespec_storage', $filespec); $factory_mock = $this->getMockBuilder('\phpbb\files\factory') ->disableOriginalConstructor() ->getMock(); $factory_mock->expects($this->any()) ->method('get') ->willReturn($filespec); - $this->container->set('files.types.local', new \phpbb\files\types\local( + $this->container->set('files.types.local_storage', new \phpbb\files\types\local_storage( $factory_mock, $this->language, $this->php_ini, @@ -240,6 +249,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->mimetype_guesser, $this->phpbb_dispatcher, $this->plupload, + $this->storage, $this->user, $this->phpbb_root_path ); @@ -336,7 +346,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case */ public function test_image_upload($is_image, $plupload_active, $config_data, $expected) { - $filespec = $this->getMockBuilder('\phpbb\files\filespec') + $filespec = $this->getMockBuilder('\phpbb\files\filespec_storage') ->setMethods(array( 'init_error', 'is_image', @@ -344,11 +354,9 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case 'is_uploaded', )) ->setConstructorArgs(array( - $this->filesystem, $this->language, $this->php_ini, new \FastImageSize\FastImageSize(), - $this->phpbb_root_path, $this->mimetype_guesser, $this->plupload )) @@ -370,14 +378,14 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $filespec->expects($this->any()) ->method('move_file') ->willReturn(false); - $this->container->set('files.filespec', $filespec); + $this->container->set('files.filespec_storage', $filespec); $factory_mock = $this->getMockBuilder('\phpbb\files\factory') ->disableOriginalConstructor() ->getMock(); $factory_mock->expects($this->any()) ->method('get') ->willReturn($filespec); - $this->container->set('files.types.local', new \phpbb\files\types\local( + $this->container->set('files.types.local_storage', new \phpbb\files\types\local_storage( $factory_mock, $this->language, $this->php_ini, @@ -406,6 +414,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->mimetype_guesser, $this->phpbb_dispatcher, $plupload, + $this->storage, $this->user, $this->phpbb_root_path ); From ef43dbdcca1c48dc43dcd033ce9b2ee8e67ce89b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Wed, 9 Aug 2017 15:54:03 +0200 Subject: [PATCH 03/10] [ticket/15286] Update use storage in avatars PHPBB3-15286 --- .../default/container/services_attachment.yml | 4 +-- phpBB/download/file.php | 1 + phpBB/includes/functions.php | 2 -- phpBB/includes/functions_acp.php | 1 - phpBB/includes/functions_content.php | 4 ++- phpBB/includes/functions_convert.php | 2 +- phpBB/includes/functions_posting.php | 4 ++- phpBB/includes/functions_privmsgs.php | 4 ++- phpBB/phpbb/attachment/delete.php | 35 ++++++++----------- phpBB/phpbb/attachment/upload.php | 24 +------------ tests/attachment/delete_test.php | 16 ++++++--- tests/attachment/upload_test.php | 9 ++--- 12 files changed, 41 insertions(+), 65 deletions(-) diff --git a/phpBB/config/default/container/services_attachment.yml b/phpBB/config/default/container/services_attachment.yml index 9253726496..5b614d2f60 100644 --- a/phpBB/config/default/container/services_attachment.yml +++ b/phpBB/config/default/container/services_attachment.yml @@ -6,9 +6,8 @@ services: - '@config' - '@dbal.conn' - '@dispatcher' - - '@filesystem' - '@attachment.resync' - - '%core.root_path%' + - '@storage.attachment' attachment.manager: class: phpbb\attachment\manager @@ -38,4 +37,3 @@ services: - '@plupload' - '@storage.attachment' - '@user' - - '%core.root_path%' diff --git a/phpBB/download/file.php b/phpBB/download/file.php index a885c886cd..23658db5d3 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -309,6 +309,7 @@ else } else { + // // Determine the 'presenting'-method if ($download_mode == PHYSICAL_LINK) { diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index d61f3bc653..2d3da96a4a 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -4438,7 +4438,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id = 'T_AVATAR_GALLERY_PATH' => "{$web_path}{$config['avatar_gallery_path']}/", 'T_ICONS_PATH' => "{$web_path}{$config['icons_path']}/", 'T_RANKS_PATH' => "{$web_path}{$config['ranks_path']}/", - 'T_UPLOAD_PATH' => "{$web_path}{$config['upload_path']}/", 'T_STYLESHEET_LINK' => "{$web_path}styles/" . rawurlencode($user->style['style_path']) . '/theme/stylesheet.css?assets_version=' . $config['assets_version'], 'T_STYLESHEET_LANG_LINK'=> "{$web_path}styles/" . rawurlencode($user->style['style_path']) . '/theme/' . $user->lang_name . '/stylesheet.css?assets_version=' . $config['assets_version'], 'T_FONT_AWESOME_LINK' => !empty($config['allow_cdn']) && !empty($config['load_font_awesome_url']) ? $config['load_font_awesome_url'] : "{$web_path}assets/css/font-awesome.min.css?assets_version=" . $config['assets_version'], @@ -4455,7 +4454,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id = 'T_AVATAR_GALLERY' => $config['avatar_gallery_path'], 'T_ICONS' => $config['icons_path'], 'T_RANKS' => $config['ranks_path'], - 'T_UPLOAD' => $config['upload_path'], 'SITE_LOGO_IMG' => $user->img('site_logo'), )); diff --git a/phpBB/includes/functions_acp.php b/phpBB/includes/functions_acp.php index b3bde79339..3b4be2c344 100644 --- a/phpBB/includes/functions_acp.php +++ b/phpBB/includes/functions_acp.php @@ -91,7 +91,6 @@ function adm_page_header($page_title) 'T_AVATAR_GALLERY_PATH' => "{$phpbb_root_path}{$config['avatar_gallery_path']}/", 'T_ICONS_PATH' => "{$phpbb_root_path}{$config['icons_path']}/", 'T_RANKS_PATH' => "{$phpbb_root_path}{$config['ranks_path']}/", - 'T_UPLOAD_PATH' => "{$phpbb_root_path}{$config['upload_path']}/", 'T_FONT_AWESOME_LINK' => !empty($config['allow_cdn']) && !empty($config['load_font_awesome_url']) ? $config['load_font_awesome_url'] : "{$phpbb_root_path}assets/css/font-awesome.min.css?assets_version=" . $config['assets_version'], 'T_ASSETS_VERSION' => $config['assets_version'], diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index 40d44cfe7b..6d6f09da63 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -1163,7 +1163,7 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a // Some basics... $attachment['extension'] = strtolower(trim($attachment['extension'])); - $filename = $phpbb_root_path . $config['upload_path'] . '/' . utf8_basename($attachment['physical_filename']); + $filename = utf8_basename($attachment['physical_filename']); $upload_icon = ''; @@ -1219,6 +1219,7 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a { if ($config['img_link_width'] || $config['img_link_height']) { + // $dimension = @getimagesize($filename); // If the dimensions could not be determined or the image being 0x0 we display it as a link for safety purposes @@ -1283,6 +1284,7 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a // Macromedia Flash Files case ATTACHMENT_CATEGORY_FLASH: + // list($width, $height) = @getimagesize($filename); $block_array += array( diff --git a/phpBB/includes/functions_convert.php b/phpBB/includes/functions_convert.php index 2cfbe9541d..7d0150fb5b 100644 --- a/phpBB/includes/functions_convert.php +++ b/phpBB/includes/functions_convert.php @@ -496,7 +496,7 @@ function import_attachment_files($category_name = '') $sql = 'SELECT config_value AS upload_path FROM ' . CONFIG_TABLE . " - WHERE config_name = 'upload_path'"; + WHERE config_name = 'storage\\attachment\\config\\path'"; $result = $db->sql_query($sql); $config['upload_path'] = $db->sql_fetchfield('upload_path'); $db->sql_freeresult($result); diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index b95926acb4..68980d79af 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -1442,6 +1442,8 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll_ary, &$data { global $db, $auth, $user, $config, $phpEx, $phpbb_root_path, $phpbb_container, $phpbb_dispatcher, $phpbb_log, $request; + $attachment_storage = $phpbb_container->get('storage.avatar'); + $poll = $poll_ary; $data = $data_ary; /** @@ -2030,7 +2032,7 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll_ary, &$data else { // insert attachment into db - if (!@file_exists($phpbb_root_path . $config['upload_path'] . '/' . utf8_basename($orphan_rows[$attach_row['attach_id']]['physical_filename']))) + if (!$attachment_storage->exists(utf8_basename($orphan_rows[$attach_row['attach_id']]['physical_filename']))) { continue; } diff --git a/phpBB/includes/functions_privmsgs.php b/phpBB/includes/functions_privmsgs.php index 444bf2c7e0..7a56a67d74 100644 --- a/phpBB/includes/functions_privmsgs.php +++ b/phpBB/includes/functions_privmsgs.php @@ -1614,6 +1614,8 @@ function submit_pm($mode, $subject, &$data_ary, $put_in_outbox = true) { global $db, $auth, $config, $user, $phpbb_root_path, $phpbb_container, $phpbb_dispatcher, $request; + $attachment_storage = $phpbb_container->get('storage.attachment'); + // We do not handle erasing pms here if ($mode == 'delete') { @@ -1881,7 +1883,7 @@ function submit_pm($mode, $subject, &$data_ary, $put_in_outbox = true) else { // insert attachment into db - if (!@file_exists($phpbb_root_path . $config['upload_path'] . '/' . utf8_basename($orphan_rows[$attach_row['attach_id']]['physical_filename']))) + if (!$attachment_storage->exists(utf8_basename($orphan_rows[$attach_row['attach_id']]['physical_filename']))) { continue; } diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index 922f24b5dc..a248d5a20b 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -16,7 +16,7 @@ namespace phpbb\attachment; use \phpbb\config\config; use \phpbb\db\driver\driver_interface; use \phpbb\event\dispatcher; -use \phpbb\filesystem\filesystem; +use \phpbb\storage\storage; /** * Attachment delete class @@ -32,14 +32,11 @@ class delete /** @var dispatcher */ protected $dispatcher; - /** @var filesystem */ - protected $filesystem; - /** @var resync */ protected $resync; - /** @var string phpBB root path */ - protected $phpbb_root_path; + /** @var storage */ + protected $storage; /** @var array Attachement IDs */ protected $ids; @@ -71,18 +68,15 @@ class delete * @param config $config * @param driver_interface $db * @param dispatcher $dispatcher - * @param filesystem $filesystem * @param resync $resync - * @param string $phpbb_root_path + * @param storage $storage */ - public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, filesystem $filesystem, resync $resync, $phpbb_root_path) + public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, resync $resync, $storage) { $this->config = $config; $this->db = $db; $this->dispatcher = $dispatcher; - $this->filesystem = $filesystem; - $this->resync = $resync; - $this->phpbb_root_path = $phpbb_root_path; + $this->storage = $storage; } /** @@ -161,8 +155,8 @@ class delete return 0; } - // Delete attachments from filesystem - $this->remove_from_filesystem(); + // Delete attachments from storage + $this->remove_from_storage(); // If we do not resync, we do not need to adjust any message, post, topic or user entries if (!$resync) @@ -327,9 +321,9 @@ class delete } /** - * Delete attachments from filesystem + * Delete attachments from storage */ - protected function remove_from_filesystem() + protected function remove_from_storage() { $space_removed = $files_removed = 0; @@ -388,7 +382,7 @@ class delete } /** - * Delete attachment from filesystem + * Delete attachment from storage * * @param string $filename Filename of attachment * @param string $mode Delete mode @@ -412,17 +406,16 @@ class delete } $filename = ($mode == 'thumbnail') ? 'thumb_' . utf8_basename($filename) : utf8_basename($filename); - $filepath = $this->phpbb_root_path . $this->config['upload_path'] . '/' . $filename; try { - if ($this->filesystem->exists($filepath)) + if ($this->storage->exists($filename)) { - $this->filesystem->remove($this->phpbb_root_path . $this->config['upload_path'] . '/' . $filename); + $this->storage->remove($filename); return true; } } - catch (\phpbb\filesystem\exception\filesystem_exception $exception) + catch (\phpbb\storage\exception\exception $exception) { // Fail is covered by return statement below } diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index 00f944dd5b..b707921684 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -81,9 +81,8 @@ class upload * @param dispatcher $phpbb_dispatcher * @param plupload $plupload * @param user $user - * @param $phpbb_root_path */ - public function __construct(auth $auth, service $cache, config $config, \phpbb\files\upload $files_upload, language $language, guesser $mimetype_guesser, dispatcher $phpbb_dispatcher, plupload $plupload, storage $storage, user $user, $phpbb_root_path) + public function __construct(auth $auth, service $cache, config $config, \phpbb\files\upload $files_upload, language $language, guesser $mimetype_guesser, dispatcher $phpbb_dispatcher, plupload $plupload, storage $storage, user $user) { $this->auth = $auth; $this->cache = $cache; @@ -95,7 +94,6 @@ class upload $this->plupload = $plupload; $this->storage = $storage; $this->user = $user; - $this->phpbb_root_path = $phpbb_root_path; } /** @@ -309,26 +307,6 @@ class upload */ protected function check_disk_space() { - if ($free_space = @disk_free_space($this->phpbb_root_path . $this->config['upload_path'])) - { - if ($free_space <= $this->file->get('filesize')) - { - if ($this->auth->acl_get('a_')) - { - $this->file_data['error'][] = $this->language->lang('ATTACH_DISK_FULL'); - } - else - { - $this->file_data['error'][] = $this->language->lang('ATTACH_QUOTA_REACHED'); - } - $this->file_data['post_attach'] = false; - - $this->file->remove(); - - return false; - } - } - return true; } diff --git a/tests/attachment/delete_test.php b/tests/attachment/delete_test.php index 5ea9f26ea0..73007e120d 100644 --- a/tests/attachment/delete_test.php +++ b/tests/attachment/delete_test.php @@ -30,8 +30,6 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case /** @var \phpbb\attachment\delete */ protected $attachment_delete; - protected $phpbb_root_path; - public function getDataSet() { return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/resync.xml'); @@ -54,9 +52,17 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case $this->filesystem->expects($this->any()) ->method('exists') ->willReturn(true); - $this->phpbb_root_path = $phpbb_root_path; + $local_adapter = new \phpbb\storage\adapter\local($this->filesystem, $phpbb_root_path); + $local_adapter->configure(['path' => 'files']); + $adapter_factory_mock = $this->getMockBuilder('\phpbb\storage\adapter_factory') + ->disableOriginalConstructor() + ->getMock(); + $adapter_factory_mock->expects($this->any()) + ->method('get') + ->willReturn($local_adapter); + $this->storage = new \phpbb\storage\storage($adapter_factory_mock, 'attachment'); $this->dispatcher = new \phpbb_mock_event_dispatcher(); - $this->attachment_delete = new \phpbb\attachment\delete($this->config, $this->db, $this->dispatcher, $this->filesystem, $this->resync, $phpbb_root_path); + $this->attachment_delete = new \phpbb\attachment\delete($this->config, $this->db, $this->dispatcher, $this->resync, $this->storage); } public function data_attachment_delete() @@ -121,7 +127,7 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case ->method('exists') ->willReturn($exists_success); - $this->attachment_delete = new \phpbb\attachment\delete($this->config, $this->db, $this->dispatcher, $this->filesystem, $this->resync, $this->phpbb_root_path); + $this->attachment_delete = new \phpbb\attachment\delete($this->config, $this->db, $this->dispatcher, $this->resync, $this->storage); $this->assertSame($expected, $this->attachment_delete->unlink_attachment('foobar')); } } diff --git a/tests/attachment/upload_test.php b/tests/attachment/upload_test.php index f728830a26..f9fe050c7a 100644 --- a/tests/attachment/upload_test.php +++ b/tests/attachment/upload_test.php @@ -157,8 +157,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->phpbb_dispatcher, $this->plupload, $this->storage, - $this->user, - $this->phpbb_root_path + $this->user ); } @@ -250,8 +249,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->phpbb_dispatcher, $this->plupload, $this->storage, - $this->user, - $this->phpbb_root_path + $this->user ); $filedata = $this->upload->upload('foobar', 1, true); @@ -415,8 +413,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->phpbb_dispatcher, $plupload, $this->storage, - $this->user, - $this->phpbb_root_path + $this->user ); $filedata = $this->upload->upload('foobar', 1, true, '', false, array( From a176fa56ef30483a28fa396f37a2351bfd283d97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Wed, 9 Aug 2017 14:22:03 +0200 Subject: [PATCH 04/10] [ticket/15286] Fix tests PHPBB3-15286 --- phpBB/phpbb/attachment/delete.php | 5 +-- tests/attachment/delete_test.php | 33 +++++-------------- tests/attachment/upload_test.php | 12 +++---- tests/content_visibility/delete_post_test.php | 10 +++++- 4 files changed, 25 insertions(+), 35 deletions(-) diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index a248d5a20b..5dddb06310 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -71,11 +71,12 @@ class delete * @param resync $resync * @param storage $storage */ - public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, resync $resync, $storage) + public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, resync $resync, storage $storage) { $this->config = $config; $this->db = $db; $this->dispatcher = $dispatcher; + $this->resync = $resync; $this->storage = $storage; } @@ -411,7 +412,7 @@ class delete { if ($this->storage->exists($filename)) { - $this->storage->remove($filename); + $this->storage->delete($filename); return true; } } diff --git a/tests/attachment/delete_test.php b/tests/attachment/delete_test.php index 73007e120d..f5d307c5af 100644 --- a/tests/attachment/delete_test.php +++ b/tests/attachment/delete_test.php @@ -27,6 +27,9 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case /** @var \phpbb\attachment\resync */ protected $resync; + /** @var \phpbb\storage\storage */ + protected $storage; + /** @var \phpbb\attachment\delete */ protected $attachment_delete; @@ -52,15 +55,13 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case $this->filesystem->expects($this->any()) ->method('exists') ->willReturn(true); - $local_adapter = new \phpbb\storage\adapter\local($this->filesystem, $phpbb_root_path); - $local_adapter->configure(['path' => 'files']); - $adapter_factory_mock = $this->getMockBuilder('\phpbb\storage\adapter_factory') - ->disableOriginalConstructor() - ->getMock(); + $adapter = new \phpbb\storage\adapter\local($this->filesystem, $phpbb_root_path); + $adapter->configure(['path' => 'files']); + $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) ->method('get') - ->willReturn($local_adapter); - $this->storage = new \phpbb\storage\storage($adapter_factory_mock, 'attachment'); + ->willReturn($adapter); + $this->storage = new \phpbb\storage\storage($adapter_factory_mock, ''); $this->dispatcher = new \phpbb_mock_event_dispatcher(); $this->attachment_delete = new \phpbb\attachment\delete($this->config, $this->db, $this->dispatcher, $this->resync, $this->storage); } @@ -109,24 +110,6 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case */ public function test_attachment_delete_success($remove_success, $exists_success, $expected, $throw_exception = false) { - $this->filesystem = $this->createMock('\phpbb\filesystem\filesystem', array('remove', 'exists')); - if ($throw_exception) - { - $this->filesystem->expects($this->any()) - ->method('remove') - ->willThrowException(new \phpbb\filesystem\exception\filesystem_exception);; - } - else - { - $this->filesystem->expects($this->any()) - ->method('remove') - ->willReturn($remove_success); - } - - $this->filesystem->expects($this->any()) - ->method('exists') - ->willReturn($exists_success); - $this->attachment_delete = new \phpbb\attachment\delete($this->config, $this->db, $this->dispatcher, $this->resync, $this->storage); $this->assertSame($expected, $this->attachment_delete->unlink_attachment('foobar')); } diff --git a/tests/attachment/upload_test.php b/tests/attachment/upload_test.php index f9fe050c7a..11f862c277 100644 --- a/tests/attachment/upload_test.php +++ b/tests/attachment/upload_test.php @@ -99,15 +99,13 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->mimetype_guesser = new \phpbb\mimetype\guesser($guessers); $this->plupload = new \phpbb\plupload\plupload($phpbb_root_path, $this->config, $this->request, new \phpbb\user($this->language, '\phpbb\datetime'), $this->php_ini, $this->mimetype_guesser); - $local_adapter = new \phpbb\storage\adapter\local($this->filesystem, $phpbb_root_path); - $local_adapter->configure(['path' => 'files']); - $adapter_factory_mock = $this->getMockBuilder('\phpbb\storage\adapter_factory') - ->disableOriginalConstructor() - ->getMock(); + $adapter = new \phpbb\storage\adapter\local($this->filesystem, $phpbb_root_path); + $adapter->configure(['path' => 'files']); + $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) ->method('get') - ->willReturn($local_adapter); - $this->storage = new \phpbb\storage\storage($adapter_factory_mock, 'attachment'); + ->willReturn($adapter); + $this->storage = new \phpbb\storage\storage($adapter_factory_mock, ''); $factory_mock = $this->getMockBuilder('\phpbb\files\factory') ->disableOriginalConstructor() diff --git a/tests/content_visibility/delete_post_test.php b/tests/content_visibility/delete_post_test.php index 4f978219c2..46c2f05aab 100644 --- a/tests/content_visibility/delete_post_test.php +++ b/tests/content_visibility/delete_post_test.php @@ -298,6 +298,14 @@ class phpbb_content_visibility_delete_post_test extends phpbb_database_test_case $db = $this->new_dbal(); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), $phpbb_root_path); + $adapter->configure(['path' => 'files']); + $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); + $adapter_factory_mock->expects($this->any()) + ->method('get') + ->willReturn($adapter); + $storage = new \phpbb\storage\storage($adapter_factory_mock, ''); + // Create auth mock $auth = $this->createMock('\phpbb\auth\auth'); $auth->expects($this->any()) @@ -309,7 +317,7 @@ class phpbb_content_visibility_delete_post_test extends phpbb_database_test_case $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); $lang = new \phpbb\language\language($lang_loader); $user = new \phpbb\user($lang, '\phpbb\datetime'); - $attachment_delete = new \phpbb\attachment\delete($config, $db, new \phpbb_mock_event_dispatcher(), new \phpbb\filesystem\filesystem(), new \phpbb\attachment\resync($db), $phpbb_root_path); + $attachment_delete = new \phpbb\attachment\delete($config, $db, new \phpbb_mock_event_dispatcher(), new \phpbb\attachment\resync($db), $storage); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); From 5111d8a3397dcbf2ed7d24655c94b04ae8056bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Wed, 9 Aug 2017 22:23:23 +0200 Subject: [PATCH 05/10] [ticket/15286] Update tests PHPBB3-15286 --- phpBB/includes/functions_content.php | 31 +++++++++++++------ phpBB/includes/functions_posting.php | 2 +- phpBB/phpbb/attachment/upload.php | 18 +++++------ tests/attachment/delete_test.php | 19 +++++++++++- tests/attachment/upload_test.php | 2 +- tests/content_visibility/delete_post_test.php | 2 +- tests/functions_user/delete_user_test.php | 11 ++++++- tests/notification/submit_post_base.php | 10 ++++++ tests/privmsgs/delete_user_pms_test.php | 11 ++++++- 9 files changed, 81 insertions(+), 25 deletions(-) diff --git a/phpBB/includes/functions_content.php b/phpBB/includes/functions_content.php index 6d6f09da63..97263af5f9 100644 --- a/phpBB/includes/functions_content.php +++ b/phpBB/includes/functions_content.php @@ -1086,6 +1086,9 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a global $template, $cache, $user, $phpbb_dispatcher; global $extensions, $config, $phpbb_root_path, $phpEx; + global $phpbb_container; + + $attachment_storage = $phpbb_container->get('storage.attachment'); // $compiled_attachments = array(); @@ -1219,18 +1222,16 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a { if ($config['img_link_width'] || $config['img_link_height']) { - // - $dimension = @getimagesize($filename); + try + { + $file_info = $storage_attachment->file_info($filename); - // If the dimensions could not be determined or the image being 0x0 we display it as a link for safety purposes - if ($dimension === false || empty($dimension[0]) || empty($dimension[1])) + $display_cat = ($file_info->image_width <= $config['img_link_width'] && $file_info->image_height <= $config['img_link_height']) ? ATTACHMENT_CATEGORY_IMAGE : ATTACHMENT_CATEGORY_NONE; + } + catch (\Exception $e) { $display_cat = ATTACHMENT_CATEGORY_NONE; } - else - { - $display_cat = ($dimension[0] <= $config['img_link_width'] && $dimension[1] <= $config['img_link_height']) ? ATTACHMENT_CATEGORY_IMAGE : ATTACHMENT_CATEGORY_NONE; - } } } else @@ -1284,8 +1285,18 @@ function parse_attachments($forum_id, &$message, &$attachments, &$update_count_a // Macromedia Flash Files case ATTACHMENT_CATEGORY_FLASH: - // - list($width, $height) = @getimagesize($filename); + try + { + $file_info = $storage_attachment->file_info($filename); + + $width = $file_info->image_width; + $height = $file_info->image_height; + } + catch (\Exception $e) + { + $width = 0; + $height = 0; + } $block_array += array( 'S_FLASH_FILE' => true, diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index 68980d79af..e6f328f102 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -1442,7 +1442,7 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll_ary, &$data { global $db, $auth, $user, $config, $phpEx, $phpbb_root_path, $phpbb_container, $phpbb_dispatcher, $phpbb_log, $request; - $attachment_storage = $phpbb_container->get('storage.avatar'); + $attachment_storage = $phpbb_container->get('storage.attachment'); $poll = $poll_ary; $data = $data_ary; diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index b707921684..89c2b8d804 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -161,6 +161,15 @@ class upload // Make sure the image category only holds valid images... $this->check_image($is_image); + if (count($this->file->error)) + { + $this->file->remove($this->storage); + $this->file_data['error'] = array_merge($this->file_data['error'], $this->file->error); + $this->file_data['post_attach'] = false; + + return $this->file_data; + } + $this->fill_file_data(); $filedata = $this->file_data; @@ -194,15 +203,6 @@ class upload // Only then perform additional image checks. $this->file->move_file($this->storage, false, !$is_image); - if (count($this->file->error)) - { - $this->file->remove($this->storage); - $this->file_data['error'] = array_merge($this->file_data['error'], $this->file->error); - $this->file_data['post_attach'] = false; - - return $this->file_data; - } - return $this->file_data; } diff --git a/tests/attachment/delete_test.php b/tests/attachment/delete_test.php index f5d307c5af..a22b31e1a0 100644 --- a/tests/attachment/delete_test.php +++ b/tests/attachment/delete_test.php @@ -55,7 +55,7 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case $this->filesystem->expects($this->any()) ->method('exists') ->willReturn(true); - $adapter = new \phpbb\storage\adapter\local($this->filesystem, $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) @@ -110,6 +110,23 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case */ public function test_attachment_delete_success($remove_success, $exists_success, $expected, $throw_exception = false) { + $this->storage = $this->createMock('\phpbb\storage\storage', array('delete', 'exists')); + if ($throw_exception) + { + $this->storage->expects($this->any()) + ->method('delete') + ->willThrowException(new \phpbb\storage\exception\exception);; + } + else + { + $this->storage->expects($this->any()) + ->method('delete') + ->willReturn($remove_success); + } + $this->storage->expects($this->any()) + ->method('exists') + ->willReturn($exists_success); + $this->attachment_delete = new \phpbb\attachment\delete($this->config, $this->db, $this->dispatcher, $this->resync, $this->storage); $this->assertSame($expected, $this->attachment_delete->unlink_attachment('foobar')); } diff --git a/tests/attachment/upload_test.php b/tests/attachment/upload_test.php index 11f862c277..0cb8fcc450 100644 --- a/tests/attachment/upload_test.php +++ b/tests/attachment/upload_test.php @@ -99,7 +99,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->mimetype_guesser = new \phpbb\mimetype\guesser($guessers); $this->plupload = new \phpbb\plupload\plupload($phpbb_root_path, $this->config, $this->request, new \phpbb\user($this->language, '\phpbb\datetime'), $this->php_ini, $this->mimetype_guesser); - $adapter = new \phpbb\storage\adapter\local($this->filesystem, $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) diff --git a/tests/content_visibility/delete_post_test.php b/tests/content_visibility/delete_post_test.php index 46c2f05aab..054d5d6877 100644 --- a/tests/content_visibility/delete_post_test.php +++ b/tests/content_visibility/delete_post_test.php @@ -298,7 +298,7 @@ class phpbb_content_visibility_delete_post_test extends phpbb_database_test_case $db = $this->new_dbal(); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) diff --git a/tests/functions_user/delete_user_test.php b/tests/functions_user/delete_user_test.php index 25042d9f1b..3f02bb53a5 100644 --- a/tests/functions_user/delete_user_test.php +++ b/tests/functions_user/delete_user_test.php @@ -34,8 +34,17 @@ class phpbb_functions_user_delete_user_test extends phpbb_database_test_case $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); $phpbb_container = new phpbb_mock_container_builder(); $phpbb_container->set('notification_manager', new phpbb_mock_notification_manager()); + + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter->configure(['path' => 'files']); + $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); + $adapter_factory_mock->expects($this->any()) + ->method('get') + ->willReturn($adapter); + $storage = new \phpbb\storage\storage($adapter_factory_mock, ''); + // Works as a workaround for tests - $phpbb_container->set('attachment.manager', new \phpbb\attachment\delete($config, $db, new \phpbb_mock_event_dispatcher(), new \phpbb\filesystem\filesystem(), new \phpbb\attachment\resync($db), $phpbb_root_path)); + $phpbb_container->set('attachment.manager', new \phpbb\attachment\delete($config, $db, new \phpbb_mock_event_dispatcher(), new \phpbb\attachment\resync($db), $storage)); $phpbb_container->set( 'auth.provider.db', new phpbb_mock_auth_provider() diff --git a/tests/notification/submit_post_base.php b/tests/notification/submit_post_base.php index baa90f29b1..5a25682b73 100644 --- a/tests/notification/submit_post_base.php +++ b/tests/notification/submit_post_base.php @@ -91,6 +91,15 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c // Language $lang = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); + // Storage + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter->configure(['path' => 'files']); + $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); + $adapter_factory_mock->expects($this->any()) + ->method('get') + ->willReturn($adapter); + $storage = new \phpbb\storage\storage($adapter_factory_mock, ''); + // User $user = $this->createMock('\phpbb\user', array(), array( $lang, @@ -125,6 +134,7 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c $phpbb_container->set('cache', $cache); $phpbb_container->set('text_formatter.utils', new \phpbb\textformatter\s9e\utils()); $phpbb_container->set('dispatcher', $phpbb_dispatcher); + $phpbb_container->set('storage.attachment', $storage); $phpbb_container->setParameter('core.root_path', $phpbb_root_path); $phpbb_container->setParameter('core.php_ext', $phpEx); $phpbb_container->setParameter('tables.notifications', 'phpbb_notifications'); diff --git a/tests/privmsgs/delete_user_pms_test.php b/tests/privmsgs/delete_user_pms_test.php index 9d6ba7a917..0c3609207c 100644 --- a/tests/privmsgs/delete_user_pms_test.php +++ b/tests/privmsgs/delete_user_pms_test.php @@ -91,8 +91,17 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case $phpbb_container = new phpbb_mock_container_builder(); $phpbb_container->set('notification_manager', new phpbb_mock_notification_manager()); + + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter->configure(['path' => 'files']); + $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); + $adapter_factory_mock->expects($this->any()) + ->method('get') + ->willReturn($adapter); + $storage = new \phpbb\storage\storage($adapter_factory_mock, ''); + // Works as a workaround for tests - $phpbb_container->set('attachment.manager', new \phpbb\attachment\delete(new \phpbb\config\config(array()), $db, new \phpbb_mock_event_dispatcher(), new \phpbb\filesystem\filesystem(), new \phpbb\attachment\resync($db), $phpbb_root_path)); + $phpbb_container->set('attachment.manager', new \phpbb\attachment\delete(new \phpbb\config\config(array()), $db, new \phpbb_mock_event_dispatcher(), new \phpbb\attachment\resync($db), $storage)); phpbb_delete_user_pms($delete_user); From 3c295abd66121d84a10487bca91ceea10713ea78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Fri, 11 Aug 2017 21:55:46 +0200 Subject: [PATCH 06/10] [ticket/15286] Remove download_mode PHPBB3-15286 --- phpBB/download/file.php | 24 +----------- phpBB/includes/acp/acp_attachments.php | 5 --- phpBB/includes/constants.php | 5 --- phpBB/install/convertors/convert_phpbb20.php | 1 - phpBB/install/schemas/schema_data.sql | 12 +++--- phpBB/phpbb/cache/service.php | 1 - .../v330/remove_attachment_download_mode.php | 39 +++++++++++++++++++ tests/attachment/fixtures/resync.xml | 2 - 8 files changed, 47 insertions(+), 42 deletions(-) create mode 100644 phpBB/phpbb/db/migration/data/v330/remove_attachment_download_mode.php diff --git a/phpBB/download/file.php b/phpBB/download/file.php index 23658db5d3..dd9df06f08 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -254,7 +254,6 @@ else } } - $download_mode = (int) $extensions[$attachment['extension']]['download_mode']; $display_cat = $extensions[$attachment['extension']]['display_cat']; if (($display_cat == ATTACHMENT_CATEGORY_IMAGE || $display_cat == ATTACHMENT_CATEGORY_THUMB) && !$user->optionget('viewimg')) @@ -274,7 +273,6 @@ else * @var int attach_id The attachment ID * @var array attachment Array with attachment data * @var int display_cat Attachment category - * @var int download_mode File extension specific download mode * @var array extensions Array with file extensions data * @var string mode Download mode * @var bool thumbnail Flag indicating if the file is a thumbnail @@ -285,7 +283,6 @@ else 'attach_id', 'attachment', 'display_cat', - 'download_mode', 'extensions', 'mode', 'thumbnail', @@ -309,24 +306,7 @@ else } else { - // - // Determine the 'presenting'-method - if ($download_mode == PHYSICAL_LINK) - { - // This presenting method should no longer be used - if (!@is_dir($phpbb_root_path . $config['upload_path'])) - { - send_status_line(500, 'Internal Server Error'); - trigger_error($user->lang['PHYSICAL_DOWNLOAD_NOT_POSSIBLE']); - } - - redirect($phpbb_root_path . $config['upload_path'] . '/' . $attachment['physical_filename']); - file_gc(); - } - else - { - send_file_to_browser($attachment, $display_cat); - file_gc(); - } + send_file_to_browser($attachment, $display_cat); + file_gc(); } } diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index c735c6265b..acb5d7d6e5 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -586,11 +586,6 @@ class acp_attachments 'allow_in_pm' => ($allow_in_pm) ? 1 : 0, ); - if ($action == 'add') - { - $group_ary['download_mode'] = INLINE_LINK; - } - $sql = ($action == 'add') ? 'INSERT INTO ' . EXTENSION_GROUPS_TABLE . ' ' : 'UPDATE ' . EXTENSION_GROUPS_TABLE . ' SET '; $sql .= $db->sql_build_array((($action == 'add') ? 'INSERT' : 'UPDATE'), $group_ary); $sql .= ($action == 'edit') ? " WHERE group_id = $group_id" : ''; diff --git a/phpBB/includes/constants.php b/phpBB/includes/constants.php index 6460a1ae7c..2fa11e4d86 100644 --- a/phpBB/includes/constants.php +++ b/phpBB/includes/constants.php @@ -157,11 +157,6 @@ define('FULL_FOLDER_NONE', -3); define('FULL_FOLDER_DELETE', -2); define('FULL_FOLDER_HOLD', -1); -// Download Modes - Attachments -define('INLINE_LINK', 1); -// This mode is only used internally to allow modders extending the attachment functionality -define('PHYSICAL_LINK', 2); - // Confirm types define('CONFIRM_REG', 1); define('CONFIRM_LOGIN', 2); diff --git a/phpBB/install/convertors/convert_phpbb20.php b/phpBB/install/convertors/convert_phpbb20.php index d0885dc620..60f3550651 100644 --- a/phpBB/install/convertors/convert_phpbb20.php +++ b/phpBB/install/convertors/convert_phpbb20.php @@ -439,7 +439,6 @@ if (!$get_info) array('group_name', 'extension_groups.group_name', array('function1' => 'phpbb_set_encoding', 'function2' => 'utf8_htmlspecialchars')), array('cat_id', 'extension_groups.cat_id', 'phpbb_attachment_category'), array('allow_group', 'extension_groups.allow_group', ''), - array('download_mode', 1, ''), array('upload_icon', '', ''), array('max_filesize', 'extension_groups.max_filesize', ''), array('allowed_forums', 'extension_groups.forum_permissions', 'phpbb_attachment_forum_perms'), diff --git a/phpBB/install/schemas/schema_data.sql b/phpBB/install/schemas/schema_data.sql index c802480eea..c6a60af67b 100644 --- a/phpBB/install/schemas/schema_data.sql +++ b/phpBB/install/schemas/schema_data.sql @@ -730,12 +730,12 @@ INSERT INTO phpbb_reports_reasons (reason_title, reason_description, reason_orde INSERT INTO phpbb_reports_reasons (reason_title, reason_description, reason_order) VALUES ('other', '{L_REPORT_OTHER}', 4); # -- extension_groups -INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, download_mode, upload_icon, max_filesize, allowed_forums) VALUES ('IMAGES', 1, 1, 1, '', 0, ''); -INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, download_mode, upload_icon, max_filesize, allowed_forums) VALUES ('ARCHIVES', 0, 1, 1, '', 0, ''); -INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, download_mode, upload_icon, max_filesize, allowed_forums) VALUES ('PLAIN_TEXT', 0, 0, 1, '', 0, ''); -INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, download_mode, upload_icon, max_filesize, allowed_forums) VALUES ('DOCUMENTS', 0, 0, 1, '', 0, ''); -INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, download_mode, upload_icon, max_filesize, allowed_forums) VALUES ('FLASH_FILES', 5, 0, 1, '', 0, ''); -INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, download_mode, upload_icon, max_filesize, allowed_forums) VALUES ('DOWNLOADABLE_FILES', 0, 0, 1, '', 0, ''); +INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, upload_icon, max_filesize, allowed_forums) VALUES ('IMAGES', 1, 1, '', 0, ''); +INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, upload_icon, max_filesize, allowed_forums) VALUES ('ARCHIVES', 0, 1, '', 0, ''); +INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, upload_icon, max_filesize, allowed_forums) VALUES ('PLAIN_TEXT', 0, 0, '', 0, ''); +INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, upload_icon, max_filesize, allowed_forums) VALUES ('DOCUMENTS', 0, 0, '', 0, ''); +INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, upload_icon, max_filesize, allowed_forums) VALUES ('FLASH_FILES', 5, 0, '', 0, ''); +INSERT INTO phpbb_extension_groups (group_name, cat_id, allow_group, upload_icon, max_filesize, allowed_forums) VALUES ('DOWNLOADABLE_FILES', 0, 0, '', 0, ''); # -- extensions INSERT INTO phpbb_extensions (group_id, extension) VALUES (1, 'gif'); diff --git a/phpBB/phpbb/cache/service.php b/phpBB/phpbb/cache/service.php index 502ae27625..2d4e55bfbb 100644 --- a/phpBB/phpbb/cache/service.php +++ b/phpBB/phpbb/cache/service.php @@ -214,7 +214,6 @@ class service $extensions[$extension] = array( 'display_cat' => (int) $row['cat_id'], - 'download_mode' => (int) $row['download_mode'], 'upload_icon' => trim($row['upload_icon']), 'max_filesize' => (int) $row['max_filesize'], 'allow_group' => $row['allow_group'], diff --git a/phpBB/phpbb/db/migration/data/v330/remove_attachment_download_mode.php b/phpBB/phpbb/db/migration/data/v330/remove_attachment_download_mode.php new file mode 100644 index 0000000000..f943d7fa25 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v330/remove_attachment_download_mode.php @@ -0,0 +1,39 @@ +<?php +/** +* +* This file is part of the phpBB Forum Software package. +* +* @copyright (c) phpBB Limited <https://www.phpbb.com> +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace phpbb\db\migration\data\v330; + +class remove_attachment_download_mode extends \phpbb\db\migration\migration +{ + public function update_data() + { + return array( + 'drop_columns' => array( + $this->table_prefix . 'extension_groups' => array( + 'download_mode', + ), + ), + ); + } + + public function revert_schema() + { + return array( + 'add_columns' => array( + $this->table_prefix . 'extension_groups' => array( + 'download_mode' => array('BOOL', '1'), + ), + ), + ); + } +} diff --git a/tests/attachment/fixtures/resync.xml b/tests/attachment/fixtures/resync.xml index af04701b4a..b06d2db3e2 100644 --- a/tests/attachment/fixtures/resync.xml +++ b/tests/attachment/fixtures/resync.xml @@ -58,7 +58,6 @@ <table name="phpbb_extension_groups"> <column>cat_id</column> <column>group_id</column> - <column>download_mode</column> <column>upload_icon</column> <column>max_filesize</column> <column>allow_group</column> @@ -66,7 +65,6 @@ <column>allowed_forums</column> <column>group_name</column> <row> - <value>1</value> <value>1</value> <value>1</value> <value> </value> From 6bd01d1506a2e3cffe7d264705ee1128791ae68a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Sun, 20 Aug 2017 21:44:40 +0200 Subject: [PATCH 07/10] [ticket/15286] Add event to allow redirects PHPBB3-15286 --- phpBB/download/file.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/phpBB/download/file.php b/phpBB/download/file.php index dd9df06f08..e0e55a5b95 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -266,6 +266,8 @@ else $display_cat = ATTACHMENT_CATEGORY_NONE; } + $redirect = ''; + /** * Event to modify data before sending file to browser * @@ -278,6 +280,7 @@ else * @var bool thumbnail Flag indicating if the file is a thumbnail * @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', @@ -286,6 +289,7 @@ else 'extensions', 'mode', 'thumbnail', + 'redirect', ); extract($phpbb_dispatcher->trigger_event('core.download_file_send_to_browser_before', compact($vars))); @@ -306,7 +310,14 @@ else } else { - send_file_to_browser($attachment, $display_cat); + if (!empty($redirect)) + { + redirect($redirect, false, true); + } + else { + send_file_to_browser($attachment, $display_cat); + } + file_gc(); } } From f80c59cb05f8b2de609fdf227416b5c6526efb79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Tue, 31 Oct 2017 17:35:25 +0100 Subject: [PATCH 08/10] [ticket/15286] Fix tests after rebase PHPBB3-15286 --- phpBB/download/file.php | 4 +++- phpBB/phpbb/attachment/upload.php | 9 +++++++++ tests/attachment/delete_test.php | 2 +- tests/attachment/upload_test.php | 2 +- tests/content_visibility/delete_post_test.php | 2 +- tests/functions_user/delete_user_test.php | 2 +- tests/notification/submit_post_base.php | 2 +- tests/privmsgs/delete_user_pms_test.php | 2 +- 8 files changed, 18 insertions(+), 7 deletions(-) diff --git a/phpBB/download/file.php b/phpBB/download/file.php index e0e55a5b95..e03a1bba35 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -278,6 +278,7 @@ else * @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 @@ -314,7 +315,8 @@ else { redirect($redirect, false, true); } - else { + else + { send_file_to_browser($attachment, $display_cat); } diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index 89c2b8d804..8d5d5b1a12 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -203,6 +203,15 @@ class upload // Only then perform additional image checks. $this->file->move_file($this->storage, false, !$is_image); + if (count($this->file->error)) + { + $this->file->remove($this->storage); + $this->file_data['error'] = array_merge($this->file_data['error'], $this->file->error); + $this->file_data['post_attach'] = false; + + return $this->file_data; + } + return $this->file_data; } diff --git a/tests/attachment/delete_test.php b/tests/attachment/delete_test.php index a22b31e1a0..e539edd30d 100644 --- a/tests/attachment/delete_test.php +++ b/tests/attachment/delete_test.php @@ -55,7 +55,7 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case $this->filesystem->expects($this->any()) ->method('exists') ->willReturn(true); - $adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) diff --git a/tests/attachment/upload_test.php b/tests/attachment/upload_test.php index 0cb8fcc450..2d577db107 100644 --- a/tests/attachment/upload_test.php +++ b/tests/attachment/upload_test.php @@ -99,7 +99,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->mimetype_guesser = new \phpbb\mimetype\guesser($guessers); $this->plupload = new \phpbb\plupload\plupload($phpbb_root_path, $this->config, $this->request, new \phpbb\user($this->language, '\phpbb\datetime'), $this->php_ini, $this->mimetype_guesser); - $adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) diff --git a/tests/content_visibility/delete_post_test.php b/tests/content_visibility/delete_post_test.php index 054d5d6877..d4feeaad22 100644 --- a/tests/content_visibility/delete_post_test.php +++ b/tests/content_visibility/delete_post_test.php @@ -298,7 +298,7 @@ class phpbb_content_visibility_delete_post_test extends phpbb_database_test_case $db = $this->new_dbal(); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) diff --git a/tests/functions_user/delete_user_test.php b/tests/functions_user/delete_user_test.php index 3f02bb53a5..b51dabbba0 100644 --- a/tests/functions_user/delete_user_test.php +++ b/tests/functions_user/delete_user_test.php @@ -35,7 +35,7 @@ class phpbb_functions_user_delete_user_test extends phpbb_database_test_case $phpbb_container = new phpbb_mock_container_builder(); $phpbb_container->set('notification_manager', new phpbb_mock_notification_manager()); - $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) diff --git a/tests/notification/submit_post_base.php b/tests/notification/submit_post_base.php index 5a25682b73..d882c106ac 100644 --- a/tests/notification/submit_post_base.php +++ b/tests/notification/submit_post_base.php @@ -92,7 +92,7 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c $lang = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); // Storage - $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) diff --git a/tests/privmsgs/delete_user_pms_test.php b/tests/privmsgs/delete_user_pms_test.php index 0c3609207c..b35510113a 100644 --- a/tests/privmsgs/delete_user_pms_test.php +++ b/tests/privmsgs/delete_user_pms_test.php @@ -92,7 +92,7 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case $phpbb_container = new phpbb_mock_container_builder(); $phpbb_container->set('notification_manager', new phpbb_mock_notification_manager()); - $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), $phpbb_root_path); + $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $adapter->configure(['path' => 'files']); $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); $adapter_factory_mock->expects($this->any()) From ed2165342f86f74907bca7759f3909a841b430d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Thu, 8 Feb 2018 19:41:27 +0100 Subject: [PATCH 09/10] [ticket/15286] Remove thumbnail if there is an error PHPBB3-15286 --- phpBB/phpbb/attachment/upload.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index 8d5d5b1a12..82a7578380 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -206,6 +206,14 @@ class upload if (count($this->file->error)) { $this->file->remove($this->storage); + + // Remove thumbnail if exists + $thumbnail_file = 'thumb_' . $this->file->get('realname'); + if ($this->storage->exists($thumbnail_file)) + { + $this->storage->delete($thumbnail_file); + } + $this->file_data['error'] = array_merge($this->file_data['error'], $this->file->error); $this->file_data['post_attach'] = false; From 246cc015079ba7b8a0c4841f89de460abfa5a9fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Tue, 8 May 2018 14:17:17 +0200 Subject: [PATCH 10/10] [ticket/15286] Remove double semicolon PHPBB3-15286 --- tests/attachment/delete_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/attachment/delete_test.php b/tests/attachment/delete_test.php index e539edd30d..cefe076bbf 100644 --- a/tests/attachment/delete_test.php +++ b/tests/attachment/delete_test.php @@ -115,7 +115,7 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case { $this->storage->expects($this->any()) ->method('delete') - ->willThrowException(new \phpbb\storage\exception\exception);; + ->willThrowException(new \phpbb\storage\exception\exception); } else {