diff options
-rw-r--r-- | phpBB/config/default/container/services_files.yml | 1 | ||||
-rw-r--r-- | phpBB/phpbb/files/filespec.php | 11 | ||||
-rw-r--r-- | tests/files/upload_test.php | 1 | ||||
-rw-r--r-- | tests/functional/fileupload_remote_test.php | 2 | ||||
-rw-r--r-- | tests/upload/filespec_test.php | 119 | ||||
-rw-r--r-- | tests/upload/fileupload_test.php | 3 |
6 files changed, 126 insertions, 11 deletions
diff --git a/phpBB/config/default/container/services_files.yml b/phpBB/config/default/container/services_files.yml index a4136a0573..cfdade37df 100644 --- a/phpBB/config/default/container/services_files.yml +++ b/phpBB/config/default/container/services_files.yml @@ -14,6 +14,7 @@ services: - @filesystem - @language - @php_ini + - @upload_imagesize - %core.root_path% - @mimetype.guesser - @plupload diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 580016b281..6ce54a4789 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -75,6 +75,9 @@ class filespec /** @var \bantu\IniGetWrapper\IniGetWrapper ini_get() wrapper class */ protected $php_ini; + /** @var \fastImageSize\fastImageSize */ + protected $imagesize; + /** @var language Language class */ protected $language; @@ -97,11 +100,12 @@ class filespec * @param \phpbb\mimetype\guesser $mimetype_guesser Mime type guesser * @param \phpbb\plupload\plupload $plupload Plupload */ - public function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, language $language, \bantu\IniGetWrapper\IniGetWrapper $php_ini, $phpbb_root_path, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) + public function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, language $language, \bantu\IniGetWrapper\IniGetWrapper $php_ini, \fastImageSize\fastImageSize $imagesize, $phpbb_root_path, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { $this->filesystem = $phpbb_filesystem; $this->language = $language; $this->php_ini = $php_ini; + $this->imagesize = $imagesize; $this->phpbb_root_path = $phpbb_root_path; $this->plupload = $plupload; $this->mimetype_guesser = $mimetype_guesser; @@ -500,10 +504,7 @@ class filespec { $this->width = $this->height = 0; - // Get imagesize class - $imagesize = new \fastImageSize\fastImageSize(); - - $this->image_info = $imagesize->getImageSize($this->destination_file, $this->mimetype); + $this->image_info = $this->imagesize->getImageSize($this->destination_file, $this->mimetype); if ($this->image_info !== false) { diff --git a/tests/files/upload_test.php b/tests/files/upload_test.php index da9d78afbc..0d02926702 100644 --- a/tests/files/upload_test.php +++ b/tests/files/upload_test.php @@ -59,6 +59,7 @@ class phpbb_files_upload_test extends phpbb_test_case $this->filesystem, $this->language, $this->php_ini, + new \fastImageSize\fastImageSize(), $phpbb_root_path, new \phpbb\mimetype\guesser(array( 'mimetype.extension_guesser' => new \phpbb\mimetype\extension_guesser(), diff --git a/tests/functional/fileupload_remote_test.php b/tests/functional/fileupload_remote_test.php index 29701e072a..1484acc4d5 100644 --- a/tests/functional/fileupload_remote_test.php +++ b/tests/functional/fileupload_remote_test.php @@ -57,7 +57,7 @@ class phpbb_functional_fileupload_remote_test extends phpbb_functional_test_case $this->php_ini = new \bantu\IniGetWrapper\IniGetWrapper; $container = new phpbb_mock_container_builder(); - $container->set('files.filespec', new \phpbb\files\filespec($this->filesystem, $this->language, $this->php_ini, $this->phpbb_root_path)); + $container->set('files.filespec', new \phpbb\files\filespec($this->filesystem, $this->language, $this->php_ini, new \fastImageSize\fastImageSize(), $this->phpbb_root_path)); $this->factory = new \phpbb\files\factory($container); $container->set('files.factory', $this->factory); $container->set('files.types.remote', new \phpbb\files\types\remote($this->factory, $this->language, $this->php_ini, $this->request, $phpbb_root_path)); diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php index a53c27f045..10a442cd1d 100644 --- a/tests/upload/filespec_test.php +++ b/tests/upload/filespec_test.php @@ -93,7 +93,7 @@ class phpbb_filespec_test extends phpbb_test_case 'error' => '', ); - $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, $this->phpbb_root_path, $this->mimetype_guesser); + $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, new \fastImageSize\fastImageSize(), $this->phpbb_root_path, $this->mimetype_guesser); return $filespec->set_upload_ary(array_merge($upload_ary, $override)); } @@ -114,7 +114,7 @@ class phpbb_filespec_test extends phpbb_test_case public function test_empty_upload_ary() { - $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, $this->phpbb_root_path, $this->mimetype_guesser); + $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, new \fastImageSize\fastImageSize(), $this->phpbb_root_path, $this->mimetype_guesser); $this->assertInstanceOf('\phpbb\files\filespec', $filespec->set_upload_ary(array())); $this->assertTrue($filespec->init_error()); } @@ -262,7 +262,7 @@ class phpbb_filespec_test extends phpbb_test_case */ public function test_clean_filename_avatar($filename, $expected, $mode, $prefix = '', $user_id = '') { - $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, $this->phpbb_root_path, $this->mimetype_guesser); + $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, new \fastImageSize\fastImageSize(), $this->phpbb_root_path, $this->mimetype_guesser); if ($filename) { @@ -403,6 +403,117 @@ class phpbb_filespec_test extends phpbb_test_case $this->assertFalse($filespec->move_file('foo')); } + public function data_move_file_copy() + { + return array( + array('gif_copy', true, false, array()), + array('gif_copy', true, true, array()), + array('foo_bar', false, false, array('GENERAL_UPLOAD_ERROR')), + array('foo_bar', false, true, array('GENERAL_UPLOAD_ERROR')), + ); + } + + /** + * @dataProvider data_move_file_copy + */ + public function test_move_file_copy($tmp_name, $move_success, $safe_mode_on, $expected_error) + { + // Initialise a blank filespec object for use with trivial methods + $upload_ary = array( + 'name' => 'gif_moved', + 'type' => 'image/gif', + 'size' => '', + 'tmp_name' => $this->path . 'copies/' . $tmp_name, + 'error' => '', + ); + + $php_ini = $this->getMockBuilder('\bantu\IniGetWrapper\IniGetWrapper') + ->getMock(); + $php_ini->expects($this->any()) + ->method('getBool') + ->with($this->anything()) + ->willReturn($safe_mode_on); + $upload = new phpbb_mock_fileupload(); + $upload->max_filesize = self::UPLOAD_MAX_FILESIZE; + $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, $php_ini, new \fastImageSize\fastImagesize, '', $this->mimetype_guesser); + $filespec->set_upload_ary($upload_ary); + $filespec->local = false; + $filespec->extension = 'gif'; + $filespec->set_upload_namespace($upload); + + $this->assertEquals($move_success, $filespec->move_file($this->path . 'copies')); + $this->assertEquals($filespec->file_moved, file_exists($this->path . 'copies/gif_moved')); + $this->assertSame($expected_error, $filespec->error); + } + + public function data_move_file_imagesize() + { + return array( + array( + array( + 'width' => 2, + 'height' => 2, + 'type' => IMAGETYPE_GIF, + ), + array() + ), + array( + array( + 'width' => 2, + 'height' => 2, + 'type' => -1, + ), + array('Image file type -1 for mimetype image/gif not supported.') + ), + array( + array( + 'width' => 0, + 'height' => 0, + 'type' => IMAGETYPE_GIF, + ), + array('The image file you tried to attach is invalid.') + ), + array( + false, + array('It was not possible to determine the dimensions of the image. Please verify that the URL you entered is correct.') + ) + ); + } + + /** + * @dataProvider data_move_file_imagesize + */ + public function test_move_file_imagesize($imagesize_return, $expected_error) + { + // Initialise a blank filespec object for use with trivial methods + $upload_ary = array( + 'name' => 'gif_moved', + 'type' => 'image/gif', + 'size' => '', + 'tmp_name' => $this->path . 'copies/gif_copy', + 'error' => '', + ); + + $imagesize = $this->getMockBuilder('\fastImageSize\fastImageSize') + ->getMock(); + $imagesize->expects($this->any()) + ->method('getImageSize') + ->with($this->anything()) + ->willReturn($imagesize_return); + + $upload = new phpbb_mock_fileupload(); + $upload->max_filesize = self::UPLOAD_MAX_FILESIZE; + $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, $imagesize, '', $this->mimetype_guesser); + $filespec->set_upload_ary($upload_ary); + $filespec->local = false; + $filespec->extension = 'gif'; + $filespec->set_upload_namespace($upload); + + $this->assertEquals(true, $filespec->move_file($this->path . 'copies')); + $this->assertEquals($filespec->file_moved, file_exists($this->path . 'copies/gif_moved')); + $this->assertSame($expected_error, $filespec->error); + } + /** * @dataProvider clean_filename_variables */ @@ -419,7 +530,7 @@ class phpbb_filespec_test extends phpbb_test_case public function test_is_uploaded() { - $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, $this->phpbb_root_path, null); + $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, new \bantu\IniGetWrapper\IniGetWrapper, new \fastImageSize\fastImageSize(), $this->phpbb_root_path, null); $reflection_filespec = new ReflectionClass($filespec); $plupload_property = $reflection_filespec->getProperty('plupload'); $plupload_property->setAccessible(true); diff --git a/tests/upload/fileupload_test.php b/tests/upload/fileupload_test.php index 8675567e22..bb3092528b 100644 --- a/tests/upload/fileupload_test.php +++ b/tests/upload/fileupload_test.php @@ -72,6 +72,7 @@ class phpbb_fileupload_test extends phpbb_test_case $this->filesystem, $this->language, $this->php_ini, + new \fastImageSize\fastImageSize(), $phpbb_root_path, new \phpbb\mimetype\guesser(array( 'mimetype.extension_guesser' => new \phpbb\mimetype\extension_guesser(), @@ -131,7 +132,7 @@ class phpbb_fileupload_test extends phpbb_test_case $upload = new \phpbb\files\upload($this->filesystem, $this->factory, $this->language, $this->php_ini, $this->request, $this->phpbb_root_path); $upload->set_allowed_extensions(array('jpg')) ->set_max_filesize(1000); - $file = new \phpbb\files\filespec($this->filesystem, $this->language, $this->php_ini, $this->phpbb_root_path); + $file = new \phpbb\files\filespec($this->filesystem, $this->language, $this->php_ini, new \fastImageSize\fastImageSize(), $this->phpbb_root_path); $file->set_upload_ary(array( 'size' => 50, 'tmp_name' => dirname(__FILE__) . '/fixture/disallowed', |