[ticket/16955] Clean up filesystem and files classes

PHPBB3-16955

phpBB/phpbb/file_downloader.php

@@ -77,7 +77,7 @@ class file_downloader
 
 				$stream_meta_data = stream_get_meta_data($socket);
 
-				if (!empty($stream_meta_data['timed_out']) || time() >= $timer_stop)
+				if ($stream_meta_data['timed_out'] || time() >= $timer_stop)
 				{
 					throw new \phpbb\exception\runtime_exception('FSOCK_TIMEOUT');
 				}

phpBB/phpbb/files/factory.php

@@ -46,7 +46,7 @@ class factory
 
 		try
 		{
-			$service = $this->container->get($name);
+			$service = $this->container->get($name) ?? false;
 		}
 		catch (\Exception $e)
 		{

phpBB/phpbb/files/filespec.php

@@ -329,7 +329,7 @@ class filespec
 	 * Get mime type
 	 *
 	 * @param string $filename Filename that needs to be checked
-	 * @return string Mime type of supplied filename
+	 * @return string|false Mime type of supplied filename or false if mimetype could not be guessed
 	 */
 	public function get_mimetype($filename)
 	{

phpBB/phpbb/files/filespec_storage.php

@@ -99,7 +99,7 @@ class filespec_storage
 	 *
 	 * @param array $upload_ary Upload ary
 	 *
-	 * @return filespec This instance of the filespec class
+	 * @return filespec_storage This instance of the filespec class
 	 */
 	public function set_upload_ary($upload_ary)
 	{
@@ -142,7 +142,7 @@ class filespec_storage
 	 *
 	 * @param upload $namespace Instance of upload class
 	 *
-	 * @return filespec This instance of the filespec class
+	 * @return filespec_storage This instance of the filespec class
 	 */
 	public function set_upload_namespace($namespace)
 	{
@@ -166,7 +166,7 @@ class filespec_storage
 	 *
 	 * @param mixed $error Content for error array
 	 *
-	 * @return \phpbb\files\filespec This instance of the filespec class
+	 * @return filespec_storage This instance of the filespec class
 	 */
 	public function set_error($error)
 	{
@@ -313,7 +313,7 @@ class filespec_storage
 	 * Get mime type
 	 *
 	 * @param string $filename Filename that needs to be checked
-	 * @return string Mime type of supplied filename
+	 * @return string|false Mime type of supplied filename or false if mimetype could not be guessed
 	 */
 	public function get_mimetype($filename)
 	{

phpBB/phpbb/files/types/base.php

@@ -27,9 +27,10 @@ abstract class base implements type_interface
 	/**
 	 * Check if upload exceeds maximum file size
 	 *
-	 * @param \phpbb\files\filespec $file Filespec object
+	 * @template filespec_type of \phpbb\files\filespec|\phpbb\files\filespec_storage
+	 * @param filespec_type $file Filespec object
 	 *
-	 * @return \phpbb\files\filespec Returns same filespec instance
+	 * @return filespec_type Returns same filespec instance
 	 */
 	public function check_upload_size($file)
 	{
@@ -47,7 +48,14 @@ abstract class base implements type_interface
 				$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));
+			if (empty($max_filesize))
+			{
+				$file->error[] = $this->language->lang($this->upload->error_prefix . 'PHP_SIZE_NA');
+			}
+			else
+			{
+				$file->error[] = $this->language->lang($this->upload->error_prefix . 'PHP_SIZE_OVERRUN', $max_filesize, $this->language->lang($unit));
+			}
 		}
 
 		return $file;

phpBB/phpbb/files/types/form.php

@@ -18,7 +18,7 @@ use phpbb\files\factory;
 use phpbb\files\filespec;
 use phpbb\language\language;
 use phpbb\plupload\plupload;
-use phpbb\request\request_interface;
+use phpbb\request\request;
 
 class form extends base
 {
@@ -28,7 +28,7 @@ class form extends base
 	/** @var plupload */
 	protected $plupload;
 
-	/** @var request_interface */
+	/** @var request */
 	protected $request;
 
 	/**
@@ -38,9 +38,9 @@ class form extends base
 	 * @param language			$language	Language class
 	 * @param IniGetWrapper		$php_ini	ini_get() wrapper
 	 * @param plupload			$plupload	Plupload
-	 * @param request_interface	$request	Request object
+	 * @param request	$request	Request object
 	 */
-	public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, plupload $plupload, request_interface $request)
+	public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, plupload $plupload, request $request)
 	{
 		$this->factory = $factory;
 		$this->language = $language;

phpBB/phpbb/files/types/form_storage.php

@@ -18,7 +18,7 @@ use phpbb\files\factory;
 use phpbb\files\filespec;
 use phpbb\language\language;
 use phpbb\plupload\plupload;
-use phpbb\request\request_interface;
+use phpbb\request\request;
 
 class form_storage extends base
 {
@@ -28,7 +28,7 @@ class form_storage extends base
 	/** @var plupload */
 	protected $plupload;
 
-	/** @var request_interface */
+	/** @var request */
 	protected $request;
 
 	/**
@@ -38,9 +38,9 @@ class form_storage extends base
 	 * @param language			$language	Language class
 	 * @param IniGetWrapper		$php_ini	ini_get() wrapper
 	 * @param plupload			$plupload	Plupload
-	 * @param request_interface	$request	Request object
+	 * @param request			$request	Request object
 	 */
-	public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, plupload $plupload, request_interface $request)
+	public function __construct(factory $factory, language $language, IniGetWrapper $php_ini, plupload $plupload, request $request)
 	{
 		$this->factory = $factory;
 		$this->language = $language;

phpBB/phpbb/files/types/local.php

@@ -65,8 +65,8 @@ class local extends base
 		$upload = $this->get_upload_ary($source_file, $filedata);
 
 		/** @var filespec $file */
-		$file = $this->factory->get('filespec')
+		$file = $this->factory->get('filespec');
-			->set_upload_ary($upload)
+		$file->set_upload_ary($upload)
 			->set_upload_namespace($this->upload);
 
 		if ($file->init_error())

phpBB/phpbb/files/types/local_storage.php

@@ -15,7 +15,7 @@ namespace phpbb\files\types;
 
 use bantu\IniGetWrapper\IniGetWrapper;
 use phpbb\files\factory;
-use phpbb\files\filespec;
+use phpbb\files\filespec_storage;
 use phpbb\language\language;
 use phpbb\request\request_interface;
 
@@ -67,15 +67,15 @@ class local_storage extends base
 	 * @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
+	 * @return filespec_storage Object "filespec_storage" is returned, all further operations can be done with this object
 	 */
 	protected function local_upload($source_file, $filedata = false)
 	{
 		$upload = $this->get_upload_ary($source_file, $filedata);
 
-		/** @var filespec $file */
+		/** @var filespec_storage $file */
-		$file = $this->factory->get('filespec_storage')
+		$file = $this->factory->get('filespec_storage');
-			->set_upload_ary($upload)
+		$file->set_upload_ary($upload)
 			->set_upload_namespace($this->upload);
 
 		if ($file->init_error())
@@ -85,6 +85,7 @@ class local_storage extends base
 		}
 
 		// PHP Upload file size check
+		/** @var filespec_storage $file */
 		$file = $this->check_upload_size($file);
 		if (count($file->error))
 		{

phpBB/phpbb/files/types/type_interface.php

@@ -21,7 +21,7 @@ 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
+	 * @return \phpbb\files\filespec_storage|\phpbb\files\filespec|bool Filespec instance if upload is
 	 *                                    successful or false if not
 	 */
 	public function upload();

phpBB/phpbb/files/upload.php

@@ -14,7 +14,7 @@
 namespace phpbb\files;
 
 use phpbb\language\language;
-use phpbb\request\request_interface;
+use phpbb\request\request;
 
 /**
  * File upload class
@@ -55,7 +55,7 @@ class upload
 	/** @var language Language class */
 	protected $language;
 
-	/** @var request_interface Request class */
+	/** @var request Request class */
 	protected $request;
 
 	/**
@@ -64,9 +64,9 @@ class upload
 	 * @param factory $factory Files factory
 	 * @param language $language Language class
 	 * @param \bantu\IniGetWrapper\IniGetWrapper $php_ini ini_get() wrapper
-	 * @param request_interface $request Request class
+	 * @param request $request Request class
 	 */
-	public function __construct(factory $factory, language $language, \bantu\IniGetWrapper\IniGetWrapper $php_ini, request_interface $request)
+	public function __construct(factory $factory, language $language, \bantu\IniGetWrapper\IniGetWrapper $php_ini, request $request)
 	{
 		$this->factory = $factory;
 		$this->language = $language;
@@ -194,7 +194,7 @@ class upload
 	 *
 	 * @param string $errorcode Error code to assign
 	 *
-	 * @return string Error string
+	 * @return string|false Error string or false if error code is not supported
 	 * @access public
 	 */
 	public function assign_internal_error($errorcode)
@@ -250,7 +250,7 @@ class upload
 	/**
 	 * Perform common file checks
 	 *
-	 * @param filespec_storage $file Instance of filespec class
+	 * @param filespec_storage|filespec $file Instance of filespec class
 	 */
 	public function common_checks($file)
 	{
@@ -296,7 +296,7 @@ class upload
 	/**
 	 * Check for allowed dimension
 	 *
-	 * @param filespec_storage $file Instance of filespec class
+	 * @param filespec_storage|filespec $file Instance of filespec class
 	 *
 	 * @return bool True if dimensions are valid or no constraints set, false
 	 *			if not

phpBB/phpbb/filesystem/filesystem.php

@@ -329,7 +329,7 @@ class filesystem implements filesystem_interface
 	/**
 	 * {@inheritdoc}
 	 */
-	public function phpbb_chmod($files, $perms = null, $recursive = false, $force_chmod_link = false)
+	public function phpbb_chmod($file, $perms = null, $recursive = false, $force_chmod_link = false)
 	{
 		if (is_null($perms))
 		{
@@ -374,26 +374,26 @@ class filesystem implements filesystem_interface
 		{
 			try
 			{
-				foreach ($this->to_iterator($files) as $file)
+				foreach ($this->to_iterator($file) as $current_file)
 				{
-					$file_uid = @fileowner($file);
+					$file_uid = @fileowner($current_file);
-					$file_gid = @filegroup($file);
+					$file_gid = @filegroup($current_file);
 
 					// Change owner
 					if ($file_uid !== $this->chmod_info['common_owner'])
 					{
-						$this->chown($file, $this->chmod_info['common_owner'], $recursive);
+						$this->chown($current_file, $this->chmod_info['common_owner'], $recursive);
 					}
 
 					// Change group
 					if ($file_gid !== $this->chmod_info['common_group'])
 					{
-						$this->chgrp($file, $this->chmod_info['common_group'], $recursive);
+						$this->chgrp($current_file, $this->chmod_info['common_group'], $recursive);
 					}
 
 					clearstatcache();
-					$file_uid = @fileowner($file);
+					$file_uid = @fileowner($current_file);
-					$file_gid = @filegroup($file);
+					$file_gid = @filegroup($current_file);
 				}
 			}
 			catch (filesystem_exception $e)
@@ -431,9 +431,9 @@ class filesystem implements filesystem_interface
 			case 'owner':
 				try
 				{
-					$this->chmod($files, $perms, $recursive, $force_chmod_link);
+					$this->chmod($file, $perms, $recursive, $force_chmod_link);
 					clearstatcache();
-					if ($this->is_readable($files) && $this->is_writable($files))
+					if ($this->is_readable($file) && $this->is_writable($file))
 					{
 						break;
 					}
@@ -445,9 +445,9 @@ class filesystem implements filesystem_interface
 			case 'group':
 				try
 				{
-					$this->chmod($files, $perms, $recursive, $force_chmod_link);
+					$this->chmod($file, $perms, $recursive, $force_chmod_link);
 					clearstatcache();
-					if ((!($perms & self::CHMOD_READ) || $this->is_readable($files, $recursive)) && (!($perms & self::CHMOD_WRITE) || $this->is_writable($files, $recursive)))
+					if ((!($perms & self::CHMOD_READ) || $this->is_readable($file, $recursive)) && (!($perms & self::CHMOD_WRITE) || $this->is_writable($file, $recursive)))
 					{
 						break;
 					}
@@ -458,7 +458,7 @@ class filesystem implements filesystem_interface
 				}
 			case 'other':
 			default:
-				$this->chmod($files, $perms, $recursive, $force_chmod_link);
+				$this->chmod($file, $perms, $recursive, $force_chmod_link);
 			break;
 		}
 	}

phpBB/phpbb/filesystem/filesystem_interface.php

@@ -241,7 +241,7 @@ interface filesystem_interface
 	 *
 	 * @param ?string	$path	Path to resolve
 	 *
-	 * @return string	Resolved path
+	 * @return string|false	Resolved path or false if path could not be resolved
 	 */
 	public function realpath($path);
 

phpBB/phpbb/filesystem/helper.php

@@ -69,7 +69,7 @@ class helper
 	 * Try to resolve real path when PHP's realpath failes to do so
 	 *
 	 * @param string	$path
-	 * @return bool|string
+	 * @return string|false
 	 */
 	protected static function phpbb_own_realpath($path)
 	{
@@ -175,7 +175,7 @@ class helper
 	 *
 	 * @param string	$path	Path to resolve
 	 *
-	 * @return string	Resolved path
+	 * @return string|false	Resolved path or false if path could not be resolved
 	 */
 	public static function realpath($path)
 	{