From 697ac5f4aa151b06ed65f8352652443bf297682a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 15:06:24 +0200 Subject: [PATCH] [ticket/13904] Use language class instead of global user in filespec PHPBB3-13904 --- .../default/container/services_files.yml | 1 + phpBB/phpbb/files/filespec.php | 45 ++++++++++--------- tests/functional/fileupload_remote_test.php | 10 ++--- tests/upload/filespec_test.php | 19 ++++---- tests/upload/fileupload_test.php | 12 ++--- 5 files changed, 41 insertions(+), 46 deletions(-) diff --git a/phpBB/config/default/container/services_files.yml b/phpBB/config/default/container/services_files.yml index c275b3f205..aabb5583d1 100644 --- a/phpBB/config/default/container/services_files.yml +++ b/phpBB/config/default/container/services_files.yml @@ -12,6 +12,7 @@ services: scope: prototype arguments: - @filesystem + - @language - @mimetype.guesser - @plupload diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index d091e975d5..5e685615d7 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -13,6 +13,8 @@ namespace phpbb\files; +use \phpbb\language\language; + /** * Responsible for holding all file relevant information, as well as doing file-specific operations. * The {@link fileupload fileupload class} can be used to upload several files, each of them being this object to operate further on. @@ -81,18 +83,23 @@ class filespec */ protected $mimetype_guesser; + /** @var \phpbb\language\language Language class */ + protected $language; + /** * File upload class * * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem + * @param \phpbb\language\language $language * @param \phpbb\mimetype\guesser $mimetype_guesser * @param \phpbb\plupload\plupload $plupload */ - function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) + function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, language $language, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { $this->plupload = $plupload; $this->mimetype_guesser = $mimetype_guesser; $this->filesystem = $phpbb_filesystem; + $this->language = $language; } /** @@ -383,7 +390,7 @@ class filespec */ function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = false) { - global $user, $phpbb_root_path; + global $phpbb_root_path; if (sizeof($this->error)) { @@ -410,7 +417,7 @@ class filespec if (file_exists($this->destination_file) && !$overwrite) { @unlink($this->filename); - $this->error[] = $user->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); + $this->error[] = $this->language->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); $this->file_moved = false; return false; } @@ -429,7 +436,7 @@ class filespec { if (!@move_uploaded_file($this->filename, $this->destination_file)) { - $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR'], $this->destination_file); + $this->error[] = $this->language->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); } } @@ -441,7 +448,7 @@ class filespec { if (!@copy($this->filename, $this->destination_file)) { - $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR'], $this->destination_file); + $this->error[] = $this->language->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); } } @@ -451,7 +458,7 @@ class filespec if (!@copy($this->filename, $this->destination_file)) { - $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR'], $this->destination_file); + $this->error[] = $this->language->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); } break; @@ -502,23 +509,23 @@ class filespec { if (!isset($types[$this->image_info['type']])) { - $this->error[] = $user->lang('IMAGE_FILETYPE_INVALID', $this->image_info['type'], $this->mimetype); + $this->error[] = $this->language->lang('IMAGE_FILETYPE_INVALID', $this->image_info['type'], $this->mimetype); } else { - $this->error[] = $user->lang('IMAGE_FILETYPE_MISMATCH', $types[$this->image_info['type']][0], $this->extension); + $this->error[] = $this->language->lang('IMAGE_FILETYPE_MISMATCH', $types[$this->image_info['type']][0], $this->extension); } } // Make sure the dimensions match a valid image if (empty($this->width) || empty($this->height)) { - $this->error[] = $user->lang['ATTACHED_IMAGE_NOT_IMAGE']; + $this->error[] = $this->language->lang('ATTACHED_IMAGE_NOT_IMAGE'); } } else { - $this->error[] = $user->lang['UNABLE_GET_IMAGE_SIZE']; + $this->error[] = $this->language->lang('UNABLE_GET_IMAGE_SIZE'); } } @@ -536,8 +543,6 @@ class filespec */ function additional_checks() { - global $user; - if (!$this->file_moved) { return false; @@ -548,20 +553,20 @@ class filespec { $max_filesize = get_formatted_filesize($this->upload->max_filesize, false); - $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit']); + $this->error[] = $this->language->lang($this->upload->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit']); return false; } if (!$this->upload->valid_dimensions($this)) { - $this->error[] = $user->lang($this->upload->error_prefix . 'WRONG_SIZE', - $user->lang('PIXELS', (int) $this->upload->min_width), - $user->lang('PIXELS', (int) $this->upload->min_height), - $user->lang('PIXELS', (int) $this->upload->max_width), - $user->lang('PIXELS', (int) $this->upload->max_height), - $user->lang('PIXELS', (int) $this->width), - $user->lang('PIXELS', (int) $this->height)); + $this->error[] = $this->language->lang($this->upload->error_prefix . 'WRONG_SIZE', + $this->language->lang('PIXELS', (int) $this->upload->min_width), + $this->language->lang('PIXELS', (int) $this->upload->min_height), + $this->language->lang('PIXELS', (int) $this->upload->max_width), + $this->language->lang('PIXELS', (int) $this->upload->max_height), + $this->language->lang('PIXELS', (int) $this->width), + $this->language->lang('PIXELS', (int) $this->height)); return false; } diff --git a/tests/functional/fileupload_remote_test.php b/tests/functional/fileupload_remote_test.php index 7301297096..ae4de26df4 100644 --- a/tests/functional/fileupload_remote_test.php +++ b/tests/functional/fileupload_remote_test.php @@ -32,8 +32,7 @@ class phpbb_functional_fileupload_remote_test extends phpbb_functional_test_case // URL // Global $config required by unique_id - // Global $user required by fileupload::remote_upload - global $config, $user, $phpbb_root_path, $phpEx; + global $config, $phpbb_root_path, $phpEx; if (!is_array($config)) { @@ -43,16 +42,13 @@ class phpbb_functional_fileupload_remote_test extends phpbb_functional_test_case $config['rand_seed'] = ''; $config['rand_seed_last_update'] = time() + 600; - $user = new phpbb_mock_user(); - $user->lang = new phpbb_mock_lang(); $this->filesystem = new \phpbb\filesystem\filesystem(); + $this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); $container = new phpbb_mock_container_builder(); - $container->set('files.filespec', new \phpbb\files\filespec($this->filesystem)); + $container->set('files.filespec', new \phpbb\files\filespec($this->filesystem, $this->language)); $this->factory = new \phpbb\files\factory($container); $container->set('files.factory', $this->factory); - - $this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); } public function tearDown() diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php index 10577ec8f3..b28adc3f28 100644 --- a/tests/upload/filespec_test.php +++ b/tests/upload/filespec_test.php @@ -25,12 +25,13 @@ class phpbb_filespec_test extends phpbb_test_case private $filesystem; public $path; + /** @var \phpbb\language\language */ + protected $language; + protected function setUp() { // Global $config required by unique_id - // Global $user required by filespec::additional_checks and - // filespec::move_file - global $config, $user, $phpbb_filesystem; + global $config, $phpbb_root_path, $phpEx; if (!is_array($config)) { @@ -44,9 +45,6 @@ class phpbb_filespec_test extends phpbb_test_case // See: phpBB/install/schemas/schema_data.sql $config['mime_triggers'] = 'body|head|html|img|plaintext|a href|pre|script|table|title'; - $user = new phpbb_mock_user(); - $user->lang = new phpbb_mock_lang(); - $this->config = &$config; $this->path = __DIR__ . '/fixture/'; @@ -75,8 +73,9 @@ class phpbb_filespec_test extends phpbb_test_case $guessers[2]->set_priority(-2); $guessers[3]->set_priority(-2); $this->mimetype_guesser = new \phpbb\mimetype\guesser($guessers); + $this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); - $this->filesystem = $phpbb_filesystem = new \phpbb\filesystem\filesystem(); + $this->filesystem = new \phpbb\filesystem\filesystem(); } private function get_filespec($override = array()) @@ -90,15 +89,13 @@ class phpbb_filespec_test extends phpbb_test_case 'error' => '', ); - $filespec = new \phpbb\files\filespec($this->filesystem, $this->mimetype_guesser); + $filespec = new \phpbb\files\filespec($this->filesystem, $this->language, $this->mimetype_guesser); return $filespec->set_upload_ary(array_merge($upload_ary, $override)); } protected function tearDown() { - global $user; $this->config = array(); - $user = null; $iterator = new DirectoryIterator($this->path . 'copies'); foreach ($iterator as $fileinfo) @@ -289,7 +286,7 @@ class phpbb_filespec_test extends phpbb_test_case array('txt_copy', 'txt_as_img', 'image/jpg', 'txt', false, true), array('txt_copy_2', 'txt_moved', 'text/plain', 'txt', false, true), array('jpg_copy', 'jpg_moved', 'image/png', 'jpg', false, true), - array('png_copy', 'png_moved', 'image/png', 'jpg', 'IMAGE_FILETYPE_MISMATCH png jpg', true), + array('png_copy', 'png_moved', 'image/png', 'jpg', 'Image file type mismatch: expected extension png but extension jpg given.', true), ); } diff --git a/tests/upload/fileupload_test.php b/tests/upload/fileupload_test.php index 87e10ac954..56e29a3ac2 100644 --- a/tests/upload/fileupload_test.php +++ b/tests/upload/fileupload_test.php @@ -33,9 +33,8 @@ class phpbb_fileupload_test extends phpbb_test_case protected function setUp() { // Global $config required by unique_id - // Global $user required by several functions dealing with translations // Global $request required by form_upload, local_upload and is_valid - global $config, $user, $request, $phpbb_filesystem, $phpbb_root_path, $phpEx; + global $config, $request, $phpbb_root_path, $phpEx; if (!is_array($config)) { @@ -45,23 +44,20 @@ class phpbb_fileupload_test extends phpbb_test_case $config['rand_seed'] = ''; $config['rand_seed_last_update'] = time() + 600; - $user = new phpbb_mock_user(); - $user->lang = new phpbb_mock_lang(); - $request = new phpbb_mock_request(); - $this->filesystem = $phpbb_filesystem = new \phpbb\filesystem\filesystem(); + $this->filesystem = new \phpbb\filesystem\filesystem(); + $this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); $this->container = new phpbb_mock_container_builder($phpbb_root_path, $phpEx); $this->container->set('files.filespec', new \phpbb\files\filespec( $this->filesystem, + $this->language, new \phpbb\mimetype\guesser(array( 'mimetype.extension_guesser' => new \phpbb\mimetype\extension_guesser(), )))); $this->factory = new \phpbb\files\factory($this->container); - $this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); - $this->path = __DIR__ . '/fixture/'; }