[ticket/13904] Move form_upload to its own class and define type classes

PHPBB3-13904

phpBB/config/default/container/services_files.yml

@@ -26,3 +26,10 @@ services:
             - @language
             - @request
             - %core.root_path%
+
+    files.types.form:
+        class: phpbb\files\types\form
+        arguments:
+            - @files.factory
+            - @plupload
+            - @request

phpBB/includes/functions_posting.php

@@ -433,7 +433,7 @@ function upload_attachment($form_name, $forum_id, $local = false, $local_storage
 	$upload->set_allowed_extensions(array_keys($extensions['_allowed_']));
 
 	/** @var \phpbb\files\filespec $file */
-	$file = ($local) ? $upload->local_upload($local_storage, $local_filedata) : $upload->form_upload($form_name, $plupload);
+	$file = ($local) ? $upload->local_upload($local_storage, $local_filedata) : $upload->handle_upload('form', $form_name);
 
 	if ($file->init_error())
 	{

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
 		{

phpBB/phpbb/files/types/form.php

@@ -0,0 +1,151 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @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;
+	}
+}

phpBB/phpbb/files/types/type_interface.php

@@ -0,0 +1,38 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @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);
+}

phpBB/phpbb/files/upload.php

@@ -182,87 +182,21 @@ class upload
 	}
 
 	/**
-	 * Form upload method
+	 * Handle upload based on type
-	 * 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 string $type Upload type
-	 * @param \phpbb\plupload\plupload $plupload The plupload object
 	 *
-	 * @return filespec $file Object "filespec" is returned, all further operations can be done with this object
+	 * @return \phpbb\files\filespec|bool A filespec instance if upload was
-	 * @access public
+	 *		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);
+		$args = func_get_args();
-		unset($upload['local_mode']);
+		array_shift($args);
+		$type_class = $this->factory->get('types.' . $type)
+			->set_upload($this);
 
-		if ($plupload)
+		return (is_object($type_class)) ? call_user_func_array(array($type_class, 'upload'), $args) : false;
-		{
-			$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;
-
-				$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;
 	}
 
 	/**
@@ -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)
 		{