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'); 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/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); } } 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 70df8043c8..132c16230d 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); @@ -138,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) @@ -331,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) @@ -363,7 +348,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) { @@ -435,7 +420,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; } /** diff --git a/phpBB/phpbb/storage/storage.php b/phpBB/phpbb/storage/storage.php index 7f9a7fdbb0..7ba26d6b95 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 @@ -106,6 +107,11 @@ class storage */ public function put_contents($path, $content) { + if ($this->exists($path)) + { + throw new exception('STORAGE_FILE_EXISTS', $path); + } + $this->get_adapter()->put_contents($path, $content); $this->track_file($path); } @@ -123,19 +129,25 @@ 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); } /** * 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. + * @return bool Returns true if the file/directory exist, false otherwise */ - public function exists($path) + public function exists($path, $full_check = false) { - return $this->get_adapter()->exists($path); + return ($this->is_tracked($path) && (!$full_check || $this->get_adapter()->exists($path))); } /** @@ -143,10 +155,16 @@ 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) { + if (!$this->exists($path)) + { + throw new exception('STORAGE_FILE_NO_EXIST', $path); + } + $this->get_adapter()->delete($path); $this->untrack_file($path); } @@ -157,11 +175,22 @@ 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) { + 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); + } + $this->get_adapter()->rename($path_orig, $path_dest); $this->track_rename($path_orig, $path_dest); } @@ -172,11 +201,22 @@ 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) { + 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); + } + $this->get_adapter()->copy($path_orig, $path_dest); $this->track_file($path_dest); } @@ -186,12 +226,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(); @@ -216,10 +262,21 @@ 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); + } + + if (!is_resource($resource)) + { + throw new exception('STORAGE_INVALID_RESOURCE'); + } + $adapter = $this->get_adapter(); if ($adapter instanceof stream_interface) @@ -242,6 +299,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(), @@ -256,7 +318,10 @@ class storage if (!$row) { - $file = $this->file_info($path); + // 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); $sql_ary['filesize'] = $file->size; $sql = 'INSERT INTO ' . $this->storage_table . $this->db->sql_build_array('INSERT', $sql_ary); @@ -295,6 +360,29 @@ 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( + 'file_path' => $path, + 'storage' => $this->get_name(), + ); + + $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 * @@ -316,11 +404,17 @@ 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 */ 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); } 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));