From 1d43e15c6064bae3f7caf1c72fb4e3d110a61d7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Fri, 15 Jun 2018 15:54:16 +0200 Subject: [PATCH 01/18] [ticket/15692] Move checks if file exist from adapter to storage PHPBB3-15692 --- .../storage/adapter/adapter_interface.php | 12 +- phpBB/phpbb/storage/adapter/local.php | 5 - phpBB/phpbb/storage/storage.php | 103 ++++++++++++++++-- 3 files changed, 96 insertions(+), 24 deletions(-) diff --git a/phpBB/phpbb/storage/adapter/adapter_interface.php b/phpBB/phpbb/storage/adapter/adapter_interface.php index 6d8b561c25..9c97f34a11 100644 --- a/phpBB/phpbb/storage/adapter/adapter_interface.php +++ b/phpBB/phpbb/storage/adapter/adapter_interface.php @@ -28,8 +28,7 @@ interface adapter_interface * @param string path The file to be written to. * @param string content The data to write into the file. * - * @throws \phpbb\storage\exception\exception When the file already exists - * When the file cannot be written + * @throws \phpbb\storage\exception\exception When the file cannot be written */ public function put_contents($path, $content); @@ -38,8 +37,7 @@ interface adapter_interface * * @param string $path The file to read * - * @throws \phpbb\storage\exception\exception When the file doesn't exist - * When cannot read file contents + * @throws \phpbb\storage\exception\exception When cannot read file contents * * @return string Returns file contents * @@ -70,8 +68,7 @@ interface adapter_interface * @param string $path_orig The original file/direcotry * @param string $path_dest The target file/directory * - * @throws \phpbb\storage\exception\exception When target exists - * When file/directory cannot be renamed + * @throws \phpbb\storage\exception\exception When file/directory cannot be renamed */ public function rename($path_orig, $path_dest); @@ -81,8 +78,7 @@ interface adapter_interface * @param string $path_orig The original filename * @param string $path_dest The target filename * - * @throws \phpbb\storage\exception\exception When target exists - * When the file cannot be copied + * @throws \phpbb\storage\exception\exception When the file cannot be copied */ public function copy($path_orig, $path_dest); diff --git a/phpBB/phpbb/storage/adapter/local.php b/phpBB/phpbb/storage/adapter/local.php index c7fbe544e9..7f55590fd8 100644 --- a/phpBB/phpbb/storage/adapter/local.php +++ b/phpBB/phpbb/storage/adapter/local.php @@ -118,11 +118,6 @@ class local implements adapter_interface, stream_interface { $this->ensure_directory_exists($path); - if ($this->exists($path)) - { - throw new exception('STORAGE_FILE_EXISTS', $path); - } - try { $this->filesystem->dump_file($this->root_path . $this->get_path($path) . $this->get_filename($path), $content); diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 97d71048ee..b9c20ed7cf 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -106,8 +106,22 @@ class storage */ public function put_contents($path, $content) { - $this->get_adapter()->put_contents($path, $content); - $this->track_file($path); + if ($this->exists($path)) + { + throw new exception('STORAGE_FILE_EXISTS', $path); + } + + try + { + $this->get_adapter()->put_contents($path, $content); + $this->track_file($path); + } + catch (\Exception $e) + { + $this->get_adapter()->delete($path); + $this->untrack_file($path); + throw $e; + } } /** @@ -123,6 +137,11 @@ class storage */ public function get_contents($path) { + if (!$this->exists($path)) + { + throw new exception('STORAGE_FILE_NO_EXIST', $path); + } + return $this->get_adapter()->get_contents($path); } @@ -147,6 +166,11 @@ class storage */ public function delete($path) { + if (!$this->exists($path)) + { + throw new exception('STORAGE_FILE_NO_EXIST', $path); + } + $this->get_adapter()->delete($path); $this->untrack_file($path); } @@ -157,13 +181,31 @@ class storage * @param string $path_orig The original file/direcotry * @param string $path_dest The target file/directory * - * @throws \phpbb\storage\exception\exception When target exists + * @throws \phpbb\storage\exception\exception When the file doesn't exist + * When target exists * When file/directory cannot be renamed */ public function rename($path_orig, $path_dest) { - $this->get_adapter()->rename($path_orig, $path_dest); - $this->track_rename($path_orig, $path_dest); + if (!$this->exists($path_orig)) + { + throw new exception('STORAGE_FILE_NO_EXIST', $path_orig); + } + + if ($this->exists($path_dest)) + { + throw new exception('STORAGE_FILE_EXISTS', $path_dest); + } + + try { + $this->get_adapter()->rename($path_orig, $path_dest); + $this->track_rename($path_orig, $path_dest); + } + catch (\Exception $e) + { + $this->untrack_file($path_dest); + throw $e; + } } /** @@ -172,13 +214,32 @@ class storage * @param string $path_orig The original filename * @param string $path_dest The target filename * - * @throws \phpbb\storage\exception\exception When target exists + * @throws \phpbb\storage\exception\exception When the file doesn't exist + * When target exists * When the file cannot be copied */ public function copy($path_orig, $path_dest) { - $this->get_adapter()->copy($path_orig, $path_dest); - $this->track_file($path_dest); + if (!$this->exists($path_orig)) + { + throw new exception('STORAGE_FILE_NO_EXIST', $path_orig); + } + + if ($this->exists($path_dest)) + { + throw new exception('STORAGE_FILE_EXISTS', $path_dest); + } + + try + { + $this->get_adapter()->copy($path_orig, $path_dest); + $this->track_file($path_dest); + } + catch (\Exception $e) + { + $this->untrack_file($path_dest); + throw $e; + } } /** @@ -186,12 +247,18 @@ class storage * * @param string $path File to read * - * @throws \phpbb\storage\exception\exception When unable to open file + * @throws \phpbb\storage\exception\exception When the file doesn't exist + * When unable to open file * * @return resource Returns a file pointer */ public function read_stream($path) { + if (!$this->exists($path)) + { + throw new exception('STORAGE_FILE_NO_EXIST', $path); + } + $stream = null; $adapter = $this->get_adapter(); @@ -217,15 +284,29 @@ class storage * @param string $path The target file * @param resource $resource The resource * - * @throws \phpbb\storage\exception\exception When target file cannot be created + * @throws \phpbb\storage\exception\exception When the file exist + * When target file cannot be created */ public function write_stream($path, $resource) { + if ($this->exists($path)) + { + throw new exception('STORAGE_FILE_EXISTS', $path); + } + $adapter = $this->get_adapter(); if ($adapter instanceof stream_interface) { - $adapter->write_stream($path, $resource); + try + { + $adapter->write_stream($path, $resource); + } + catch (\Exception $e) + { + $this->get_adapter()->delete($path); + $this->untrack_file($path); + } } else { From 4fed28577902a363749dc3f63df1c02f06d6d051 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Fri, 15 Jun 2018 15:58:16 +0200 Subject: [PATCH 02/18] [ticket/15692] Reduce storage api calls PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index b9c20ed7cf..f2fabdcb94 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -154,7 +154,7 @@ class storage */ public function exists($path) { - return $this->get_adapter()->exists($path); + return $this->is_tracked($path); } /** @@ -376,6 +376,23 @@ class storage $this->cache->destroy('_storage_' . $this->get_name() . '_numfiles'); } + public function is_tracked($path) + { + $sql_ary = array( + 'file_path' => $path, + 'storage' => $this->get_name(), + ); + + // Get file, if exist update filesize, if not add new record + $sql = 'SELECT file_id FROM ' . $this->storage_table . ' + WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); + $result = $this->db->sql_query($sql); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + return ($row) ? true : false; + } + /** * Rename tracked file * From 3ebbb8b837bbf98d1965fb0e7e52bc432957e8a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Fri, 15 Jun 2018 18:29:27 +0200 Subject: [PATCH 03/18] [ticket/15692] Add missing import PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index f2fabdcb94..4097d7ac82 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -15,6 +15,7 @@ namespace phpbb\storage; use phpbb\cache\driver\driver_interface as cache; use phpbb\db\driver\driver_interface as db; +use phpbb\storage\exception\exception; /** * @internal Experimental From 5934bdaaf108b59129be5ffbf7260267742790ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Sat, 16 Jun 2018 17:42:58 +0200 Subject: [PATCH 04/18] [ticket/15692] Close db connection after use storage PHPBB3-15692 --- phpBB/includes/functions_download.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index 6e25c0fbdb..b599e78a39 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -223,9 +223,6 @@ function send_file_to_browser($attachment, $category) } } - // Close the db connection before sending the file etc. - file_gc(false); - if (!set_modified_headers($attachment['filetime'], $user->browser)) { if ($size) @@ -238,6 +235,9 @@ function send_file_to_browser($attachment, $category) $fp = $storage->read_stream($filename); + // Close the db connection before sending the file etc. + file_gc(false); + if ($fp !== false) { $output = fopen('php://output', 'w+b'); From 9650763a19d0fd653e1b323d86973cafa2dc3b4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Sat, 16 Jun 2018 17:46:19 +0200 Subject: [PATCH 05/18] [ticket/15692] Allow to check if file exists using adapter too PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 4097d7ac82..d3e2024a50 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -153,9 +153,9 @@ class storage * * @return bool Returns true if the file/directory exist, false otherwise. */ - public function exists($path) + public function exists($path, $full_check = false) { - return $this->is_tracked($path); + return ($this->is_tracked($path) && $full_check && $this->get_adapter()->exists($path)); } /** From d65e60d1a6afe6f13313e16de14c62fdd6b78f4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Thu, 21 Jun 2018 04:56:25 +0200 Subject: [PATCH 06/18] [ticket/15692] Fix exists condition and add more checks if file exists PHPBB3-15692 --- phpBB/phpbb/storage/adapter/local.php | 2 +- phpBB/phpbb/storage/storage.php | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/storage/adapter/local.php b/phpBB/phpbb/storage/adapter/local.php index 7f55590fd8..da3950186e 100644 --- a/phpBB/phpbb/storage/adapter/local.php +++ b/phpBB/phpbb/storage/adapter/local.php @@ -356,7 +356,7 @@ class local implements adapter_interface, stream_interface */ public function file_size($path) { - $size = filesize($this->root_path . $this->get_path($path) . $this->get_filename($path)); + $size = @filesize($this->root_path . $this->get_path($path) . $this->get_filename($path)); if ($size === null) { diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index d3e2024a50..4a31f87e82 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -155,7 +155,7 @@ class storage */ public function exists($path, $full_check = false) { - return ($this->is_tracked($path) && $full_check && $this->get_adapter()->exists($path)); + return ($this->is_tracked($path) && ($full_check ? $this->get_adapter()->exists($path) : true)); } /** @@ -324,6 +324,11 @@ class storage */ public function track_file($path, $update = false) { + if (!$this->get_adapter()->exists($path)) + { + throw new exception('STORAGE_FILE_NO_EXIST', $path); + } + $sql_ary = array( 'file_path' => $path, 'storage' => $this->get_name(), @@ -338,7 +343,7 @@ class storage if (!$row) { - $file = $this->file_info($path); + $file = new file_info($this->get_adapter(), $path); $sql_ary['filesize'] = $file->size; $sql = 'INSERT INTO ' . $this->storage_table . $this->db->sql_build_array('INSERT', $sql_ary); @@ -420,6 +425,11 @@ class storage */ public function file_info($path) { + if (!$this->exists($path)) + { + throw new exception('STORAGE_FILE_NO_EXIST', $path); + } + return new file_info($this->get_adapter(), $path); } From d40bda0e08ee3e63dd97ec1aa50bf6e15246c575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Thu, 21 Jun 2018 05:11:31 +0200 Subject: [PATCH 07/18] [ticket/15692] Modify condition PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 4a31f87e82..deab3b708a 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -155,7 +155,7 @@ class storage */ public function exists($path, $full_check = false) { - return ($this->is_tracked($path) && ($full_check ? $this->get_adapter()->exists($path) : true)); + return ($this->is_tracked($path) && !$full_check || $this->get_adapter()->exists($path)); } /** From 69c1f5713cb72cc98bd0304c6e73d1b6b84a05fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Thu, 21 Jun 2018 05:29:26 +0200 Subject: [PATCH 08/18] [ticket/15692] Modify variable PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index deab3b708a..fab84891f9 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -305,7 +305,7 @@ class storage } catch (\Exception $e) { - $this->get_adapter()->delete($path); + $adapter->delete($path); $this->untrack_file($path); } } From 071d5fe9971b33a57b52852743415435b02a44ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Tue, 7 Aug 2018 00:16:37 +0200 Subject: [PATCH 09/18] [ticket/15692] Remove useless code PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 46 +++++---------------------------- 1 file changed, 7 insertions(+), 39 deletions(-) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index fab84891f9..545f342e82 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -112,17 +112,8 @@ class storage throw new exception('STORAGE_FILE_EXISTS', $path); } - try - { - $this->get_adapter()->put_contents($path, $content); - $this->track_file($path); - } - catch (\Exception $e) - { - $this->get_adapter()->delete($path); - $this->untrack_file($path); - throw $e; - } + $this->get_adapter()->put_contents($path, $content); + $this->track_file($path); } /** @@ -198,15 +189,8 @@ class storage throw new exception('STORAGE_FILE_EXISTS', $path_dest); } - try { - $this->get_adapter()->rename($path_orig, $path_dest); - $this->track_rename($path_orig, $path_dest); - } - catch (\Exception $e) - { - $this->untrack_file($path_dest); - throw $e; - } + $this->get_adapter()->rename($path_orig, $path_dest); + $this->track_rename($path_orig, $path_dest); } /** @@ -231,16 +215,8 @@ class storage throw new exception('STORAGE_FILE_EXISTS', $path_dest); } - try - { - $this->get_adapter()->copy($path_orig, $path_dest); - $this->track_file($path_dest); - } - catch (\Exception $e) - { - $this->untrack_file($path_dest); - throw $e; - } + $this->get_adapter()->copy($path_orig, $path_dest); + $this->track_file($path_dest); } /** @@ -299,15 +275,7 @@ class storage if ($adapter instanceof stream_interface) { - try - { - $adapter->write_stream($path, $resource); - } - catch (\Exception $e) - { - $adapter->delete($path); - $this->untrack_file($path); - } + $adapter->write_stream($path, $resource); } else { From 5fbe929a1d5b4c8354406245db743c597f052a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Tue, 7 Aug 2018 00:18:25 +0200 Subject: [PATCH 10/18] [ticket/15692] Check if argument is a valid resource PHPBB3-15692 --- phpBB/language/en/common.php | 1 + phpBB/phpbb/storage/storage.php | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index c7325043b2..1bd8ac1e44 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -745,6 +745,7 @@ $lang = array_merge($lang, array( 'STORAGE_CANNOT_CREATE_DIR' => 'Can not create directory.', 'STORAGE_CANNOT_OPEN_FILE' => 'Can not open file.', 'STORAGE_CANNOT_CREATE_FILE' => 'Can not create file.', + 'STORAGE_INVALID_RESOURCE' => 'Resource is invalid.', 'TB' => 'TB', 'TERMS_LINK' => 'Terms', diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 545f342e82..25c81075c4 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -271,6 +271,11 @@ class storage throw new exception('STORAGE_FILE_EXISTS', $path); } + if (!is_resource($resource)) + { + throw new exception('STORAGE_INVALID_RESOURCE'); + } + $adapter = $this->get_adapter(); if ($adapter instanceof stream_interface) From d0b7875b1881b84cd23f323a6b88318e70c159e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Tue, 7 Aug 2018 00:23:47 +0200 Subject: [PATCH 11/18] [ticket/15692] Update tests PHPBB3-15692 --- tests/attachment/delete_test.php | 18 ++---------------- tests/attachment/upload_test.php | 13 ++++--------- tests/avatar/manager_test.php | 13 ++----------- tests/notification/submit_post_base.php | 1 - tests/privmsgs/delete_user_pms_test.php | 11 +---------- 5 files changed, 9 insertions(+), 47 deletions(-) diff --git a/tests/attachment/delete_test.php b/tests/attachment/delete_test.php index 35dbf5d47d..67adc50edf 100644 --- a/tests/attachment/delete_test.php +++ b/tests/attachment/delete_test.php @@ -21,9 +21,6 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case /** @var \phpbb\db\driver\driver_interface */ protected $db; - /** @var \phpbb\filesystem\filesystem */ - protected $filesystem; - /** @var \phpbb\attachment\resync */ protected $resync; @@ -47,22 +44,11 @@ class phpbb_attachment_delete_test extends \phpbb_database_test_case $cache = $this->createMock('\phpbb\cache\driver\driver_interface'); $this->config = new \phpbb\config\config(array()); $this->db = $this->new_dbal(); - $db_mock = $this->createMock('\phpbb\db\driver\driver_interface'); $this->resync = new \phpbb\attachment\resync($this->db); - $this->filesystem = $this->createMock('\phpbb\filesystem\filesystem', array('remove', 'exists')); - $this->filesystem->expects($this->any()) - ->method('remove') - ->willReturn(false); - $this->filesystem->expects($this->any()) + $this->storage = $this->createMock('\phpbb\storage\storage'); + $this->storage->expects($this->any()) ->method('exists') ->willReturn(true); - $adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); - $adapter->configure(['path' => 'files']); - $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); - $adapter_factory_mock->expects($this->any()) - ->method('get') - ->willReturn($adapter); - $this->storage = new \phpbb\storage\storage($db_mock, $cache, $adapter_factory_mock, '', ''); $this->dispatcher = new \phpbb_mock_event_dispatcher(); $this->attachment_delete = new \phpbb\attachment\delete($this->config, $this->db, $this->dispatcher, $this->resync, $this->storage); } diff --git a/tests/attachment/upload_test.php b/tests/attachment/upload_test.php index f1df81bc8a..2a0bf0fa40 100644 --- a/tests/attachment/upload_test.php +++ b/tests/attachment/upload_test.php @@ -86,9 +86,7 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case )); $config = $this->config; $this->db = $this->new_dbal(); - $db_mock = $this->createMock('\phpbb\db\driver\driver_interface'); $this->cache = new \phpbb\cache\service(new \phpbb\cache\driver\dummy(), $this->config, $this->db, $phpbb_root_path, $phpEx); - $cache_mock = $this->createMock('\phpbb\cache\driver\driver_interface'); $this->request = $this->createMock('\phpbb\request\request'); $this->filesystem = new \phpbb\filesystem\filesystem(); @@ -105,13 +103,10 @@ class phpbb_attachment_upload_test extends \phpbb_database_test_case $this->mimetype_guesser = new \phpbb\mimetype\guesser($guessers); $this->plupload = new \phpbb\plupload\plupload($phpbb_root_path, $this->config, $this->request, new \phpbb\user($this->language, '\phpbb\datetime'), $this->php_ini, $this->mimetype_guesser); - $adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); - $adapter->configure(['path' => 'files']); - $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); - $adapter_factory_mock->expects($this->any()) - ->method('get') - ->willReturn($adapter); - $this->storage = new \phpbb\storage\storage($db_mock, $cache_mock, $adapter_factory_mock, '', ''); + $this->storage = $this->createMock('\phpbb\storage\storage'); + $this->storage->expects($this->any()) + ->method('free_space') + ->willReturn(1024*1024); // 1gb $factory_mock = $this->getMockBuilder('\phpbb\files\factory') ->disableOriginalConstructor() diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index c390eac883..8248223390 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -30,25 +30,16 @@ class phpbb_avatar_manager_test extends \phpbb_database_test_case global $phpbb_root_path, $phpEx; // Mock phpbb_container - - $cache = $this->createMock('\phpbb\cache\driver\driver_interface'); $phpbb_container = $this->createMock('Symfony\Component\DependencyInjection\ContainerInterface'); $phpbb_container->expects($this->any()) ->method('get') ->will($this->returnArgument(0)); - $filesystem = new \phpbb\filesystem\filesystem(); - $adapter = new \phpbb\storage\adapter\local($filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); - $adapter->configure(['path' => 'images/avatars/upload']); - $db = $this->createMock('\phpbb\db\driver\driver_interface'); - $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); - $adapter_factory_mock->expects($this->any()) - ->method('get') - ->willReturn($adapter); - $storage = new \phpbb\storage\storage($db, $cache, $adapter_factory_mock, '', ''); + $storage = $this->createMock('\phpbb\storage\storage'); // Prepare dependencies for avatar manager and driver $this->config = new \phpbb\config\config(array()); + $cache = $this->createMock('\phpbb\cache\driver\driver_interface'); $path_helper = new \phpbb\path_helper( new \phpbb\symfony_request( new phpbb_mock_request() diff --git a/tests/notification/submit_post_base.php b/tests/notification/submit_post_base.php index 255d52bde0..c011011ba7 100644 --- a/tests/notification/submit_post_base.php +++ b/tests/notification/submit_post_base.php @@ -56,7 +56,6 @@ abstract class phpbb_notification_submit_post_base extends phpbb_database_test_c // Database $this->db = $this->new_dbal(); $db = $this->db; - $db_mock = $this->createMock('\phpbb\db\driver\driver_interface'); // Auth $auth = $this->createMock('\phpbb\auth\auth'); diff --git a/tests/privmsgs/delete_user_pms_test.php b/tests/privmsgs/delete_user_pms_test.php index 50761d72ca..f1d717f03c 100644 --- a/tests/privmsgs/delete_user_pms_test.php +++ b/tests/privmsgs/delete_user_pms_test.php @@ -87,21 +87,12 @@ class phpbb_privmsgs_delete_user_pms_test extends phpbb_database_test_case { global $db, $phpbb_container, $phpbb_root_path; - $cache = $this->createMock('\phpbb\cache\driver\driver_interface'); - $db = $this->new_dbal(); - $db_mock = $this->createMock('\phpbb\db\driver\driver_interface'); $phpbb_container = new phpbb_mock_container_builder(); $phpbb_container->set('notification_manager', new phpbb_mock_notification_manager()); - $adapter = new \phpbb\storage\adapter\local(new \phpbb\filesystem\filesystem(), new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); - $adapter->configure(['path' => 'files']); - $adapter_factory_mock = $this->createMock('\phpbb\storage\adapter_factory'); - $adapter_factory_mock->expects($this->any()) - ->method('get') - ->willReturn($adapter); - $storage = new \phpbb\storage\storage($db_mock, $cache, $adapter_factory_mock, '', ''); + $storage = $this->createMock('\phpbb\storage\storage'); // Works as a workaround for tests $phpbb_container->set('attachment.manager', new \phpbb\attachment\delete(new \phpbb\config\config(array()), $db, new \phpbb_mock_event_dispatcher(), new \phpbb\attachment\resync($db), $storage)); From 115749a9ecc74c2ade96e7cf8c6d23c341a29374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Tue, 7 Aug 2018 00:35:58 +0200 Subject: [PATCH 12/18] [ticket/15692] Update docblock PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 25c81075c4..98cecb5936 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -142,7 +142,7 @@ class storage * * @param string $path file/directory to check * - * @return bool Returns true if the file/directory exist, false otherwise. + * @return bool Returns true if the file/directory exist, false otherwise */ public function exists($path, $full_check = false) { @@ -154,7 +154,8 @@ class storage * * @param string $path file/directory to remove * - * @throws \phpbb\storage\exception\exception When removal fails. + * @throws \phpbb\storage\exception\exception When removal fails + * When the file doesn't exist */ public function delete($path) { @@ -355,6 +356,13 @@ class storage $this->cache->destroy('_storage_' . $this->get_name() . '_numfiles'); } + /** + * Check if a file is tracked + * + * @param string $path The file + * + * @return bool True if file is tracked + */ public function is_tracked($path) { $sql_ary = array( @@ -393,6 +401,7 @@ class storage * @param string $path The file * * @throws \phpbb\storage\exception\not_implemented When the adapter doesnt implement the method + * When the file doesn't exist * * @return \phpbb\storage\file_info Returns file_info object */ From d5df62597b31aca30381deb1a1768c395f588769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Tue, 7 Aug 2018 01:42:29 +0200 Subject: [PATCH 13/18] [ticket/15692] Remove duplicated checks PHPBB3-15692 --- phpBB/phpbb/storage/adapter/local.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/phpBB/phpbb/storage/adapter/local.php b/phpBB/phpbb/storage/adapter/local.php index da3950186e..2ea8b8b03e 100644 --- a/phpBB/phpbb/storage/adapter/local.php +++ b/phpBB/phpbb/storage/adapter/local.php @@ -133,11 +133,6 @@ class local implements adapter_interface, stream_interface */ public function get_contents($path) { - if (!$this->exists($path)) - { - throw new exception('STORAGE_FILE_NO_EXIST', $path); - } - $content = @file_get_contents($this->root_path . $this->get_path($path) . $this->get_filename($path)); if ($content === false) @@ -326,11 +321,6 @@ class local implements adapter_interface, stream_interface { $this->ensure_directory_exists($path); - if ($this->exists($path)) - { - throw new exception('STORAGE_FILE_EXISTS', $path); - } - $stream = @fopen($this->root_path . $this->get_path($path) . $this->get_filename($path), 'w+b'); if (!$stream) From 0c1bf3c561beb7892a7fe008e27742784e420d83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Thu, 9 Aug 2018 02:28:58 +0200 Subject: [PATCH 14/18] [ticket/15692] Fix full_check in exist method PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 98cecb5936..1f22090745 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -146,7 +146,7 @@ class storage */ public function exists($path, $full_check = false) { - return ($this->is_tracked($path) && !$full_check || $this->get_adapter()->exists($path)); + return ($this->is_tracked($path) && (!$full_check || $this->get_adapter()->exists($path))); } /** From 252bb7704c7b05860b50c27df97f6b46b3b0ef2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Mon, 20 Aug 2018 23:58:24 +0200 Subject: [PATCH 15/18] [ticket/15692] Add comment PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 1f22090745..1bd693b6fa 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -317,6 +317,9 @@ class storage if (!$row) { + // Don't call the method, because it check's if the file is tracked + // and is not (for now). This method check if the file exists using the adapter + // at the beginning. $file = new file_info($this->get_adapter(), $path); $sql_ary['filesize'] = $file->size; From 8d89f20b81167520ee36aa78925b1dfe7eeffbbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Tue, 28 Aug 2018 00:32:06 +0200 Subject: [PATCH 16/18] [ticket/15692] Append / to board url PHPBB3-15692 --- phpBB/phpbb/storage/adapter/local.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/storage/adapter/local.php b/phpBB/phpbb/storage/adapter/local.php index 2ea8b8b03e..2b4b550aff 100644 --- a/phpBB/phpbb/storage/adapter/local.php +++ b/phpBB/phpbb/storage/adapter/local.php @@ -418,7 +418,7 @@ class local implements adapter_interface, stream_interface */ public function get_link($path) { - return generate_board_url() . $this->path . $path; + return generate_board_url() . '/' . $this->path . $path; } /** From 4734e56b5bfd66a5778365b5ec023b1fb7a88fe5 Mon Sep 17 00:00:00 2001 From: rubencm <rubencm@gmail.com> Date: Thu, 13 Sep 2018 12:39:16 +0000 Subject: [PATCH 17/18] [ticket/15692] Update comments PHPBB3-15692 --- phpBB/phpbb/storage/storage.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 1bd693b6fa..c8fcdacbf7 100644 --- a/phpBB/phpbb/storage/storage.php +++ b/phpBB/phpbb/storage/storage.php @@ -140,7 +140,8 @@ class storage /** * Checks the existence of files or directories * - * @param string $path file/directory to check + * @param string $path file/directory to check + * @param bool $full_check check in the filesystem too * * @return bool Returns true if the file/directory exist, false otherwise */ @@ -317,7 +318,7 @@ class storage if (!$row) { - // Don't call the method, because it check's if the file is tracked + // Don't call the file_info method, because it check's if the file is tracked // and is not (for now). This method check if the file exists using the adapter // at the beginning. $file = new file_info($this->get_adapter(), $path); @@ -373,7 +374,6 @@ class storage 'storage' => $this->get_name(), ); - // Get file, if exist update filesize, if not add new record $sql = 'SELECT file_id FROM ' . $this->storage_table . ' WHERE ' . $this->db->sql_build_array('SELECT', $sql_ary); $result = $this->db->sql_query($sql); From 212882fe70f7ea3fb740038d66b5db741af54588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Calvo?= <rubencm@gmail.com> Date: Sun, 16 Sep 2018 08:57:01 +0000 Subject: [PATCH 18/18] [ticket/15692] Don't try to track files that don't exist PHPBB3-15692 --- .../db/migration/data/v330/storage_track.php | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v330/storage_track.php b/phpBB/phpbb/db/migration/data/v330/storage_track.php index 7463e4641b..5abd875ae9 100644 --- a/phpBB/phpbb/db/migration/data/v330/storage_track.php +++ b/phpBB/phpbb/db/migration/data/v330/storage_track.php @@ -86,7 +86,14 @@ class storage_track extends \phpbb\db\migration\container_aware_migration $ext = substr(strrchr($filename, '.'), 1); $filename = (int) $filename; - $storage->track_file($this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $filename . '.' . $ext); + try + { + $storage->track_file($this->config['avatar_salt'] . '_' . ($avatar_group ? 'g' : '') . $filename . '.' . $ext); + } + catch (\phpbb\storage\exception\exception $e) + { + // If file don't exist, don't track it + } } $this->db->sql_freeresult($result); } @@ -103,11 +110,25 @@ class storage_track extends \phpbb\db\migration\container_aware_migration while ($row = $this->db->sql_fetchrow($result)) { - $storage->track_file($row['physical_filename']); + try + { + $storage->track_file($row['physical_filename']); + } + catch (\phpbb\storage\exception\exception $e) + { + // If file don't exist, don't track it + } if ($row['thumbnail'] == 1) { - $storage->track_file('thumb_' . $row['physical_filename']); + try + { + $storage->track_file('thumb_' . $row['physical_filename']); + } + catch (\phpbb\storage\exception\exception $e) + { + // If file don't exist, don't track it + } } } $this->db->sql_freeresult($result); @@ -125,8 +146,16 @@ class storage_track extends \phpbb\db\migration\container_aware_migration while ($row = $this->db->sql_fetchrow($result)) { - $storage->track_file($row['filename']); + try + { + $storage->track_file($row['filename']); + } + catch (\phpbb\storage\exception\exception $e) + { + // If file don't exist, don't track it + } } + $this->db->sql_freeresult($result); } }