From e50718008a44953181e133de1e37304ed6a5994f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 19 Sep 2015 16:39:17 +0200 Subject: [ticket/14168] Add attachment upload class PHPBB3-14168 --- phpBB/phpbb/attachment/upload.php | 261 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 phpBB/phpbb/attachment/upload.php (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php new file mode 100644 index 0000000000..a175dee0b3 --- /dev/null +++ b/phpBB/phpbb/attachment/upload.php @@ -0,0 +1,261 @@ + + * @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\attachment; + +use phpbb\auth\auth; +use \phpbb\cache\service; +use \phpbb\config\config; +use \phpbb\event\dispatcher; +use \phpbb\language\language; +use \phpbb\mimetype\guesser; +use \phpbb\plupload\plupload; +use \phpbb\user; + +/** + * Attachment upload class + */ + +class upload +{ + /** @var auth */ + protected $auth; + + /** @var service */ + protected $cache; + + /** @var config */ + protected $config; + + /** @var \phpbb\files\upload Upload class */ + protected $files_upload; + + /** @var \phpbb\language\language */ + protected $language; + + /** @var guesser Mimetype guesser */ + protected $mimetype_guesser; + + /** @var dispatcher */ + protected $phpbb_dispatcher; + + /** @var plupload Plupload */ + protected $plupload; + + /** @var user */ + protected $user; + + /** + * Constructor for attachments upload class + * + * @param auth $auth + * @param service $cache + * @param config $config + * @param \phpbb\files\upload $files_upload + * @param language $language + * @param guesser $mimetype_guesser + * @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, user $user, $phpbb_root_path) + { + $this->auth = $auth; + $this->cache = $cache; + $this->config = $config; + $this->files_upload = $files_upload; + $this->language = $language; + $this->mimetype_guesser = $mimetype_guesser; + $this->phpbb_dispatcher = $phpbb_dispatcher; + $this->plupload = $plupload; + $this->user = $user; + $this->phpbb_root_path = $phpbb_root_path; + } + + /** + * Upload Attachment - filedata is generated here + * Uses upload class + * + * @param string $form_name The form name of the file upload input + * @param int $forum_id The id of the forum + * @param bool $local Whether the file is local or not + * @param string $local_storage The path to the local file + * @param bool $is_message Whether it is a PM or not + * @param array $local_filedata An file data object created for the local file + * + * @return object filespec + */ + public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = false) + { + $filedata = array( + 'error' => array() + ); + + if ($this->config['check_attachment_content'] && isset($this->config['mime_triggers'])) + { + $this->files_upload->set_disallowed_content(explode('|', $this->config['mime_triggers'])); + } + else if (!$this->config['check_attachment_content']) + { + $this->files_upload->set_disallowed_content(array()); + } + + $filedata['post_attach'] = $local || $this->files_upload->is_valid($form_name); + + if (!$filedata['post_attach']) + { + $filedata['error'][] = $this->user->lang['NO_UPLOAD_FORM_FOUND']; + return $filedata; + } + + $extensions = $this->cache->obtain_attach_extensions((($is_message) ? false : (int) $forum_id)); + $this->files_upload->set_allowed_extensions(array_keys($extensions['_allowed_'])); + + /** @var \phpbb\files\filespec $file */ + $file = ($local) ? $this->files_upload->handle_upload('files.types.local', $local_storage, $local_filedata) : $this->files_upload->handle_upload('files.types.form', $form_name); + + if ($file->init_error()) + { + $filedata['post_attach'] = false; + return $filedata; + } + + // Whether the uploaded file is in the image category + $is_image = (isset($extensions[$file->get('extension')]['display_cat'])) ? $extensions[$file->get('extension')]['display_cat'] == ATTACHMENT_CATEGORY_IMAGE : false; + + if (!$this->auth->acl_get('a_') && !$this->auth->acl_get('m_', $forum_id)) + { + // Check Image Size, if it is an image + if ($is_image) + { + $file->upload->set_allowed_dimensions(0, 0, $this->config['img_max_width'], $this->config['img_max_height']); + } + + // Admins and mods are allowed to exceed the allowed filesize + if (!empty($extensions[$file->get('extension')]['max_filesize'])) + { + $allowed_filesize = $extensions[$file->get('extension')]['max_filesize']; + } + else + { + $allowed_filesize = ($is_message) ? $this->config['max_filesize_pm'] : $this->config['max_filesize']; + } + + $file->upload->set_max_filesize($allowed_filesize); + } + + $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. + $file->move_file($this->config['upload_path'], false, !$is_image); + + // Do we have to create a thumbnail? + $filedata['thumbnail'] = ($is_image && $this->config['img_create_thumbnail']) ? 1 : 0; + + if (sizeof($file->error)) + { + $file->remove(); + $filedata['error'] = array_merge($filedata['error'], $file->error); + $filedata['post_attach'] = false; + + return $filedata; + } + + // Make sure the image category only holds valid images... + if ($is_image && !$file->is_image()) + { + $file->remove(); + + if ($this->plupload && $this->plupload->is_active()) + { + $this->plupload->emit_error(104, 'ATTACHED_IMAGE_NOT_IMAGE'); + } + + // If this error occurs a user tried to exploit an IE Bug by renaming extensions + // Since the image category is displaying content inline we need to catch this. + trigger_error($this->user->lang['ATTACHED_IMAGE_NOT_IMAGE']); + } + + $filedata['filesize'] = $file->get('filesize'); + $filedata['mimetype'] = $file->get('mimetype'); + $filedata['extension'] = $file->get('extension'); + $filedata['physical_filename'] = $file->get('realname'); + $filedata['real_filename'] = $file->get('uploadname'); + $filedata['filetime'] = time(); + + /** + * Event to modify uploaded file before submit to the post + * + * @event core.modify_uploaded_file + * @var array filedata Array containing uploaded file data + * @var bool is_image Flag indicating if the file is an image + * @since 3.1.0-RC3 + */ + $vars = array( + 'filedata', + 'is_image', + ); + extract($this->phpbb_dispatcher->trigger_event('core.modify_uploaded_file', compact($vars))); + + // Check our complete quota + if ($this->config['attachment_quota']) + { + if (intval($this->config['upload_dir_size']) + $file->get('filesize') > $this->config['attachment_quota']) + { + $filedata['error'][] = $this->user->lang['ATTACH_QUOTA_REACHED']; + $filedata['post_attach'] = false; + + $file->remove(); + + return $filedata; + } + } + + // Check free disk space + if ($free_space = @disk_free_space($this->phpbb_root_path . $this->config['upload_path'])) + { + if ($free_space <= $file->get('filesize')) + { + if ($this->auth->acl_get('a_')) + { + $filedata['error'][] = $this->user->lang['ATTACH_DISK_FULL']; + } + else + { + $filedata['error'][] = $this->user->lang['ATTACH_QUOTA_REACHED']; + } + $filedata['post_attach'] = false; + + $file->remove(); + + return $filedata; + } + } + + // Create Thumbnail + if ($filedata['thumbnail']) + { + $source = $file->get('destination_file'); + $destination = $file->get('destination_path') . '/thumb_' . $file->get('realname'); + + if (!create_thumbnail($source, $destination, $file->get('mimetype'))) + { + $filedata['thumbnail'] = 0; + } + } + + return $filedata; + } +} -- cgit v1.2.1 From 07c8e21e8033f35483b5c96653f616b6f094138e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 19 Sep 2015 16:45:51 +0200 Subject: [ticket/14168] Use language class and fix incorrect docblock PHPBB3-14168 --- phpBB/phpbb/attachment/upload.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index a175dee0b3..c7de323699 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -96,7 +96,7 @@ class upload * * @return object filespec */ - public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = false) + public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = array()) { $filedata = array( 'error' => array() @@ -115,7 +115,7 @@ class upload if (!$filedata['post_attach']) { - $filedata['error'][] = $this->user->lang['NO_UPLOAD_FORM_FOUND']; + $filedata['error'][] = $this->language->lang('NO_UPLOAD_FORM_FOUND'); return $filedata; } @@ -185,7 +185,7 @@ class upload // If this error occurs a user tried to exploit an IE Bug by renaming extensions // Since the image category is displaying content inline we need to catch this. - trigger_error($this->user->lang['ATTACHED_IMAGE_NOT_IMAGE']); + trigger_error($this->language->lang('ATTACHED_IMAGE_NOT_IMAGE')); } $filedata['filesize'] = $file->get('filesize'); @@ -214,7 +214,7 @@ class upload { if (intval($this->config['upload_dir_size']) + $file->get('filesize') > $this->config['attachment_quota']) { - $filedata['error'][] = $this->user->lang['ATTACH_QUOTA_REACHED']; + $filedata['error'][] = $this->language->lang('ATTACH_QUOTA_REACHED'); $filedata['post_attach'] = false; $file->remove(); @@ -230,11 +230,11 @@ class upload { if ($this->auth->acl_get('a_')) { - $filedata['error'][] = $this->user->lang['ATTACH_DISK_FULL']; + $filedata['error'][] = $this->language->lang('ATTACH_DISK_FULL'); } else { - $filedata['error'][] = $this->user->lang['ATTACH_QUOTA_REACHED']; + $filedata['error'][] = $this->language->lang('ATTACH_QUOTA_REACHED'); } $filedata['post_attach'] = false; -- cgit v1.2.1 From 6c1cd26b7a3602932f3571eebcd5a21d1ce8ae51 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 19 Sep 2015 16:50:21 +0200 Subject: [ticket/14168] Split thumbnail creation to separate method PHPBB3-14168 --- phpBB/phpbb/attachment/upload.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index c7de323699..fa6fbad60d 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -245,6 +245,21 @@ class upload } // Create Thumbnail + $filedata = $this->create_thumbnail($file, $filedata); + + return $filedata; + } + + /** + * Create thumbnail for file if necessary + * + * @param \phpbb\files\filespec $file + * @param array $filedata File's filedata + * + * @return array Updated $filedata + */ + protected function create_thumbnail(\phpbb\files\filespec $file, $filedata) + { if ($filedata['thumbnail']) { $source = $file->get('destination_file'); -- cgit v1.2.1 From e10aa45470889cba22b51ac4f43582ecfa131310 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 20 Sep 2015 00:51:51 +0200 Subject: [ticket/14168] Further split up attachment upload class PHPBB3-14168 --- phpBB/phpbb/attachment/upload.php | 130 +++++++++++++++++++++++--------------- 1 file changed, 78 insertions(+), 52 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index fa6fbad60d..2880e9e42e 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -55,6 +55,12 @@ class upload /** @var user */ protected $user; + /** @var \phpbb\files\filespec Current filespec instance */ + private $file; + + /** @var array Extensions array */ + private $extensions; + /** * Constructor for attachments upload class * @@ -102,14 +108,7 @@ class upload 'error' => array() ); - if ($this->config['check_attachment_content'] && isset($this->config['mime_triggers'])) - { - $this->files_upload->set_disallowed_content(explode('|', $this->config['mime_triggers'])); - } - else if (!$this->config['check_attachment_content']) - { - $this->files_upload->set_disallowed_content(array()); - } + $this->init_files_upload($forum_id, $is_message); $filedata['post_attach'] = $local || $this->files_upload->is_valid($form_name); @@ -119,80 +118,64 @@ class upload return $filedata; } - $extensions = $this->cache->obtain_attach_extensions((($is_message) ? false : (int) $forum_id)); - $this->files_upload->set_allowed_extensions(array_keys($extensions['_allowed_'])); - - /** @var \phpbb\files\filespec $file */ - $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', $local_storage, $local_filedata) : $this->files_upload->handle_upload('files.types.form', $form_name); - if ($file->init_error()) + if ($this->file->init_error()) { $filedata['post_attach'] = false; return $filedata; } // Whether the uploaded file is in the image category - $is_image = (isset($extensions[$file->get('extension')]['display_cat'])) ? $extensions[$file->get('extension')]['display_cat'] == ATTACHMENT_CATEGORY_IMAGE : false; + $is_image = (isset($this->extensions[$this->file->get('extension')]['display_cat'])) ? $this->extensions[$this->file->get('extension')]['display_cat'] == ATTACHMENT_CATEGORY_IMAGE : false; if (!$this->auth->acl_get('a_') && !$this->auth->acl_get('m_', $forum_id)) { // Check Image Size, if it is an image if ($is_image) { - $file->upload->set_allowed_dimensions(0, 0, $this->config['img_max_width'], $this->config['img_max_height']); + $this->file->upload->set_allowed_dimensions(0, 0, $this->config['img_max_width'], $this->config['img_max_height']); } // Admins and mods are allowed to exceed the allowed filesize - if (!empty($extensions[$file->get('extension')]['max_filesize'])) + if (!empty($this->extensions[$this->file->get('extension')]['max_filesize'])) { - $allowed_filesize = $extensions[$file->get('extension')]['max_filesize']; + $allowed_filesize = $this->extensions[$this->file->get('extension')]['max_filesize']; } else { $allowed_filesize = ($is_message) ? $this->config['max_filesize_pm'] : $this->config['max_filesize']; } - $file->upload->set_max_filesize($allowed_filesize); + $this->file->upload->set_max_filesize($allowed_filesize); } - $file->clean_filename('unique', $this->user->data['user_id'] . '_'); + $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. - $file->move_file($this->config['upload_path'], false, !$is_image); + $this->file->move_file($this->config['upload_path'], false, !$is_image); // Do we have to create a thumbnail? $filedata['thumbnail'] = ($is_image && $this->config['img_create_thumbnail']) ? 1 : 0; - if (sizeof($file->error)) + if (sizeof($this->file->error)) { - $file->remove(); - $filedata['error'] = array_merge($filedata['error'], $file->error); + $this->file->remove(); + $filedata['error'] = array_merge($filedata['error'], $this->file->error); $filedata['post_attach'] = false; return $filedata; } // Make sure the image category only holds valid images... - if ($is_image && !$file->is_image()) - { - $file->remove(); - - if ($this->plupload && $this->plupload->is_active()) - { - $this->plupload->emit_error(104, 'ATTACHED_IMAGE_NOT_IMAGE'); - } - - // If this error occurs a user tried to exploit an IE Bug by renaming extensions - // Since the image category is displaying content inline we need to catch this. - trigger_error($this->language->lang('ATTACHED_IMAGE_NOT_IMAGE')); - } + $this->check_image($is_image); - $filedata['filesize'] = $file->get('filesize'); - $filedata['mimetype'] = $file->get('mimetype'); - $filedata['extension'] = $file->get('extension'); - $filedata['physical_filename'] = $file->get('realname'); - $filedata['real_filename'] = $file->get('uploadname'); + $filedata['filesize'] = $this->file->get('filesize'); + $filedata['mimetype'] = $this->file->get('mimetype'); + $filedata['extension'] = $this->file->get('extension'); + $filedata['physical_filename'] = $this->file->get('realname'); + $filedata['real_filename'] = $this->file->get('uploadname'); $filedata['filetime'] = time(); /** @@ -212,12 +195,12 @@ class upload // Check our complete quota if ($this->config['attachment_quota']) { - if (intval($this->config['upload_dir_size']) + $file->get('filesize') > $this->config['attachment_quota']) + if (intval($this->config['upload_dir_size']) + $this->file->get('filesize') > $this->config['attachment_quota']) { $filedata['error'][] = $this->language->lang('ATTACH_QUOTA_REACHED'); $filedata['post_attach'] = false; - $file->remove(); + $this->file->remove(); return $filedata; } @@ -226,7 +209,7 @@ class upload // Check free disk space if ($free_space = @disk_free_space($this->phpbb_root_path . $this->config['upload_path'])) { - if ($free_space <= $file->get('filesize')) + if ($free_space <= $this->file->get('filesize')) { if ($this->auth->acl_get('a_')) { @@ -238,14 +221,14 @@ class upload } $filedata['post_attach'] = false; - $file->remove(); + $this->file->remove(); return $filedata; } } // Create Thumbnail - $filedata = $this->create_thumbnail($file, $filedata); + $filedata = $this->create_thumbnail($filedata); return $filedata; } @@ -253,19 +236,18 @@ class upload /** * Create thumbnail for file if necessary * - * @param \phpbb\files\filespec $file * @param array $filedata File's filedata * * @return array Updated $filedata */ - protected function create_thumbnail(\phpbb\files\filespec $file, $filedata) + protected function create_thumbnail($filedata) { if ($filedata['thumbnail']) { - $source = $file->get('destination_file'); - $destination = $file->get('destination_path') . '/thumb_' . $file->get('realname'); + $source = $this->file->get('destination_file'); + $destination = $this->file->get('destination_path') . '/thumb_' . $this->file->get('realname'); - if (!create_thumbnail($source, $destination, $file->get('mimetype'))) + if (!create_thumbnail($source, $destination, $this->file->get('mimetype'))) { $filedata['thumbnail'] = 0; } @@ -273,4 +255,48 @@ class upload return $filedata; } + + /** + * Init files upload class + * + * @param int $forum_id Forum ID + * @param bool $is_message Whether attachment is inside PM or not + */ + protected function init_files_upload($forum_id, $is_message) + { + if ($this->config['check_attachment_content'] && isset($this->config['mime_triggers'])) + { + $this->files_upload->set_disallowed_content(explode('|', $this->config['mime_triggers'])); + } + else if (!$this->config['check_attachment_content']) + { + $this->files_upload->set_disallowed_content(array()); + } + + $this->extensions = $this->cache->obtain_attach_extensions((($is_message) ? false : (int) $forum_id)); + $this->files_upload->set_allowed_extensions(array_keys($this->extensions['_allowed_'])); + } + + /** + * Check if uploaded file is really an image + * + * @param bool $is_image Whether file is image + */ + protected function check_image($is_image) + { + // Make sure the image category only holds valid images... + if ($is_image && !$this->file->is_image()) + { + $this->file->remove(); + + if ($this->plupload && $this->plupload->is_active()) + { + $this->plupload->emit_error(104, 'ATTACHED_IMAGE_NOT_IMAGE'); + } + + // If this error occurs a user tried to exploit an IE Bug by renaming extensions + // Since the image category is displaying content inline we need to catch this. + trigger_error($this->language->lang('ATTACHED_IMAGE_NOT_IMAGE')); + } + } } -- cgit v1.2.1 From a60beb6f2f9d75e34c67a53c9b2332e15fe21b4a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 20 Sep 2015 12:18:58 +0200 Subject: [ticket/14168] Further split up attachment upload class PHPBB3-14168 --- phpBB/phpbb/attachment/upload.php | 155 +++++++++++++++++++++++--------------- 1 file changed, 94 insertions(+), 61 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index 2880e9e42e..d57b33b137 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -58,6 +58,11 @@ class upload /** @var \phpbb\files\filespec Current filespec instance */ private $file; + /** @var array File data */ + private $file_data = array( + 'error' => array() + ); + /** @var array Extensions array */ private $extensions; @@ -104,26 +109,22 @@ class upload */ public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = array()) { - $filedata = array( - 'error' => array() - ); - $this->init_files_upload($forum_id, $is_message); - $filedata['post_attach'] = $local || $this->files_upload->is_valid($form_name); + $this->file_data['post_attach'] = $local || $this->files_upload->is_valid($form_name); - if (!$filedata['post_attach']) + if (!$this->file_data['post_attach']) { - $filedata['error'][] = $this->language->lang('NO_UPLOAD_FORM_FOUND'); - return $filedata; + $this->file_data['error'][] = $this->language->lang('NO_UPLOAD_FORM_FOUND'); + 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); if ($this->file->init_error()) { - $filedata['post_attach'] = false; - return $filedata; + $this->file_data['post_attach'] = false; + return $this->file_data; } // Whether the uploaded file is in the image category @@ -157,26 +158,23 @@ class upload $this->file->move_file($this->config['upload_path'], false, !$is_image); // Do we have to create a thumbnail? - $filedata['thumbnail'] = ($is_image && $this->config['img_create_thumbnail']) ? 1 : 0; + $this->file_data['thumbnail'] = ($is_image && $this->config['img_create_thumbnail']) ? 1 : 0; if (sizeof($this->file->error)) { $this->file->remove(); - $filedata['error'] = array_merge($filedata['error'], $this->file->error); - $filedata['post_attach'] = false; + $this->file_data['error'] = array_merge($this->file_data['error'], $this->file->error); + $this->file_data['post_attach'] = false; - return $filedata; + return $this->file_data; } // Make sure the image category only holds valid images... $this->check_image($is_image); - $filedata['filesize'] = $this->file->get('filesize'); - $filedata['mimetype'] = $this->file->get('mimetype'); - $filedata['extension'] = $this->file->get('extension'); - $filedata['physical_filename'] = $this->file->get('realname'); - $filedata['real_filename'] = $this->file->get('uploadname'); - $filedata['filetime'] = time(); + $this->fill_file_data(); + + $filedata = $this->file_data; /** * Event to modify uploaded file before submit to the post @@ -191,69 +189,38 @@ class upload 'is_image', ); extract($this->phpbb_dispatcher->trigger_event('core.modify_uploaded_file', compact($vars))); + $this->file_data = $filedata; + unset($filedata); - // Check our complete quota - if ($this->config['attachment_quota']) - { - if (intval($this->config['upload_dir_size']) + $this->file->get('filesize') > $this->config['attachment_quota']) - { - $filedata['error'][] = $this->language->lang('ATTACH_QUOTA_REACHED'); - $filedata['post_attach'] = false; - - $this->file->remove(); - - return $filedata; - } - } - - // Check free disk space - if ($free_space = @disk_free_space($this->phpbb_root_path . $this->config['upload_path'])) + // Check for attachment quota and free space + if (!$this->check_attach_quota() || !$this->check_disk_space()) { - if ($free_space <= $this->file->get('filesize')) - { - if ($this->auth->acl_get('a_')) - { - $filedata['error'][] = $this->language->lang('ATTACH_DISK_FULL'); - } - else - { - $filedata['error'][] = $this->language->lang('ATTACH_QUOTA_REACHED'); - } - $filedata['post_attach'] = false; - - $this->file->remove(); - - return $filedata; - } + return $this->file_data; } // Create Thumbnail - $filedata = $this->create_thumbnail($filedata); + $this->create_thumbnail(); - return $filedata; + return $this->file_data; } /** * Create thumbnail for file if necessary * - * @param array $filedata File's filedata - * * @return array Updated $filedata */ - protected function create_thumbnail($filedata) + protected function create_thumbnail() { - if ($filedata['thumbnail']) + if ($this->file_data['thumbnail']) { $source = $this->file->get('destination_file'); $destination = $this->file->get('destination_path') . '/thumb_' . $this->file->get('realname'); if (!create_thumbnail($source, $destination, $this->file->get('mimetype'))) { - $filedata['thumbnail'] = 0; + $this->file_data['thumbnail'] = 0; } } - - return $filedata; } /** @@ -299,4 +266,70 @@ class upload trigger_error($this->language->lang('ATTACHED_IMAGE_NOT_IMAGE')); } } + + /** + * Check if attachment quota was reached + * + * @return bool False if attachment quota was reached, true if not + */ + protected function check_attach_quota() + { + if ($this->config['attachment_quota']) + { + if (intval($this->config['upload_dir_size']) + $this->file->get('filesize') > $this->config['attachment_quota']) + { + $this->file_data['error'][] = $this->language->lang('ATTACH_QUOTA_REACHED'); + $this->file_data['post_attach'] = false; + + $this->file->remove(); + + return false; + } + } + + return true; + } + + /** + * Check if there is enough free space available on disk + * + * @return bool True if disk space is available, false if not + */ + 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; + } + + /** + * Fills file data with file information and current time as filetime + */ + protected function fill_file_data() + { + $this->file_data['filesize'] = $this->file->get('filesize'); + $this->file_data['mimetype'] = $this->file->get('mimetype'); + $this->file_data['extension'] = $this->file->get('extension'); + $this->file_data['physical_filename'] = $this->file->get('realname'); + $this->file_data['real_filename'] = $this->file->get('uploadname'); + $this->file_data['filetime'] = time(); + } } -- cgit v1.2.1 From d1030450f5f2ddca88f1ef17cc7b949541e64bd8 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 21 Sep 2015 08:54:57 +0200 Subject: [ticket/14168] Move function for attachment deletion into class PHPBB3-14168 --- phpBB/phpbb/attachment/delete.php | 398 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 398 insertions(+) create mode 100644 phpBB/phpbb/attachment/delete.php (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php new file mode 100644 index 0000000000..a2fe1a8cdf --- /dev/null +++ b/phpBB/phpbb/attachment/delete.php @@ -0,0 +1,398 @@ + + * @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\attachment; + +use \phpbb\config\config; +use \phpbb\db\driver\driver_interface; +use \phpbb\event\dispatcher; + +/** + * Attachment delete class + */ + +class delete +{ + /** @var \phpbb\config\config */ + protected $config; + + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + + /** @var \phpbb\event\dispatcher */ + protected $dispatcher; + + /** @var array Attachement IDs */ + protected $ids; + + /** + * Attachment delete class constructor + * + * @param config $config + * @param driver_interface $db + * @param dispatcher>>>>>>> 85b6020... [ticket/14168] Move function for attachment deletion into class + */ + public function __construct(config $config, driver_interface $db, dispatcher $dispatcher) + { + $this->config = $config; + $this->db = $db; + $this->dispatcher = $dispatcher; + } + + /** + * Delete Attachments + * + * @param string $mode can be: post|message|topic|attach|user + * @param mixed $ids can be: post_ids, message_ids, topic_ids, attach_ids, user_ids + * @param bool $resync set this to false if you are deleting posts or topics + * + * @return int|bool Number of deleted attachments or false if something + * went wrong during attachment deletion + */ + function delete($mode, $ids, $resync = true) + { + if (!$this->set_attachment_ids($ids)) + { + return false; + } + + $sql_where = ''; + + switch ($mode) + { + case 'post': + case 'message': + $sql_id = 'post_msg_id'; + $sql_where = ' AND in_message = ' . ($mode == 'message' ? 1 : 0); + break; + + case 'topic': + $sql_id = 'topic_id'; + break; + + case 'user': + $sql_id = 'poster_id'; + break; + + case 'attach': + default: + $sql_id = 'attach_id'; + break; + } + + $post_ids = $message_ids = $topic_ids = $physical = array(); + + /** + * Perform additional actions before collecting data for attachment(s) deletion + * + * @event core.delete_attachments_collect_data_before + * @var string mode Variable containing attachments deletion mode, can be: post|message|topic|attach|user + * @var mixed ids Array or comma separated list of ids corresponding to the mode + * @var bool resync Flag indicating if posts/messages/topics should be synchronized + * @var string sql_id The field name to collect/delete data for depending on the mode + * @since 3.1.7-RC1 + */ + $vars = array( + 'mode', + 'ids', + 'resync', + 'sql_id', + ); + extract($this->dispatcher->trigger_event('core.delete_attachments_collect_data_before', compact($vars))); + + // Collect post and topic ids for later use if we need to touch remaining entries (if resync is enabled) + $sql = 'SELECT post_msg_id, topic_id, in_message, physical_filename, thumbnail, filesize, is_orphan + FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $this->db->sql_in_set($sql_id, $ids); + + $sql .= $sql_where; + + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + // We only need to store post/message/topic ids if resync is enabled and the file is not orphaned + if ($resync && !$row['is_orphan']) + { + if (!$row['in_message']) + { + $post_ids[] = $row['post_msg_id']; + $topic_ids[] = $row['topic_id']; + } + else + { + $message_ids[] = $row['post_msg_id']; + } + } + + $physical[] = array('filename' => $row['physical_filename'], 'thumbnail' => $row['thumbnail'], 'filesize' => $row['filesize'], 'is_orphan' => $row['is_orphan']); + } + $this->db->sql_freeresult($result); + + /** + * Perform additional actions before attachment(s) deletion + * + * @event core.delete_attachments_before + * @var string mode Variable containing attachments deletion mode, can be: post|message|topic|attach|user + * @var mixed ids Array or comma separated list of ids corresponding to the mode + * @var bool resync Flag indicating if posts/messages/topics should be synchronized + * @var string sql_id The field name to collect/delete data for depending on the mode + * @var array post_ids Array with post ids for deleted attachment(s) + * @var array topic_ids Array with topic ids for deleted attachment(s) + * @var array message_ids Array with private message ids for deleted attachment(s) + * @var array physical Array with deleted attachment(s) physical file(s) data + * @since 3.1.7-RC1 + */ + $vars = array( + 'mode', + 'ids', + 'resync', + 'sql_id', + 'post_ids', + 'topic_ids', + 'message_ids', + 'physical', + ); + extract($this->dispatcher->trigger_event('core.delete_attachments_before', compact($vars))); + + // Delete attachments + $sql = 'DELETE FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $this->db->sql_in_set($sql_id, $ids); + + $sql .= $sql_where; + + $this->db->sql_query($sql); + $num_deleted = $this->db->sql_affectedrows(); + + /** + * Perform additional actions after attachment(s) deletion from the database + * + * @event core.delete_attachments_from_database_after + * @var string mode Variable containing attachments deletion mode, can be: post|message|topic|attach|user + * @var mixed ids Array or comma separated list of ids corresponding to the mode + * @var bool resync Flag indicating if posts/messages/topics should be synchronized + * @var string sql_id The field name to collect/delete data for depending on the mode + * @var array post_ids Array with post ids for deleted attachment(s) + * @var array topic_ids Array with topic ids for deleted attachment(s) + * @var array message_ids Array with private message ids for deleted attachment(s) + * @var array physical Array with deleted attachment(s) physical file(s) data + * @var int num_deleted The number of deleted attachment(s) from the database + * @since 3.1.7-RC1 + */ + $vars = array( + 'mode', + 'ids', + 'resync', + 'sql_id', + 'post_ids', + 'topic_ids', + 'message_ids', + 'physical', + 'num_deleted', + ); + extract($this->dispatcher->trigger_event('core.delete_attachments_from_database_after', compact($vars))); + + if (!$num_deleted) + { + return 0; + } + + // Delete attachments from filesystem + $space_removed = $files_removed = 0; + foreach ($physical as $file_ary) + { + if (phpbb_unlink($file_ary['filename'], 'file', true) && !$file_ary['is_orphan']) + { + // Only non-orphaned files count to the file size + $space_removed += $file_ary['filesize']; + $files_removed++; + } + + if ($file_ary['thumbnail']) + { + phpbb_unlink($file_ary['filename'], 'thumbnail', true); + } + } + + /** + * Perform additional actions after attachment(s) deletion from the filesystem + * + * @event core.delete_attachments_from_filesystem_after + * @var string mode Variable containing attachments deletion mode, can be: post|message|topic|attach|user + * @var mixed ids Array or comma separated list of ids corresponding to the mode + * @var bool resync Flag indicating if posts/messages/topics should be synchronized + * @var string sql_id The field name to collect/delete data for depending on the mode + * @var array post_ids Array with post ids for deleted attachment(s) + * @var array topic_ids Array with topic ids for deleted attachment(s) + * @var array message_ids Array with private message ids for deleted attachment(s) + * @var array physical Array with deleted attachment(s) physical file(s) data + * @var int num_deleted The number of deleted attachment(s) from the database + * @var int space_removed The size of deleted files(s) from the filesystem + * @var int files_removed The number of deleted file(s) from the filesystem + * @since 3.1.7-RC1 + */ + $vars = array( + 'mode', + 'ids', + 'resync', + 'sql_id', + 'post_ids', + 'topic_ids', + 'message_ids', + 'physical', + 'num_deleted', + 'space_removed', + 'files_removed', + ); + extract($this->dispatcher->trigger_event('core.delete_attachments_from_filesystem_after', compact($vars))); + + if ($space_removed || $files_removed) + { + $this->config->increment('upload_dir_size', $space_removed * (-1), false); + $this->config->increment('num_files', $files_removed * (-1), false); + } + + // If we do not resync, we do not need to adjust any message, post, topic or user entries + if (!$resync) + { + return $num_deleted; + } + + // No more use for the original ids + unset($ids); + + // Now, we need to resync posts, messages, topics. We go through every one of them + $post_ids = array_unique($post_ids); + $message_ids = array_unique($message_ids); + $topic_ids = array_unique($topic_ids); + + // Update post indicators for posts now no longer having attachments + if (sizeof($post_ids)) + { + // Just check which posts are still having an assigned attachment not orphaned by querying the attachments table + $sql = 'SELECT post_msg_id + FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $this->db->sql_in_set('post_msg_id', $post_ids) . ' + AND in_message = 0 + AND is_orphan = 0'; + $result = $this->db->sql_query($sql); + + $remaining_ids = array(); + while ($row = $this->db->sql_fetchrow($result)) + { + $remaining_ids[] = $row['post_msg_id']; + } + $this->db->sql_freeresult($result); + + // Now only unset those ids remaining + $post_ids = array_diff($post_ids, $remaining_ids); + + if (sizeof($post_ids)) + { + $sql = 'UPDATE ' . POSTS_TABLE . ' + SET post_attachment = 0 + WHERE ' . $this->db->sql_in_set('post_id', $post_ids); + $this->db->sql_query($sql); + } + } + + // Update message table if messages are affected + if (sizeof($message_ids)) + { + // Just check which messages are still having an assigned attachment not orphaned by querying the attachments table + $sql = 'SELECT post_msg_id + FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $this->db->sql_in_set('post_msg_id', $message_ids) . ' + AND in_message = 1 + AND is_orphan = 0'; + $result = $this->db->sql_query($sql); + + $remaining_ids = array(); + while ($row = $this->db->sql_fetchrow($result)) + { + $remaining_ids[] = $row['post_msg_id']; + } + $this->db->sql_freeresult($result); + + // Now only unset those ids remaining + $message_ids = array_diff($message_ids, $remaining_ids); + + if (sizeof($message_ids)) + { + $sql = 'UPDATE ' . PRIVMSGS_TABLE . ' + SET message_attachment = 0 + WHERE ' . $this->db->sql_in_set('msg_id', $message_ids); + $this->db->sql_query($sql); + } + } + + // Now update the topics. This is a bit trickier, because there could be posts still having attachments within the topic + if (sizeof($topic_ids)) + { + // Just check which topics are still having an assigned attachment not orphaned by querying the attachments table (much less entries expected) + $sql = 'SELECT topic_id + FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $this->db->sql_in_set('topic_id', $topic_ids) . ' + AND is_orphan = 0'; + $result = $this->db->sql_query($sql); + + $remaining_ids = array(); + while ($row = $this->db->sql_fetchrow($result)) + { + $remaining_ids[] = $row['topic_id']; + } + $this->db->sql_freeresult($result); + + // Now only unset those ids remaining + $topic_ids = array_diff($topic_ids, $remaining_ids); + + if (sizeof($topic_ids)) + { + $sql = 'UPDATE ' . TOPICS_TABLE . ' + SET topic_attachment = 0 + WHERE ' . $this->db->sql_in_set('topic_id', $topic_ids); + $this->db->sql_query($sql); + } + } + + return $num_deleted; + } + + /** + * Set attachment IDs + * + * @param array $ids + * + * @return bool True if attachment IDs were set, false if not + */ + protected function set_attachment_ids($ids) + { + // 0 is as bad as an empty array + if (empty($ids)) + { + return false; + } + + if (is_array($ids)) + { + $ids = array_unique($ids); + $this->ids = array_map('intval', $ids); + } + else + { + $this->ids = array((int) $ids); + } + + return true; + } +} \ No newline at end of file -- cgit v1.2.1 From ebfdd69525cc10446fba6c6e8600b44e1124a64d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 21 Sep 2015 10:28:09 +0200 Subject: [ticket/14168] Further split up attachment delete class PHPBB3-14168 --- phpBB/phpbb/attachment/delete.php | 379 ++++++++++++++++++++++---------------- 1 file changed, 218 insertions(+), 161 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index a2fe1a8cdf..7a168c2243 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -35,6 +35,27 @@ class delete /** @var array Attachement IDs */ protected $ids; + /** @var string SQL ID string */ + private $sql_id; + + /** @var string SQL where string */ + private $sql_where = ''; + + /** @var int Number of deleted items */ + private $num_deleted; + + /** @var array Post IDs */ + private $post_ids = array(); + + /** @var array Message IDs */ + private $message_ids = array(); + + /** @var array Topic IDs */ + private $topic_ids = array(); + + /** @var array Info of physical file */ + private $physical = array(); + /** * Attachment delete class constructor * @@ -66,31 +87,7 @@ class delete return false; } - $sql_where = ''; - - switch ($mode) - { - case 'post': - case 'message': - $sql_id = 'post_msg_id'; - $sql_where = ' AND in_message = ' . ($mode == 'message' ? 1 : 0); - break; - - case 'topic': - $sql_id = 'topic_id'; - break; - - case 'user': - $sql_id = 'poster_id'; - break; - - case 'attach': - default: - $sql_id = 'attach_id'; - break; - } - - $post_ids = $message_ids = $topic_ids = $physical = array(); + $this->set_sql_constraints($mode); /** * Perform additional actions before collecting data for attachment(s) deletion @@ -111,68 +108,10 @@ class delete extract($this->dispatcher->trigger_event('core.delete_attachments_collect_data_before', compact($vars))); // Collect post and topic ids for later use if we need to touch remaining entries (if resync is enabled) - $sql = 'SELECT post_msg_id, topic_id, in_message, physical_filename, thumbnail, filesize, is_orphan - FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set($sql_id, $ids); - - $sql .= $sql_where; - - $result = $this->db->sql_query($sql); - - while ($row = $this->db->sql_fetchrow($result)) - { - // We only need to store post/message/topic ids if resync is enabled and the file is not orphaned - if ($resync && !$row['is_orphan']) - { - if (!$row['in_message']) - { - $post_ids[] = $row['post_msg_id']; - $topic_ids[] = $row['topic_id']; - } - else - { - $message_ids[] = $row['post_msg_id']; - } - } - - $physical[] = array('filename' => $row['physical_filename'], 'thumbnail' => $row['thumbnail'], 'filesize' => $row['filesize'], 'is_orphan' => $row['is_orphan']); - } - $this->db->sql_freeresult($result); - - /** - * Perform additional actions before attachment(s) deletion - * - * @event core.delete_attachments_before - * @var string mode Variable containing attachments deletion mode, can be: post|message|topic|attach|user - * @var mixed ids Array or comma separated list of ids corresponding to the mode - * @var bool resync Flag indicating if posts/messages/topics should be synchronized - * @var string sql_id The field name to collect/delete data for depending on the mode - * @var array post_ids Array with post ids for deleted attachment(s) - * @var array topic_ids Array with topic ids for deleted attachment(s) - * @var array message_ids Array with private message ids for deleted attachment(s) - * @var array physical Array with deleted attachment(s) physical file(s) data - * @since 3.1.7-RC1 - */ - $vars = array( - 'mode', - 'ids', - 'resync', - 'sql_id', - 'post_ids', - 'topic_ids', - 'message_ids', - 'physical', - ); - extract($this->dispatcher->trigger_event('core.delete_attachments_before', compact($vars))); + $this->collect_attachment_info($resync); - // Delete attachments - $sql = 'DELETE FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set($sql_id, $ids); - - $sql .= $sql_where; - - $this->db->sql_query($sql); - $num_deleted = $this->db->sql_affectedrows(); + // Delete attachments from database + $this->delete_attachments_from_db(); /** * Perform additional actions after attachment(s) deletion from the database @@ -202,87 +141,31 @@ class delete ); extract($this->dispatcher->trigger_event('core.delete_attachments_from_database_after', compact($vars))); - if (!$num_deleted) + if (!$this->num_deleted) { return 0; } // Delete attachments from filesystem - $space_removed = $files_removed = 0; - foreach ($physical as $file_ary) - { - if (phpbb_unlink($file_ary['filename'], 'file', true) && !$file_ary['is_orphan']) - { - // Only non-orphaned files count to the file size - $space_removed += $file_ary['filesize']; - $files_removed++; - } - - if ($file_ary['thumbnail']) - { - phpbb_unlink($file_ary['filename'], 'thumbnail', true); - } - } - - /** - * Perform additional actions after attachment(s) deletion from the filesystem - * - * @event core.delete_attachments_from_filesystem_after - * @var string mode Variable containing attachments deletion mode, can be: post|message|topic|attach|user - * @var mixed ids Array or comma separated list of ids corresponding to the mode - * @var bool resync Flag indicating if posts/messages/topics should be synchronized - * @var string sql_id The field name to collect/delete data for depending on the mode - * @var array post_ids Array with post ids for deleted attachment(s) - * @var array topic_ids Array with topic ids for deleted attachment(s) - * @var array message_ids Array with private message ids for deleted attachment(s) - * @var array physical Array with deleted attachment(s) physical file(s) data - * @var int num_deleted The number of deleted attachment(s) from the database - * @var int space_removed The size of deleted files(s) from the filesystem - * @var int files_removed The number of deleted file(s) from the filesystem - * @since 3.1.7-RC1 - */ - $vars = array( - 'mode', - 'ids', - 'resync', - 'sql_id', - 'post_ids', - 'topic_ids', - 'message_ids', - 'physical', - 'num_deleted', - 'space_removed', - 'files_removed', - ); - extract($this->dispatcher->trigger_event('core.delete_attachments_from_filesystem_after', compact($vars))); - - if ($space_removed || $files_removed) - { - $this->config->increment('upload_dir_size', $space_removed * (-1), false); - $this->config->increment('num_files', $files_removed * (-1), false); - } + $this->delete_attachments_from_filesystem(); // If we do not resync, we do not need to adjust any message, post, topic or user entries if (!$resync) { - return $num_deleted; + return $this->num_deleted; } // No more use for the original ids unset($ids); // Now, we need to resync posts, messages, topics. We go through every one of them - $post_ids = array_unique($post_ids); - $message_ids = array_unique($message_ids); - $topic_ids = array_unique($topic_ids); - // Update post indicators for posts now no longer having attachments - if (sizeof($post_ids)) + if (sizeof($this->post_ids)) { // Just check which posts are still having an assigned attachment not orphaned by querying the attachments table $sql = 'SELECT post_msg_id FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set('post_msg_id', $post_ids) . ' + WHERE ' . $this->db->sql_in_set('post_msg_id', $this->post_ids) . ' AND in_message = 0 AND is_orphan = 0'; $result = $this->db->sql_query($sql); @@ -295,24 +178,24 @@ class delete $this->db->sql_freeresult($result); // Now only unset those ids remaining - $post_ids = array_diff($post_ids, $remaining_ids); + $this->post_ids = array_diff($this->post_ids, $remaining_ids); - if (sizeof($post_ids)) + if (sizeof($this->post_ids)) { $sql = 'UPDATE ' . POSTS_TABLE . ' SET post_attachment = 0 - WHERE ' . $this->db->sql_in_set('post_id', $post_ids); + WHERE ' . $this->db->sql_in_set('post_id', $this->post_ids); $this->db->sql_query($sql); } } // Update message table if messages are affected - if (sizeof($message_ids)) + if (sizeof($this->message_ids)) { // Just check which messages are still having an assigned attachment not orphaned by querying the attachments table $sql = 'SELECT post_msg_id FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set('post_msg_id', $message_ids) . ' + WHERE ' . $this->db->sql_in_set('post_msg_id', $this->message_ids) . ' AND in_message = 1 AND is_orphan = 0'; $result = $this->db->sql_query($sql); @@ -325,24 +208,24 @@ class delete $this->db->sql_freeresult($result); // Now only unset those ids remaining - $message_ids = array_diff($message_ids, $remaining_ids); + $this->message_ids = array_diff($this->message_ids, $remaining_ids); - if (sizeof($message_ids)) + if (sizeof($this->message_ids)) { $sql = 'UPDATE ' . PRIVMSGS_TABLE . ' SET message_attachment = 0 - WHERE ' . $this->db->sql_in_set('msg_id', $message_ids); + WHERE ' . $this->db->sql_in_set('msg_id', $this->message_ids); $this->db->sql_query($sql); } } // Now update the topics. This is a bit trickier, because there could be posts still having attachments within the topic - if (sizeof($topic_ids)) + if (sizeof($this->topic_ids)) { // Just check which topics are still having an assigned attachment not orphaned by querying the attachments table (much less entries expected) $sql = 'SELECT topic_id FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set('topic_id', $topic_ids) . ' + WHERE ' . $this->db->sql_in_set('topic_id', $this->topic_ids) . ' AND is_orphan = 0'; $result = $this->db->sql_query($sql); @@ -354,18 +237,18 @@ class delete $this->db->sql_freeresult($result); // Now only unset those ids remaining - $topic_ids = array_diff($topic_ids, $remaining_ids); + $this->topic_ids = array_diff($this->topic_ids, $remaining_ids); - if (sizeof($topic_ids)) + if (sizeof($this->topic_ids)) { $sql = 'UPDATE ' . TOPICS_TABLE . ' SET topic_attachment = 0 - WHERE ' . $this->db->sql_in_set('topic_id', $topic_ids); + WHERE ' . $this->db->sql_in_set('topic_id', $this->topic_ids); $this->db->sql_query($sql); } } - return $num_deleted; + return $this->num_deleted; } /** @@ -395,4 +278,178 @@ class delete return true; } + + /** + * Set SQL constraints based on mode + * + * @param string $mode Delete mode; can be: post|message|topic|attach|user + */ + private function set_sql_constraints($mode) + { + switch ($mode) + { + case 'post': + case 'message': + $this->sql_id = 'post_msg_id'; + $this->sql_where = ' AND in_message = ' . ($mode == 'message' ? 1 : 0); + break; + + case 'topic': + $this->sql_id = 'topic_id'; + break; + + case 'user': + $this->sql_id = 'poster_id'; + break; + + case 'attach': + default: + $this->sql_id = 'attach_id'; + break; + } + } + + /** + * Collect info about attachment IDs + * + * @param bool $resync Whether topics/posts should be resynced after delete + */ + protected function collect_attachment_info($resync) + { + // Collect post and topic ids for later use if we need to touch remaining entries (if resync is enabled) + $sql = 'SELECT post_msg_id, topic_id, in_message, physical_filename, thumbnail, filesize, is_orphan + FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $this->db->sql_in_set($this->sql_id, $this->ids); + + $sql .= $this->sql_where; + + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + // We only need to store post/message/topic ids if resync is enabled and the file is not orphaned + if ($resync && !$row['is_orphan']) + { + if (!$row['in_message']) + { + $this->post_ids[] = $row['post_msg_id']; + $this->topic_ids[] = $row['topic_id']; + } + else + { + $this->message_ids[] = $row['post_msg_id']; + } + } + + $this->physical[] = array('filename' => $row['physical_filename'], 'thumbnail' => $row['thumbnail'], 'filesize' => $row['filesize'], 'is_orphan' => $row['is_orphan']); + } + $this->db->sql_freeresult($result); + + // IDs should be unique + $this->post_ids = array_unique($this->post_ids); + $this->message_ids = array_unique($this->message_ids); + $this->topic_ids = array_unique($this->topic_ids); + } + + /** + * Delete attachments from database table + */ + protected function delete_attachments_from_db() + { + /** + * Perform additional actions before attachment(s) deletion + * + * @event core.delete_attachments_before + * @var string mode Variable containing attachments deletion mode, can be: post|message|topic|attach|user + * @var mixed ids Array or comma separated list of ids corresponding to the mode + * @var bool resync Flag indicating if posts/messages/topics should be synchronized + * @var string sql_id The field name to collect/delete data for depending on the mode + * @var array post_ids Array with post ids for deleted attachment(s) + * @var array topic_ids Array with topic ids for deleted attachment(s) + * @var array message_ids Array with private message ids for deleted attachment(s) + * @var array physical Array with deleted attachment(s) physical file(s) data + * @since 3.1.7-RC1 + */ + $vars = array( + 'mode', + 'ids', + 'resync', + 'sql_id', + 'post_ids', + 'topic_ids', + 'message_ids', + 'physical', + ); + extract($this->dispatcher->trigger_event('core.delete_attachments_before', compact($vars))); + + // Delete attachments + $sql = 'DELETE FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $this->db->sql_in_set($this->sql_id, $this->ids); + + $sql .= $this->sql_where; + + $this->db->sql_query($sql); + $this->num_deleted = $this->db->sql_affectedrows(); + } + + /** + * Delete attachments from filesystem + */ + protected function delete_attachments_from_filesystem() + { + $space_removed = $files_removed = 0; + + foreach ($this->physical as $file_ary) + { + if (phpbb_unlink($file_ary['filename'], 'file', true) && !$file_ary['is_orphan']) + { + // Only non-orphaned files count to the file size + $space_removed += $file_ary['filesize']; + $files_removed++; + } + + if ($file_ary['thumbnail']) + { + phpbb_unlink($file_ary['filename'], 'thumbnail', true); + } + } + + /** + * Perform additional actions after attachment(s) deletion from the filesystem + * + * @event core.delete_attachments_from_filesystem_after + * @var string mode Variable containing attachments deletion mode, can be: post|message|topic|attach|user + * @var mixed ids Array or comma separated list of ids corresponding to the mode + * @var bool resync Flag indicating if posts/messages/topics should be synchronized + * @var string sql_id The field name to collect/delete data for depending on the mode + * @var array post_ids Array with post ids for deleted attachment(s) + * @var array topic_ids Array with topic ids for deleted attachment(s) + * @var array message_ids Array with private message ids for deleted attachment(s) + * @var array physical Array with deleted attachment(s) physical file(s) data + * @var int num_deleted The number of deleted attachment(s) from the database + * @var int space_removed The size of deleted files(s) from the filesystem + * @var int files_removed The number of deleted file(s) from the filesystem + * @since 3.1.7-RC1 + */ + $vars = array( + 'mode', + 'ids', + 'resync', + 'sql_id', + 'post_ids', + 'topic_ids', + 'message_ids', + 'physical', + 'num_deleted', + 'space_removed', + 'files_removed', + ); + extract($this->dispatcher->trigger_event('core.delete_attachments_from_filesystem_after', compact($vars))); + + if ($space_removed || $files_removed) + { + $this->config->increment('upload_dir_size', $space_removed * (-1), false); + $this->config->increment('num_files', $files_removed * (-1), false); + } + } } \ No newline at end of file -- cgit v1.2.1 From 40117c77304468120245ae05b6b99f1d2a68c9f6 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 21 Sep 2015 10:56:32 +0200 Subject: [ticket/14168] Add attachment resync class PHPBB3-14168 --- phpBB/phpbb/attachment/delete.php | 98 ++++-------------------------- phpBB/phpbb/attachment/resync.php | 123 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 87 deletions(-) create mode 100644 phpBB/phpbb/attachment/resync.php (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index 7a168c2243..4ef320eab4 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -32,6 +32,9 @@ class delete /** @var \phpbb\event\dispatcher */ protected $dispatcher; + /** @var \phpbb\attachment\resync */ + protected $resync; + /** @var array Attachement IDs */ protected $ids; @@ -61,13 +64,15 @@ class delete * * @param config $config * @param driver_interface $db - * @param dispatcher>>>>>>> 85b6020... [ticket/14168] Move function for attachment deletion into class + * @param dispatcher $dispatcher + * @param resync $resync */ - public function __construct(config $config, driver_interface $db, dispatcher $dispatcher) + public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, resync $resync) { $this->config = $config; $this->db = $db; $this->dispatcher = $dispatcher; + $this->resync = $resync; } /** @@ -158,95 +163,14 @@ class delete // No more use for the original ids unset($ids); - // Now, we need to resync posts, messages, topics. We go through every one of them // Update post indicators for posts now no longer having attachments - if (sizeof($this->post_ids)) - { - // Just check which posts are still having an assigned attachment not orphaned by querying the attachments table - $sql = 'SELECT post_msg_id - FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set('post_msg_id', $this->post_ids) . ' - AND in_message = 0 - AND is_orphan = 0'; - $result = $this->db->sql_query($sql); - - $remaining_ids = array(); - while ($row = $this->db->sql_fetchrow($result)) - { - $remaining_ids[] = $row['post_msg_id']; - } - $this->db->sql_freeresult($result); - - // Now only unset those ids remaining - $this->post_ids = array_diff($this->post_ids, $remaining_ids); - - if (sizeof($this->post_ids)) - { - $sql = 'UPDATE ' . POSTS_TABLE . ' - SET post_attachment = 0 - WHERE ' . $this->db->sql_in_set('post_id', $this->post_ids); - $this->db->sql_query($sql); - } - } + $this->resync->resync('post', $this->post_ids); // Update message table if messages are affected - if (sizeof($this->message_ids)) - { - // Just check which messages are still having an assigned attachment not orphaned by querying the attachments table - $sql = 'SELECT post_msg_id - FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set('post_msg_id', $this->message_ids) . ' - AND in_message = 1 - AND is_orphan = 0'; - $result = $this->db->sql_query($sql); - - $remaining_ids = array(); - while ($row = $this->db->sql_fetchrow($result)) - { - $remaining_ids[] = $row['post_msg_id']; - } - $this->db->sql_freeresult($result); - - // Now only unset those ids remaining - $this->message_ids = array_diff($this->message_ids, $remaining_ids); - - if (sizeof($this->message_ids)) - { - $sql = 'UPDATE ' . PRIVMSGS_TABLE . ' - SET message_attachment = 0 - WHERE ' . $this->db->sql_in_set('msg_id', $this->message_ids); - $this->db->sql_query($sql); - } - } + $this->resync->resync('message', $this->message_ids); // Now update the topics. This is a bit trickier, because there could be posts still having attachments within the topic - if (sizeof($this->topic_ids)) - { - // Just check which topics are still having an assigned attachment not orphaned by querying the attachments table (much less entries expected) - $sql = 'SELECT topic_id - FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set('topic_id', $this->topic_ids) . ' - AND is_orphan = 0'; - $result = $this->db->sql_query($sql); - - $remaining_ids = array(); - while ($row = $this->db->sql_fetchrow($result)) - { - $remaining_ids[] = $row['topic_id']; - } - $this->db->sql_freeresult($result); - - // Now only unset those ids remaining - $this->topic_ids = array_diff($this->topic_ids, $remaining_ids); - - if (sizeof($this->topic_ids)) - { - $sql = 'UPDATE ' . TOPICS_TABLE . ' - SET topic_attachment = 0 - WHERE ' . $this->db->sql_in_set('topic_id', $this->topic_ids); - $this->db->sql_query($sql); - } - } + $this->resync->resync('topic', $this->topic_ids); return $this->num_deleted; } @@ -384,7 +308,7 @@ class delete // Delete attachments $sql = 'DELETE FROM ' . ATTACHMENTS_TABLE . ' - WHERE ' . $this->db->sql_in_set($this->sql_id, $this->ids); + WHERE ' . $this->db->sql_in_set($this->sql_id, $this->ids); $sql .= $this->sql_where; diff --git a/phpBB/phpbb/attachment/resync.php b/phpBB/phpbb/attachment/resync.php new file mode 100644 index 0000000000..c247fcce7a --- /dev/null +++ b/phpBB/phpbb/attachment/resync.php @@ -0,0 +1,123 @@ + + * @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\attachment; + +use \phpbb\db\driver\driver_interface; + +/** + * Attachment delete class + */ + +class resync +{ + /** @var driver_interface */ + protected $db; + + /** @var string Attachment table SQL ID */ + private $attach_sql_id; + + /** @var string Resync table SQL ID */ + private $resync_sql_id; + + /** @var string Resync SQL table */ + private $resync_table; + + /** @var string SQL where statement */ + private $sql_where; + + /** + * Constructor for attachment resync class + * + * @param driver_interface $db Database driver + */ + public function __construct(driver_interface $db) + { + $this->db = $db; + } + + /** + * Set type constraints for attachment resync + * + * @param string $type Type of resync; can be: message|post|topic + */ + protected function set_type_constraints($type) + { + switch ($type) + { + case 'message': + $this->attach_sql_id = 'post_msg_id'; + $this->sql_where = ' AND in_message = 1 + AND is_orphan = 0'; + $this->resync_table = PRIVMSGS_TABLE; + $this->resync_sql_id = 'msg_id'; + break; + + case 'post': + $this->attach_sql_id = 'post_msg_id'; + $this->sql_where = ' AND in_message = 0 + AND is_orphan = 0'; + $this->resync_table = POSTS_TABLE; + $this->resync_sql_id = 'post_id'; + break; + + case 'topic': + $this->attach_sql_id = 'topic_id'; + $this->sql_where = ' AND is_orphan = 0'; + $this->resync_table = TOPICS_TABLE; + $this->resync_sql_id = 'topic_id'; + break; + } + } + + /** + * Resync specified type + * + * @param string $type Type of resync + * @param array $ids IDs to resync + */ + public function resync($type, $ids) + { + if (empty($type) || !is_array($ids) || !sizeof($ids) || !in_array($type, array('post', 'topic', 'message'))) + { + return; + } + + // Just check which elements are still having an assigned attachment + // not orphaned by querying the attachments table + $sql = 'SELECT ' . $this->attach_sql_id . ' + FROM ' . ATTACHMENTS_TABLE . ' + WHERE ' . $this->db->sql_in_set($this->attach_sql_id, $ids) + . $this->sql_where; + $result = $this->db->sql_query($sql); + + $remaining_ids = array(); + while ($row = $this->db->sql_fetchrow($result)) + { + $remaining_ids[] = $row[$this->attach_sql_id]; + } + $this->db->sql_freeresult($result); + + // Now only unset those ids remaining + $ids = array_diff($ids, $remaining_ids); + + if (sizeof($ids)) + { + $sql = 'UPDATE ' . POSTS_TABLE . ' + SET ' . $type . '_attachment = 0 + WHERE ' . $this->db->sql_in_set($this->resync_sql_id, $ids); + $this->db->sql_query($sql); + } + } + +} \ No newline at end of file -- cgit v1.2.1 From e158db5daa8b9eedfea9d17b5d678320ac7f8f61 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 21 Sep 2015 11:01:01 +0200 Subject: [ticket/14168] Minor coding style fixes PHPBB3-14168 --- phpBB/phpbb/attachment/delete.php | 2 +- phpBB/phpbb/attachment/resync.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index 4ef320eab4..32469c124c 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -376,4 +376,4 @@ class delete $this->config->increment('num_files', $files_removed * (-1), false); } } -} \ No newline at end of file +} diff --git a/phpBB/phpbb/attachment/resync.php b/phpBB/phpbb/attachment/resync.php index c247fcce7a..9abf961c6b 100644 --- a/phpBB/phpbb/attachment/resync.php +++ b/phpBB/phpbb/attachment/resync.php @@ -120,4 +120,4 @@ class resync } } -} \ No newline at end of file +} -- cgit v1.2.1 From cebc064f4c15b9c53210cc3729756b4d7888041b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 21 Sep 2015 14:51:39 +0200 Subject: [ticket/14168] Coding guidelines fixes PHPBB3-14168 --- phpBB/phpbb/attachment/delete.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index 32469c124c..4c863a2205 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -216,20 +216,20 @@ class delete case 'message': $this->sql_id = 'post_msg_id'; $this->sql_where = ' AND in_message = ' . ($mode == 'message' ? 1 : 0); - break; + break; case 'topic': $this->sql_id = 'topic_id'; - break; + break; case 'user': $this->sql_id = 'poster_id'; - break; + break; case 'attach': default: $this->sql_id = 'attach_id'; - break; + break; } } -- cgit v1.2.1 From 583f42a2aa5ba8132e9e516b363ba0180fe46289 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 21 Sep 2015 16:28:53 +0200 Subject: [ticket/14168] Add tests for attachment resync class PHPBB3-14168 --- phpBB/phpbb/attachment/resync.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/resync.php b/phpBB/phpbb/attachment/resync.php index 9abf961c6b..adb642fe97 100644 --- a/phpBB/phpbb/attachment/resync.php +++ b/phpBB/phpbb/attachment/resync.php @@ -93,6 +93,8 @@ class resync return; } + $this->set_type_constraints($type); + // Just check which elements are still having an assigned attachment // not orphaned by querying the attachments table $sql = 'SELECT ' . $this->attach_sql_id . ' @@ -113,7 +115,7 @@ class resync if (sizeof($ids)) { - $sql = 'UPDATE ' . POSTS_TABLE . ' + $sql = 'UPDATE ' . $this->resync_table . ' SET ' . $type . '_attachment = 0 WHERE ' . $this->db->sql_in_set($this->resync_sql_id, $ids); $this->db->sql_query($sql); -- cgit v1.2.1 From b00c7511f022dfafb32f570db1f5fe762a62ef21 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 21 Sep 2015 22:16:23 +0200 Subject: [ticket/14168] Add tests for attachment delete class PHPBB3-14168 --- phpBB/phpbb/attachment/delete.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index 4c863a2205..372495aef0 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -85,7 +85,7 @@ class delete * @return int|bool Number of deleted attachments or false if something * went wrong during attachment deletion */ - function delete($mode, $ids, $resync = true) + public function delete($mode, $ids, $resync = true) { if (!$this->set_attachment_ids($ids)) { -- cgit v1.2.1 From 04786313892df25f5adce555ebae6b2e5ad4d222 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 23 Sep 2015 10:17:38 +0200 Subject: [ticket/14168] Move phpbb_unlink() into attachment delete class PHPBB3-14168 --- phpBB/phpbb/attachment/delete.php | 70 ++++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 8 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index 372495aef0..ad385861a1 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -16,6 +16,7 @@ namespace phpbb\attachment; use \phpbb\config\config; use \phpbb\db\driver\driver_interface; use \phpbb\event\dispatcher; +use \phpbb\filesystem\filesystem; /** * Attachment delete class @@ -23,18 +24,24 @@ use \phpbb\event\dispatcher; class delete { - /** @var \phpbb\config\config */ + /** @var config */ protected $config; - /** @var \phpbb\db\driver\driver_interface */ + /** @var driver_interface */ protected $db; /** @var \phpbb\event\dispatcher */ protected $dispatcher; - /** @var \phpbb\attachment\resync */ + /** @var filesystem */ + protected $filesystem; + + /** @var resync */ protected $resync; + /** @var string phpBB root path */ + protected $phpbb_root_path; + /** @var array Attachement IDs */ protected $ids; @@ -65,14 +72,18 @@ class delete * @param config $config * @param driver_interface $db * @param dispatcher $dispatcher + * @param filesystem $filesystem * @param resync $resync + * @param string $phpbb_root_path */ - public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, resync $resync) + public function __construct(config $config, driver_interface $db, dispatcher $dispatcher, filesystem $filesystem, resync $resync, $phpbb_root_path) { $this->config = $config; $this->db = $db; $this->dispatcher = $dispatcher; + $this->filesystem = $filesystem; $this->resync = $resync; + $this->phpbb_root_path = $phpbb_root_path; } /** @@ -152,7 +163,7 @@ class delete } // Delete attachments from filesystem - $this->delete_attachments_from_filesystem(); + $this->remove_from_filesystem(); // If we do not resync, we do not need to adjust any message, post, topic or user entries if (!$resync) @@ -319,13 +330,13 @@ class delete /** * Delete attachments from filesystem */ - protected function delete_attachments_from_filesystem() + protected function remove_from_filesystem() { $space_removed = $files_removed = 0; foreach ($this->physical as $file_ary) { - if (phpbb_unlink($file_ary['filename'], 'file', true) && !$file_ary['is_orphan']) + if ($this->unlink_attachment($file_ary['filename'], 'file', true) && !$file_ary['is_orphan']) { // Only non-orphaned files count to the file size $space_removed += $file_ary['filesize']; @@ -334,7 +345,7 @@ class delete if ($file_ary['thumbnail']) { - phpbb_unlink($file_ary['filename'], 'thumbnail', true); + $this->unlink_attachment($file_ary['filename'], 'thumbnail', true); } } @@ -376,4 +387,47 @@ class delete $this->config->increment('num_files', $files_removed * (-1), false); } } + + /** + * Delete attachment from filesystem + * + * @param string $filename Filename of attachment + * @param string $mode Delete mode + * @param bool $entry_removed Whether entry was removed. Defaults to false + * @return bool True if file was removed, false if not + */ + public function unlink_attachment($filename, $mode = 'file', $entry_removed = false) + { + // Because of copying topics or modifications a physical filename could be assigned more than once. If so, do not remove the file itself. + $sql = 'SELECT COUNT(attach_id) AS num_entries + FROM ' . ATTACHMENTS_TABLE . " + WHERE physical_filename = '" . $this->db->sql_escape(utf8_basename($filename)) . "'"; + $result = $this->db->sql_query($sql); + $num_entries = (int) $this->db->sql_fetchfield('num_entries'); + $this->db->sql_freeresult($result); + + // Do not remove file if at least one additional entry with the same name exist. + if (($entry_removed && $num_entries > 0) || (!$entry_removed && $num_entries > 1)) + { + return false; + } + + $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)) + { + $this->filesystem->remove($this->phpbb_root_path . $this->config['upload_path'] . '/' . $filename); + return true; + } + } + catch (\phpbb\filesystem\exception\filesystem_exception $exception) + { + // Fail is covered by return statement below + } + + return false; + } } -- cgit v1.2.1 From 36ea105236c87e84ca2b9f0a193b5b8721e8cf97 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 4 Oct 2015 11:10:07 +0200 Subject: [ticket/14168] Move image check and don't use trigger_error() PHPBB3-14168 --- phpBB/phpbb/attachment/upload.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index d57b33b137..ae8d70c18e 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -160,6 +160,9 @@ class upload // 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 (sizeof($this->file->error)) { $this->file->remove(); @@ -169,9 +172,6 @@ class upload return $this->file_data; } - // Make sure the image category only holds valid images... - $this->check_image($is_image); - $this->fill_file_data(); $filedata = $this->file_data; @@ -263,7 +263,7 @@ class upload // If this error occurs a user tried to exploit an IE Bug by renaming extensions // Since the image category is displaying content inline we need to catch this. - trigger_error($this->language->lang('ATTACHED_IMAGE_NOT_IMAGE')); + $this->file->set_error($this->language->lang('ATTACHED_IMAGE_NOT_IMAGE')); } } -- cgit v1.2.1 From 80e3c79f383366915b799675c6bcd3e15715780e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 7 Oct 2015 11:04:15 +0200 Subject: [ticket/14168] Minor coding style fixes PHPBB3-14168 --- phpBB/phpbb/attachment/delete.php | 3 +-- phpBB/phpbb/attachment/resync.php | 1 - phpBB/phpbb/attachment/upload.php | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/delete.php b/phpBB/phpbb/attachment/delete.php index ad385861a1..e093da8865 100644 --- a/phpBB/phpbb/attachment/delete.php +++ b/phpBB/phpbb/attachment/delete.php @@ -21,7 +21,6 @@ use \phpbb\filesystem\filesystem; /** * Attachment delete class */ - class delete { /** @var config */ @@ -189,7 +188,7 @@ class delete /** * Set attachment IDs * - * @param array $ids + * @param mixed $ids ID or array of IDs * * @return bool True if attachment IDs were set, false if not */ diff --git a/phpBB/phpbb/attachment/resync.php b/phpBB/phpbb/attachment/resync.php index adb642fe97..699d37340d 100644 --- a/phpBB/phpbb/attachment/resync.php +++ b/phpBB/phpbb/attachment/resync.php @@ -18,7 +18,6 @@ use \phpbb\db\driver\driver_interface; /** * Attachment delete class */ - class resync { /** @var driver_interface */ diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index ae8d70c18e..2d49e05b71 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -25,7 +25,6 @@ use \phpbb\user; /** * Attachment upload class */ - class upload { /** @var auth */ -- cgit v1.2.1 From 22b36d9d0e50fdc820912cca0164fccc7b5785a1 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Oct 2015 08:03:53 +0200 Subject: [ticket/14168] Use correct docblock PHPBB3-14168 --- phpBB/phpbb/attachment/resync.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/resync.php b/phpBB/phpbb/attachment/resync.php index 699d37340d..6c2e0a8b0d 100644 --- a/phpBB/phpbb/attachment/resync.php +++ b/phpBB/phpbb/attachment/resync.php @@ -16,7 +16,7 @@ namespace phpbb\attachment; use \phpbb\db\driver\driver_interface; /** - * Attachment delete class + * Attachment resync class */ class resync { -- cgit v1.2.1 From c78dbd2eea2dae14b076d8d800474c8715982277 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Oct 2015 09:45:28 +0200 Subject: [ticket/14168] Add attachment manager service PHPBB3-14168 --- phpBB/phpbb/attachment/manager.php | 99 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 phpBB/phpbb/attachment/manager.php (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/manager.php b/phpBB/phpbb/attachment/manager.php new file mode 100644 index 0000000000..0aeadf3e3e --- /dev/null +++ b/phpBB/phpbb/attachment/manager.php @@ -0,0 +1,99 @@ + + * @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\attachment; + +/** + * Attachment manager + */ +class manager +{ + /** @var delete Attachment delete class */ + protected $delete; + + /** @var resync Attachment resync class */ + protected $resync; + + /** @var upload Attachment upload class */ + protected $upload; + + /** + * Constructor for attachment manager + * + * @param delete $delete Attachment delete class + * @param resync $resync Attachment resync class + * @param upload $upload Attachment upload class + */ + public function __construct(delete $delete, resync $resync, upload $upload) + { + $this->delete = $delete; + $this->resync = $resync; + $this->upload = $upload; + } + + /** + * Wrapper method for deleting attachments + * + * @param string $mode can be: post|message|topic|attach|user + * @param mixed $ids can be: post_ids, message_ids, topic_ids, attach_ids, user_ids + * @param bool $resync set this to false if you are deleting posts or topics + * + * @return int|bool Number of deleted attachments or false if something + * went wrong during attachment deletion + */ + public function delete($mode, $id, $resync = true) + { + $this->delete->delete($mode, $id, $resync); + } + + /** + * Wrapper method for deleting attachments from filesystem + * + * @param string $filename Filename of attachment + * @param string $mode Delete mode + * @param bool $entry_removed Whether entry was removed. Defaults to false + * @return bool True if file was removed, false if not + */ + public function unlink($filename, $mode = 'file', $entry_removed = false) + { + $this->delete->unlink_attachment($filename, $mode, $entry_removed); + } + + /** + * Wrapper method for resyncing specified type + * + * @param string $type Type of resync + * @param array $ids IDs to resync + */ + public function resync($type, $ids) + { + $this->resync->resync($type, $ids); + } + + /** + * Wrapper method for uploading attachment + * + * @param string $form_name The form name of the file upload input + * @param int $forum_id The id of the forum + * @param bool $local Whether the file is local or not + * @param string $local_storage The path to the local file + * @param bool $is_message Whether it is a PM or not + * @param array $local_filedata An file data object created for the local file + * + * @return object filespec + */ + public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = []) + { + $this->upload->upload($form_name, $forum_id, $local, $local_storage, $is_message, $local_filedata); + } +} -- cgit v1.2.1 From 53008c87828746c316019e758641a2a4e37f522b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Oct 2015 12:14:19 +0200 Subject: [ticket/14168] Fix tabs in manager and add test file PHPBB3-14168 --- phpBB/phpbb/attachment/manager.php | 142 ++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 71 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/manager.php b/phpBB/phpbb/attachment/manager.php index 0aeadf3e3e..414a8edfdd 100644 --- a/phpBB/phpbb/attachment/manager.php +++ b/phpBB/phpbb/attachment/manager.php @@ -18,82 +18,82 @@ namespace phpbb\attachment; */ class manager { - /** @var delete Attachment delete class */ - protected $delete; + /** @var delete Attachment delete class */ + protected $delete; - /** @var resync Attachment resync class */ - protected $resync; + /** @var resync Attachment resync class */ + protected $resync; - /** @var upload Attachment upload class */ - protected $upload; + /** @var upload Attachment upload class */ + protected $upload; - /** - * Constructor for attachment manager - * - * @param delete $delete Attachment delete class - * @param resync $resync Attachment resync class - * @param upload $upload Attachment upload class - */ - public function __construct(delete $delete, resync $resync, upload $upload) - { - $this->delete = $delete; - $this->resync = $resync; - $this->upload = $upload; - } + /** + * Constructor for attachment manager + * + * @param delete $delete Attachment delete class + * @param resync $resync Attachment resync class + * @param upload $upload Attachment upload class + */ + public function __construct(delete $delete, resync $resync, upload $upload) + { + $this->delete = $delete; + $this->resync = $resync; + $this->upload = $upload; + } - /** - * Wrapper method for deleting attachments - * - * @param string $mode can be: post|message|topic|attach|user - * @param mixed $ids can be: post_ids, message_ids, topic_ids, attach_ids, user_ids - * @param bool $resync set this to false if you are deleting posts or topics - * - * @return int|bool Number of deleted attachments or false if something - * went wrong during attachment deletion - */ - public function delete($mode, $id, $resync = true) - { - $this->delete->delete($mode, $id, $resync); - } + /** + * Wrapper method for deleting attachments + * + * @param string $mode can be: post|message|topic|attach|user + * @param mixed $ids can be: post_ids, message_ids, topic_ids, attach_ids, user_ids + * @param bool $resync set this to false if you are deleting posts or topics + * + * @return int|bool Number of deleted attachments or false if something + * went wrong during attachment deletion + */ + public function delete($mode, $id, $resync = true) + { + return $this->delete->delete($mode, $id, $resync); + } - /** - * Wrapper method for deleting attachments from filesystem - * - * @param string $filename Filename of attachment - * @param string $mode Delete mode - * @param bool $entry_removed Whether entry was removed. Defaults to false - * @return bool True if file was removed, false if not - */ - public function unlink($filename, $mode = 'file', $entry_removed = false) - { - $this->delete->unlink_attachment($filename, $mode, $entry_removed); - } + /** + * Wrapper method for deleting attachments from filesystem + * + * @param string $filename Filename of attachment + * @param string $mode Delete mode + * @param bool $entry_removed Whether entry was removed. Defaults to false + * @return bool True if file was removed, false if not + */ + public function unlink($filename, $mode = 'file', $entry_removed = false) + { + return $this->delete->unlink_attachment($filename, $mode, $entry_removed); + } - /** - * Wrapper method for resyncing specified type - * - * @param string $type Type of resync - * @param array $ids IDs to resync - */ - public function resync($type, $ids) - { - $this->resync->resync($type, $ids); - } + /** + * Wrapper method for resyncing specified type + * + * @param string $type Type of resync + * @param array $ids IDs to resync + */ + public function resync($type, $ids) + { + $this->resync->resync($type, $ids); + } - /** - * Wrapper method for uploading attachment - * - * @param string $form_name The form name of the file upload input - * @param int $forum_id The id of the forum - * @param bool $local Whether the file is local or not - * @param string $local_storage The path to the local file - * @param bool $is_message Whether it is a PM or not - * @param array $local_filedata An file data object created for the local file - * - * @return object filespec - */ - public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = []) - { - $this->upload->upload($form_name, $forum_id, $local, $local_storage, $is_message, $local_filedata); - } + /** + * Wrapper method for uploading attachment + * + * @param string $form_name The form name of the file upload input + * @param int $forum_id The id of the forum + * @param bool $local Whether the file is local or not + * @param string $local_storage The path to the local file + * @param bool $is_message Whether it is a PM or not + * @param array $local_filedata An file data object created for the local file + * + * @return object filespec + */ + public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = []) + { + return $this->upload->upload($form_name, $forum_id, $local, $local_storage, $is_message, $local_filedata); + } } -- cgit v1.2.1 From a0167ad41037d7fe9214ca36c4c7097177361a6b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 9 Oct 2015 14:35:40 +0200 Subject: [ticket/14168] Fix docblock in manager PHPBB3-14168 --- phpBB/phpbb/attachment/manager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/manager.php b/phpBB/phpbb/attachment/manager.php index 414a8edfdd..9dcd3c5c92 100644 --- a/phpBB/phpbb/attachment/manager.php +++ b/phpBB/phpbb/attachment/manager.php @@ -51,9 +51,9 @@ class manager * @return int|bool Number of deleted attachments or false if something * went wrong during attachment deletion */ - public function delete($mode, $id, $resync = true) + public function delete($mode, $ids, $resync = true) { - return $this->delete->delete($mode, $id, $resync); + return $this->delete->delete($mode, $ids, $resync); } /** -- cgit v1.2.1 From bb2634b5e5bdee53118eff8cdba3fea73932ff47 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 12 Oct 2015 12:11:26 +0200 Subject: [ticket/14168] Correctly state return type of upload and upload_attachment PHPBB3-14168 --- phpBB/phpbb/attachment/manager.php | 2 +- phpBB/phpbb/attachment/upload.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/attachment/manager.php b/phpBB/phpbb/attachment/manager.php index 9dcd3c5c92..3c47171b2f 100644 --- a/phpBB/phpbb/attachment/manager.php +++ b/phpBB/phpbb/attachment/manager.php @@ -90,7 +90,7 @@ class manager * @param bool $is_message Whether it is a PM or not * @param array $local_filedata An file data object created for the local file * - * @return object filespec + * @return array File data array */ public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = []) { diff --git a/phpBB/phpbb/attachment/upload.php b/phpBB/phpbb/attachment/upload.php index 2d49e05b71..957558768b 100644 --- a/phpBB/phpbb/attachment/upload.php +++ b/phpBB/phpbb/attachment/upload.php @@ -104,7 +104,7 @@ class upload * @param bool $is_message Whether it is a PM or not * @param array $local_filedata An file data object created for the local file * - * @return object filespec + * @return array File data array */ public function upload($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = array()) { -- cgit v1.2.1