diff options
-rw-r--r-- | phpBB/config/avatars.yml | 1 | ||||
-rw-r--r-- | phpBB/includes/functions_posting.php | 5 | ||||
-rw-r--r-- | phpBB/includes/functions_upload.php | 60 | ||||
-rw-r--r-- | phpBB/includes/message_parser.php | 20 | ||||
-rw-r--r-- | phpBB/phpbb/avatar/driver/upload.php | 30 | ||||
-rw-r--r-- | phpBB/posting.php | 2 | ||||
-rw-r--r-- | tests/avatar/manager_test.php | 17 | ||||
-rw-r--r-- | tests/functional/fileupload_form_test.php | 4 | ||||
-rw-r--r-- | tests/upload/filespec_test.php | 39 |
9 files changed, 134 insertions, 44 deletions
diff --git a/phpBB/config/avatars.yml b/phpBB/config/avatars.yml index d22a5db2ae..8e5b1fdbfe 100644 --- a/phpBB/config/avatars.yml +++ b/phpBB/config/avatars.yml @@ -45,6 +45,7 @@ services: - %core.root_path% - %core.php_ext% - @path_helper + - @mimetype.guesser - @cache.driver calls: - [set_name, [avatar.driver.upload]] diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index b3f164a84d..f7e33725ec 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -398,11 +398,12 @@ function posting_gen_topic_types($forum_id, $cur_topic_type = POST_NORMAL) * @param string $local_storage The path to the local file * @param bool $is_message Whether it is a PM or not * @param \filespec $local_filedata A filespec object created for the local file +* @param \phpbb\mimetype\guesser $mimetype_guesser The mimetype guesser object if used * @param \phpbb\plupload\plupload $plupload The plupload object if one is being used * * @return object filespec */ -function upload_attachment($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = false, \phpbb\plupload\plupload $plupload = null) +function upload_attachment($form_name, $forum_id, $local = false, $local_storage = '', $is_message = false, $local_filedata = false, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { global $auth, $user, $config, $db, $cache; global $phpbb_root_path, $phpEx, $phpbb_dispatcher; @@ -434,7 +435,7 @@ function upload_attachment($form_name, $forum_id, $local = false, $local_storage $extensions = $cache->obtain_attach_extensions((($is_message) ? false : (int) $forum_id)); $upload->set_allowed_extensions(array_keys($extensions['_allowed_'])); - $file = ($local) ? $upload->local_upload($local_storage, $local_filedata) : $upload->form_upload($form_name, $plupload); + $file = ($local) ? $upload->local_upload($local_storage, $local_filedata, $mimetype_guesser) : $upload->form_upload($form_name, $mimetype_guesser, $plupload); if ($file->init_error) { diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index 0847c3a550..e658e607ec 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -53,10 +53,16 @@ class filespec protected $plupload; /** + * phpBB Mimetype guesser + * @var \phpbb\mimetype\guesser + */ + protected $mimetype_guesser; + + /** * File Class * @access private */ - function filespec($upload_ary, $upload_namespace, \phpbb\plupload\plupload $plupload = null) + function filespec($upload_ary, $upload_namespace, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { if (!isset($upload_ary)) { @@ -76,7 +82,7 @@ class filespec if (!$this->mimetype) { - $this->mimetype = 'application/octetstream'; + $this->mimetype = 'application/octet-stream'; } $this->extension = strtolower(self::get_extension($this->realname)); @@ -90,6 +96,7 @@ class filespec $this->local = (isset($upload_ary['local_mode'])) ? true : false; $this->upload = $upload_namespace; $this->plupload = $plupload; + $this->mimetype_guesser = $mimetype_guesser; } /** @@ -215,25 +222,19 @@ class filespec } /** - * Get mimetype. Utilize mime_content_type if the function exist. - * Not used at the moment... + * Get mimetype + * + * @param string $filename Filename that needs to be checked + * @return string Mimetype of supplied filename */ function get_mimetype($filename) { - $mimetype = ''; - - if (function_exists('mime_content_type')) - { - $mimetype = mime_content_type($filename); - } - - // Some browsers choke on a mimetype of application/octet-stream - if (!$mimetype || $mimetype == 'application/octet-stream') + if ($this->mimetype_guesser !== null) { - $mimetype = 'application/octetstream'; + $this->mimetype = $this->mimetype_guesser->guess($filename); } - return $mimetype; + return $this->mimetype; } /** @@ -372,6 +373,9 @@ class filespec // Try to get real filesize from destination folder $this->filesize = (@filesize($this->destination_file)) ? @filesize($this->destination_file) : $this->filesize; + // Get mimetype of supplied file + $this->mimetype = $this->get_mimetype($this->destination_file); + if ($this->is_image() && !$skip_image_check) { $this->width = $this->height = 0; @@ -583,7 +587,7 @@ class fileupload * @return object $file Object "filespec" is returned, all further operations can be done with this object * @access public */ - function form_upload($form_name, \phpbb\plupload\plupload $plupload = null) + function form_upload($form_name, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { global $user, $request; @@ -599,7 +603,7 @@ class fileupload } } - $file = new filespec($upload, $this, $plupload); + $file = new filespec($upload, $this, $mimetype_guesser, $plupload); if ($file->init_error) { @@ -659,7 +663,7 @@ class fileupload /** * Move file from another location to phpBB */ - function local_upload($source_file, $filedata = false) + function local_upload($source_file, $filedata = false, \phpbb\mimetype\guesser $mimetype_guesser = null) { global $user, $request; @@ -672,20 +676,6 @@ class fileupload { $upload['name'] = utf8_basename($source_file); $upload['size'] = 0; - $mimetype = ''; - - if (function_exists('mime_content_type')) - { - $mimetype = mime_content_type($source_file); - } - - // Some browsers choke on a mimetype of application/octet-stream - if (!$mimetype || $mimetype == 'application/octet-stream') - { - $mimetype = 'application/octetstream'; - } - - $upload['type'] = $mimetype; } else { @@ -694,7 +684,7 @@ class fileupload $upload['type'] = $filedata['type']; } - $file = new filespec($upload, $this); + $file = new filespec($upload, $this, $mimetype_guesser); if ($file->init_error) { @@ -752,7 +742,7 @@ class fileupload * @return object $file Object "filespec" is returned, all further operations can be done with this object * @access public */ - function remote_upload($upload_url) + function remote_upload($upload_url, \phpbb\mimetype\guesser $mimetype_guesser = null) { global $user, $phpbb_root_path; @@ -931,7 +921,7 @@ class fileupload $upload_ary['tmp_name'] = $filename; - $file = new filespec($upload_ary, $this); + $file = new filespec($upload_ary, $this, $mimetype_guesser); $this->common_checks($file); return $file; diff --git a/phpBB/includes/message_parser.php b/phpBB/includes/message_parser.php index da27d8900d..92ace7b585 100644 --- a/phpBB/includes/message_parser.php +++ b/phpBB/includes/message_parser.php @@ -1083,6 +1083,12 @@ class parse_message extends bbcode_firstpass protected $plupload; /** + * The mimetype guesser object used for attachment mimetypes + * @var \phpbb\mimetype\guesser + */ + protected $mimetype_guesser; + + /** * Init - give message here or manually */ function parse_message($message = '') @@ -1560,7 +1566,7 @@ class parse_message extends bbcode_firstpass { if ($num_attachments < $cfg['max_attachments'] || $auth->acl_gets('m_', 'a_', $forum_id)) { - $filedata = upload_attachment($form_name, $forum_id, false, '', $is_message, false, $this->plupload); + $filedata = upload_attachment($form_name, $forum_id, false, '', $is_message, false, $this->mimetype_guesser, $this->plupload); $error = array_merge($error, $filedata['error']); if (!sizeof($error)) @@ -1792,4 +1798,16 @@ class parse_message extends bbcode_firstpass { $this->plupload = $plupload; } + + /** + * Setter function for passing the mimetype_guesser object + * + * @param \phpbb\mimetype\guesser $mimetype_guesser The mimetype_guesser object + * + * @return null + */ + public function set_mimetype_guesser(\phpbb\mimetype\guesser $mimetype_guesser) + { + $this->mimetype_guesser = $mimetype_guesser; + } } diff --git a/phpBB/phpbb/avatar/driver/upload.php b/phpBB/phpbb/avatar/driver/upload.php index c43004f340..edc5941602 100644 --- a/phpBB/phpbb/avatar/driver/upload.php +++ b/phpBB/phpbb/avatar/driver/upload.php @@ -19,6 +19,32 @@ namespace phpbb\avatar\driver; class upload extends \phpbb\avatar\driver\driver { /** + * @var \phpbb\mimetype\guesser + */ + protected $mimetype_guesser; + + /** + * Construct a driver object + * + * @param \phpbb\config\config $config phpBB configuration + * @param \phpbb\request\request $request Request object + * @param string $phpbb_root_path Path to the phpBB root + * @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\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) + { + $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->cache = $cache; + } + + /** * {@inheritdoc} */ public function get_data($row, $ignore_config = false) @@ -70,7 +96,7 @@ class upload extends \phpbb\avatar\driver\driver if (!empty($upload_file['name'])) { - $file = $upload->form_upload('avatar_upload_file'); + $file = $upload->form_upload('avatar_upload_file', $this->mimetype_guesser); } else if (!empty($this->config['allow_avatar_remote_upload']) && !empty($url)) { @@ -100,7 +126,7 @@ class upload extends \phpbb\avatar\driver\driver return false; } - $file = $upload->remote_upload($url); + $file = $upload->remote_upload($url, $this->mimetype_guesser); } else { diff --git a/phpBB/posting.php b/phpBB/posting.php index 70df052099..6638caa94b 100644 --- a/phpBB/posting.php +++ b/phpBB/posting.php @@ -523,7 +523,9 @@ $orig_poll_options_size = sizeof($post_data['poll_options']); $message_parser = new parse_message(); $plupload = $phpbb_container->get('plupload'); +$mimetype_guesser = $phpbb_container->get('mimetype.guesser'); $message_parser->set_plupload($plupload); +$message_parser->set_mimetype_guesser($mimetype_guesser); if (isset($post_data['post_text'])) { diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index de505e2c9f..d8099b40d4 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -43,6 +43,14 @@ class phpbb_avatar_manager_test extends \phpbb_test_case $phpEx ); + $guessers = array( + new \Symfony\Component\HttpFoundation\File\MimeType\FileinfoMimeTypeGuesser(), + new \Symfony\Component\HttpFoundation\File\MimeType\FileBinaryMimeTypeGuesser(), + new \phpbb\mimetype\extension_guesser, + new \phpbb\mimetype\content_guesser, + ); + $guesser = new \phpbb\mimetype\guesser($guessers); + // $this->avatar_foobar will be needed later on $this->avatar_foobar = $this->getMock('\phpbb\avatar\driver\foobar', array('get_name'), array($config, $phpbb_root_path, $phpEx, $path_helper, $cache)); $this->avatar_foobar->expects($this->any()) @@ -57,7 +65,14 @@ class phpbb_avatar_manager_test extends \phpbb_test_case foreach ($this->avatar_drivers() as $driver) { - $cur_avatar = $this->getMock('\phpbb\avatar\driver\\' . $driver, array('get_name'), array($config, $phpbb_root_path, $phpEx, $path_helper, $cache)); + if ($driver !== 'upload') + { + $cur_avatar = $this->getMock('\phpbb\avatar\driver\\' . $driver, array('get_name'), array($config, $phpbb_root_path, $phpEx, $path_helper, $cache)); + } + else + { + $cur_avatar = $this->getMock('\phpbb\avatar\driver\\' . $driver, array('get_name'), array($config, $phpbb_root_path, $phpEx, $path_helper, $guesser, $cache)); + } $cur_avatar->expects($this->any()) ->method('get_name') ->will($this->returnValue('avatar.driver.' . $driver)); diff --git a/tests/functional/fileupload_form_test.php b/tests/functional/fileupload_form_test.php index b9d55fbd3c..29036c821e 100644 --- a/tests/functional/fileupload_form_test.php +++ b/tests/functional/fileupload_form_test.php @@ -109,9 +109,9 @@ class phpbb_functional_fileupload_form_test extends phpbb_functional_test_case $crawler = $this->upload_file('disallowed.jpg', 'image/jpeg'); - // Hitting the UNABLE_GET_IMAGE_SIZE error means we passed the + // Hitting the ATTACHED_IMAGE_NOT_IMAGE error means we passed the // DISALLOWED_CONTENT check - $this->assertEquals($this->lang('UNABLE_GET_IMAGE_SIZE'), $crawler->filter('p.error')->text()); + $this->assertContains($this->lang('ATTACHED_IMAGE_NOT_IMAGE'), $crawler->text()); } public function test_too_large() diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php index 5e333213f4..d8fa82e2b5 100644 --- a/tests/upload/filespec_test.php +++ b/tests/upload/filespec_test.php @@ -65,6 +65,16 @@ class phpbb_filespec_test extends phpbb_test_case copy($fileinfo->getPathname(), $this->path . 'copies/' . $fileinfo->getFilename() . '_copy_2'); } } + + $guessers = array( + new \Symfony\Component\HttpFoundation\File\MimeType\FileinfoMimeTypeGuesser(), + new \Symfony\Component\HttpFoundation\File\MimeType\FileBinaryMimeTypeGuesser(), + new \phpbb\mimetype\content_guesser(), + new \phpbb\mimetype\extension_guesser(), + ); + $guessers[2]->set_priority(-2); + $guessers[3]->set_priority(-2); + $this->mimetype_guesser = new \phpbb\mimetype\guesser($guessers); } private function get_filespec($override = array()) @@ -78,7 +88,7 @@ class phpbb_filespec_test extends phpbb_test_case 'error' => '', ); - return new filespec(array_merge($upload_ary, $override), null); + return new filespec(array_merge($upload_ary, $override), null, $this->mimetype_guesser); } protected function tearDown() @@ -222,6 +232,9 @@ class phpbb_filespec_test extends phpbb_test_case array('png', 'image/png', true), array('tif', 'image/tif', true), array('txt', 'text/plain', false), + array('jpg', 'application/octet-stream', false), + array('gif', 'application/octetstream', false), + array('png', 'application/mime', false), ); } @@ -234,6 +247,30 @@ class phpbb_filespec_test extends phpbb_test_case $this->assertEquals($expected, $filespec->is_image()); } + public function is_image_get_mimetype() + { + return array( + array('gif', 'image/gif', true), + array('jpg', 'image/jpg', true), + array('png', 'image/png', true), + array('tif', 'image/tif', true), + array('txt', 'text/plain', false), + array('jpg', 'application/octet-stream', true), + array('gif', 'application/octetstream', true), + array('png', 'application/mime', true), + ); + } + + /** + * @dataProvider is_image_get_mimetype + */ + public function test_is_image_get_mimetype($filename, $mimetype, $expected) + { + $filespec = $this->get_filespec(array('tmp_name' => $this->path . $filename, 'type' => $mimetype)); + $filespec->get_mimetype($this->path . $filename); + $this->assertEquals($expected, $filespec->is_image()); + } + public function move_file_variables() { return array( |