From e0efd5ee576b10be1b826ee3971212a4bdd0b498 Mon Sep 17 00:00:00 2001 From: javiexin <javiexin@gmail.com> Date: Fri, 3 Jul 2015 21:39:36 +0200 Subject: [PATCH 1/6] [ticket/13981] Add events to capture avatar deletion or overwriting PHPBB3-13981 --- phpBB/phpbb/avatar/driver/upload.php | 60 ++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 7 deletions(-) diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index ee36243844..a1ea339371 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -80,6 +80,8 @@ class upload extends \phpbb\avatar\driver\driver */ public function process_form($request, $template, $user, $row, &$error) { + global $phpbb_dispatcher; + if (!$this->can_upload()) { return false; @@ -151,13 +153,34 @@ class upload extends \phpbb\avatar\driver\driver $destination = ''; } - // Move file and overwrite any existing image - $file->move_file($destination, true); + /** + * Before overwriting an existing avatar with a newly uploaded avatar + * + * @event core.avatar_driver_upload_overwrite_before + * @var string destination Destination directory where the file is going to be moved + * @var string prefix Prefix for the avatar filename + * @var array row Array with avatar row data + * @var array error Array of errors, if filled in by this event file will not be moved + * @since 3.1.6-RC1 + */ + $vars = array( + 'destination', + 'prefix', + 'row', + 'error', + ); + extract($phpbb_dispatcher->trigger_event('core.avatar_driver_upload_overwrite_before', compact($vars))); - if (sizeof($file->error)) + if (!sizeof($error)) + { + // Move file and overwrite any existing image + $file->move_file($destination, true); + } + + $error = array_merge($error, $file->error); + if (sizeof($error)) { $file->remove(); - $error = array_merge($error, $file->error); return false; } @@ -185,10 +208,33 @@ class upload extends \phpbb\avatar\driver\driver */ public function delete($row) { - $ext = substr(strrchr($row['avatar'], '.'), 1); - $filename = $this->phpbb_root_path . $this->config['avatar_path'] . '/' . $this->config['avatar_salt'] . '_' . $row['id'] . '.' . $ext; + global $phpbb_dispatcher; - if (file_exists($filename)) + $error = array(); + $destination = $this->config['avatar_path']; + $prefix = $this->config['avatar_salt'] . '_'; + $ext = substr(strrchr($row['avatar'], '.'), 1); + $filename = $this->phpbb_root_path . $destination . '/' . $prefix . $row['id'] . '.' . $ext; + + /** + * Before deleting an existing avatar + * + * @event core.avatar_driver_upload_delete_before + * @var string destination Destination directory where the file is going to be deleted + * @var string prefix Prefix for the avatar filename + * @var array row Array with avatar row data + * @var array error Array of errors, if filled in by this event file will not be deleted + * @since 3.1.6-RC1 + */ + $vars = array( + 'destination', + 'prefix', + 'row', + 'error', + ); + extract($phpbb_dispatcher->trigger_event('core.avatar_driver_upload_delete_before', compact($vars))); + + if (!sizeof($error) && file_exists($filename)) { @unlink($filename); } From 4b54df8d45210eed4374d5c4ce22be233c7a34c0 Mon Sep 17 00:00:00 2001 From: javiexin <javiexin@gmail.com> Date: Thu, 9 Jul 2015 16:33:56 +0200 Subject: [PATCH 2/6] [ticket/13981] Add events to capture avatar deletion or overwriting An event to capture overwriting, and another to capture deletion. Includes better error processing. PHPBB3-13981 --- phpBB/phpbb/avatar/driver/upload.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index a1ea339371..cc8662d811 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -139,6 +139,15 @@ class upload extends \phpbb\avatar\driver\driver $prefix = $this->config['avatar_salt'] . '_'; $file->clean_filename('avatar', $prefix, $row['id']); + // If there was an error during upload, then abort operation + if (sizeof($file->error)) + { + $file->remove(); + $error = $file->error; + return false; + } + + // Calculate new destination $destination = $this->config['avatar_path']; // Adjust destination path (no trailing slash) @@ -177,6 +186,7 @@ class upload extends \phpbb\avatar\driver\driver $file->move_file($destination, true); } + // If there was an error during move, then clean up leftovers $error = array_merge($error, $file->error); if (sizeof($error)) { From 4d3188ba57ae59c1f1f2eec8f5f70b8e1477c1af Mon Sep 17 00:00:00 2001 From: javiexin <javiexin@gmail.com> Date: Fri, 17 Jul 2015 19:40:33 +0200 Subject: [PATCH 3/6] [ticket/13981] Add events to capture avatar deletion or overwriting An event to capture overwriting, and another to capture deletion. Includes better error processing. Replaced global by dependency injection. PHPBB3-13981 --- phpBB/config/avatar.yml | 1 + phpBB/phpbb/avatar/driver/upload.php | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/phpBB/config/avatar.yml b/phpBB/config/avatar.yml index 5292489715..b0dca137b0 100644 --- a/phpBB/config/avatar.yml +++ b/phpBB/config/avatar.yml @@ -60,6 +60,7 @@ services: - %core.php_ext% - @path_helper - @mimetype.guesser + - @dispatcher - @cache.driver calls: - [set_name, [avatar.driver.upload]] diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index cc8662d811..05a52cd222 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -23,6 +23,11 @@ class upload extends \phpbb\avatar\driver\driver */ protected $mimetype_guesser; + /** + * @var \phpbb\event\dispatcher_interface + */ + protected $dispatcher; + /** * Construct a driver object * @@ -31,15 +36,17 @@ class upload extends \phpbb\avatar\driver\driver * @param string $php_ext PHP file extension * @param \phpbb_path_helper $path_helper phpBB path helper * @param \phpbb\mimetype\guesser $mimetype_guesser Mimetype guesser + * @param \phpbb\event\dispatcher_interface $dispatcher phpBB Event dispatcher object * @param \phpbb\cache\driver\driver_interface $cache Cache driver */ - public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\path_helper $path_helper, \phpbb\mimetype\guesser $mimetype_guesser, \phpbb\cache\driver\driver_interface $cache = null) + public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\path_helper $path_helper, \phpbb\mimetype\guesser $mimetype_guesser, \phpbb\cache\driver\driver_interface $dispatcher, \phpbb\cache\driver\driver_interface $cache = null) { $this->config = $config; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; $this->path_helper = $path_helper; $this->mimetype_guesser = $mimetype_guesser; + $this->dispatcher = $dispatcher; $this->cache = $cache; } @@ -80,8 +87,6 @@ class upload extends \phpbb\avatar\driver\driver */ public function process_form($request, $template, $user, $row, &$error) { - global $phpbb_dispatcher; - if (!$this->can_upload()) { return false; @@ -178,7 +183,7 @@ class upload extends \phpbb\avatar\driver\driver 'row', 'error', ); - extract($phpbb_dispatcher->trigger_event('core.avatar_driver_upload_overwrite_before', compact($vars))); + extract($this->dispatcher->trigger_event('core.avatar_driver_upload_overwrite_before', compact($vars))); if (!sizeof($error)) { @@ -218,7 +223,6 @@ class upload extends \phpbb\avatar\driver\driver */ public function delete($row) { - global $phpbb_dispatcher; $error = array(); $destination = $this->config['avatar_path']; @@ -242,7 +246,7 @@ class upload extends \phpbb\avatar\driver\driver 'row', 'error', ); - extract($phpbb_dispatcher->trigger_event('core.avatar_driver_upload_delete_before', compact($vars))); + extract($this->dispatcher->trigger_event('core.avatar_driver_upload_delete_before', compact($vars))); if (!sizeof($error) && file_exists($filename)) { From 11256cd1679f5764548976455cc89c10d8b1b48f Mon Sep 17 00:00:00 2001 From: javiexin <javiexin@gmail.com> Date: Fri, 17 Jul 2015 23:03:19 +0200 Subject: [PATCH 4/6] [ticket/13981] Add events to capture avatar deletion or overwriting An event to capture overwriting, and another to capture deletion. Includes better error processing. Replaced global by dependency injection. Fix typo. PHPBB3-13981 --- phpBB/phpbb/avatar/driver/upload.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index 05a52cd222..2393b4ed94 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -39,7 +39,7 @@ class upload extends \phpbb\avatar\driver\driver * @param \phpbb\event\dispatcher_interface $dispatcher phpBB Event dispatcher object * @param \phpbb\cache\driver\driver_interface $cache Cache driver */ - public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\path_helper $path_helper, \phpbb\mimetype\guesser $mimetype_guesser, \phpbb\cache\driver\driver_interface $dispatcher, \phpbb\cache\driver\driver_interface $cache = null) + public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\path_helper $path_helper, \phpbb\mimetype\guesser $mimetype_guesser, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\cache\driver\driver_interface $cache = null) { $this->config = $config; $this->phpbb_root_path = $phpbb_root_path; From e37b3495f02ecf895f2db1bdf2404f0c2be89073 Mon Sep 17 00:00:00 2001 From: javiexin <javiexin@gmail.com> Date: Thu, 23 Jul 2015 14:46:51 +0200 Subject: [PATCH 5/6] [ticket/13981] Add events to capture avatar deletion or overwriting An event to capture overwriting, and another to capture deletion. Includes better error processing. Replaced global by dependency injection. Fix Travis tests. PHPBB3-13981 --- tests/avatar/manager_test.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index a109a7b5de..9b97fa6a68 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -56,6 +56,8 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case ); $guesser = new \phpbb\mimetype\guesser($guessers); + $dispatcher = new phpbb_mock_event_dispatcher(); + // $this->avatar_foobar will be needed later on $this->avatar_foobar = $this->getMock('\phpbb\avatar\driver\foobar', array('get_name'), array($this->config, $phpbb_root_path, $phpEx, $path_helper, $cache)); $this->avatar_foobar->expects($this->any()) @@ -76,7 +78,7 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case } else { - $cur_avatar = $this->getMock('\phpbb\avatar\driver\\' . $driver, array('get_name'), array($this->config, $phpbb_root_path, $phpEx, $path_helper, $guesser, $cache)); + $cur_avatar = $this->getMock('\phpbb\avatar\driver\\' . $driver, array('get_name'), array($this->config, $phpbb_root_path, $phpEx, $path_helper, $guesser, $dispatcher, $cache)); } $cur_avatar->expects($this->any()) ->method('get_name') From c3d77edd836a0fd026f7ea644b87ab39cb24986e Mon Sep 17 00:00:00 2001 From: javiexin <javiexin@gmail.com> Date: Fri, 24 Jul 2015 10:52:00 +0200 Subject: [PATCH 6/6] [ticket/13981] Add events to capture avatar deletion or overwriting An event to capture new avatar moving in place (and maybe overwriting existing avatar), and another to capture deletion. Includes better error processing. Rename event. PHPBB3-13981 --- phpBB/phpbb/avatar/driver/upload.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index 2393b4ed94..f6a5324f0d 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -168,9 +168,9 @@ class upload extends \phpbb\avatar\driver\driver } /** - * Before overwriting an existing avatar with a newly uploaded avatar + * Before moving new file in place (and eventually overwriting the existing avatar with the newly uploaded avatar) * - * @event core.avatar_driver_upload_overwrite_before + * @event core.avatar_driver_upload_move_file_before * @var string destination Destination directory where the file is going to be moved * @var string prefix Prefix for the avatar filename * @var array row Array with avatar row data @@ -183,7 +183,7 @@ class upload extends \phpbb\avatar\driver\driver 'row', 'error', ); - extract($this->dispatcher->trigger_event('core.avatar_driver_upload_overwrite_before', compact($vars))); + extract($this->dispatcher->trigger_event('core.avatar_driver_upload_move_file_before', compact($vars))); if (!sizeof($error)) {