From b971f20a5d9f6f163731b533fdb1242ebd27ff8f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 31 May 2015 13:29:49 +0200 Subject: [ticket/13904] Add initial factory for handling file classes PHPBB3-13904 --- phpBB/phpbb/files/factory.php | 79 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 phpBB/phpbb/files/factory.php (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/factory.php b/phpBB/phpbb/files/factory.php new file mode 100644 index 0000000000..9385ad678e --- /dev/null +++ b/phpBB/phpbb/files/factory.php @@ -0,0 +1,79 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files; + +class factory +{ + /** + * @var \Symfony\Component\DependencyInjection\ContainerInterface + */ + private $container; + + /** + * Constructor + * + * @param \Symfony\Component\DependencyInjection\ContainerInterface $container + */ + public function __construct(\Symfony\Component\DependencyInjection\ContainerInterface $container) + { + $this->container = $container; + } + + /** + * Get files service + * + * @param string $name Service name + * + * @return object|false Requested service or false if service could not be + * found by the container + */ + public function get($name) + { + $service = false; + + try + { + $service = $this->container->get($name); + } + catch (\Exception $e) + { + // do nothing + } + + return $service; + } + + /** + * Magic function for handling get calls, e.g. get_fileupload() or + * get_filespec() and turning them into call for files. services like + * files.fileupload. + * + * @param string $name Name of called function + * @param mixed $arguments Possible supplied arguments + * + * @return object|false Requested service or false if service could not be + * found by the container + */ + public function __call($name, $arguments) + { + if (substr($name, 0, 4) === 'get_') + { + return $this->get('files.' . substr($name, 4)); + } + else + { + return false; + } + } +} -- cgit v1.2.1 From 557f1a89d512e6c4f4c96033091ab5b429825e6d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 14 Aug 2015 09:01:29 +0200 Subject: [ticket/13904] Add filespec class to files classes PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 480 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 480 insertions(+) create mode 100644 phpBB/phpbb/files/filespec.php (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php new file mode 100644 index 0000000000..f74e91b0d5 --- /dev/null +++ b/phpBB/phpbb/files/filespec.php @@ -0,0 +1,480 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files; + +/** + * 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. + */ +class filespec +{ + var $filename = ''; + var $realname = ''; + var $uploadname = ''; + var $mimetype = ''; + var $extension = ''; + var $filesize = 0; + var $width = 0; + var $height = 0; + var $image_info = array(); + + var $destination_file = ''; + var $destination_path = ''; + + var $file_moved = false; + var $init_error = false; + var $local = false; + + var $error = array(); + + var $upload = ''; + + /** + * @var \phpbb\filesystem\filesystem_interface + */ + protected $filesystem; + + /** + * The plupload object + * @var \phpbb\plupload\plupload + */ + protected $plupload; + + /** + * phpBB Mimetype guesser + * @var \phpbb\mimetype\guesser + */ + protected $mimetype_guesser; + + /** + * File Class + * @access private + */ + function filespec($upload_ary, $upload_namespace, \phpbb\filesystem\filesystem_interface $phpbb_filesystem, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) + { + if (!isset($upload_ary)) + { + $this->init_error = true; + return; + } + + $this->filename = $upload_ary['tmp_name']; + $this->filesize = $upload_ary['size']; + $name = (STRIP) ? stripslashes($upload_ary['name']) : $upload_ary['name']; + $name = trim(utf8_basename($name)); + $this->realname = $this->uploadname = $name; + $this->mimetype = $upload_ary['type']; + + // Opera adds the name to the mime type + $this->mimetype = (strpos($this->mimetype, '; name') !== false) ? str_replace(strstr($this->mimetype, '; name'), '', $this->mimetype) : $this->mimetype; + + if (!$this->mimetype) + { + $this->mimetype = 'application/octet-stream'; + } + + $this->extension = strtolower(self::get_extension($this->realname)); + + // Try to get real filesize from temporary folder (not always working) ;) + $this->filesize = (@filesize($this->filename)) ? @filesize($this->filename) : $this->filesize; + + $this->width = $this->height = 0; + $this->file_moved = false; + + $this->local = (isset($upload_ary['local_mode'])) ? true : false; + $this->upload = $upload_namespace; + $this->plupload = $plupload; + $this->mimetype_guesser = $mimetype_guesser; + $this->filesystem = $phpbb_filesystem; + } + + /** + * Cleans destination filename + * + * @param real|unique|unique_ext $mode real creates a realname, filtering some characters, lowering every character. Unique creates an unique filename + * @param string $prefix Prefix applied to filename + * @param string $user_id The user_id is only needed for when cleaning a user's avatar + * @access public + */ + function clean_filename($mode = 'unique', $prefix = '', $user_id = '') + { + if ($this->init_error) + { + return; + } + + switch ($mode) + { + case 'real': + // Remove every extension from filename (to not let the mime bug being exposed) + if (strpos($this->realname, '.') !== false) + { + $this->realname = substr($this->realname, 0, strpos($this->realname, '.')); + } + + // Replace any chars which may cause us problems with _ + $bad_chars = array("'", "\\", ' ', '/', ':', '*', '?', '"', '<', '>', '|'); + + $this->realname = rawurlencode(str_replace($bad_chars, '_', strtolower($this->realname))); + $this->realname = preg_replace("/%(\w{2})/", '_', $this->realname); + + $this->realname = $prefix . $this->realname . '.' . $this->extension; + break; + + case 'unique': + $this->realname = $prefix . md5(unique_id()); + break; + + case 'avatar': + $this->extension = strtolower($this->extension); + $this->realname = $prefix . $user_id . '.' . $this->extension; + + break; + + case 'unique_ext': + default: + $this->realname = $prefix . md5(unique_id()) . '.' . $this->extension; + break; + } + } + + /** + * Get property from file object + */ + function get($property) + { + if ($this->init_error || !isset($this->$property)) + { + return false; + } + + return $this->$property; + } + + /** + * Check if file is an image (mimetype) + * + * @return true if it is an image, false if not + */ + function is_image() + { + return (strpos($this->mimetype, 'image/') === 0); + } + + /** + * Check if the file got correctly uploaded + * + * @return true if it is a valid upload, false if not + */ + function is_uploaded() + { + $is_plupload = $this->plupload && $this->plupload->is_active(); + + if (!$this->local && !$is_plupload && !is_uploaded_file($this->filename)) + { + return false; + } + + if (($this->local || $is_plupload) && !file_exists($this->filename)) + { + return false; + } + + return true; + } + + /** + * Remove file + */ + function remove() + { + if ($this->file_moved) + { + @unlink($this->destination_file); + } + } + + /** + * Get file extension + * + * @param string Filename that needs to be checked + * @return string Extension of the supplied filename + */ + static public function get_extension($filename) + { + $filename = utf8_basename($filename); + + if (strpos($filename, '.') === false) + { + return ''; + } + + $filename = explode('.', $filename); + return array_pop($filename); + } + + /** + * Get mimetype + * + * @param string $filename Filename that needs to be checked + * @return string Mimetype of supplied filename + */ + function get_mimetype($filename) + { + if ($this->mimetype_guesser !== null) + { + $mimetype = $this->mimetype_guesser->guess($filename, $this->uploadname); + + if ($mimetype !== 'application/octet-stream') + { + $this->mimetype = $mimetype; + } + } + + return $this->mimetype; + } + + /** + * Get filesize + */ + function get_filesize($filename) + { + return @filesize($filename); + } + + + /** + * Check the first 256 bytes for forbidden content + */ + function check_content($disallowed_content) + { + if (empty($disallowed_content)) + { + return true; + } + + $fp = @fopen($this->filename, 'rb'); + + if ($fp !== false) + { + $ie_mime_relevant = fread($fp, 256); + fclose($fp); + foreach ($disallowed_content as $forbidden) + { + if (stripos($ie_mime_relevant, '<' . $forbidden) !== false) + { + return false; + } + } + } + return true; + } + + /** + * Move file to destination folder + * The phpbb_root_path variable will be applied to the destination path + * + * @param string $destination Destination path, for example $config['avatar_path'] + * @param bool $overwrite If set to true, an already existing file will be overwritten + * @param bool $skip_image_check If set to true, the check for the file to be a valid image is skipped + * @param string $chmod Permission mask for chmodding the file after a successful move. The mode entered here reflects the mode defined by {@link phpbb_chmod()} + * + * @access public + */ + function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = false) + { + global $user, $phpbb_root_path; + + if (sizeof($this->error)) + { + return false; + } + + $chmod = ($chmod === false) ? CHMOD_READ | CHMOD_WRITE : $chmod; + + // We need to trust the admin in specifying valid upload directories and an attacker not being able to overwrite it... + $this->destination_path = $phpbb_root_path . $destination; + + // Check if the destination path exist... + if (!file_exists($this->destination_path)) + { + @unlink($this->filename); + return false; + } + + $upload_mode = (@ini_get('open_basedir') || @ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'on') ? 'move' : 'copy'; + $upload_mode = ($this->local) ? 'local' : $upload_mode; + $this->destination_file = $this->destination_path . '/' . utf8_basename($this->realname); + + // Check if the file already exist, else there is something wrong... + 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->file_moved = false; + return false; + } + else + { + if (file_exists($this->destination_file)) + { + @unlink($this->destination_file); + } + + switch ($upload_mode) + { + case 'copy': + + if (!@copy($this->filename, $this->destination_file)) + { + if (!@move_uploaded_file($this->filename, $this->destination_file)) + { + $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR'], $this->destination_file); + } + } + + break; + + case 'move': + + if (!@move_uploaded_file($this->filename, $this->destination_file)) + { + if (!@copy($this->filename, $this->destination_file)) + { + $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR'], $this->destination_file); + } + } + + break; + + case 'local': + + if (!@copy($this->filename, $this->destination_file)) + { + $this->error[] = sprintf($user->lang[$this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR'], $this->destination_file); + } + + break; + } + + // Remove temporary filename + @unlink($this->filename); + + if (sizeof($this->error)) + { + return false; + } + + try + { + $this->filesystem->phpbb_chmod($this->destination_file, $chmod); + } + catch (\phpbb\filesystem\exception\filesystem_exception $e) + { + // Do nothing + } + } + + // 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; + + // Get imagesize class + $imagesize = new \fastImageSize\fastImageSize(); + + $this->image_info = $imagesize->getImageSize($this->destination_file, $this->mimetype); + + if ($this->image_info !== false) + { + $this->width = $this->image_info['width']; + $this->height = $this->image_info['height']; + + // Check image type + $types = fileupload::image_types(); + + if (!isset($types[$this->image_info['type']]) || !in_array($this->extension, $types[$this->image_info['type']])) + { + if (!isset($types[$this->image_info['type']])) + { + $this->error[] = $user->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); + } + } + + // Make sure the dimensions match a valid image + if (empty($this->width) || empty($this->height)) + { + $this->error[] = $user->lang['ATTACHED_IMAGE_NOT_IMAGE']; + } + } + else + { + $this->error[] = $user->lang['UNABLE_GET_IMAGE_SIZE']; + } + } + + $this->file_moved = true; + $this->additional_checks(); + unset($this->upload); + + return true; + } + + /** + * Performing additional checks + */ + function additional_checks() + { + global $user; + + if (!$this->file_moved) + { + return false; + } + + // Filesize is too big or it's 0 if it was larger than the maxsize in the upload form + if ($this->upload->max_filesize && ($this->get('filesize') > $this->upload->max_filesize || $this->filesize == 0)) + { + $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']); + + 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)); + + return false; + } + + return true; + } +} -- cgit v1.2.1 From c72d6a71bb71b5abeae05b4a6494ffb166624179 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 31 May 2015 14:02:02 +0200 Subject: [ticket/13904] Modify constructor to be instantiatable by container PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index f74e91b0d5..4e52d13d01 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -33,7 +33,6 @@ class filespec var $destination_path = ''; var $file_moved = false; - var $init_error = false; var $local = false; var $error = array(); @@ -61,14 +60,19 @@ class filespec * File Class * @access private */ - function filespec($upload_ary, $upload_namespace, \phpbb\filesystem\filesystem_interface $phpbb_filesystem, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) + function filespec(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { - if (!isset($upload_ary)) - { - $this->init_error = true; - return; - } + // @todo call this via files + //$this->set_upload_ary($upload_ary); + //$this->set_upload_namespace($upload_namespace); + $this->plupload = $plupload; + $this->mimetype_guesser = $mimetype_guesser; + $this->filesystem = $phpbb_filesystem; + } + + public function set_upload_ary($upload_ary) + { $this->filename = $upload_ary['tmp_name']; $this->filesize = $upload_ary['size']; $name = (STRIP) ? stripslashes($upload_ary['name']) : $upload_ary['name']; @@ -93,10 +97,21 @@ class filespec $this->file_moved = false; $this->local = (isset($upload_ary['local_mode'])) ? true : false; - $this->upload = $upload_namespace; - $this->plupload = $plupload; - $this->mimetype_guesser = $mimetype_guesser; - $this->filesystem = $phpbb_filesystem; + } + + public function set_upload_namespace($namespace) + { + $this->upload = $namespace; + } + + /** + * Check if class members were not properly initalised yet + * + * @return bool True if there was an init error, false if not + */ + protected function init_error() + { + return !isset($upload_ary); } /** @@ -109,7 +124,7 @@ class filespec */ function clean_filename($mode = 'unique', $prefix = '', $user_id = '') { - if ($this->init_error) + if ($this->init_error()) { return; } @@ -154,7 +169,7 @@ class filespec */ function get($property) { - if ($this->init_error || !isset($this->$property)) + if ($this->init_error() || !isset($this->$property)) { return false; } -- cgit v1.2.1 From 0cbb713cc2a6249cb12507db7d0fa78ce8663ae6 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 31 May 2015 14:47:57 +0200 Subject: [ticket/13904] Fix uploading for use with new filespec class PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 4e52d13d01..8501b217f7 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -60,7 +60,7 @@ class filespec * File Class * @access private */ - function filespec(\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, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { // @todo call this via files //$this->set_upload_ary($upload_ary); @@ -109,9 +109,9 @@ class filespec * * @return bool True if there was an init error, false if not */ - protected function init_error() + public function init_error() { - return !isset($upload_ary); + return !isset($this->filename); } /** @@ -422,7 +422,7 @@ class filespec $this->height = $this->image_info['height']; // Check image type - $types = fileupload::image_types(); + $types = \fileupload::image_types(); if (!isset($types[$this->image_info['type']]) || !in_array($this->extension, $types[$this->image_info['type']])) { -- cgit v1.2.1 From 92e49cd0acef56b78fda3bcffebb7a0958891b82 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 1 Jun 2015 13:27:29 +0200 Subject: [ticket/13904] Turn filespec into prototype and improve init methods PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 8501b217f7..3f50488e7c 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -97,11 +97,15 @@ class filespec $this->file_moved = false; $this->local = (isset($upload_ary['local_mode'])) ? true : false; + + return $this; } public function set_upload_namespace($namespace) { $this->upload = $namespace; + + return $this; } /** -- cgit v1.2.1 From d2be8e1503db3686c62b23973511c61dea7a6616 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 1 Jun 2015 13:30:17 +0200 Subject: [ticket/13904] Add fileupload class to files classes PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 655 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 655 insertions(+) create mode 100644 phpBB/phpbb/files/upload.php (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php new file mode 100644 index 0000000000..7501247e06 --- /dev/null +++ b/phpBB/phpbb/files/upload.php @@ -0,0 +1,655 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files; + +/** + * File upload class + * Init class (all parameters optional and able to be set/overwritten separately) - scope is global and valid for all uploads + */ +class upload +{ + var $allowed_extensions = array(); + var $disallowed_content = array('body', 'head', 'html', 'img', 'plaintext', 'a href', 'pre', 'script', 'table', 'title'); + var $max_filesize = 0; + var $min_width = 0; + var $min_height = 0; + var $max_width = 0; + var $max_height = 0; + var $error_prefix = ''; + + /** @var int Timeout for remote upload */ + var $upload_timeout = 6; + + /** + * @var \phpbb\filesystem\filesystem_interface + */ + protected $filesystem; + + /** + * Init file upload class. + * + * @param \phpbb\filesystem\filesystem_interface $filesystem + * @param string $error_prefix Used error messages will get prefixed by this string + * @param array $allowed_extensions Array of allowed extensions, for example array('jpg', 'jpeg', 'gif', 'png') + * @param int $max_filesize Maximum filesize + * @param int $min_width Minimum image width (only checked for images) + * @param int $min_height Minimum image height (only checked for images) + * @param int $max_width Maximum image width (only checked for images) + * @param int $max_height Maximum image height (only checked for images) + * @param bool|array $disallowed_content If enabled, the first 256 bytes of the file must not + * contain any of its values. Defaults to false. + * + */ + function fileupload(\phpbb\filesystem\filesystem_interface $filesystem, $error_prefix = '', $allowed_extensions = false, $max_filesize = false, $min_width = false, $min_height = false, $max_width = false, $max_height = false, $disallowed_content = false) + { + $this->set_allowed_extensions($allowed_extensions); + $this->set_max_filesize($max_filesize); + $this->set_allowed_dimensions($min_width, $min_height, $max_width, $max_height); + $this->set_error_prefix($error_prefix); + $this->set_disallowed_content($disallowed_content); + $this->filesystem = $filesystem; + } + + /** + * Reset vars + */ + function reset_vars() + { + $this->max_filesize = 0; + $this->min_width = $this->min_height = $this->max_width = $this->max_height = 0; + $this->error_prefix = ''; + $this->allowed_extensions = array(); + $this->disallowed_content = array(); + } + + /** + * Set allowed extensions + */ + function set_allowed_extensions($allowed_extensions) + { + if ($allowed_extensions !== false && is_array($allowed_extensions)) + { + $this->allowed_extensions = $allowed_extensions; + } + } + + /** + * Set allowed dimensions + */ + function set_allowed_dimensions($min_width, $min_height, $max_width, $max_height) + { + $this->min_width = (int) $min_width; + $this->min_height = (int) $min_height; + $this->max_width = (int) $max_width; + $this->max_height = (int) $max_height; + } + + /** + * Set maximum allowed filesize + */ + function set_max_filesize($max_filesize) + { + if ($max_filesize !== false && (int) $max_filesize) + { + $this->max_filesize = (int) $max_filesize; + } + } + + /** + * Set disallowed strings + */ + function set_disallowed_content($disallowed_content) + { + if ($disallowed_content !== false && is_array($disallowed_content)) + { + $this->disallowed_content = array_diff($disallowed_content, array('')); + } + } + + /** + * Set error prefix + */ + function set_error_prefix($error_prefix) + { + $this->error_prefix = $error_prefix; + } + + /** + * Form upload method + * Upload file from users harddisk + * + * @param string $form_name Form name assigned to the file input field (if it is an array, the key has to be specified) + * @param \phpbb\mimetype\guesser $mimetype_guesser Mimetype guesser + * @param \phpbb\plupload\plupload $plupload The plupload object + * + * @return object $file Object "filespec" is returned, all further operations can be done with this object + * @access public + */ + function form_upload($form_name, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) + { + global $user, $request, $phpbb_container; + + $upload = $request->file($form_name); + unset($upload['local_mode']); + + if ($plupload) + { + $result = $plupload->handle_upload($form_name); + if (is_array($result)) + { + $upload = array_merge($upload, $result); + } + } + + /** @var \phpbb\files\filespec $file */ + $file = $phpbb_container->get('files.filespec') + ->set_upload_ary($upload) + ->set_upload_namespace($this); + + if ($file->init_error()) + { + $file->error[] = ''; + return $file; + } + + // Error array filled? + if (isset($upload['error'])) + { + $error = $this->assign_internal_error($upload['error']); + + if ($error !== false) + { + $file->error[] = $error; + return $file; + } + } + + // Check if empty file got uploaded (not catched by is_uploaded_file) + if (isset($upload['size']) && $upload['size'] == 0) + { + $file->error[] = $user->lang[$this->error_prefix . 'EMPTY_FILEUPLOAD']; + return $file; + } + + // PHP Upload filesize exceeded + if ($file->get('filename') == 'none') + { + $max_filesize = @ini_get('upload_max_filesize'); + $unit = 'MB'; + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_filesize, -1, 1)); + $max_filesize = (int) $max_filesize; + + $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); + } + + $file->error[] = (empty($max_filesize)) ? $user->lang[$this->error_prefix . 'PHP_SIZE_NA'] : sprintf($user->lang[$this->error_prefix . 'PHP_SIZE_OVERRUN'], $max_filesize, $user->lang[$unit]); + return $file; + } + + // Not correctly uploaded + if (!$file->is_uploaded()) + { + $file->error[] = $user->lang[$this->error_prefix . 'NOT_UPLOADED']; + return $file; + } + + $this->common_checks($file); + + return $file; + } + + /** + * Move file from another location to phpBB + */ + function local_upload($source_file, $filedata = false, \phpbb\mimetype\guesser $mimetype_guesser = null) + { + global $user, $request, $phpbb_container; + + $upload = array(); + + $upload['local_mode'] = true; + $upload['tmp_name'] = $source_file; + + if ($filedata === false) + { + $upload['name'] = utf8_basename($source_file); + $upload['size'] = 0; + } + else + { + $upload['name'] = $filedata['realname']; + $upload['size'] = $filedata['size']; + $upload['type'] = $filedata['type']; + } + + /** @var \phpbb\files\filespec $file */ + $file = $phpbb_container->get('files.filespec') + ->set_upload_ary($upload) + ->set_upload_namespace($this); + + if ($file->init_error()) + { + $file->error[] = ''; + return $file; + } + + if (isset($upload['error'])) + { + $error = $this->assign_internal_error($upload['error']); + + if ($error !== false) + { + $file->error[] = $error; + return $file; + } + } + + // PHP Upload filesize exceeded + if ($file->get('filename') == 'none') + { + $max_filesize = @ini_get('upload_max_filesize'); + $unit = 'MB'; + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_filesize, -1, 1)); + $max_filesize = (int) $max_filesize; + + $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); + } + + $file->error[] = (empty($max_filesize)) ? $user->lang[$this->error_prefix . 'PHP_SIZE_NA'] : sprintf($user->lang[$this->error_prefix . 'PHP_SIZE_OVERRUN'], $max_filesize, $user->lang[$unit]); + return $file; + } + + // Not correctly uploaded + if (!$file->is_uploaded()) + { + $file->error[] = $user->lang[$this->error_prefix . 'NOT_UPLOADED']; + return $file; + } + + $this->common_checks($file); + $request->overwrite('local', $upload, \phpbb\request\request_interface::FILES); + + return $file; + } + + /** + * Remote upload method + * Uploads file from given url + * + * @param string $upload_url URL pointing to file to upload, for example http://www.foobar.com/example.gif + * @param \phpbb\mimetype\guesser $mimetype_guesser Mimetype guesser + * @return object $file Object "filespec" is returned, all further operations can be done with this object + * @access public + */ + function remote_upload($upload_url, \phpbb\mimetype\guesser $mimetype_guesser = null) + { + global $user, $phpbb_root_path, $phpbb_container; + + $upload_ary = array(); + $upload_ary['local_mode'] = true; + + if (!preg_match('#^(https?://).*?\.(' . implode('|', $this->allowed_extensions) . ')$#i', $upload_url, $match)) + { + $file = new fileerror($user->lang[$this->error_prefix . 'URL_INVALID']); + return $file; + } + + if (empty($match[2])) + { + $file = new fileerror($user->lang[$this->error_prefix . 'URL_INVALID']); + return $file; + } + + $url = parse_url($upload_url); + + $host = $url['host']; + $path = $url['path']; + $port = (!empty($url['port'])) ? (int) $url['port'] : 80; + + $upload_ary['type'] = 'application/octet-stream'; + + $url['path'] = explode('.', $url['path']); + $ext = array_pop($url['path']); + + $url['path'] = implode('', $url['path']); + $upload_ary['name'] = utf8_basename($url['path']) . (($ext) ? '.' . $ext : ''); + $filename = $url['path']; + $filesize = 0; + + $remote_max_filesize = $this->max_filesize; + if (!$remote_max_filesize) + { + $max_filesize = @ini_get('upload_max_filesize'); + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_filesize, -1, 1)); + $remote_max_filesize = (int) $max_filesize; + + switch ($unit) + { + case 'g': + $remote_max_filesize *= 1024; + // no break + case 'm': + $remote_max_filesize *= 1024; + // no break + case 'k': + $remote_max_filesize *= 1024; + // no break + } + } + } + + $errno = 0; + $errstr = ''; + + if (!($fsock = @fsockopen($host, $port, $errno, $errstr))) + { + $file = new fileerror($user->lang[$this->error_prefix . 'NOT_UPLOADED']); + return $file; + } + + // Make sure $path not beginning with / + if (strpos($path, '/') === 0) + { + $path = substr($path, 1); + } + + fputs($fsock, 'GET /' . $path . " HTTP/1.1\r\n"); + fputs($fsock, "HOST: " . $host . "\r\n"); + fputs($fsock, "Connection: close\r\n\r\n"); + + // Set a proper timeout for the socket + socket_set_timeout($fsock, $this->upload_timeout); + + $get_info = false; + $data = ''; + $length = false; + $timer_stop = time() + $this->upload_timeout; + + while ((!$length || $filesize < $length) && !@feof($fsock)) + { + if ($get_info) + { + if ($length) + { + // Don't attempt to read past end of file if server indicated length + $block = @fread($fsock, min($length - $filesize, 1024)); + } + else + { + $block = @fread($fsock, 1024); + } + + $filesize += strlen($block); + + if ($remote_max_filesize && $filesize > $remote_max_filesize) + { + $max_filesize = get_formatted_filesize($remote_max_filesize, false); + + $file = new fileerror(sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit'])); + return $file; + } + + $data .= $block; + } + else + { + $line = @fgets($fsock, 1024); + + if ($line == "\r\n") + { + $get_info = true; + } + else + { + if (stripos($line, 'content-type: ') !== false) + { + $upload_ary['type'] = rtrim(str_replace('content-type: ', '', strtolower($line))); + } + else if ($this->max_filesize && stripos($line, 'content-length: ') !== false) + { + $length = (int) str_replace('content-length: ', '', strtolower($line)); + + if ($remote_max_filesize && $length && $length > $remote_max_filesize) + { + $max_filesize = get_formatted_filesize($remote_max_filesize, false); + + $file = new fileerror(sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit'])); + return $file; + } + } + else if (stripos($line, '404 not found') !== false) + { + $file = new fileerror($user->lang[$this->error_prefix . 'URL_NOT_FOUND']); + return $file; + } + } + } + + $stream_meta_data = stream_get_meta_data($fsock); + + // Cancel upload if we exceed timeout + if (!empty($stream_meta_data['timed_out']) || time() >= $timer_stop) + { + $file = new fileerror($user->lang[$this->error_prefix . 'REMOTE_UPLOAD_TIMEOUT']); + return $file; + } + } + @fclose($fsock); + + if (empty($data)) + { + $file = new fileerror($user->lang[$this->error_prefix . 'EMPTY_REMOTE_DATA']); + return $file; + } + + $tmp_path = (!@ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'off') ? false : $phpbb_root_path . 'cache'; + $filename = tempnam($tmp_path, unique_id() . '-'); + + if (!($fp = @fopen($filename, 'wb'))) + { + $file = new fileerror($user->lang[$this->error_prefix . 'NOT_UPLOADED']); + return $file; + } + + $upload_ary['size'] = fwrite($fp, $data); + fclose($fp); + unset($data); + + $upload_ary['tmp_name'] = $filename; + + /** @var \phpbb\files\filespec $file */ + $file = $phpbb_container->get('files.filespec') + ->set_upload_ary($upload_ary) + ->set_upload_namespace($this); + $this->common_checks($file); + + return $file; + } + + /** + * Assign internal error + * @access private + */ + function assign_internal_error($errorcode) + { + global $user; + + switch ($errorcode) + { + case 1: + $max_filesize = @ini_get('upload_max_filesize'); + $unit = 'MB'; + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_filesize, -1, 1)); + $max_filesize = (int) $max_filesize; + + $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); + } + + $error = (empty($max_filesize)) ? $user->lang[$this->error_prefix . 'PHP_SIZE_NA'] : sprintf($user->lang[$this->error_prefix . 'PHP_SIZE_OVERRUN'], $max_filesize, $user->lang[$unit]); + break; + + case 2: + $max_filesize = get_formatted_filesize($this->max_filesize, false); + + $error = sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit']); + break; + + case 3: + $error = $user->lang[$this->error_prefix . 'PARTIAL_UPLOAD']; + break; + + case 4: + $error = $user->lang[$this->error_prefix . 'NOT_UPLOADED']; + break; + + case 6: + $error = 'Temporary folder could not be found. Please check your PHP installation.'; + break; + + default: + $error = false; + break; + } + + return $error; + } + + /** + * Perform common checks + */ + function common_checks(&$file) + { + global $user; + + // Filesize is too big or it's 0 if it was larger than the maxsize in the upload form + if ($this->max_filesize && ($file->get('filesize') > $this->max_filesize || $file->get('filesize') == 0)) + { + $max_filesize = get_formatted_filesize($this->max_filesize, false); + + $file->error[] = sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit']); + } + + // check Filename + if (preg_match("#[\\/:*?\"<>|]#i", $file->get('realname'))) + { + $file->error[] = sprintf($user->lang[$this->error_prefix . 'INVALID_FILENAME'], $file->get('realname')); + } + + // Invalid Extension + if (!$this->valid_extension($file)) + { + $file->error[] = sprintf($user->lang[$this->error_prefix . 'DISALLOWED_EXTENSION'], $file->get('extension')); + } + + // MIME Sniffing + if (!$this->valid_content($file)) + { + $file->error[] = sprintf($user->lang[$this->error_prefix . 'DISALLOWED_CONTENT']); + } + } + + /** + * Check for allowed extension + */ + function valid_extension(&$file) + { + return (in_array($file->get('extension'), $this->allowed_extensions)) ? true : false; + } + + /** + * Check for allowed dimension + */ + function valid_dimensions(&$file) + { + if (!$this->max_width && !$this->max_height && !$this->min_width && !$this->min_height) + { + return true; + } + + if (($file->get('width') > $this->max_width && $this->max_width) || + ($file->get('height') > $this->max_height && $this->max_height) || + ($file->get('width') < $this->min_width && $this->min_width) || + ($file->get('height') < $this->min_height && $this->min_height)) + { + return false; + } + + return true; + } + + /** + * Check if form upload is valid + */ + function is_valid($form_name) + { + global $request; + $upload = $request->file($form_name); + + return (!empty($upload) && $upload['name'] !== 'none'); + } + + + /** + * Check for bad content (IE mime-sniffing) + */ + function valid_content(&$file) + { + return ($file->check_content($this->disallowed_content)); + } + + /** + * Get image type/extension mapping + * + * @return array Array containing the image types and their extensions + */ + static public function image_types() + { + $result = array( + IMAGETYPE_GIF => array('gif'), + IMAGETYPE_JPEG => array('jpg', 'jpeg'), + IMAGETYPE_PNG => array('png'), + IMAGETYPE_SWF => array('swf'), + IMAGETYPE_PSD => array('psd'), + IMAGETYPE_BMP => array('bmp'), + IMAGETYPE_TIFF_II => array('tif', 'tiff'), + IMAGETYPE_TIFF_MM => array('tif', 'tiff'), + IMAGETYPE_JPC => array('jpg', 'jpeg'), + IMAGETYPE_JP2 => array('jpg', 'jpeg'), + IMAGETYPE_JPX => array('jpg', 'jpeg'), + IMAGETYPE_JB2 => array('jpg', 'jpeg'), + IMAGETYPE_IFF => array('iff'), + IMAGETYPE_WBMP => array('wbmp'), + IMAGETYPE_XBM => array('xbm'), + ); + + if (defined('IMAGETYPE_SWC')) + { + $result[IMAGETYPE_SWC] = array('swc'); + } + + return $result; + } +} -- cgit v1.2.1 From 1af6f052d80e693d289258d490c1187a064093b9 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 14 Aug 2015 09:03:55 +0200 Subject: [ticket/13904] Load upload class using factory PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 7501247e06..8666b857a5 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -40,24 +40,15 @@ class upload * Init file upload class. * * @param \phpbb\filesystem\filesystem_interface $filesystem - * @param string $error_prefix Used error messages will get prefixed by this string - * @param array $allowed_extensions Array of allowed extensions, for example array('jpg', 'jpeg', 'gif', 'png') - * @param int $max_filesize Maximum filesize - * @param int $min_width Minimum image width (only checked for images) - * @param int $min_height Minimum image height (only checked for images) - * @param int $max_width Maximum image width (only checked for images) - * @param int $max_height Maximum image height (only checked for images) - * @param bool|array $disallowed_content If enabled, the first 256 bytes of the file must not - * contain any of its values. Defaults to false. * */ - function fileupload(\phpbb\filesystem\filesystem_interface $filesystem, $error_prefix = '', $allowed_extensions = false, $max_filesize = false, $min_width = false, $min_height = false, $max_width = false, $max_height = false, $disallowed_content = false) + public function __construct(\phpbb\filesystem\filesystem_interface $filesystem) { - $this->set_allowed_extensions($allowed_extensions); - $this->set_max_filesize($max_filesize); - $this->set_allowed_dimensions($min_width, $min_height, $max_width, $max_height); - $this->set_error_prefix($error_prefix); - $this->set_disallowed_content($disallowed_content); +// $this->set_allowed_extensions($allowed_extensions); +// $this->set_max_filesize($max_filesize); +// $this->set_allowed_dimensions($min_width, $min_height, $max_width, $max_height); +// $this->set_error_prefix($error_prefix); +// $this->set_disallowed_content($disallowed_content); $this->filesystem = $filesystem; } @@ -82,6 +73,8 @@ class upload { $this->allowed_extensions = $allowed_extensions; } + + return $this; } /** @@ -93,6 +86,8 @@ class upload $this->min_height = (int) $min_height; $this->max_width = (int) $max_width; $this->max_height = (int) $max_height; + + return $this; } /** @@ -104,6 +99,8 @@ class upload { $this->max_filesize = (int) $max_filesize; } + + return $this; } /** @@ -115,6 +112,8 @@ class upload { $this->disallowed_content = array_diff($disallowed_content, array('')); } + + return $this; } /** @@ -123,6 +122,8 @@ class upload function set_error_prefix($error_prefix) { $this->error_prefix = $error_prefix; + + return $this; } /** -- cgit v1.2.1 From 8c2cbc0599b9d62fde730b2891c73c9c053682e7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 1 Jun 2015 13:51:05 +0200 Subject: [ticket/13904] Remove magic method from factory and allow short names PHPBB3-13904 --- phpBB/phpbb/files/factory.php | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/factory.php b/phpBB/phpbb/files/factory.php index 9385ad678e..508c50c6ce 100644 --- a/phpBB/phpbb/files/factory.php +++ b/phpBB/phpbb/files/factory.php @@ -35,13 +35,15 @@ class factory * * @param string $name Service name * - * @return object|false Requested service or false if service could not be + * @return object|bool Requested service or false if service could not be * found by the container */ public function get($name) { $service = false; + $name = (strpos($name, 'files.') === false) ? 'files.' . $name : $name; + try { $service = $this->container->get($name); @@ -53,27 +55,4 @@ class factory return $service; } - - /** - * Magic function for handling get calls, e.g. get_fileupload() or - * get_filespec() and turning them into call for files. services like - * files.fileupload. - * - * @param string $name Name of called function - * @param mixed $arguments Possible supplied arguments - * - * @return object|false Requested service or false if service could not be - * found by the container - */ - public function __call($name, $arguments) - { - if (substr($name, 0, 4) === 'get_') - { - return $this->get('files.' . substr($name, 4)); - } - else - { - return false; - } - } } -- cgit v1.2.1 From 186b4495b625a40459618f01784b814a29f98ae0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 1 Jun 2015 13:52:01 +0200 Subject: [ticket/13904] Allow using factory for none files. classes PHPBB3-13904 --- phpBB/phpbb/files/factory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/factory.php b/phpBB/phpbb/files/factory.php index 508c50c6ce..b28a7ab659 100644 --- a/phpBB/phpbb/files/factory.php +++ b/phpBB/phpbb/files/factory.php @@ -42,7 +42,7 @@ class factory { $service = false; - $name = (strpos($name, 'files.') === false) ? 'files.' . $name : $name; + $name = (strpos($name, 'files.') === false && strpos($name, '.') === false) ? 'files.' . $name : $name; try { -- cgit v1.2.1 From a96e7a8ec6efa483b47dca3395ee2de608cfc675 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 1 Jun 2015 13:57:17 +0200 Subject: [ticket/13904] Get rid of useless parameters and variables PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 8666b857a5..1892d22adf 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -131,13 +131,12 @@ class upload * Upload file from users harddisk * * @param string $form_name Form name assigned to the file input field (if it is an array, the key has to be specified) - * @param \phpbb\mimetype\guesser $mimetype_guesser Mimetype guesser * @param \phpbb\plupload\plupload $plupload The plupload object * * @return object $file Object "filespec" is returned, all further operations can be done with this object * @access public */ - function form_upload($form_name, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) + function form_upload($form_name, \phpbb\plupload\plupload $plupload = null) { global $user, $request, $phpbb_container; @@ -216,7 +215,7 @@ class upload /** * Move file from another location to phpBB */ - function local_upload($source_file, $filedata = false, \phpbb\mimetype\guesser $mimetype_guesser = null) + function local_upload($source_file, $filedata = false) { global $user, $request, $phpbb_container; @@ -295,11 +294,10 @@ class upload * Uploads file from given url * * @param string $upload_url URL pointing to file to upload, for example http://www.foobar.com/example.gif - * @param \phpbb\mimetype\guesser $mimetype_guesser Mimetype guesser * @return object $file Object "filespec" is returned, all further operations can be done with this object * @access public */ - function remote_upload($upload_url, \phpbb\mimetype\guesser $mimetype_guesser = null) + function remote_upload($upload_url) { global $user, $phpbb_root_path, $phpbb_container; -- cgit v1.2.1 From 2915647a546b4c0733a0e1a0cdc924272e41615b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 00:29:03 +0200 Subject: [ticket/13904] Fix filespec tests PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 3f50488e7c..2fdba2d793 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -426,7 +426,7 @@ class filespec $this->height = $this->image_info['height']; // Check image type - $types = \fileupload::image_types(); + $types = upload::image_types(); if (!isset($types[$this->image_info['type']]) || !in_array($this->extension, $types[$this->image_info['type']])) { -- cgit v1.2.1 From eb11973ea8af41623a9e6e6c320cb1fb0df7a222 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 14 Aug 2015 09:13:27 +0200 Subject: [ticket/13904] Use factory instead of container and add factory to services PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 1892d22adf..7ca29efe1a 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -36,20 +36,20 @@ class upload */ protected $filesystem; + /** @var \phpbb\files\factory Files factory */ + protected $factory; + /** * Init file upload class. * * @param \phpbb\filesystem\filesystem_interface $filesystem + * @param \phpbb\files\factory $factory Files factory * */ - public function __construct(\phpbb\filesystem\filesystem_interface $filesystem) + public function __construct(\phpbb\filesystem\filesystem_interface $filesystem, factory $factory) { -// $this->set_allowed_extensions($allowed_extensions); -// $this->set_max_filesize($max_filesize); -// $this->set_allowed_dimensions($min_width, $min_height, $max_width, $max_height); -// $this->set_error_prefix($error_prefix); -// $this->set_disallowed_content($disallowed_content); $this->filesystem = $filesystem; + $this->factory = $factory; } /** @@ -138,7 +138,7 @@ class upload */ function form_upload($form_name, \phpbb\plupload\plupload $plupload = null) { - global $user, $request, $phpbb_container; + global $user, $request; $upload = $request->file($form_name); unset($upload['local_mode']); @@ -153,7 +153,7 @@ class upload } /** @var \phpbb\files\filespec $file */ - $file = $phpbb_container->get('files.filespec') + $file = $this->factory->get('filespec') ->set_upload_ary($upload) ->set_upload_namespace($this); @@ -217,7 +217,7 @@ class upload */ function local_upload($source_file, $filedata = false) { - global $user, $request, $phpbb_container; + global $user, $request; $upload = array(); @@ -237,7 +237,7 @@ class upload } /** @var \phpbb\files\filespec $file */ - $file = $phpbb_container->get('files.filespec') + $file = $this->factory->get('filespec') ->set_upload_ary($upload) ->set_upload_namespace($this); @@ -299,7 +299,7 @@ class upload */ function remote_upload($upload_url) { - global $user, $phpbb_root_path, $phpbb_container; + global $user, $phpbb_root_path; $upload_ary = array(); $upload_ary['local_mode'] = true; @@ -477,7 +477,7 @@ class upload $upload_ary['tmp_name'] = $filename; /** @var \phpbb\files\filespec $file */ - $file = $phpbb_container->get('files.filespec') + $file = $this->factory->get('filespec') ->set_upload_ary($upload_ary) ->set_upload_namespace($this); $this->common_checks($file); -- cgit v1.2.1 From a53825ad760cc8437d8c26eb1f947622c0fcf229 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 11:48:55 +0200 Subject: [ticket/13904] No longer use fileerror class for extending filespec class PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 14 ++++++++++++++ phpBB/phpbb/files/upload.php | 27 +++++++++------------------ 2 files changed, 23 insertions(+), 18 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 2fdba2d793..d14c9a226d 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -118,6 +118,20 @@ class filespec return !isset($this->filename); } + /** + * Set error in error array + * + * @param mixed $error Content for error array + * + * @return \phpbb\files\filespec This instance of the filespec class + */ + public function set_error($error) + { + $this->error[] = $error; + + return $this; + } + /** * Cleans destination filename * diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 7ca29efe1a..291cdb266c 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -306,14 +306,12 @@ class upload if (!preg_match('#^(https?://).*?\.(' . implode('|', $this->allowed_extensions) . ')$#i', $upload_url, $match)) { - $file = new fileerror($user->lang[$this->error_prefix . 'URL_INVALID']); - return $file; + return $this->factory->get('filespec')->set_error($user->lang[$this->error_prefix . 'URL_INVALID']); } if (empty($match[2])) { - $file = new fileerror($user->lang[$this->error_prefix . 'URL_INVALID']); - return $file; + return $this->factory->get('filespec')->set_error($user->lang[$this->error_prefix . 'URL_INVALID']);e; } $url = parse_url($upload_url); @@ -362,8 +360,7 @@ class upload if (!($fsock = @fsockopen($host, $port, $errno, $errstr))) { - $file = new fileerror($user->lang[$this->error_prefix . 'NOT_UPLOADED']); - return $file; + return $this->factory->get('filespec')->set_error($user->lang[$this->error_prefix . 'NOT_UPLOADED']); } // Make sure $path not beginning with / @@ -404,8 +401,7 @@ class upload { $max_filesize = get_formatted_filesize($remote_max_filesize, false); - $file = new fileerror(sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit'])); - return $file; + return $this->factory->get('filespec')->set_error(sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit'])); } $data .= $block; @@ -432,14 +428,12 @@ class upload { $max_filesize = get_formatted_filesize($remote_max_filesize, false); - $file = new fileerror(sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit'])); - return $file; + return $this->factory->get('filespec')->set_error(sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit'])); } } else if (stripos($line, '404 not found') !== false) { - $file = new fileerror($user->lang[$this->error_prefix . 'URL_NOT_FOUND']); - return $file; + return $this->factory->get('filespec')->set_error($this->error_prefix . 'URL_NOT_FOUND'); } } } @@ -449,16 +443,14 @@ class upload // Cancel upload if we exceed timeout if (!empty($stream_meta_data['timed_out']) || time() >= $timer_stop) { - $file = new fileerror($user->lang[$this->error_prefix . 'REMOTE_UPLOAD_TIMEOUT']); - return $file; + return $this->factory->get('filespec')->set_error($this->error_prefix . 'REMOTE_UPLOAD_TIMEOUT'); } } @fclose($fsock); if (empty($data)) { - $file = new fileerror($user->lang[$this->error_prefix . 'EMPTY_REMOTE_DATA']); - return $file; + return $this->factory->get('filespec')->set_error($this->error_prefix . 'EMPTY_REMOTE_DATA'); } $tmp_path = (!@ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'off') ? false : $phpbb_root_path . 'cache'; @@ -466,8 +458,7 @@ class upload if (!($fp = @fopen($filename, 'wb'))) { - $file = new fileerror($user->lang[$this->error_prefix . 'NOT_UPLOADED']); - return $file; + return $this->factory->get('filespec')->set_error($this->error_prefix . 'NOT_UPLOADED'); } $upload_ary['size'] = fwrite($fp, $data); -- cgit v1.2.1 From e4546ad03c0c0130e60d164f3741cc57c33b8980 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 13:23:51 +0200 Subject: [ticket/13904] Improve doc blocks in upload class PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 85 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 74 insertions(+), 11 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 291cdb266c..f253fc762d 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -19,21 +19,34 @@ namespace phpbb\files; */ class upload { + /** @var array Allowed file extensions */ var $allowed_extensions = array(); + + /** @var array Disallowed content */ var $disallowed_content = array('body', 'head', 'html', 'img', 'plaintext', 'a href', 'pre', 'script', 'table', 'title'); + + /** @var int Maximum filesize */ var $max_filesize = 0; + + /** @var int Minimum width of images */ var $min_width = 0; + + /** @var int Minimum height of images */ var $min_height = 0; + + /** @var int Maximum width of images */ var $max_width = 0; + + /** @var int Maximum height of images */ var $max_height = 0; + + /** @var string Prefix for language variables of errors */ var $error_prefix = ''; /** @var int Timeout for remote upload */ var $upload_timeout = 6; - /** - * @var \phpbb\filesystem\filesystem_interface - */ + /** @var \phpbb\filesystem\filesystem_interface */ protected $filesystem; /** @var \phpbb\files\factory Files factory */ @@ -44,7 +57,6 @@ class upload * * @param \phpbb\filesystem\filesystem_interface $filesystem * @param \phpbb\files\factory $factory Files factory - * */ public function __construct(\phpbb\filesystem\filesystem_interface $filesystem, factory $factory) { @@ -66,6 +78,10 @@ class upload /** * Set allowed extensions + * + * @param array $allowed_extensions Allowed file extensions + * + * @return \phpbb\files\upload This instance of upload */ function set_allowed_extensions($allowed_extensions) { @@ -79,6 +95,13 @@ class upload /** * Set allowed dimensions + * + * @param int $min_width Minimum image width + * @param int $min_height Minimum image height + * @param int $max_width Maximum image width + * @param int $max_height Maximum image height + * + * @return \phpbb\files\upload This instance of upload */ function set_allowed_dimensions($min_width, $min_height, $max_width, $max_height) { @@ -91,7 +114,11 @@ class upload } /** - * Set maximum allowed filesize + * Set maximum allowed file size + * + * @param int $max_filesize Maximum file size + * + * @return \phpbb\files\upload This instance of upload */ function set_max_filesize($max_filesize) { @@ -105,6 +132,10 @@ class upload /** * Set disallowed strings + * + * @param array $disallowed_content Disallowed content + * + * @return \phpbb\files\upload This instance of upload */ function set_disallowed_content($disallowed_content) { @@ -118,6 +149,10 @@ class upload /** * Set error prefix + * + * @param string $error_prefix Prefix for language variables of errors + * + * @return \phpbb\files\upload This instance of upload */ function set_error_prefix($error_prefix) { @@ -133,7 +168,7 @@ class upload * @param string $form_name Form name assigned to the file input field (if it is an array, the key has to be specified) * @param \phpbb\plupload\plupload $plupload The plupload object * - * @return object $file Object "filespec" is returned, all further operations can be done with this object + * @return filespec $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) @@ -152,7 +187,7 @@ class upload } } - /** @var \phpbb\files\filespec $file */ + /** @var filespec $file */ $file = $this->factory->get('filespec') ->set_upload_ary($upload) ->set_upload_namespace($this); @@ -214,6 +249,11 @@ class upload /** * Move file from another location to phpBB + * + * @param string $source_file Filename of source file + * @param array|bool $filedata Array with filedata or false + * + * @return filespec Object "filespec" is returned, all further operations can be done with this object */ function local_upload($source_file, $filedata = false) { @@ -236,7 +276,7 @@ class upload $upload['type'] = $filedata['type']; } - /** @var \phpbb\files\filespec $file */ + /** @var filespec $file */ $file = $this->factory->get('filespec') ->set_upload_ary($upload) ->set_upload_namespace($this); @@ -294,7 +334,7 @@ class upload * Uploads file from given url * * @param string $upload_url URL pointing to file to upload, for example http://www.foobar.com/example.gif - * @return object $file Object "filespec" is returned, all further operations can be done with this object + * @return filespec $file Object "filespec" is returned, all further operations can be done with this object * @access public */ function remote_upload($upload_url) @@ -467,7 +507,7 @@ class upload $upload_ary['tmp_name'] = $filename; - /** @var \phpbb\files\filespec $file */ + /** @var filespec $file */ $file = $this->factory->get('filespec') ->set_upload_ary($upload_ary) ->set_upload_namespace($this); @@ -478,6 +518,10 @@ class upload /** * Assign internal error + * + * @param string $errorcode Error code to assign + * + * @return string Error string * @access private */ function assign_internal_error($errorcode) @@ -528,7 +572,9 @@ class upload } /** - * Perform common checks + * Perform common file checks + * + * @param filespec $file Instance of filespec class */ function common_checks(&$file) { @@ -563,6 +609,10 @@ class upload /** * Check for allowed extension + * + * @param filespec $file Instance of filespec class + * + * @return bool True if extension is allowed, false if not */ function valid_extension(&$file) { @@ -571,6 +621,11 @@ class upload /** * Check for allowed dimension + * + * @param filespec $file Instance of filespec class + * + * @return bool True if dimensions are valid or no constraints set, false + * if not */ function valid_dimensions(&$file) { @@ -592,6 +647,10 @@ class upload /** * Check if form upload is valid + * + * @param string $form_name Name of form + * + * @return bool True if form upload is valid, false if not */ function is_valid($form_name) { @@ -604,6 +663,10 @@ class upload /** * Check for bad content (IE mime-sniffing) + * + * @param filespec $file Instance of filespec class + * + * @return bool True if content is valid, false if not */ function valid_content(&$file) { -- cgit v1.2.1 From 0121e60cd73963043047e1b29b8d94ea9aa684e3 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 13:33:10 +0200 Subject: [ticket/13904] Use language class instead of user global in upload PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 55 +++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 26 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index f253fc762d..e37f90e820 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -13,6 +13,8 @@ namespace phpbb\files; +use \phpbb\language\language; + /** * File upload class * Init class (all parameters optional and able to be set/overwritten separately) - scope is global and valid for all uploads @@ -52,16 +54,21 @@ class upload /** @var \phpbb\files\factory Files factory */ protected $factory; + /** @var \phpbb\language\language Language class */ + protected $language; + /** * Init file upload class. * * @param \phpbb\filesystem\filesystem_interface $filesystem * @param \phpbb\files\factory $factory Files factory + * @param \phpbb\language\language $language Language class */ - public function __construct(\phpbb\filesystem\filesystem_interface $filesystem, factory $factory) + public function __construct(\phpbb\filesystem\filesystem_interface $filesystem, factory $factory, language $language) { $this->filesystem = $filesystem; $this->factory = $factory; + $this->language = $language; } /** @@ -173,7 +180,7 @@ class upload */ function form_upload($form_name, \phpbb\plupload\plupload $plupload = null) { - global $user, $request; + global $request; $upload = $request->file($form_name); unset($upload['local_mode']); @@ -213,7 +220,7 @@ class upload // Check if empty file got uploaded (not catched by is_uploaded_file) if (isset($upload['size']) && $upload['size'] == 0) { - $file->error[] = $user->lang[$this->error_prefix . 'EMPTY_FILEUPLOAD']; + $file->error[] = $this->language->lang($this->error_prefix . 'EMPTY_FILEUPLOAD'); return $file; } @@ -231,14 +238,14 @@ class upload $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); } - $file->error[] = (empty($max_filesize)) ? $user->lang[$this->error_prefix . 'PHP_SIZE_NA'] : sprintf($user->lang[$this->error_prefix . 'PHP_SIZE_OVERRUN'], $max_filesize, $user->lang[$unit]); + $file->error[] = (empty($max_filesize)) ? $this->language->lang($this->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); return $file; } // Not correctly uploaded if (!$file->is_uploaded()) { - $file->error[] = $user->lang[$this->error_prefix . 'NOT_UPLOADED']; + $file->error[] = $this->language->lang($this->error_prefix . 'NOT_UPLOADED'); return $file; } @@ -257,7 +264,7 @@ class upload */ function local_upload($source_file, $filedata = false) { - global $user, $request; + global $request; $upload = array(); @@ -312,14 +319,14 @@ class upload $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); } - $file->error[] = (empty($max_filesize)) ? $user->lang[$this->error_prefix . 'PHP_SIZE_NA'] : sprintf($user->lang[$this->error_prefix . 'PHP_SIZE_OVERRUN'], $max_filesize, $user->lang[$unit]); + $file->error[] = (empty($max_filesize)) ?$this->language->lang($this->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); return $file; } // Not correctly uploaded if (!$file->is_uploaded()) { - $file->error[] = $user->lang[$this->error_prefix . 'NOT_UPLOADED']; + $file->error[] = $this->language->lang($this->error_prefix . 'NOT_UPLOADED'); return $file; } @@ -339,19 +346,19 @@ class upload */ function remote_upload($upload_url) { - global $user, $phpbb_root_path; + global $phpbb_root_path; $upload_ary = array(); $upload_ary['local_mode'] = true; if (!preg_match('#^(https?://).*?\.(' . implode('|', $this->allowed_extensions) . ')$#i', $upload_url, $match)) { - return $this->factory->get('filespec')->set_error($user->lang[$this->error_prefix . 'URL_INVALID']); + return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'URL_INVALID')); } if (empty($match[2])) { - return $this->factory->get('filespec')->set_error($user->lang[$this->error_prefix . 'URL_INVALID']);e; + return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'URL_INVALID')); } $url = parse_url($upload_url); @@ -400,7 +407,7 @@ class upload if (!($fsock = @fsockopen($host, $port, $errno, $errstr))) { - return $this->factory->get('filespec')->set_error($user->lang[$this->error_prefix . 'NOT_UPLOADED']); + return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'NOT_UPLOADED')); } // Make sure $path not beginning with / @@ -441,7 +448,7 @@ class upload { $max_filesize = get_formatted_filesize($remote_max_filesize, false); - return $this->factory->get('filespec')->set_error(sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit'])); + return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); } $data .= $block; @@ -468,7 +475,7 @@ class upload { $max_filesize = get_formatted_filesize($remote_max_filesize, false); - return $this->factory->get('filespec')->set_error(sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit'])); + return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); } } else if (stripos($line, '404 not found') !== false) @@ -526,8 +533,6 @@ class upload */ function assign_internal_error($errorcode) { - global $user; - switch ($errorcode) { case 1: @@ -542,21 +547,21 @@ class upload $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); } - $error = (empty($max_filesize)) ? $user->lang[$this->error_prefix . 'PHP_SIZE_NA'] : sprintf($user->lang[$this->error_prefix . 'PHP_SIZE_OVERRUN'], $max_filesize, $user->lang[$unit]); + $error = (empty($max_filesize)) ? $this->language->lang($this->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); break; case 2: $max_filesize = get_formatted_filesize($this->max_filesize, false); - $error = sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit']); + $error = $this->language->lang($this->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit']); break; case 3: - $error = $user->lang[$this->error_prefix . 'PARTIAL_UPLOAD']; + $error = $this->language->lang($this->error_prefix . 'PARTIAL_UPLOAD'); break; case 4: - $error = $user->lang[$this->error_prefix . 'NOT_UPLOADED']; + $error = $this->language->lang($this->error_prefix . 'NOT_UPLOADED'); break; case 6: @@ -578,32 +583,30 @@ class upload */ function common_checks(&$file) { - global $user; - // Filesize is too big or it's 0 if it was larger than the maxsize in the upload form if ($this->max_filesize && ($file->get('filesize') > $this->max_filesize || $file->get('filesize') == 0)) { $max_filesize = get_formatted_filesize($this->max_filesize, false); - $file->error[] = sprintf($user->lang[$this->error_prefix . 'WRONG_FILESIZE'], $max_filesize['value'], $max_filesize['unit']); + $file->error[] = $this->language->lang($this->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit']); } // check Filename if (preg_match("#[\\/:*?\"<>|]#i", $file->get('realname'))) { - $file->error[] = sprintf($user->lang[$this->error_prefix . 'INVALID_FILENAME'], $file->get('realname')); + $file->error[] = $this->language->lang($this->error_prefix . 'INVALID_FILENAME', $file->get('realname')); } // Invalid Extension if (!$this->valid_extension($file)) { - $file->error[] = sprintf($user->lang[$this->error_prefix . 'DISALLOWED_EXTENSION'], $file->get('extension')); + $file->error[] = $this->language->lang($this->error_prefix . 'DISALLOWED_EXTENSION', $file->get('extension')); } // MIME Sniffing if (!$this->valid_content($file)) { - $file->error[] = sprintf($user->lang[$this->error_prefix . 'DISALLOWED_CONTENT']); + $file->error[] = $this->language->lang($this->error_prefix . 'DISALLOWED_CONTENT'); } } -- cgit v1.2.1 From 6541e4cb17d3014151d469b870eac5637ed23071 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 14:52:09 +0200 Subject: [ticket/13904] Improve doc blocks in filespec class PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 94 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 76 insertions(+), 18 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index d14c9a226d..d091e975d5 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -19,25 +19,50 @@ namespace phpbb\files; */ class filespec { + /** @var string File name */ var $filename = ''; + + /** @var string Real name of file */ var $realname = ''; + + /** @var string Upload name of file */ var $uploadname = ''; + + /** @var string Mimetype of file */ var $mimetype = ''; + + /** @var string File extension */ var $extension = ''; + + /** @var int File size */ var $filesize = 0; + + /** @var int Width of file */ var $width = 0; + + /** @var int Height of file */ var $height = 0; + + /** @var array Image info including type and size */ var $image_info = array(); + /** @var string Destination file name */ var $destination_file = ''; + + /** @var string Destination file path */ var $destination_path = ''; + /** @var bool Whether file was moved */ var $file_moved = false; + + /** @var bool Whether file is local */ var $local = false; + /** @var array Error array */ var $error = array(); - var $upload = ''; + /** @var upload Instance of upload class */ + var $upload; /** * @var \phpbb\filesystem\filesystem_interface @@ -57,20 +82,26 @@ class filespec protected $mimetype_guesser; /** - * File Class - * @access private + * File upload class + * + * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem + * @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) { - // @todo call this via files - //$this->set_upload_ary($upload_ary); - //$this->set_upload_namespace($upload_namespace); - $this->plupload = $plupload; $this->mimetype_guesser = $mimetype_guesser; $this->filesystem = $phpbb_filesystem; } + /** + * Set upload ary + * + * @param array $upload_ary Upload ary + * + * @return filespec This instance of the filespec class + */ public function set_upload_ary($upload_ary) { $this->filename = $upload_ary['tmp_name']; @@ -101,6 +132,13 @@ class filespec return $this; } + /** + * Set the upload namespace + * + * @param upload $namespace Instance of upload class + * + * @return filespec This instance of the filespec class + */ public function set_upload_namespace($namespace) { $this->upload = $namespace; @@ -109,7 +147,7 @@ class filespec } /** - * Check if class members were not properly initalised yet + * Check if class members were not properly initialised yet * * @return bool True if there was an init error, false if not */ @@ -135,10 +173,13 @@ class filespec /** * Cleans destination filename * - * @param real|unique|unique_ext $mode real creates a realname, filtering some characters, lowering every character. Unique creates an unique filename + * @param string $mode Either real, unique, or unique_ext. Real creates a + * realname, filtering some characters, lowering every + * character. Unique creates a unique filename. * @param string $prefix Prefix applied to filename * @param string $user_id The user_id is only needed for when cleaning a user's avatar - * @access public + * + *@access public */ function clean_filename($mode = 'unique', $prefix = '', $user_id = '') { @@ -184,6 +225,10 @@ class filespec /** * Get property from file object + * + * @param string $property Name of property + * + * @return mixed Content of property */ function get($property) { @@ -196,9 +241,9 @@ class filespec } /** - * Check if file is an image (mimetype) + * Check if file is an image (mime type) * - * @return true if it is an image, false if not + * @return bool true if it is an image, false if not */ function is_image() { @@ -208,7 +253,7 @@ class filespec /** * Check if the file got correctly uploaded * - * @return true if it is a valid upload, false if not + * @return bool true if it is a valid upload, false if not */ function is_uploaded() { @@ -241,7 +286,8 @@ class filespec /** * Get file extension * - * @param string Filename that needs to be checked + * @param string $filename Filename that needs to be checked + * * @return string Extension of the supplied filename */ static public function get_extension($filename) @@ -258,10 +304,10 @@ class filespec } /** - * Get mimetype + * Get mime type * * @param string $filename Filename that needs to be checked - * @return string Mimetype of supplied filename + * @return string Mime type of supplied filename */ function get_mimetype($filename) { @@ -279,7 +325,11 @@ class filespec } /** - * Get filesize + * Get file size + * + * @param string $filename File name of file to check + * + * @return int File size */ function get_filesize($filename) { @@ -289,6 +339,10 @@ class filespec /** * Check the first 256 bytes for forbidden content + * + * @param array $disallowed_content Array containg disallowed content + * + * @return bool False if disallowed content found, true if not */ function check_content($disallowed_content) { @@ -321,8 +375,10 @@ class filespec * @param string $destination Destination path, for example $config['avatar_path'] * @param bool $overwrite If set to true, an already existing file will be overwritten * @param bool $skip_image_check If set to true, the check for the file to be a valid image is skipped - * @param string $chmod Permission mask for chmodding the file after a successful move. The mode entered here reflects the mode defined by {@link phpbb_chmod()} + * @param string|bool $chmod Permission mask for chmodding the file after a successful move. + * The mode entered here reflects the mode defined by {@link phpbb_chmod()} * + * @return bool True if file was moved, false if not * @access public */ function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = false) @@ -475,6 +531,8 @@ class filespec /** * Performing additional checks + * + * @return bool False if issue was found, true if not */ function additional_checks() { -- cgit v1.2.1 From 697ac5f4aa151b06ed65f8352652443bf297682a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 15:06:24 +0200 Subject: [ticket/13904] Use language class instead of global user in filespec PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 45 +++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 20 deletions(-) (limited to 'phpBB/phpbb/files') 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; } -- cgit v1.2.1 From 47f8f2cc88bdcd40087c8e391be1d33d36a2d308 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 15:24:38 +0200 Subject: [ticket/13904] Pass request service to upload instead of using global PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index e37f90e820..e62c29883a 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -13,7 +13,10 @@ namespace phpbb\files; +use \phpbb\filesystem\filesystem_interface; use \phpbb\language\language; +use \phpbb\plupload\plupload; +use \phpbb\request\request_interface; /** * File upload class @@ -57,18 +60,23 @@ class upload /** @var \phpbb\language\language Language class */ protected $language; + /** @var \phpbb\request\request_interface Request class */ + protected $request; + /** * Init file upload class. * * @param \phpbb\filesystem\filesystem_interface $filesystem * @param \phpbb\files\factory $factory Files factory * @param \phpbb\language\language $language Language class + * @param \phpbb\request\request_interface $request Request class */ - public function __construct(\phpbb\filesystem\filesystem_interface $filesystem, factory $factory, language $language) + public function __construct(filesystem_interface $filesystem, factory $factory, language $language, request_interface $request) { $this->filesystem = $filesystem; $this->factory = $factory; $this->language = $language; + $this->request = $request; } /** @@ -178,11 +186,9 @@ class upload * @return filespec $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, plupload $plupload = null) { - global $request; - - $upload = $request->file($form_name); + $upload = $this->request->file($form_name); unset($upload['local_mode']); if ($plupload) @@ -264,8 +270,6 @@ class upload */ function local_upload($source_file, $filedata = false) { - global $request; - $upload = array(); $upload['local_mode'] = true; @@ -331,7 +335,7 @@ class upload } $this->common_checks($file); - $request->overwrite('local', $upload, \phpbb\request\request_interface::FILES); + $this->request->overwrite('local', $upload, request_interface::FILES); return $file; } @@ -657,8 +661,7 @@ class upload */ function is_valid($form_name) { - global $request; - $upload = $request->file($form_name); + $upload = $this->request->file($form_name); return (!empty($upload) && $upload['name'] !== 'none'); } -- cgit v1.2.1 From 52652ca1824e91ecfe7549167aebd92c314af678 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 15:29:32 +0200 Subject: [ticket/13904] Remove phpbb_root_path global from upload class PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index e62c29883a..38aad5a3bd 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -63,6 +63,9 @@ class upload /** @var \phpbb\request\request_interface Request class */ protected $request; + /** @var string phpBB root path */ + protected $phpbb_root_path; + /** * Init file upload class. * @@ -70,13 +73,15 @@ class upload * @param \phpbb\files\factory $factory Files factory * @param \phpbb\language\language $language Language class * @param \phpbb\request\request_interface $request Request class + * @param string $phpbb_root_path phpBB root path */ - public function __construct(filesystem_interface $filesystem, factory $factory, language $language, request_interface $request) + public function __construct(filesystem_interface $filesystem, factory $factory, language $language, request_interface $request, $phpbb_root_path) { $this->filesystem = $filesystem; $this->factory = $factory; $this->language = $language; $this->request = $request; + $this->phpbb_root_path = $phpbb_root_path; } /** @@ -350,8 +355,6 @@ class upload */ function remote_upload($upload_url) { - global $phpbb_root_path; - $upload_ary = array(); $upload_ary['local_mode'] = true; @@ -504,7 +507,7 @@ class upload return $this->factory->get('filespec')->set_error($this->error_prefix . 'EMPTY_REMOTE_DATA'); } - $tmp_path = (!@ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'off') ? false : $phpbb_root_path . 'cache'; + $tmp_path = (!@ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'off') ? false : $this->phpbb_root_path . 'cache'; $filename = tempnam($tmp_path, unique_id() . '-'); if (!($fp = @fopen($filename, 'wb'))) -- cgit v1.2.1 From b871dbcf1f2d0483cbe19cddf94a5bdc9659ab00 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 2 Jun 2015 15:46:41 +0200 Subject: [ticket/13904] Remove phpbb_root_path global from filespec class PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 5e685615d7..736610f6c2 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -86,20 +86,25 @@ class filespec /** @var \phpbb\language\language Language class */ protected $language; + /** @var string phpBB root path */ + protected $phpbb_root_path; + /** * 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 + * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem Filesystem + * @param \phpbb\language\language $language Language + * @param string $phpbb_root_path phpBB root path + * @param \phpbb\mimetype\guesser $mimetype_guesser Mime type guesser + * @param \phpbb\plupload\plupload $plupload Plupload */ - function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, language $language, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) + function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, language $language, $phpbb_root_path, \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; + $this->phpbb_root_path = $phpbb_root_path; } /** @@ -390,8 +395,6 @@ class filespec */ function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = false) { - global $phpbb_root_path; - if (sizeof($this->error)) { return false; @@ -400,7 +403,7 @@ class filespec $chmod = ($chmod === false) ? CHMOD_READ | CHMOD_WRITE : $chmod; // We need to trust the admin in specifying valid upload directories and an attacker not being able to overwrite it... - $this->destination_path = $phpbb_root_path . $destination; + $this->destination_path = $this->phpbb_root_path . $destination; // Check if the destination path exist... if (!file_exists($this->destination_path)) -- cgit v1.2.1 From 11b2c938c6c3a6a14465f04ed356fbd013276143 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 14 Jul 2015 16:15:39 +0200 Subject: [ticket/13904] Move form_upload to its own class and define type classes PHPBB3-13904 --- phpBB/phpbb/files/factory.php | 2 +- phpBB/phpbb/files/types/form.php | 151 +++++++++++++++++++++++++++++ phpBB/phpbb/files/types/type_interface.php | 38 ++++++++ phpBB/phpbb/files/upload.php | 90 +++-------------- 4 files changed, 202 insertions(+), 79 deletions(-) create mode 100644 phpBB/phpbb/files/types/form.php create mode 100644 phpBB/phpbb/files/types/type_interface.php (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/factory.php b/phpBB/phpbb/files/factory.php index b28a7ab659..508c50c6ce 100644 --- a/phpBB/phpbb/files/factory.php +++ b/phpBB/phpbb/files/factory.php @@ -42,7 +42,7 @@ class factory { $service = false; - $name = (strpos($name, 'files.') === false && strpos($name, '.') === false) ? 'files.' . $name : $name; + $name = (strpos($name, 'files.') === false) ? 'files.' . $name : $name; try { diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php new file mode 100644 index 0000000000..5c5c332906 --- /dev/null +++ b/phpBB/phpbb/files/types/form.php @@ -0,0 +1,151 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files\types; + +use \phpbb\files\factory; +use \phpbb\files\filespec; +use \phpbb\files\upload; +use \phpbb\plupload\plupload; +use \phpbb\request\request_interface; + +class form implements type_interface +{ + /** @var factory Files factory */ + protected $factory; + + /** @var plupload */ + protected $plupload; + + /** @var request_interface */ + protected $request; + + /** @var upload */ + protected $upload; + + /** + * Construct a form upload type + * + * @param factory $factory + * @param request_interface $request + */ + public function __construct(factory $factory, plupload $plupload, request_interface $request) + { + $this->factory = $factory; + $this->plupload = $plupload; + $this->request = $request; + } + + /** + * {@inheritdoc} + */ + public function upload() + { + $args = func_get_args(); + return $this->form_upload($args[0], $args[1]); + } + + /** + * {@inheritdoc} + */ + public function set_upload(upload $upload) + { + $this->upload = $upload; + + return $this; + } + + /** + * Form upload method + * Upload file from users harddisk + * + * @param string $form_name Form name assigned to the file input field (if it is an array, the key has to be specified) + * @param plupload $plupload The plupload object + * + * @return filespec $file Object "filespec" is returned, all further operations can be done with this object + * @access public + */ + protected function form_upload($form_name, plupload $plupload = null) + { + $upload = $this->request->file($form_name); + unset($upload['local_mode']); + + if ($plupload) + { + $result = $plupload->handle_upload($form_name); + if (is_array($result)) + { + $upload = array_merge($upload, $result); + } + } + + /** @var filespec $file */ + $file = $this->factory->get('filespec') + ->set_upload_ary($upload) + ->set_upload_namespace($this->upload); + + if ($file->init_error()) + { + $file->error[] = ''; + return $file; + } + + // Error array filled? + if (isset($upload['error'])) + { + $error = $this->upload->assign_internal_error($upload['error']); + + if ($error !== false) + { + $file->error[] = $error; + return $file; + } + } + + // Check if empty file got uploaded (not catched by is_uploaded_file) + if (isset($upload['size']) && $upload['size'] == 0) + { + $file->error[] = $this->language->lang($this->error_prefix . 'EMPTY_FILEUPLOAD'); + return $file; + } + + // PHP Upload filesize exceeded + if ($file->get('filename') == 'none') + { + $max_filesize = @ini_get('upload_max_filesize'); + $unit = 'MB'; + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_filesize, -1, 1)); + $max_filesize = (int) $max_filesize; + + $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); + } + + $file->error[] = (empty($max_filesize)) ? $this->language->lang($this->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); + return $file; + } + + // Not correctly uploaded + if (!$file->is_uploaded()) + { + $file->error[] = $this->language->lang($this->error_prefix . 'NOT_UPLOADED'); + return $file; + } + + $this->upload->common_checks($file); + + return $file; + } +} diff --git a/phpBB/phpbb/files/types/type_interface.php b/phpBB/phpbb/files/types/type_interface.php new file mode 100644 index 0000000000..adfbb75293 --- /dev/null +++ b/phpBB/phpbb/files/types/type_interface.php @@ -0,0 +1,38 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files\types; + +use \phpbb\files\upload; + +interface type_interface +{ + /** + * Handle upload for upload types. Arguments passed to this method will be + * handled by the upload type classes themselves. + * + * @return \phpbb\files\filespec|bool Filespec instance if upload is + * successful or false if not + */ + public function upload(); + + /** + * Set upload instance + * Needs to be executed before every upload. + * + * @param upload $upload Upload instance + * + * @return type_interface Returns itself + */ + public function set_upload(upload $upload); +} diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 38aad5a3bd..09f2b9408d 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -182,87 +182,21 @@ class upload } /** - * Form upload method - * Upload file from users harddisk + * Handle upload based on type * - * @param string $form_name Form name assigned to the file input field (if it is an array, the key has to be specified) - * @param \phpbb\plupload\plupload $plupload The plupload object + * @param string $type Upload type * - * @return filespec $file Object "filespec" is returned, all further operations can be done with this object - * @access public + * @return \phpbb\files\filespec|bool A filespec instance if upload was + * successful, false if there were issues or the type is not supported */ - function form_upload($form_name, plupload $plupload = null) + public function handle_upload($type) { - $upload = $this->request->file($form_name); - unset($upload['local_mode']); - - if ($plupload) - { - $result = $plupload->handle_upload($form_name); - if (is_array($result)) - { - $upload = array_merge($upload, $result); - } - } - - /** @var filespec $file */ - $file = $this->factory->get('filespec') - ->set_upload_ary($upload) - ->set_upload_namespace($this); - - if ($file->init_error()) - { - $file->error[] = ''; - return $file; - } - - // Error array filled? - if (isset($upload['error'])) - { - $error = $this->assign_internal_error($upload['error']); - - if ($error !== false) - { - $file->error[] = $error; - return $file; - } - } - - // Check if empty file got uploaded (not catched by is_uploaded_file) - if (isset($upload['size']) && $upload['size'] == 0) - { - $file->error[] = $this->language->lang($this->error_prefix . 'EMPTY_FILEUPLOAD'); - return $file; - } - - // PHP Upload filesize exceeded - if ($file->get('filename') == 'none') - { - $max_filesize = @ini_get('upload_max_filesize'); - $unit = 'MB'; - - if (!empty($max_filesize)) - { - $unit = strtolower(substr($max_filesize, -1, 1)); - $max_filesize = (int) $max_filesize; + $args = func_get_args(); + array_shift($args); + $type_class = $this->factory->get('types.' . $type) + ->set_upload($this); - $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); - } - - $file->error[] = (empty($max_filesize)) ? $this->language->lang($this->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); - return $file; - } - - // Not correctly uploaded - if (!$file->is_uploaded()) - { - $file->error[] = $this->language->lang($this->error_prefix . 'NOT_UPLOADED'); - return $file; - } - - $this->common_checks($file); - - return $file; + return (is_object($type_class)) ? call_user_func_array(array($type_class, 'upload'), $args) : false; } /** @@ -536,9 +470,9 @@ class upload * @param string $errorcode Error code to assign * * @return string Error string - * @access private + * @access public */ - function assign_internal_error($errorcode) + public function assign_internal_error($errorcode) { switch ($errorcode) { -- cgit v1.2.1 From f9b69e73d4b754c7c5bab52d16b2df98812e5570 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 13:35:38 +0200 Subject: [ticket/13904] Fix minor coding issues and don't use form_upload anymore PHPBB3-13904 --- phpBB/phpbb/files/types/form.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index 5c5c332906..d82df46fac 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -52,7 +52,7 @@ class form implements type_interface public function upload() { $args = func_get_args(); - return $this->form_upload($args[0], $args[1]); + return $this->form_upload($args[0], (isset($args[1])) ? $args[1] : null); } /** -- cgit v1.2.1 From cf9b6ed4742f95b75125963935f4f2d9e9cfa62c Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 13:57:19 +0200 Subject: [ticket/13904] Use the class member plupload instead of argument PHPBB3-13904 --- phpBB/phpbb/files/types/form.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index d82df46fac..130a64445b 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -52,7 +52,7 @@ class form implements type_interface public function upload() { $args = func_get_args(); - return $this->form_upload($args[0], (isset($args[1])) ? $args[1] : null); + return $this->form_upload($args[0]); } /** @@ -70,19 +70,18 @@ class form implements type_interface * Upload file from users harddisk * * @param string $form_name Form name assigned to the file input field (if it is an array, the key has to be specified) - * @param plupload $plupload The plupload object * * @return filespec $file Object "filespec" is returned, all further operations can be done with this object * @access public */ - protected function form_upload($form_name, plupload $plupload = null) + protected function form_upload($form_name) { $upload = $this->request->file($form_name); unset($upload['local_mode']); - if ($plupload) + if ($this->plupload) { - $result = $plupload->handle_upload($form_name); + $result = $this->plupload->handle_upload($form_name); if (is_array($result)) { $upload = array_merge($upload, $result); -- cgit v1.2.1 From adcc901af181b6727dd7af89a3926c9923a58471 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 16:08:20 +0200 Subject: [ticket/13904] Fix minor issues and move local_upload to its own class PHPBB3-13904 --- phpBB/phpbb/files/types/form.php | 39 +++++------ phpBB/phpbb/files/types/local.php | 137 ++++++++++++++++++++++++++++++++++++++ phpBB/phpbb/files/upload.php | 80 ---------------------- 3 files changed, 152 insertions(+), 104 deletions(-) create mode 100644 phpBB/phpbb/files/types/local.php (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index 130a64445b..98d1c51d5f 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -16,14 +16,18 @@ namespace phpbb\files\types; use \phpbb\files\factory; use \phpbb\files\filespec; use \phpbb\files\upload; +use \phpbb\language\language; use \phpbb\plupload\plupload; use \phpbb\request\request_interface; -class form implements type_interface +class form extends base { /** @var factory Files factory */ protected $factory; + /** @var language */ + protected $language; + /** @var plupload */ protected $plupload; @@ -39,9 +43,10 @@ class form implements type_interface * @param factory $factory * @param request_interface $request */ - public function __construct(factory $factory, plupload $plupload, request_interface $request) + public function __construct(factory $factory, language $language, plupload $plupload, request_interface $request) { $this->factory = $factory; + $this->language = $language; $this->plupload = $plupload; $this->request = $request; } @@ -79,13 +84,10 @@ class form implements type_interface $upload = $this->request->file($form_name); unset($upload['local_mode']); - if ($this->plupload) + $result = $this->plupload->handle_upload($form_name); + if (is_array($result)) { - $result = $this->plupload->handle_upload($form_name); - if (is_array($result)) - { - $upload = array_merge($upload, $result); - } + $upload = array_merge($upload, $result); } /** @var filespec $file */ @@ -114,32 +116,21 @@ class form implements type_interface // Check if empty file got uploaded (not catched by is_uploaded_file) if (isset($upload['size']) && $upload['size'] == 0) { - $file->error[] = $this->language->lang($this->error_prefix . 'EMPTY_FILEUPLOAD'); + $file->error[] = $this->language->lang($this->upload->error_prefix . 'EMPTY_FILEUPLOAD'); return $file; } - // PHP Upload filesize exceeded - if ($file->get('filename') == 'none') + // PHP Upload filesize check + $file = $this->check_upload_size($file); + if (sizeof($file->error)) { - $max_filesize = @ini_get('upload_max_filesize'); - $unit = 'MB'; - - if (!empty($max_filesize)) - { - $unit = strtolower(substr($max_filesize, -1, 1)); - $max_filesize = (int) $max_filesize; - - $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); - } - - $file->error[] = (empty($max_filesize)) ? $this->language->lang($this->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); return $file; } // Not correctly uploaded if (!$file->is_uploaded()) { - $file->error[] = $this->language->lang($this->error_prefix . 'NOT_UPLOADED'); + $file->error[] = $this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED'); return $file; } diff --git a/phpBB/phpbb/files/types/local.php b/phpBB/phpbb/files/types/local.php new file mode 100644 index 0000000000..38876d8ab4 --- /dev/null +++ b/phpBB/phpbb/files/types/local.php @@ -0,0 +1,137 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files\types; + +use \phpbb\files\factory; +use \phpbb\files\filespec; +use \phpbb\files\upload; +use \phpbb\language\language; +use \phpbb\request\request_interface; + +class local extends base +{ + /** @var factory Files factory */ + protected $factory; + + /** @var language */ + protected $language; + + /** @var request_interface */ + protected $request; + + /** @var upload */ + protected $upload; + + /** + * Construct a form upload type + * + * @param factory $factory + * @param request_interface $request + */ + public function __construct(factory $factory, language $language, request_interface $request) + { + $this->factory = $factory; + $this->language = $language; + $this->request = $request; + } + + /** + * {@inheritdoc} + */ + public function upload() + { + $args = func_get_args(); + return $this->local_upload($args[0], isset($args[1]) ? $args[1] : false); + } + + /** + * Move file from another location to phpBB + * + * @param string $source_file Filename of source file + * @param array|bool $filedata Array with filedata or false + * + * @return filespec Object "filespec" is returned, all further operations can be done with this object + */ + protected function local_upload($source_file, $filedata = false) + { + $upload = array(); + + $upload['local_mode'] = true; + $upload['tmp_name'] = $source_file; + + if ($filedata === false) + { + $upload['name'] = utf8_basename($source_file); + $upload['size'] = 0; + } + else + { + $upload['name'] = $filedata['realname']; + $upload['size'] = $filedata['size']; + $upload['type'] = $filedata['type']; + } + + /** @var filespec $file */ + $file = $this->factory->get('filespec') + ->set_upload_ary($upload) + ->set_upload_namespace($this->upload); + + if ($file->init_error()) + { + $file->error[] = ''; + return $file; + } + + if (isset($upload['error'])) + { + $error = $this->upload->assign_internal_error($upload['error']); + + if ($error !== false) + { + $file->error[] = $error; + return $file; + } + } + + // PHP Upload filesize exceeded + if ($file->get('filename') == 'none') + { + $max_filesize = @ini_get('upload_max_filesize'); + $unit = 'MB'; + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_filesize, -1, 1)); + $max_filesize = (int) $max_filesize; + + $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); + } + + $file->error[] = (empty($max_filesize)) ?$this->language->lang($this->upload->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->upload->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); + return $file; + } + + // Not correctly uploaded + if (!$file->is_uploaded()) + { + $file->error[] = $this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED'); + return $file; + } + + $this->upload->common_checks($file); + $this->request->overwrite('local', $upload, request_interface::FILES); + + return $file; + } +} diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 09f2b9408d..471c9c378f 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -199,86 +199,6 @@ class upload return (is_object($type_class)) ? call_user_func_array(array($type_class, 'upload'), $args) : false; } - /** - * Move file from another location to phpBB - * - * @param string $source_file Filename of source file - * @param array|bool $filedata Array with filedata or false - * - * @return filespec Object "filespec" is returned, all further operations can be done with this object - */ - function local_upload($source_file, $filedata = false) - { - $upload = array(); - - $upload['local_mode'] = true; - $upload['tmp_name'] = $source_file; - - if ($filedata === false) - { - $upload['name'] = utf8_basename($source_file); - $upload['size'] = 0; - } - else - { - $upload['name'] = $filedata['realname']; - $upload['size'] = $filedata['size']; - $upload['type'] = $filedata['type']; - } - - /** @var filespec $file */ - $file = $this->factory->get('filespec') - ->set_upload_ary($upload) - ->set_upload_namespace($this); - - if ($file->init_error()) - { - $file->error[] = ''; - return $file; - } - - if (isset($upload['error'])) - { - $error = $this->assign_internal_error($upload['error']); - - if ($error !== false) - { - $file->error[] = $error; - return $file; - } - } - - // PHP Upload filesize exceeded - if ($file->get('filename') == 'none') - { - $max_filesize = @ini_get('upload_max_filesize'); - $unit = 'MB'; - - if (!empty($max_filesize)) - { - $unit = strtolower(substr($max_filesize, -1, 1)); - $max_filesize = (int) $max_filesize; - - $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); - } - - $file->error[] = (empty($max_filesize)) ?$this->language->lang($this->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); - return $file; - } - - // Not correctly uploaded - if (!$file->is_uploaded()) - { - $file->error[] = $this->language->lang($this->error_prefix . 'NOT_UPLOADED'); - return $file; - } - - $this->common_checks($file); - $this->request->overwrite('local', $upload, request_interface::FILES); - - return $file; - } - /** * Remote upload method * Uploads file from given url -- cgit v1.2.1 From 167cc58f27da138e138310dafaeff5b53ccedef0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 16:08:49 +0200 Subject: [ticket/13904] Add types base class PHPBB3-13904 --- phpBB/phpbb/files/types/base.php | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 phpBB/phpbb/files/types/base.php (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/base.php b/phpBB/phpbb/files/types/base.php new file mode 100644 index 0000000000..686ce6f695 --- /dev/null +++ b/phpBB/phpbb/files/types/base.php @@ -0,0 +1,66 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files\types; + +use \phpbb\files\filespec; +use \phpbb\files\upload; +use \phpbb\language\language; + +abstract class base implements type_interface +{ + /** @var language */ + protected $language; + + /** @var upload */ + protected $upload; + + /** + * Check if upload exceeds maximum file size + * + * @param filespec $file Filespec object + * + * @return filespec Returns same filespec instance + */ + public function check_upload_size($file) + { + // PHP Upload filesize exceeded + if ($file->get('filename') == 'none') + { + $max_filesize = @ini_get('upload_max_filesize'); + $unit = 'MB'; + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_filesize, -1, 1)); + $max_filesize = (int) $max_filesize; + + $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); + } + + $file->error[] = (empty($max_filesize)) ? $this->language->lang($this->upload->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->upload->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); + } + + return $file; + } + + /** + * {@inheritdoc} + */ + public function set_upload(upload $upload) + { + $this->upload = $upload; + + return $this; + } +} -- cgit v1.2.1 From 636a29d589e92765b5746a977b3b53c1bb4e818c Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 17:12:24 +0200 Subject: [ticket/13904] Split up local_upload method PHPBB3-13904 --- phpBB/phpbb/files/types/local.php | 47 ++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 16 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/local.php b/phpBB/phpbb/files/types/local.php index 38876d8ab4..d439f01866 100644 --- a/phpBB/phpbb/files/types/local.php +++ b/phpBB/phpbb/files/types/local.php @@ -65,22 +65,7 @@ class local extends base */ protected function local_upload($source_file, $filedata = false) { - $upload = array(); - - $upload['local_mode'] = true; - $upload['tmp_name'] = $source_file; - - if ($filedata === false) - { - $upload['name'] = utf8_basename($source_file); - $upload['size'] = 0; - } - else - { - $upload['name'] = $filedata['realname']; - $upload['size'] = $filedata['size']; - $upload['type'] = $filedata['type']; - } + $upload = $this->get_upload_ary($source_file, $filedata); /** @var filespec $file */ $file = $this->factory->get('filespec') @@ -134,4 +119,34 @@ class local extends base return $file; } + + /** + * Retrieve upload array + * + * @param string $source_file Source file name + * @param array $filedata File data array + * + * @return array Upload array + */ + protected function get_upload_ary($source_file, $filedata) + { + $upload = array(); + + $upload['local_mode'] = true; + $upload['tmp_name'] = $source_file; + + if ($filedata === false) + { + $upload['name'] = utf8_basename($source_file); + $upload['size'] = 0; + } + else + { + $upload['name'] = $filedata['realname']; + $upload['size'] = $filedata['size']; + $upload['type'] = $filedata['type']; + } + + return $upload; + } } -- cgit v1.2.1 From 5b21830ba81b5512b7c3f945a899da9103c80558 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 18:00:26 +0200 Subject: [ticket/13904] Improve docblock PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 4 ++-- phpBB/phpbb/files/types/local.php | 3 +-- phpBB/phpbb/files/upload.php | 13 ++++++------- 3 files changed, 9 insertions(+), 11 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 736610f6c2..ed64b7ff5c 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -83,7 +83,7 @@ class filespec */ protected $mimetype_guesser; - /** @var \phpbb\language\language Language class */ + /** @var language Language class */ protected $language; /** @var string phpBB root path */ @@ -93,7 +93,7 @@ class filespec * File upload class * * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem Filesystem - * @param \phpbb\language\language $language Language + * @param language $language Language * @param string $phpbb_root_path phpBB root path * @param \phpbb\mimetype\guesser $mimetype_guesser Mime type guesser * @param \phpbb\plupload\plupload $plupload Plupload diff --git a/phpBB/phpbb/files/types/local.php b/phpBB/phpbb/files/types/local.php index d439f01866..bb5994841f 100644 --- a/phpBB/phpbb/files/types/local.php +++ b/phpBB/phpbb/files/types/local.php @@ -15,7 +15,6 @@ namespace phpbb\files\types; use \phpbb\files\factory; use \phpbb\files\filespec; -use \phpbb\files\upload; use \phpbb\language\language; use \phpbb\request\request_interface; @@ -30,7 +29,7 @@ class local extends base /** @var request_interface */ protected $request; - /** @var upload */ + /** @var \phpbb\files\upload */ protected $upload; /** diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 471c9c378f..43f06c3503 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -15,7 +15,6 @@ namespace phpbb\files; use \phpbb\filesystem\filesystem_interface; use \phpbb\language\language; -use \phpbb\plupload\plupload; use \phpbb\request\request_interface; /** @@ -51,7 +50,7 @@ class upload /** @var int Timeout for remote upload */ var $upload_timeout = 6; - /** @var \phpbb\filesystem\filesystem_interface */ + /** @var filesystem_interface */ protected $filesystem; /** @var \phpbb\files\factory Files factory */ @@ -60,7 +59,7 @@ class upload /** @var \phpbb\language\language Language class */ protected $language; - /** @var \phpbb\request\request_interface Request class */ + /** @var request_interface Request class */ protected $request; /** @var string phpBB root path */ @@ -69,10 +68,10 @@ class upload /** * Init file upload class. * - * @param \phpbb\filesystem\filesystem_interface $filesystem - * @param \phpbb\files\factory $factory Files factory - * @param \phpbb\language\language $language Language class - * @param \phpbb\request\request_interface $request Request class + * @param filesystem_interface $filesystem + * @param factory $factory Files factory + * @param language $language Language class + * @param request_interface $request Request class * @param string $phpbb_root_path phpBB root path */ public function __construct(filesystem_interface $filesystem, factory $factory, language $language, request_interface $request, $phpbb_root_path) -- cgit v1.2.1 From 57ccfe0c483254e54b7b40bc1906ef946daf4f55 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 18:00:52 +0200 Subject: [ticket/13904] Move remote upload to its own type class PHPBB3-13904 --- phpBB/phpbb/files/types/remote.php | 242 +++++++++++++++++++++++++++++++++++++ phpBB/phpbb/files/upload.php | 185 ---------------------------- 2 files changed, 242 insertions(+), 185 deletions(-) create mode 100644 phpBB/phpbb/files/types/remote.php (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php new file mode 100644 index 0000000000..c70f1557e7 --- /dev/null +++ b/phpBB/phpbb/files/types/remote.php @@ -0,0 +1,242 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\files\types; + +use \phpbb\files\factory; +use \phpbb\files\filespec; +use \phpbb\files\upload; +use \phpbb\language\language; +use \phpbb\request\request_interface; + +class remote extends base +{ + /** @var factory Files factory */ + protected $factory; + + /** @var language */ + protected $language; + + /** @var request_interface */ + protected $request; + + /** @var upload */ + protected $upload; + + /** + * Construct a form upload type + * + * @param factory $factory + * @param request_interface $request + */ + public function __construct(factory $factory, language $language, request_interface $request) + { + $this->factory = $factory; + $this->language = $language; + $this->request = $request; + } + + /** + * {@inheritdoc} + */ + public function upload() + { + $args = func_get_args(); + return $this->remote_upload($args[0]); + } + + /** + * Remote upload method + * Uploads file from given url + * + * @param string $upload_url URL pointing to file to upload, for example http://www.foobar.com/example.gif + * @return filespec $file Object "filespec" is returned, all further operations can be done with this object + * @access public + */ + protected function remote_upload($upload_url) + { + $upload_ary = array(); + $upload_ary['local_mode'] = true; + + if (!preg_match('#^(https?://).*?\.(' . implode('|', $this->upload->allowed_extensions) . ')$#i', $upload_url, $match)) + { + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'URL_INVALID')); + } + + if (empty($match[2])) + { + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'URL_INVALID')); + } + + $url = parse_url($upload_url); + + $host = $url['host']; + $path = $url['path']; + $port = (!empty($url['port'])) ? (int) $url['port'] : 80; + + $upload_ary['type'] = 'application/octet-stream'; + + $url['path'] = explode('.', $url['path']); + $ext = array_pop($url['path']); + + $url['path'] = implode('', $url['path']); + $upload_ary['name'] = utf8_basename($url['path']) . (($ext) ? '.' . $ext : ''); + $filename = $url['path']; + $filesize = 0; + + $remote_max_filesize = $this->upload->max_filesize; + if (!$remote_max_filesize) + { + $max_filesize = @ini_get('upload_max_filesize'); + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_filesize, -1, 1)); + $remote_max_filesize = (int) $max_filesize; + + switch ($unit) + { + case 'g': + $remote_max_filesize *= 1024; + // no break + case 'm': + $remote_max_filesize *= 1024; + // no break + case 'k': + $remote_max_filesize *= 1024; + // no break + } + } + } + + $errno = 0; + $errstr = ''; + + if (!($fsock = @fsockopen($host, $port, $errno, $errstr))) + { + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); + } + + // Make sure $path not beginning with / + if (strpos($path, '/') === 0) + { + $path = substr($path, 1); + } + + fputs($fsock, 'GET /' . $path . " HTTP/1.1\r\n"); + fputs($fsock, "HOST: " . $host . "\r\n"); + fputs($fsock, "Connection: close\r\n\r\n"); + + // Set a proper timeout for the socket + socket_set_timeout($fsock, $this->upload->upload_timeout); + + $get_info = false; + $data = ''; + $length = false; + $timer_stop = time() + $this->upload->upload_timeout; + + while ((!$length || $filesize < $length) && !@feof($fsock)) + { + if ($get_info) + { + if ($length) + { + // Don't attempt to read past end of file if server indicated length + $block = @fread($fsock, min($length - $filesize, 1024)); + } + else + { + $block = @fread($fsock, 1024); + } + + $filesize += strlen($block); + + if ($remote_max_filesize && $filesize > $remote_max_filesize) + { + $max_filesize = get_formatted_filesize($remote_max_filesize, false); + + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); + } + + $data .= $block; + } + else + { + $line = @fgets($fsock, 1024); + + if ($line == "\r\n") + { + $get_info = true; + } + else + { + if (stripos($line, 'content-type: ') !== false) + { + $upload_ary['type'] = rtrim(str_replace('content-type: ', '', strtolower($line))); + } + else if ($this->upload->max_filesize && stripos($line, 'content-length: ') !== false) + { + $length = (int) str_replace('content-length: ', '', strtolower($line)); + + if ($remote_max_filesize && $length && $length > $remote_max_filesize) + { + $max_filesize = get_formatted_filesize($remote_max_filesize, false); + + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); + } + } + else if (stripos($line, '404 not found') !== false) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'URL_NOT_FOUND'); + } + } + } + + $stream_meta_data = stream_get_meta_data($fsock); + + // Cancel upload if we exceed timeout + if (!empty($stream_meta_data['timed_out']) || time() >= $timer_stop) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'REMOTE_UPLOAD_TIMEOUT'); + } + } + @fclose($fsock); + + if (empty($data)) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'EMPTY_REMOTE_DATA'); + } + + $tmp_path = (!@ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'off') ? false : $this->phpbb_root_path . 'cache'; + $filename = tempnam($tmp_path, unique_id() . '-'); + + if (!($fp = @fopen($filename, 'wb'))) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'NOT_UPLOADED'); + } + + $upload_ary['size'] = fwrite($fp, $data); + fclose($fp); + unset($data); + + $upload_ary['tmp_name'] = $filename; + + /** @var filespec $file */ + $file = $this->factory->get('filespec') + ->set_upload_ary($upload_ary) + ->set_upload_namespace($this); + $this->upload->common_checks($file); + + return $file; + } +} diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 43f06c3503..ceb7e1a741 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -198,191 +198,6 @@ class upload return (is_object($type_class)) ? call_user_func_array(array($type_class, 'upload'), $args) : false; } - /** - * Remote upload method - * Uploads file from given url - * - * @param string $upload_url URL pointing to file to upload, for example http://www.foobar.com/example.gif - * @return filespec $file Object "filespec" is returned, all further operations can be done with this object - * @access public - */ - function remote_upload($upload_url) - { - $upload_ary = array(); - $upload_ary['local_mode'] = true; - - if (!preg_match('#^(https?://).*?\.(' . implode('|', $this->allowed_extensions) . ')$#i', $upload_url, $match)) - { - return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'URL_INVALID')); - } - - if (empty($match[2])) - { - return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'URL_INVALID')); - } - - $url = parse_url($upload_url); - - $host = $url['host']; - $path = $url['path']; - $port = (!empty($url['port'])) ? (int) $url['port'] : 80; - - $upload_ary['type'] = 'application/octet-stream'; - - $url['path'] = explode('.', $url['path']); - $ext = array_pop($url['path']); - - $url['path'] = implode('', $url['path']); - $upload_ary['name'] = utf8_basename($url['path']) . (($ext) ? '.' . $ext : ''); - $filename = $url['path']; - $filesize = 0; - - $remote_max_filesize = $this->max_filesize; - if (!$remote_max_filesize) - { - $max_filesize = @ini_get('upload_max_filesize'); - - if (!empty($max_filesize)) - { - $unit = strtolower(substr($max_filesize, -1, 1)); - $remote_max_filesize = (int) $max_filesize; - - switch ($unit) - { - case 'g': - $remote_max_filesize *= 1024; - // no break - case 'm': - $remote_max_filesize *= 1024; - // no break - case 'k': - $remote_max_filesize *= 1024; - // no break - } - } - } - - $errno = 0; - $errstr = ''; - - if (!($fsock = @fsockopen($host, $port, $errno, $errstr))) - { - return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'NOT_UPLOADED')); - } - - // Make sure $path not beginning with / - if (strpos($path, '/') === 0) - { - $path = substr($path, 1); - } - - fputs($fsock, 'GET /' . $path . " HTTP/1.1\r\n"); - fputs($fsock, "HOST: " . $host . "\r\n"); - fputs($fsock, "Connection: close\r\n\r\n"); - - // Set a proper timeout for the socket - socket_set_timeout($fsock, $this->upload_timeout); - - $get_info = false; - $data = ''; - $length = false; - $timer_stop = time() + $this->upload_timeout; - - while ((!$length || $filesize < $length) && !@feof($fsock)) - { - if ($get_info) - { - if ($length) - { - // Don't attempt to read past end of file if server indicated length - $block = @fread($fsock, min($length - $filesize, 1024)); - } - else - { - $block = @fread($fsock, 1024); - } - - $filesize += strlen($block); - - if ($remote_max_filesize && $filesize > $remote_max_filesize) - { - $max_filesize = get_formatted_filesize($remote_max_filesize, false); - - return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); - } - - $data .= $block; - } - else - { - $line = @fgets($fsock, 1024); - - if ($line == "\r\n") - { - $get_info = true; - } - else - { - if (stripos($line, 'content-type: ') !== false) - { - $upload_ary['type'] = rtrim(str_replace('content-type: ', '', strtolower($line))); - } - else if ($this->max_filesize && stripos($line, 'content-length: ') !== false) - { - $length = (int) str_replace('content-length: ', '', strtolower($line)); - - if ($remote_max_filesize && $length && $length > $remote_max_filesize) - { - $max_filesize = get_formatted_filesize($remote_max_filesize, false); - - return $this->factory->get('filespec')->set_error($this->language->lang($this->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); - } - } - else if (stripos($line, '404 not found') !== false) - { - return $this->factory->get('filespec')->set_error($this->error_prefix . 'URL_NOT_FOUND'); - } - } - } - - $stream_meta_data = stream_get_meta_data($fsock); - - // Cancel upload if we exceed timeout - if (!empty($stream_meta_data['timed_out']) || time() >= $timer_stop) - { - return $this->factory->get('filespec')->set_error($this->error_prefix . 'REMOTE_UPLOAD_TIMEOUT'); - } - } - @fclose($fsock); - - if (empty($data)) - { - return $this->factory->get('filespec')->set_error($this->error_prefix . 'EMPTY_REMOTE_DATA'); - } - - $tmp_path = (!@ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'off') ? false : $this->phpbb_root_path . 'cache'; - $filename = tempnam($tmp_path, unique_id() . '-'); - - if (!($fp = @fopen($filename, 'wb'))) - { - return $this->factory->get('filespec')->set_error($this->error_prefix . 'NOT_UPLOADED'); - } - - $upload_ary['size'] = fwrite($fp, $data); - fclose($fp); - unset($data); - - $upload_ary['tmp_name'] = $filename; - - /** @var filespec $file */ - $file = $this->factory->get('filespec') - ->set_upload_ary($upload_ary) - ->set_upload_namespace($this); - $this->common_checks($file); - - return $file; - } - /** * Assign internal error * -- cgit v1.2.1 From 759a1a09fae5147afe346729764f050ecc67076b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 21:30:42 +0200 Subject: [ticket/13904] Fix remote upload functional tests PHPBB3-13904 --- phpBB/phpbb/files/types/remote.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index c70f1557e7..04263f9cba 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -234,7 +234,7 @@ class remote extends base /** @var filespec $file */ $file = $this->factory->get('filespec') ->set_upload_ary($upload_ary) - ->set_upload_namespace($this); + ->set_upload_namespace($this->upload); $this->upload->common_checks($file); return $file; -- cgit v1.2.1 From 0a6f54d522f7799819969fcd588a354b7beb3fa9 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 22:40:17 +0200 Subject: [ticket/13904] Modify doc blocks PHPBB3-13904 --- phpBB/phpbb/files/types/base.php | 10 ++++------ phpBB/phpbb/files/types/remote.php | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/base.php b/phpBB/phpbb/files/types/base.php index 686ce6f695..77ebdb4f32 100644 --- a/phpBB/phpbb/files/types/base.php +++ b/phpBB/phpbb/files/types/base.php @@ -13,8 +13,6 @@ namespace phpbb\files\types; -use \phpbb\files\filespec; -use \phpbb\files\upload; use \phpbb\language\language; abstract class base implements type_interface @@ -22,15 +20,15 @@ abstract class base implements type_interface /** @var language */ protected $language; - /** @var upload */ + /** @var \phpbb\files\upload */ protected $upload; /** * Check if upload exceeds maximum file size * - * @param filespec $file Filespec object + * @param \phpbb\files\filespec $file Filespec object * - * @return filespec Returns same filespec instance + * @return \phpbb\files\filespec Returns same filespec instance */ public function check_upload_size($file) { @@ -57,7 +55,7 @@ abstract class base implements type_interface /** * {@inheritdoc} */ - public function set_upload(upload $upload) + public function set_upload(\phpbb\files\upload $upload) { $this->upload = $upload; diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 04263f9cba..f2db6c798c 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -15,7 +15,6 @@ namespace phpbb\files\types; use \phpbb\files\factory; use \phpbb\files\filespec; -use \phpbb\files\upload; use \phpbb\language\language; use \phpbb\request\request_interface; @@ -30,7 +29,7 @@ class remote extends base /** @var request_interface */ protected $request; - /** @var upload */ + /** @var \phpbb\files\upload */ protected $upload; /** -- cgit v1.2.1 From a09c6d1fb760151b1a6c654b597b4578c3136be1 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 15 Jul 2015 23:10:23 +0200 Subject: [ticket/13904] Split code up and pass root path to remote upload type PHPBB3-13904 --- phpBB/phpbb/files/types/remote.php | 66 +++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 25 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index f2db6c798c..65cff8ccc7 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -32,17 +32,21 @@ class remote extends base /** @var \phpbb\files\upload */ protected $upload; + /** @var string phpBB root path */ + protected $phpbb_root_path; + /** * Construct a form upload type * * @param factory $factory * @param request_interface $request */ - public function __construct(factory $factory, language $language, request_interface $request) + public function __construct(factory $factory, language $language, request_interface $request, $phpbb_root_path) { $this->factory = $factory; $this->language = $language; $this->request = $request; + $this->phpbb_root_path = $phpbb_root_path; } /** @@ -93,30 +97,7 @@ class remote extends base $filename = $url['path']; $filesize = 0; - $remote_max_filesize = $this->upload->max_filesize; - if (!$remote_max_filesize) - { - $max_filesize = @ini_get('upload_max_filesize'); - - if (!empty($max_filesize)) - { - $unit = strtolower(substr($max_filesize, -1, 1)); - $remote_max_filesize = (int) $max_filesize; - - switch ($unit) - { - case 'g': - $remote_max_filesize *= 1024; - // no break - case 'm': - $remote_max_filesize *= 1024; - // no break - case 'k': - $remote_max_filesize *= 1024; - // no break - } - } - } + $remote_max_filesize = $this->get_max_file_size(); $errno = 0; $errstr = ''; @@ -238,4 +219,39 @@ class remote extends base return $file; } + + /** + * Get maximum file size for remote uploads + * + * @return int Maximum file size + */ + protected function get_max_file_size() + { + $max_file_size = $this->upload->max_filesize; + if (!$max_file_size) + { + $max_file_size = @ini_get('upload_max_filesize'); + + if (!empty($max_filesize)) + { + $unit = strtolower(substr($max_file_size, -1, 1)); + $max_file_size = (int) $max_filesize; + + switch ($unit) + { + case 'g': + $max_file_size *= 1024; + // no break + case 'm': + $max_file_size *= 1024; + // no break + case 'k': + $max_file_size *= 1024; + // no break + } + } + } + + return $max_file_size; + } } -- cgit v1.2.1 From 845233fc626b0d5e6d9e61039fde8e31b4dd28aa Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 16 Jul 2015 00:21:23 +0200 Subject: [ticket/13904] Improve test coverage and use constants instead of magic numbers PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index ceb7e1a741..234eb69735 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -210,7 +210,7 @@ class upload { switch ($errorcode) { - case 1: + case UPLOAD_ERR_INI_SIZE: $max_filesize = @ini_get('upload_max_filesize'); $unit = 'MB'; @@ -223,29 +223,37 @@ class upload } $error = (empty($max_filesize)) ? $this->language->lang($this->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); - break; + break; - case 2: + case UPLOAD_ERR_FORM_SIZE: $max_filesize = get_formatted_filesize($this->max_filesize, false); $error = $this->language->lang($this->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit']); - break; + break; - case 3: + case UPLOAD_ERR_PARTIAL: $error = $this->language->lang($this->error_prefix . 'PARTIAL_UPLOAD'); - break; + break; - case 4: + case UPLOAD_ERR_NO_FILE: $error = $this->language->lang($this->error_prefix . 'NOT_UPLOADED'); - break; + break; - case 6: + case UPLOAD_ERR_NO_TMP_DIR: $error = 'Temporary folder could not be found. Please check your PHP installation.'; - break; + break; + + case UPLOAD_ERR_CANT_WRITE: + $error = 'Can’t write to temporary folder.'; + break; + + case UPLOAD_ERR_EXTENSION: + $error = 'A PHP extension has stopped the file upload.'; + break; default: $error = false; - break; + break; } return $error; -- cgit v1.2.1 From 9e87e5a3437f3d88f7dac4c576a53fed53ff4bae Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 16 Jul 2015 00:23:10 +0200 Subject: [ticket/13904] Remove unused use statement PHPBB3-13904 --- phpBB/phpbb/files/types/base.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/base.php b/phpBB/phpbb/files/types/base.php index 77ebdb4f32..1ed06053a4 100644 --- a/phpBB/phpbb/files/types/base.php +++ b/phpBB/phpbb/files/types/base.php @@ -13,11 +13,9 @@ namespace phpbb\files\types; -use \phpbb\language\language; - abstract class base implements type_interface { - /** @var language */ + /** @var \phpbb\language\language */ protected $language; /** @var \phpbb\files\upload */ -- cgit v1.2.1 From 3e99816fa2f184b859d47308254aa8f07d68f1dd Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 16 Jul 2015 12:06:23 +0200 Subject: [ticket/13904] Set visibility in files and improve test coverage PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 78 +++++++++++++++++++++++------------------- phpBB/phpbb/files/upload.php | 40 +++++++++++----------- 2 files changed, 63 insertions(+), 55 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index ed64b7ff5c..e07aef9892 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -22,49 +22,52 @@ use \phpbb\language\language; class filespec { /** @var string File name */ - var $filename = ''; + protected $filename = ''; /** @var string Real name of file */ - var $realname = ''; + protected $realname = ''; /** @var string Upload name of file */ - var $uploadname = ''; + protected $uploadname = ''; /** @var string Mimetype of file */ - var $mimetype = ''; + protected $mimetype = ''; /** @var string File extension */ - var $extension = ''; + public $extension = ''; /** @var int File size */ - var $filesize = 0; + public $filesize = 0; /** @var int Width of file */ - var $width = 0; + protected $width = 0; /** @var int Height of file */ - var $height = 0; + protected $height = 0; /** @var array Image info including type and size */ - var $image_info = array(); + protected $image_info = array(); /** @var string Destination file name */ - var $destination_file = ''; + protected $destination_file = ''; /** @var string Destination file path */ - var $destination_path = ''; + protected $destination_path = ''; /** @var bool Whether file was moved */ - var $file_moved = false; + public $file_moved = false; - /** @var bool Whether file is local */ - var $local = false; + /** @var bool Whether file is local */ + public $local = false; + + /** @var bool Class initialization flag */ + protected $class_initialized = false; /** @var array Error array */ - var $error = array(); + public $error = array(); /** @var upload Instance of upload class */ - var $upload; + public $upload; /** * @var \phpbb\filesystem\filesystem_interface @@ -98,7 +101,7 @@ class filespec * @param \phpbb\mimetype\guesser $mimetype_guesser Mime type guesser * @param \phpbb\plupload\plupload $plupload Plupload */ - function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, language $language, $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, $phpbb_root_path, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { $this->plupload = $plupload; $this->mimetype_guesser = $mimetype_guesser; @@ -116,6 +119,12 @@ class filespec */ public function set_upload_ary($upload_ary) { + if (!isset($upload_ary) || !sizeof($upload_ary)) + { + return $this; + } + + $this->class_initialized = true; $this->filename = $upload_ary['tmp_name']; $this->filesize = $upload_ary['size']; $name = (STRIP) ? stripslashes($upload_ary['name']) : $upload_ary['name']; @@ -165,7 +174,7 @@ class filespec */ public function init_error() { - return !isset($this->filename); + return !$this->class_initialized; } /** @@ -193,7 +202,7 @@ class filespec * *@access public */ - function clean_filename($mode = 'unique', $prefix = '', $user_id = '') + public function clean_filename($mode = 'unique', $prefix = '', $user_id = '') { if ($this->init_error()) { @@ -216,22 +225,21 @@ class filespec $this->realname = preg_replace("/%(\w{2})/", '_', $this->realname); $this->realname = $prefix . $this->realname . '.' . $this->extension; - break; + break; case 'unique': $this->realname = $prefix . md5(unique_id()); - break; + break; case 'avatar': $this->extension = strtolower($this->extension); $this->realname = $prefix . $user_id . '.' . $this->extension; - break; + break; case 'unique_ext': default: $this->realname = $prefix . md5(unique_id()) . '.' . $this->extension; - break; } } @@ -242,7 +250,7 @@ class filespec * * @return mixed Content of property */ - function get($property) + public function get($property) { if ($this->init_error() || !isset($this->$property)) { @@ -257,7 +265,7 @@ class filespec * * @return bool true if it is an image, false if not */ - function is_image() + public function is_image() { return (strpos($this->mimetype, 'image/') === 0); } @@ -267,7 +275,7 @@ class filespec * * @return bool true if it is a valid upload, false if not */ - function is_uploaded() + public function is_uploaded() { $is_plupload = $this->plupload && $this->plupload->is_active(); @@ -287,7 +295,7 @@ class filespec /** * Remove file */ - function remove() + public function remove() { if ($this->file_moved) { @@ -321,7 +329,7 @@ class filespec * @param string $filename Filename that needs to be checked * @return string Mime type of supplied filename */ - function get_mimetype($filename) + public function get_mimetype($filename) { if ($this->mimetype_guesser !== null) { @@ -343,7 +351,7 @@ class filespec * * @return int File size */ - function get_filesize($filename) + public function get_filesize($filename) { return @filesize($filename); } @@ -356,7 +364,7 @@ class filespec * * @return bool False if disallowed content found, true if not */ - function check_content($disallowed_content) + public function check_content($disallowed_content) { if (empty($disallowed_content)) { @@ -393,7 +401,7 @@ class filespec * @return bool True if file was moved, false if not * @access public */ - function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = false) + public function move_file($destination, $overwrite = false, $skip_image_check = false, $chmod = false) { if (sizeof($this->error)) { @@ -443,7 +451,7 @@ class filespec } } - break; + break; case 'move': @@ -455,7 +463,7 @@ class filespec } } - break; + break; case 'local': @@ -464,7 +472,7 @@ class filespec $this->error[] = $this->language->lang($this->upload->error_prefix . 'GENERAL_UPLOAD_ERROR', $this->destination_file); } - break; + break; } // Remove temporary filename @@ -544,7 +552,7 @@ class filespec * * @return bool False if issue was found, true if not */ - function additional_checks() + public function additional_checks() { if (!$this->file_moved) { diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 234eb69735..397eb5af36 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -24,31 +24,31 @@ use \phpbb\request\request_interface; class upload { /** @var array Allowed file extensions */ - var $allowed_extensions = array(); + public $allowed_extensions = array(); /** @var array Disallowed content */ - var $disallowed_content = array('body', 'head', 'html', 'img', 'plaintext', 'a href', 'pre', 'script', 'table', 'title'); + protected $disallowed_content = array('body', 'head', 'html', 'img', 'plaintext', 'a href', 'pre', 'script', 'table', 'title'); /** @var int Maximum filesize */ - var $max_filesize = 0; + public $max_filesize = 0; /** @var int Minimum width of images */ - var $min_width = 0; + public $min_width = 0; /** @var int Minimum height of images */ - var $min_height = 0; + public $min_height = 0; /** @var int Maximum width of images */ - var $max_width = 0; + public $max_width = 0; /** @var int Maximum height of images */ - var $max_height = 0; + public $max_height = 0; /** @var string Prefix for language variables of errors */ - var $error_prefix = ''; + public $error_prefix = ''; /** @var int Timeout for remote upload */ - var $upload_timeout = 6; + public $upload_timeout = 6; /** @var filesystem_interface */ protected $filesystem; @@ -86,7 +86,7 @@ class upload /** * Reset vars */ - function reset_vars() + public function reset_vars() { $this->max_filesize = 0; $this->min_width = $this->min_height = $this->max_width = $this->max_height = 0; @@ -102,7 +102,7 @@ class upload * * @return \phpbb\files\upload This instance of upload */ - function set_allowed_extensions($allowed_extensions) + public function set_allowed_extensions($allowed_extensions) { if ($allowed_extensions !== false && is_array($allowed_extensions)) { @@ -122,7 +122,7 @@ class upload * * @return \phpbb\files\upload This instance of upload */ - function set_allowed_dimensions($min_width, $min_height, $max_width, $max_height) + public function set_allowed_dimensions($min_width, $min_height, $max_width, $max_height) { $this->min_width = (int) $min_width; $this->min_height = (int) $min_height; @@ -139,7 +139,7 @@ class upload * * @return \phpbb\files\upload This instance of upload */ - function set_max_filesize($max_filesize) + public function set_max_filesize($max_filesize) { if ($max_filesize !== false && (int) $max_filesize) { @@ -156,7 +156,7 @@ class upload * * @return \phpbb\files\upload This instance of upload */ - function set_disallowed_content($disallowed_content) + public function set_disallowed_content($disallowed_content) { if ($disallowed_content !== false && is_array($disallowed_content)) { @@ -173,7 +173,7 @@ class upload * * @return \phpbb\files\upload This instance of upload */ - function set_error_prefix($error_prefix) + public function set_error_prefix($error_prefix) { $this->error_prefix = $error_prefix; @@ -264,7 +264,7 @@ class upload * * @param filespec $file Instance of filespec class */ - function common_checks(&$file) + public function common_checks(&$file) { // Filesize is too big or it's 0 if it was larger than the maxsize in the upload form if ($this->max_filesize && ($file->get('filesize') > $this->max_filesize || $file->get('filesize') == 0)) @@ -300,7 +300,7 @@ class upload * * @return bool True if extension is allowed, false if not */ - function valid_extension(&$file) + public function valid_extension(&$file) { return (in_array($file->get('extension'), $this->allowed_extensions)) ? true : false; } @@ -313,7 +313,7 @@ class upload * @return bool True if dimensions are valid or no constraints set, false * if not */ - function valid_dimensions(&$file) + public function valid_dimensions(&$file) { if (!$this->max_width && !$this->max_height && !$this->min_width && !$this->min_height) { @@ -338,7 +338,7 @@ class upload * * @return bool True if form upload is valid, false if not */ - function is_valid($form_name) + public function is_valid($form_name) { $upload = $this->request->file($form_name); @@ -353,7 +353,7 @@ class upload * * @return bool True if content is valid, false if not */ - function valid_content(&$file) + public function valid_content(&$file) { return ($file->check_content($this->disallowed_content)); } -- cgit v1.2.1 From 02f94b7527298aad3bd15738676da872aa498039 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 14 Aug 2015 10:31:26 +0200 Subject: [ticket/13904] Update doc blocks PHPBB3-13904 --- phpBB/phpbb/files/types/form.php | 6 ++++-- phpBB/phpbb/files/types/local.php | 5 +++-- phpBB/phpbb/files/types/remote.php | 6 ++++-- 3 files changed, 11 insertions(+), 6 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index 98d1c51d5f..be0a425d57 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -40,8 +40,10 @@ class form extends base /** * Construct a form upload type * - * @param factory $factory - * @param request_interface $request + * @param factory $factory Files factory + * @param language $language Language class + * @param plupload $plupload Plupload + * @param request_interface $request Request object */ public function __construct(factory $factory, language $language, plupload $plupload, request_interface $request) { diff --git a/phpBB/phpbb/files/types/local.php b/phpBB/phpbb/files/types/local.php index bb5994841f..dfaabded9b 100644 --- a/phpBB/phpbb/files/types/local.php +++ b/phpBB/phpbb/files/types/local.php @@ -35,8 +35,9 @@ class local extends base /** * Construct a form upload type * - * @param factory $factory - * @param request_interface $request + * @param factory $factory Files factory + * @param language $language Language class + * @param request_interface $request Request object */ public function __construct(factory $factory, language $language, request_interface $request) { diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 65cff8ccc7..94bba8ad46 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -38,8 +38,10 @@ class remote extends base /** * Construct a form upload type * - * @param factory $factory - * @param request_interface $request + * @param factory $factory Files factory + * @param language $language Language class + * @param request_interface $request Request object + * @param string $phpbb_root_path phpBB root path */ public function __construct(factory $factory, language $language, request_interface $request, $phpbb_root_path) { -- cgit v1.2.1 From cdde86ce7e0c594fad5992789b3fae466bd526cc Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 26 Aug 2015 13:57:42 +0200 Subject: [ticket/13904] Use \phpbb\php\ini class for ini_get() PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 31 ++++++++++++++++++------------- phpBB/phpbb/files/upload.php | 9 +++++++-- 2 files changed, 25 insertions(+), 15 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index e07aef9892..34d86116c2 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -74,6 +74,15 @@ class filespec */ protected $filesystem; + /** @var \phpbb\php\ini ini_get() wrapper class */ + protected $php_ini; + + /** @var language Language class */ + protected $language; + + /** @var string phpBB root path */ + protected $phpbb_root_path; + /** * The plupload object * @var \phpbb\plupload\plupload @@ -86,28 +95,24 @@ class filespec */ protected $mimetype_guesser; - /** @var language Language class */ - protected $language; - - /** @var string phpBB root path */ - protected $phpbb_root_path; - /** * File upload class * * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem Filesystem + * @param \phpbb\php\ini $php_ini ini_get() wrapper * @param language $language Language - * @param string $phpbb_root_path phpBB root path - * @param \phpbb\mimetype\guesser $mimetype_guesser Mime type guesser - * @param \phpbb\plupload\plupload $plupload Plupload + * @param string $phpbb_root_path phpBB root path + * @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, $phpbb_root_path, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) + public function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, \phpbb\php\ini $php_ini, language $language, $phpbb_root_path, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { - $this->plupload = $plupload; - $this->mimetype_guesser = $mimetype_guesser; $this->filesystem = $phpbb_filesystem; + $this->php_ini = $php_ini; $this->language = $language; $this->phpbb_root_path = $phpbb_root_path; + $this->plupload = $plupload; + $this->mimetype_guesser = $mimetype_guesser; } /** @@ -420,7 +425,7 @@ class filespec return false; } - $upload_mode = (@ini_get('open_basedir') || @ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'on') ? 'move' : 'copy'; + $upload_mode = ($this->php_ini->get_bool('open_basedir') || $this->php_ini->get_bool('safe_mode')) ? 'move' : 'copy'; $upload_mode = ($this->local) ? 'local' : $upload_mode; $this->destination_file = $this->destination_path . '/' . utf8_basename($this->realname); diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 397eb5af36..35107ccb45 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -56,6 +56,9 @@ class upload /** @var \phpbb\files\factory Files factory */ protected $factory; + /** @var \phpbb\php\ini ini_get() wrapper */ + protected $php_ini; + /** @var \phpbb\language\language Language class */ protected $language; @@ -70,14 +73,16 @@ class upload * * @param filesystem_interface $filesystem * @param factory $factory Files factory + * @param \phpbb\php\ini $php_ini ini_get() wrapper * @param language $language Language class * @param request_interface $request Request class * @param string $phpbb_root_path phpBB root path */ - public function __construct(filesystem_interface $filesystem, factory $factory, language $language, request_interface $request, $phpbb_root_path) + public function __construct(filesystem_interface $filesystem, factory $factory, \phpbb\php\ini $php_ini, language $language, request_interface $request, $phpbb_root_path) { $this->filesystem = $filesystem; $this->factory = $factory; + $this->php_ini = $php_ini; $this->language = $language; $this->request = $request; $this->phpbb_root_path = $phpbb_root_path; @@ -211,7 +216,7 @@ class upload switch ($errorcode) { case UPLOAD_ERR_INI_SIZE: - $max_filesize = @ini_get('upload_max_filesize'); + $max_filesize = $this->php_ini->get_string('upload_max_filesize'); $unit = 'MB'; if (!empty($max_filesize)) -- cgit v1.2.1 From 36545d5cbe7188efbedf2e1f44b1a7b9617b50c1 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 26 Aug 2015 16:18:10 +0200 Subject: [ticket/13904] Switch around constructor arguments PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 20 ++++++-------------- phpBB/phpbb/files/upload.php | 6 +++--- 2 files changed, 9 insertions(+), 17 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 34d86116c2..48e12f23ef 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -69,9 +69,7 @@ class filespec /** @var upload Instance of upload class */ public $upload; - /** - * @var \phpbb\filesystem\filesystem_interface - */ + /** @var \phpbb\filesystem\filesystem_interface */ protected $filesystem; /** @var \phpbb\php\ini ini_get() wrapper class */ @@ -83,33 +81,27 @@ class filespec /** @var string phpBB root path */ protected $phpbb_root_path; - /** - * The plupload object - * @var \phpbb\plupload\plupload - */ + /** @var \phpbb\plupload\plupload The plupload object */ protected $plupload; - /** - * phpBB Mimetype guesser - * @var \phpbb\mimetype\guesser - */ + /** @var \phpbb\mimetype\guesser phpBB Mimetype guesser */ protected $mimetype_guesser; /** * File upload class * * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem Filesystem - * @param \phpbb\php\ini $php_ini ini_get() wrapper * @param language $language Language + * @param \phpbb\php\ini $php_ini ini_get() wrapper * @param string $phpbb_root_path phpBB root path * @param \phpbb\mimetype\guesser $mimetype_guesser Mime type guesser * @param \phpbb\plupload\plupload $plupload Plupload */ - public function __construct(\phpbb\filesystem\filesystem_interface $phpbb_filesystem, \phpbb\php\ini $php_ini, language $language, $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, \phpbb\php\ini $php_ini, $phpbb_root_path, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { $this->filesystem = $phpbb_filesystem; - $this->php_ini = $php_ini; $this->language = $language; + $this->php_ini = $php_ini; $this->phpbb_root_path = $phpbb_root_path; $this->plupload = $plupload; $this->mimetype_guesser = $mimetype_guesser; diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 35107ccb45..036f216d22 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -73,17 +73,17 @@ class upload * * @param filesystem_interface $filesystem * @param factory $factory Files factory - * @param \phpbb\php\ini $php_ini ini_get() wrapper * @param language $language Language class + * @param \phpbb\php\ini $php_ini ini_get() wrapper * @param request_interface $request Request class * @param string $phpbb_root_path phpBB root path */ - public function __construct(filesystem_interface $filesystem, factory $factory, \phpbb\php\ini $php_ini, language $language, request_interface $request, $phpbb_root_path) + public function __construct(filesystem_interface $filesystem, factory $factory, language $language, \phpbb\php\ini $php_ini, request_interface $request, $phpbb_root_path) { $this->filesystem = $filesystem; $this->factory = $factory; - $this->php_ini = $php_ini; $this->language = $language; + $this->php_ini = $php_ini; $this->request = $request; $this->phpbb_root_path = $phpbb_root_path; } -- cgit v1.2.1 From 16f3b8c2b9de388223cbe8ace9e1d9bcf0ba5e11 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 27 Aug 2015 10:51:10 +0200 Subject: [ticket/13904] Modify files for changes in ini wrapper PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 8 ++++---- phpBB/phpbb/files/upload.php | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 48e12f23ef..580016b281 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -72,7 +72,7 @@ class filespec /** @var \phpbb\filesystem\filesystem_interface */ protected $filesystem; - /** @var \phpbb\php\ini ini_get() wrapper class */ + /** @var \bantu\IniGetWrapper\IniGetWrapper ini_get() wrapper class */ protected $php_ini; /** @var language Language class */ @@ -92,12 +92,12 @@ class filespec * * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem Filesystem * @param language $language Language - * @param \phpbb\php\ini $php_ini ini_get() wrapper + * @param \bantu\IniGetWrapper\IniGetWrapper $php_ini ini_get() wrapper * @param string $phpbb_root_path phpBB root path * @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, \phpbb\php\ini $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, $phpbb_root_path, \phpbb\mimetype\guesser $mimetype_guesser = null, \phpbb\plupload\plupload $plupload = null) { $this->filesystem = $phpbb_filesystem; $this->language = $language; @@ -417,7 +417,7 @@ class filespec return false; } - $upload_mode = ($this->php_ini->get_bool('open_basedir') || $this->php_ini->get_bool('safe_mode')) ? 'move' : 'copy'; + $upload_mode = ($this->php_ini->getBool('open_basedir') || $this->php_ini->getBool('safe_mode')) ? 'move' : 'copy'; $upload_mode = ($this->local) ? 'local' : $upload_mode; $this->destination_file = $this->destination_path . '/' . utf8_basename($this->realname); diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 036f216d22..328dd49a06 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -56,7 +56,7 @@ class upload /** @var \phpbb\files\factory Files factory */ protected $factory; - /** @var \phpbb\php\ini ini_get() wrapper */ + /** @var \bantu\IniGetWrapper\IniGetWrapper ini_get() wrapper */ protected $php_ini; /** @var \phpbb\language\language Language class */ @@ -74,11 +74,11 @@ class upload * @param filesystem_interface $filesystem * @param factory $factory Files factory * @param language $language Language class - * @param \phpbb\php\ini $php_ini ini_get() wrapper + * @param \bantu\IniGetWrapper\IniGetWrapper $php_ini ini_get() wrapper * @param request_interface $request Request class * @param string $phpbb_root_path phpBB root path */ - public function __construct(filesystem_interface $filesystem, factory $factory, language $language, \phpbb\php\ini $php_ini, request_interface $request, $phpbb_root_path) + public function __construct(filesystem_interface $filesystem, factory $factory, language $language, \bantu\IniGetWrapper\IniGetWrapper $php_ini, request_interface $request, $phpbb_root_path) { $this->filesystem = $filesystem; $this->factory = $factory; @@ -216,7 +216,7 @@ class upload switch ($errorcode) { case UPLOAD_ERR_INI_SIZE: - $max_filesize = $this->php_ini->get_string('upload_max_filesize'); + $max_filesize = $this->php_ini->getString('upload_max_filesize'); $unit = 'MB'; if (!empty($max_filesize)) -- cgit v1.2.1 From b29b62debe4077ec688ebee9e9363db2c8614dd5 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 27 Aug 2015 12:48:06 +0200 Subject: [ticket/13904] Use ini_get() wrapper in file upload types PHPBB3-13904 --- phpBB/phpbb/files/types/base.php | 5 ++++- phpBB/phpbb/files/types/form.php | 10 ++++++++-- phpBB/phpbb/files/types/local.php | 26 +++++++++++--------------- phpBB/phpbb/files/types/remote.php | 12 +++++++++--- 4 files changed, 32 insertions(+), 21 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/base.php b/phpBB/phpbb/files/types/base.php index 1ed06053a4..3313ad040b 100644 --- a/phpBB/phpbb/files/types/base.php +++ b/phpBB/phpbb/files/types/base.php @@ -18,6 +18,9 @@ abstract class base implements type_interface /** @var \phpbb\language\language */ protected $language; + /** @var \bantu\IniGetWrapper\IniGetWrapper */ + protected $php_ini; + /** @var \phpbb\files\upload */ protected $upload; @@ -33,7 +36,7 @@ abstract class base implements type_interface // PHP Upload filesize exceeded if ($file->get('filename') == 'none') { - $max_filesize = @ini_get('upload_max_filesize'); + $max_filesize = $this->php_ini->getString('upload_max_filesize'); $unit = 'MB'; if (!empty($max_filesize)) diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index be0a425d57..b8ad62f58b 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -13,6 +13,7 @@ namespace phpbb\files\types; +use \bantu\IniGetWrapper\IniGetWrapper; use \phpbb\files\factory; use \phpbb\files\filespec; use \phpbb\files\upload; @@ -28,6 +29,9 @@ class form extends base /** @var language */ protected $language; + /** @var IniGetWrapper */ + protected $php_ini; + /** @var plupload */ protected $plupload; @@ -42,13 +46,15 @@ class form extends base * * @param factory $factory Files factory * @param language $language Language class + * @param IniGetWrapper $php_ini ini_get() wrapper * @param plupload $plupload Plupload * @param request_interface $request Request object */ - public function __construct(factory $factory, language $language, plupload $plupload, request_interface $request) + public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, plupload $plupload, request_interface $request) { $this->factory = $factory; $this->language = $language; + $this->php_ini = $php_ini; $this->plupload = $plupload; $this->request = $request; } @@ -122,7 +128,7 @@ class form extends base return $file; } - // PHP Upload filesize check + // PHP Upload file size check $file = $this->check_upload_size($file); if (sizeof($file->error)) { diff --git a/phpBB/phpbb/files/types/local.php b/phpBB/phpbb/files/types/local.php index dfaabded9b..96bfdaca7f 100644 --- a/phpBB/phpbb/files/types/local.php +++ b/phpBB/phpbb/files/types/local.php @@ -13,6 +13,7 @@ namespace phpbb\files\types; +use \bantu\IniGetWrapper\IniGetWrapper; use \phpbb\files\factory; use \phpbb\files\filespec; use \phpbb\language\language; @@ -26,6 +27,9 @@ class local extends base /** @var language */ protected $language; + /** @var IniGetWrapper */ + protected $php_ini; + /** @var request_interface */ protected $request; @@ -37,12 +41,14 @@ class local extends base * * @param factory $factory Files factory * @param language $language Language class + * @param IniGetWrapper $php_ini ini_get() wrapper * @param request_interface $request Request object */ - public function __construct(factory $factory, language $language, request_interface $request) + public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, request_interface $request) { $this->factory = $factory; $this->language = $language; + $this->php_ini = $php_ini; $this->request = $request; } @@ -89,21 +95,11 @@ class local extends base } } - // PHP Upload filesize exceeded - if ($file->get('filename') == 'none') + // PHP Upload file size check + $this->check_upload_size($file); + $file = $this->check_upload_size($file); + if (sizeof($file->error)) { - $max_filesize = @ini_get('upload_max_filesize'); - $unit = 'MB'; - - if (!empty($max_filesize)) - { - $unit = strtolower(substr($max_filesize, -1, 1)); - $max_filesize = (int) $max_filesize; - - $unit = ($unit == 'k') ? 'KB' : (($unit == 'g') ? 'GB' : 'MB'); - } - - $file->error[] = (empty($max_filesize)) ?$this->language->lang($this->upload->error_prefix . 'PHP_SIZE_NA') : $this->language->lang($this->upload->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit)); return $file; } diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 94bba8ad46..bad87243a5 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -13,6 +13,7 @@ namespace phpbb\files\types; +use \bantu\IniGetWrapper\IniGetWrapper; use \phpbb\files\factory; use \phpbb\files\filespec; use \phpbb\language\language; @@ -26,6 +27,9 @@ class remote extends base /** @var language */ protected $language; + /** @var IniGetWrapper */ + protected $php_ini; + /** @var request_interface */ protected $request; @@ -40,13 +44,15 @@ class remote extends base * * @param factory $factory Files factory * @param language $language Language class + * @param IniGetWrapper $php_ini ini_get() wrapper * @param request_interface $request Request object * @param string $phpbb_root_path phpBB root path */ - public function __construct(factory $factory, language $language, request_interface $request, $phpbb_root_path) + public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, request_interface $request, $phpbb_root_path) { $this->factory = $factory; $this->language = $language; + $this->php_ini = $php_ini; $this->request = $request; $this->phpbb_root_path = $phpbb_root_path; } @@ -199,7 +205,7 @@ class remote extends base return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'EMPTY_REMOTE_DATA'); } - $tmp_path = (!@ini_get('safe_mode') || strtolower(@ini_get('safe_mode')) == 'off') ? false : $this->phpbb_root_path . 'cache'; + $tmp_path = (!$this->php_ini->getBool('safe_mode')) ? false : $this->phpbb_root_path . 'cache'; $filename = tempnam($tmp_path, unique_id() . '-'); if (!($fp = @fopen($filename, 'wb'))) @@ -232,7 +238,7 @@ class remote extends base $max_file_size = $this->upload->max_filesize; if (!$max_file_size) { - $max_file_size = @ini_get('upload_max_filesize'); + $max_file_size = $this->php_ini->getString('upload_max_filesize'); if (!empty($max_filesize)) { -- cgit v1.2.1 From 591995267a3f1931131fa630bd3ff120476f881f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 3 Sep 2015 17:20:54 +0200 Subject: [ticket/13904] Improve test coverage of filespec class PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'phpBB/phpbb/files') 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) { -- cgit v1.2.1 From 1b9e6e352fd4a78e195d22a7dad6cd4f8f11d97c Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Sep 2015 11:34:42 +0200 Subject: [ticket/13904] Improve test coverage of form upload type PHPBB3-13904 --- phpBB/phpbb/files/types/form.php | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index b8ad62f58b..1662e0e920 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -68,16 +68,6 @@ class form extends base return $this->form_upload($args[0]); } - /** - * {@inheritdoc} - */ - public function set_upload(upload $upload) - { - $this->upload = $upload; - - return $this; - } - /** * Form upload method * Upload file from users harddisk -- cgit v1.2.1 From 82bca32015b94b140b7c810fc5d7e721e8ee341f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Sep 2015 11:35:12 +0200 Subject: [ticket/13904] Improve test coverage of remote upload type PHPBB3-13904 --- phpBB/phpbb/files/types/remote.php | 5 ----- 1 file changed, 5 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index bad87243a5..b5a0dade11 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -84,11 +84,6 @@ class remote extends base return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'URL_INVALID')); } - if (empty($match[2])) - { - return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'URL_INVALID')); - } - $url = parse_url($upload_url); $host = $url['host']; -- cgit v1.2.1 From 7a92ad596c56c25728fd6a22c3a817504e8cb347 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Sep 2015 12:19:48 +0200 Subject: [ticket/13904] Minor coding style fixes PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 1 + phpBB/phpbb/files/types/form.php | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 6ce54a4789..83007f11c0 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -96,6 +96,7 @@ class filespec * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem Filesystem * @param language $language Language * @param \bantu\IniGetWrapper\IniGetWrapper $php_ini ini_get() wrapper + * @param \fastImageSize\fastImageSize $imagesize Imagesize class * @param string $phpbb_root_path phpBB root path * @param \phpbb\mimetype\guesser $mimetype_guesser Mime type guesser * @param \phpbb\plupload\plupload $plupload Plupload diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index 1662e0e920..4cf340ee7d 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -16,7 +16,6 @@ namespace phpbb\files\types; use \bantu\IniGetWrapper\IniGetWrapper; use \phpbb\files\factory; use \phpbb\files\filespec; -use \phpbb\files\upload; use \phpbb\language\language; use \phpbb\plupload\plupload; use \phpbb\request\request_interface; @@ -38,7 +37,7 @@ class form extends base /** @var request_interface */ protected $request; - /** @var upload */ + /** @var \phpbb\files\upload */ protected $upload; /** -- cgit v1.2.1 From 00e5ff9e2e9c0dd7325f33a22888d6888af4b197 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Sep 2015 22:51:44 +0200 Subject: [ticket/13904] Add unit tests for local upload type PHPBB3-13904 --- phpBB/phpbb/files/types/local.php | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/local.php b/phpBB/phpbb/files/types/local.php index 96bfdaca7f..ff11890856 100644 --- a/phpBB/phpbb/files/types/local.php +++ b/phpBB/phpbb/files/types/local.php @@ -84,19 +84,7 @@ class local extends base return $file; } - if (isset($upload['error'])) - { - $error = $this->upload->assign_internal_error($upload['error']); - - if ($error !== false) - { - $file->error[] = $error; - return $file; - } - } - // PHP Upload file size check - $this->check_upload_size($file); $file = $this->check_upload_size($file); if (sizeof($file->error)) { -- cgit v1.2.1 From e60c8a5a8bfa1aa500c096ea90cd32fda9465f9a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Sep 2015 22:52:20 +0200 Subject: [ticket/13904] Improve code coverage PHPBB3-13904 --- phpBB/phpbb/files/types/remote.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index b5a0dade11..34ee97f1c9 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -235,10 +235,10 @@ class remote extends base { $max_file_size = $this->php_ini->getString('upload_max_filesize'); - if (!empty($max_filesize)) + if (!empty($max_file_size)) { $unit = strtolower(substr($max_file_size, -1, 1)); - $max_file_size = (int) $max_filesize; + $max_file_size = (int) $max_file_size; switch ($unit) { -- cgit v1.2.1 From 327e36a4d68ff9607967af52ef8f6a00c60343ff Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 9 Sep 2015 09:41:40 +0200 Subject: [ticket/13904] Modify files for updated fast-image-size library PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 83007f11c0..e171e7e68f 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -75,7 +75,7 @@ class filespec /** @var \bantu\IniGetWrapper\IniGetWrapper ini_get() wrapper class */ protected $php_ini; - /** @var \fastImageSize\fastImageSize */ + /** @var \FastImageSize\FastImageSize */ protected $imagesize; /** @var language Language class */ @@ -96,12 +96,12 @@ class filespec * @param \phpbb\filesystem\filesystem_interface $phpbb_filesystem Filesystem * @param language $language Language * @param \bantu\IniGetWrapper\IniGetWrapper $php_ini ini_get() wrapper - * @param \fastImageSize\fastImageSize $imagesize Imagesize class + * @param \FastImageSize\FastImageSize $imagesize Imagesize class * @param string $phpbb_root_path phpBB root path * @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, \fastImageSize\fastImageSize $imagesize, $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; -- cgit v1.2.1 From 70ad0c6a8f7d16b767aa78cde2acc9a3b3512e6f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 9 Sep 2015 10:43:12 +0200 Subject: [ticket/13904] Add language entries for error messages in upload class PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 328dd49a06..bd379924e7 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -245,15 +245,12 @@ class upload break; case UPLOAD_ERR_NO_TMP_DIR: - $error = 'Temporary folder could not be found. Please check your PHP installation.'; - break; - case UPLOAD_ERR_CANT_WRITE: - $error = 'Can’t write to temporary folder.'; + $error = $this->language->lang($this->error_prefix . 'NO_TEMP_DIR'); break; case UPLOAD_ERR_EXTENSION: - $error = 'A PHP extension has stopped the file upload.'; + $error = $this->language->lang($this->error_prefix . 'PHP_UPLOAD_STOPPED'); break; default: -- cgit v1.2.1 From 5f91f1cad85eaf7f8dc62a1a140605a46431496f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 9 Sep 2015 10:46:14 +0200 Subject: [ticket/13904] Minor coding style fixes PHPBB3-13904 --- phpBB/phpbb/files/factory.php | 2 +- phpBB/phpbb/files/filespec.php | 4 +--- phpBB/phpbb/files/types/form.php | 12 ++++++------ phpBB/phpbb/files/types/local.php | 10 +++++----- phpBB/phpbb/files/types/remote.php | 10 +++++----- phpBB/phpbb/files/types/type_interface.php | 2 +- phpBB/phpbb/files/upload.php | 6 +++--- 7 files changed, 22 insertions(+), 24 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/factory.php b/phpBB/phpbb/files/factory.php index 508c50c6ce..84b7cc9449 100644 --- a/phpBB/phpbb/files/factory.php +++ b/phpBB/phpbb/files/factory.php @@ -42,7 +42,7 @@ class factory { $service = false; - $name = (strpos($name, 'files.') === false) ? 'files.' . $name : $name; + $name = (strpos($name, '.') === false) ? 'files.' . $name : $name; try { diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index e171e7e68f..7a42e6bd50 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -13,7 +13,7 @@ namespace phpbb\files; -use \phpbb\language\language; +use phpbb\language\language; /** * Responsible for holding all file relevant information, as well as doing file-specific operations. @@ -201,8 +201,6 @@ class filespec * character. Unique creates a unique filename. * @param string $prefix Prefix applied to filename * @param string $user_id The user_id is only needed for when cleaning a user's avatar - * - *@access public */ public function clean_filename($mode = 'unique', $prefix = '', $user_id = '') { diff --git a/phpBB/phpbb/files/types/form.php b/phpBB/phpbb/files/types/form.php index 4cf340ee7d..832f090c47 100644 --- a/phpBB/phpbb/files/types/form.php +++ b/phpBB/phpbb/files/types/form.php @@ -13,12 +13,12 @@ namespace phpbb\files\types; -use \bantu\IniGetWrapper\IniGetWrapper; -use \phpbb\files\factory; -use \phpbb\files\filespec; -use \phpbb\language\language; -use \phpbb\plupload\plupload; -use \phpbb\request\request_interface; +use bantu\IniGetWrapper\IniGetWrapper; +use phpbb\files\factory; +use phpbb\files\filespec; +use phpbb\language\language; +use phpbb\plupload\plupload; +use phpbb\request\request_interface; class form extends base { diff --git a/phpBB/phpbb/files/types/local.php b/phpBB/phpbb/files/types/local.php index ff11890856..7e9210b196 100644 --- a/phpBB/phpbb/files/types/local.php +++ b/phpBB/phpbb/files/types/local.php @@ -13,11 +13,11 @@ namespace phpbb\files\types; -use \bantu\IniGetWrapper\IniGetWrapper; -use \phpbb\files\factory; -use \phpbb\files\filespec; -use \phpbb\language\language; -use \phpbb\request\request_interface; +use bantu\IniGetWrapper\IniGetWrapper; +use phpbb\files\factory; +use phpbb\files\filespec; +use phpbb\language\language; +use phpbb\request\request_interface; class local extends base { diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 34ee97f1c9..44feab0ece 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -13,11 +13,11 @@ namespace phpbb\files\types; -use \bantu\IniGetWrapper\IniGetWrapper; -use \phpbb\files\factory; -use \phpbb\files\filespec; -use \phpbb\language\language; -use \phpbb\request\request_interface; +use bantu\IniGetWrapper\IniGetWrapper; +use phpbb\files\factory; +use phpbb\files\filespec; +use phpbb\language\language; +use phpbb\request\request_interface; class remote extends base { diff --git a/phpBB/phpbb/files/types/type_interface.php b/phpBB/phpbb/files/types/type_interface.php index adfbb75293..e07078349a 100644 --- a/phpBB/phpbb/files/types/type_interface.php +++ b/phpBB/phpbb/files/types/type_interface.php @@ -13,7 +13,7 @@ namespace phpbb\files\types; -use \phpbb\files\upload; +use phpbb\files\upload; interface type_interface { diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index bd379924e7..45ff1c372f 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -13,9 +13,9 @@ namespace phpbb\files; -use \phpbb\filesystem\filesystem_interface; -use \phpbb\language\language; -use \phpbb\request\request_interface; +use phpbb\filesystem\filesystem_interface; +use phpbb\language\language; +use phpbb\request\request_interface; /** * File upload class -- cgit v1.2.1 From 40e614f56436447dffd272351e23b79c2da9fa3f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 9 Sep 2015 12:58:22 +0200 Subject: [ticket/13904] Fix tests after changes to factory PHPBB3-13904 --- phpBB/phpbb/files/upload.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index 45ff1c372f..e011e714e5 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -197,7 +197,7 @@ class upload { $args = func_get_args(); array_shift($args); - $type_class = $this->factory->get('types.' . $type) + $type_class = $this->factory->get($type) ->set_upload($this); return (is_object($type_class)) ? call_user_func_array(array($type_class, 'upload'), $args) : false; -- cgit v1.2.1 From 759dc9bb84d712c11148a9689d294c09aa0f81d4 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 13 Sep 2015 09:30:56 +0200 Subject: [ticket/13904] Set properties to protected where possible in filespec PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 7a42e6bd50..7fc9e923ea 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -34,10 +34,10 @@ class filespec protected $mimetype = ''; /** @var string File extension */ - public $extension = ''; + protected $extension = ''; /** @var int File size */ - public $filesize = 0; + protected $filesize = 0; /** @var int Width of file */ protected $width = 0; @@ -55,10 +55,10 @@ class filespec protected $destination_path = ''; /** @var bool Whether file was moved */ - public $file_moved = false; + protected $file_moved = false; /** @var bool Whether file is local */ - public $local = false; + protected $local = false; /** @var bool Class initialization flag */ protected $class_initialized = false; -- cgit v1.2.1 From 6651c1d8f4aead15f4750989670f75721390ee21 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 13 Sep 2015 09:31:23 +0200 Subject: [ticket/13904] Use filespec's get_filesize instead of calling filesize() PHPBB3-13904 --- phpBB/phpbb/files/filespec.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 7fc9e923ea..2ff2a92c83 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -145,7 +145,7 @@ class filespec $this->extension = strtolower(self::get_extension($this->realname)); // Try to get real filesize from temporary folder (not always working) ;) - $this->filesize = (@filesize($this->filename)) ? @filesize($this->filename) : $this->filesize; + $this->filesize = ($this->get_filesize($this->filename)) ?: $this->filesize; $this->width = $this->height = 0; $this->file_moved = false; @@ -494,7 +494,7 @@ class filespec } // Try to get real filesize from destination folder - $this->filesize = (@filesize($this->destination_file)) ? @filesize($this->destination_file) : $this->filesize; + $this->filesize = ($this->get_filesize($this->destination_file)) ?: $this->filesize; // Get mimetype of supplied file $this->mimetype = $this->get_mimetype($this->destination_file); -- cgit v1.2.1 From a92083169a181a6524e383de657c78af1bff2887 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Thu, 5 Nov 2015 13:52:04 -0800 Subject: [ticket/14273] Remove unused core.root_path dependency in files.upload service PHPBB3-14273 --- phpBB/phpbb/files/upload.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/upload.php b/phpBB/phpbb/files/upload.php index e011e714e5..a9bf74094d 100644 --- a/phpBB/phpbb/files/upload.php +++ b/phpBB/phpbb/files/upload.php @@ -65,9 +65,6 @@ class upload /** @var request_interface Request class */ protected $request; - /** @var string phpBB root path */ - protected $phpbb_root_path; - /** * Init file upload class. * @@ -76,16 +73,14 @@ class upload * @param language $language Language class * @param \bantu\IniGetWrapper\IniGetWrapper $php_ini ini_get() wrapper * @param request_interface $request Request class - * @param string $phpbb_root_path phpBB root path */ - public function __construct(filesystem_interface $filesystem, factory $factory, language $language, \bantu\IniGetWrapper\IniGetWrapper $php_ini, request_interface $request, $phpbb_root_path) + public function __construct(filesystem_interface $filesystem, factory $factory, language $language, \bantu\IniGetWrapper\IniGetWrapper $php_ini, request_interface $request) { $this->filesystem = $filesystem; $this->factory = $factory; $this->language = $language; $this->php_ini = $php_ini; $this->request = $request; - $this->phpbb_root_path = $phpbb_root_path; } /** -- cgit v1.2.1 From 59481da9dac23ccb8382202eb6623c3fc71030a0 Mon Sep 17 00:00:00 2001 From: Erwan Nader Date: Sun, 3 Jan 2016 11:29:53 +0100 Subject: [ticket/14386] Use of sys_get_temp_dir() PHPBB3-14386 --- phpBB/phpbb/files/types/remote.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 44feab0ece..4f4ce15c18 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -200,8 +200,7 @@ class remote extends base return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'EMPTY_REMOTE_DATA'); } - $tmp_path = (!$this->php_ini->getBool('safe_mode')) ? false : $this->phpbb_root_path . 'cache'; - $filename = tempnam($tmp_path, unique_id() . '-'); + $filename = tempnam(sys_get_temp_dir(), unique_id() . '-'); if (!($fp = @fopen($filename, 'wb'))) { -- cgit v1.2.1 From 73e6e5b77faadbb7676961bf38122d669de111db Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 3 Dec 2015 17:50:29 +0100 Subject: [ticket/13454] Remove unused variables This is the first part of the changes. More to come. PHPBB3-13454 --- phpBB/phpbb/files/types/remote.php | 1 - 1 file changed, 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 4f4ce15c18..33cbfb00ae 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -97,7 +97,6 @@ class remote extends base $url['path'] = implode('', $url['path']); $upload_ary['name'] = utf8_basename($url['path']) . (($ext) ? '.' . $ext : ''); - $filename = $url['path']; $filesize = 0; $remote_max_filesize = $this->get_max_file_size(); -- cgit v1.2.1 From cec63974c393b71770eae7d740e136b43ed1c78f Mon Sep 17 00:00:00 2001 From: Scott Dutton Date: Mon, 25 Jan 2016 21:45:43 +0000 Subject: [ticket/14431] Remote avatar uploading replace the 3.1 way with guzzle Adds guzzle as a dependency Removed a test as it wont wont with guzzle (as far as I know) PHPBB3-14431 --- phpBB/phpbb/files/types/remote.php | 122 ++++++------------------------------- 1 file changed, 19 insertions(+), 103 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index d311face98..e990149501 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -86,19 +86,6 @@ class remote extends base $url = parse_url($upload_url); - $default_port = 80; - $hostname = $url['host']; - - if ($url['scheme'] == 'https') - { - $default_port = 443; - $hostname = 'tls://' . $url['host']; - } - - $host = $url['host']; - $path = $url['path']; - $port = (!empty($url['port'])) ? (int) $url['port'] : $default_port; - $upload_ary['type'] = 'application/octet-stream'; $url['path'] = explode('.', $url['path']); @@ -110,103 +97,32 @@ class remote extends base $remote_max_filesize = $this->get_max_file_size(); - $errno = 0; - $errstr = ''; - - if (!($fsock = @fsockopen($hostname, $port, $errno, $errstr))) - { + $client = new \Guzzle\Http\Client([ + 'timeout' => $this->upload->upload_timeout, + 'connect_timeout' => $this->upload->upload_timeout, + ]); + + try { + $response = $client->get($upload_url)->send(); + } catch (\Guzzle\Http\Exception\ClientErrorResponseException $responseException) { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'URL_NOT_FOUND'); + } catch (\Guzzle\Http\Exception\CurlException $curlException) { + //curl exceptions are when the DNS fails etc + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); + } catch (\Guzzle\Http\Exception\RequestException $requestException) { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'REMOTE_UPLOAD_TIMEOUT'); + } catch (\Exception $e) { return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); } - // Make sure $path not beginning with / - if (strpos($path, '/') === 0) - { - $path = substr($path, 1); - } - - fputs($fsock, 'GET /' . $path . " HTTP/1.1\r\n"); - fputs($fsock, "HOST: " . $host . "\r\n"); - fputs($fsock, "Connection: close\r\n\r\n"); - - // Set a proper timeout for the socket - socket_set_timeout($fsock, $this->upload->upload_timeout); - - $get_info = false; - $data = ''; - $length = false; - $timer_stop = time() + $this->upload->upload_timeout; - - while ((!$length || $filesize < $length) && !@feof($fsock)) + if ($remote_max_filesize && $response->getContentType() > $remote_max_filesize) { - if ($get_info) - { - if ($length) - { - // Don't attempt to read past end of file if server indicated length - $block = @fread($fsock, min($length - $filesize, 1024)); - } - else - { - $block = @fread($fsock, 1024); - } + $max_filesize = get_formatted_filesize($remote_max_filesize, false); - $filesize += strlen($block); - - if ($remote_max_filesize && $filesize > $remote_max_filesize) - { - $max_filesize = get_formatted_filesize($remote_max_filesize, false); - - return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); - } - - $data .= $block; - } - else - { - $line = @fgets($fsock, 1024); - - if ($line == "\r\n") - { - $get_info = true; - } - else - { - if (stripos($line, 'content-type: ') !== false) - { - $upload_ary['type'] = rtrim(str_replace('content-type: ', '', strtolower($line))); - } - else if ($this->upload->max_filesize && stripos($line, 'content-length: ') !== false) - { - $length = (int) str_replace('content-length: ', '', strtolower($line)); - - if ($remote_max_filesize && $length && $length > $remote_max_filesize) - { - $max_filesize = get_formatted_filesize($remote_max_filesize, false); - - return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); - } - } - else if (stripos($line, '404 not found') !== false) - { - return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'URL_NOT_FOUND'); - } - } - } - - $stream_meta_data = stream_get_meta_data($fsock); - - // Cancel upload if we exceed timeout - if (!empty($stream_meta_data['timed_out']) || time() >= $timer_stop) - { - return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'REMOTE_UPLOAD_TIMEOUT'); - } + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); } - @fclose($fsock); - if (empty($data)) - { - return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'EMPTY_REMOTE_DATA'); - } + $data = $response->getBody(); $filename = tempnam(sys_get_temp_dir(), unique_id() . '-'); -- cgit v1.2.1 From 49dd9f021924551bf0cfc5db3962ddb50c9e98a2 Mon Sep 17 00:00:00 2001 From: Scott Dutton Date: Wed, 3 Feb 2016 05:45:24 +0000 Subject: [ticket/14431] Remote avatar uploading Fixed content length bug Ran composer update PHPBB3-14431 --- phpBB/phpbb/files/types/remote.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index e990149501..92e0e3b9bc 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -115,13 +115,19 @@ class remote extends base return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); } - if ($remote_max_filesize && $response->getContentType() > $remote_max_filesize) + $content_length = $response->getContentLength(); + if ($remote_max_filesize && $content_length > $remote_max_filesize) { $max_filesize = get_formatted_filesize($remote_max_filesize, false); return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'WRONG_FILESIZE', $max_filesize['value'], $max_filesize['unit'])); } + if ($content_length == 0) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'EMPTY_REMOTE_DATA'); + } + $data = $response->getBody(); $filename = tempnam(sys_get_temp_dir(), unique_id() . '-'); -- cgit v1.2.1 From c83db45f55858c78648a6650aec39a9fc627a50b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 3 Feb 2016 14:06:10 +0100 Subject: [ticket/14448] Use GuzzleHttp and try to verify certs PHPBB3-14448 --- phpBB/phpbb/files/types/remote.php | 48 ++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 12 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 92e0e3b9bc..f4a4fa70d1 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -93,29 +93,53 @@ class remote extends base $url['path'] = implode('', $url['path']); $upload_ary['name'] = utf8_basename($url['path']) . (($ext) ? '.' . $ext : ''); - $filesize = 0; $remote_max_filesize = $this->get_max_file_size(); - $client = new \Guzzle\Http\Client([ + $guzzle_options = [ 'timeout' => $this->upload->upload_timeout, 'connect_timeout' => $this->upload->upload_timeout, - ]); + ]; + $client = new \GuzzleHttp\Client($guzzle_options); - try { - $response = $client->get($upload_url)->send(); - } catch (\Guzzle\Http\Exception\ClientErrorResponseException $responseException) { + try + { + $response = $client->get($upload_url, $guzzle_options); + } + catch (\GuzzleHttp\Exception\ClientException $clientException) + { return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'URL_NOT_FOUND'); - } catch (\Guzzle\Http\Exception\CurlException $curlException) { - //curl exceptions are when the DNS fails etc - return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); - } catch (\Guzzle\Http\Exception\RequestException $requestException) { - return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'REMOTE_UPLOAD_TIMEOUT'); + } + catch (\GuzzleHttp\Exception\RequestException $requestException) + { + if (strpos($requestException->getMessage(), 'cURL error 28') !== false || preg_match('/408|504/', $requestException->getCode())) + { + return $this->factory->get('filespec')->set_error($this->upload->error_prefix . 'REMOTE_UPLOAD_TIMEOUT'); + } + else + { + if (strpos($requestException->getMessage(), 'cURL error 60') !== false) + { + // Work around non existent CA file + try + { + $response = $client->get($upload_url, array_merge($guzzle_options, ['verify' => false])); + } + catch (\GuzzleHttp\Exception\RequestException $requestException) + { + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); + } + } + else + { + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); + } + } } catch (\Exception $e) { return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); } - $content_length = $response->getContentLength(); + $content_length = $response->getBody()->getSize(); if ($remote_max_filesize && $content_length > $remote_max_filesize) { $max_filesize = get_formatted_filesize($remote_max_filesize, false); -- cgit v1.2.1 From fd9c05309d186332728b533467aaecad5c543c52 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 4 Feb 2016 17:10:59 +0100 Subject: [ticket/14448] Let user decide if remote upload certs should be checked Also fixed some minor issues like coding style. PHPBB3-14448 --- phpBB/phpbb/files/types/remote.php | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index f4a4fa70d1..7e5157baa9 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -14,6 +14,7 @@ namespace phpbb\files\types; use bantu\IniGetWrapper\IniGetWrapper; +use phpbb\config\config; use phpbb\files\factory; use phpbb\files\filespec; use phpbb\language\language; @@ -21,6 +22,9 @@ use phpbb\request\request_interface; class remote extends base { + /** @var config phpBB config */ + protected $config; + /** @var factory Files factory */ protected $factory; @@ -42,14 +46,16 @@ class remote extends base /** * Construct a form upload type * + * @param config $config phpBB config * @param factory $factory Files factory * @param language $language Language class * @param IniGetWrapper $php_ini ini_get() wrapper * @param request_interface $request Request object * @param string $phpbb_root_path phpBB root path */ - public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, request_interface $request, $phpbb_root_path) + public function __construct(config $config, factory $factory, language $language, IniGetWrapper $php_ini, request_interface $request, $phpbb_root_path) { + $this->config = $config; $this->factory = $factory; $this->language = $language; $this->php_ini = $php_ini; @@ -97,8 +103,9 @@ class remote extends base $remote_max_filesize = $this->get_max_file_size(); $guzzle_options = [ - 'timeout' => $this->upload->upload_timeout, - 'connect_timeout' => $this->upload->upload_timeout, + 'timeout' => $this->upload->upload_timeout, + 'connect_timeout' => $this->upload->upload_timeout, + 'verify' => !empty($this->config['remote_upload_verify']), ]; $client = new \GuzzleHttp\Client($guzzle_options); @@ -118,24 +125,11 @@ class remote extends base } else { - if (strpos($requestException->getMessage(), 'cURL error 60') !== false) - { - // Work around non existent CA file - try - { - $response = $client->get($upload_url, array_merge($guzzle_options, ['verify' => false])); - } - catch (\GuzzleHttp\Exception\RequestException $requestException) - { - return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); - } - } - else - { - return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); - } + return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); } - } catch (\Exception $e) { + } + catch (\Exception $e) + { return $this->factory->get('filespec')->set_error($this->language->lang($this->upload->error_prefix . 'NOT_UPLOADED')); } -- cgit v1.2.1 From 8cf086ef9b200f0b59348c0f0d4946f9ebc4adae Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 8 Feb 2016 13:26:19 +0100 Subject: [ticket/14448] Correctly pass verify setting if available PHPBB3-14448 --- phpBB/phpbb/files/types/remote.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/types/remote.php b/phpBB/phpbb/files/types/remote.php index 7e5157baa9..1fdba0ca32 100644 --- a/phpBB/phpbb/files/types/remote.php +++ b/phpBB/phpbb/files/types/remote.php @@ -105,7 +105,7 @@ class remote extends base $guzzle_options = [ 'timeout' => $this->upload->upload_timeout, 'connect_timeout' => $this->upload->upload_timeout, - 'verify' => !empty($this->config['remote_upload_verify']), + 'verify' => !empty($this->config['remote_upload_verify']) ? (bool) $this->config['remote_upload_verify'] : false, ]; $client = new \GuzzleHttp\Client($guzzle_options); -- cgit v1.2.1 From 1d5f5ccffcfd30a652734485b51066950bbb8a76 Mon Sep 17 00:00:00 2001 From: javiexin Date: Sun, 21 May 2017 12:58:05 +0200 Subject: [ticket/15227] Remove STRIP, as always false PHPBB3-15227 --- phpBB/phpbb/files/filespec.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/files') diff --git a/phpBB/phpbb/files/filespec.php b/phpBB/phpbb/files/filespec.php index 2ff2a92c83..f1a32ef4a8 100644 --- a/phpBB/phpbb/files/filespec.php +++ b/phpBB/phpbb/files/filespec.php @@ -129,7 +129,7 @@ class filespec $this->class_initialized = true; $this->filename = $upload_ary['tmp_name']; $this->filesize = $upload_ary['size']; - $name = (STRIP) ? stripslashes($upload_ary['name']) : $upload_ary['name']; + $name = $upload_ary['name']; $name = trim(utf8_basename($name)); $this->realname = $this->uploadname = $name; $this->mimetype = $upload_ary['type']; -- cgit v1.2.1