1
0
mirror of https://github.com/phpbb/phpbb.git synced 2025-02-25 04:23:38 +01:00

Merge branch 'develop-ascraeus' into develop

* develop-ascraeus:
  [ticket/11148] Remove unneeded variable mimetype and use type octet-stream
  [ticket/11148] Change expected output with disallowed content in test
  [ticket/11148] Always use the output of the mimetype guesser in get_mimetype
  [ticket/11148] Get rid of extra line in mimetype guesser setter doc block
  [ticket/11148] Add missing parts to docblock of get_mimetype() method
  [ticket/11148] Default to application/octet-stream if no mimetype given
  [ticket/11148] Use mimetype guesser for uploaded avatars
  [ticket/11148] Pass mimetype guesser to upload_attachment() function
  [ticket/11148] Add mimetype guesser to filespec and fileupload class
This commit is contained in:
Nils Adermann 2014-08-07 12:06:49 +02:00
commit 89a6fed91d
9 changed files with 134 additions and 44 deletions

View File

@ -45,6 +45,7 @@ services:
- %core.root_path%
- %core.php_ext%
- @path_helper
- @mimetype.guesser
- @cache.driver
calls:
- [set_name, [avatar.driver.upload]]

View File

@ -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)
{

View File

@ -52,11 +52,17 @@ 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'))
if ($this->mimetype_guesser !== null)
{
$mimetype = mime_content_type($filename);
$this->mimetype = $this->mimetype_guesser->guess($filename);
}
// Some browsers choke on a mimetype of application/octet-stream
if (!$mimetype || $mimetype == 'application/octet-stream')
{
$mimetype = 'application/octetstream';
}
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;

View File

@ -1082,6 +1082,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
*/
@ -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;
}
}

View File

@ -18,6 +18,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}
*/
@ -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
{

View File

@ -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']))
{

View File

@ -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));

View File

@ -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()

View File

@ -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(