[ticket/15276] Use storage in avatars

PHPBB3-15276

phpBB/config/default/container/services_avatar.yml

@@ -61,7 +61,7 @@ services:
             - '@config'
             - '%core.root_path%'
             - '%core.php_ext%'
-            - '@filesystem'
+            - '@storage.avatar'
             - '@path_helper'
             - '@dispatcher'
             - '@files.factory'

phpBB/config/default/container/services_files.yml

@@ -12,7 +12,6 @@ services:
             - '@language'
             - '@php_ini'
             - '@upload_imagesize'
-            - '%core.root_path%'
             - '@mimetype.guesser'
             - '@plupload'
 

phpBB/config/default/container/services_storage.yml

@@ -1,9 +1,18 @@
 services:
+
+# Storages
+    storage.avatar:
+        class: phpbb\storage\storage
+        arguments:
+            - '@storage.adapter.factory'
+            - 'avatar'
+
 # Factory
     storage.adapter.factory:
         class: phpbb\storage\adapter_factory
         arguments:
             - '@config'
+            - '@service_container'
             - '@storage.adapter_collection'
             - '@storage.provider_collection'
 

phpBB/install/schemas/schema_data.sql

@@ -55,7 +55,6 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_max_height'
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_max_width', '90');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_min_height', '20');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_min_width', '20');
-INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_path', 'images/avatars/upload');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('avatar_salt', 'phpbb_avatar');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('board_contact', 'contact@yourdomain.tld');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('board_contact_name', '');
@@ -288,6 +287,8 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_json
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_vendor_dir', 'vendor-ext/');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_enable_on_install', '0');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('exts_composer_purge_on_remove', '1');
+INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\avatar\adapter', 'phpbb\storage\provider\local');
+INSERT INTO phpbb_config (config_name, config_value) VALUES ('storage\avatar\config\path', 'images/avatars/upload');
 
 INSERT INTO phpbb_config (config_name, config_value, is_dynamic) VALUES ('cache_last_gc', '0', 1);
 INSERT INTO phpbb_config (config_name, config_value, is_dynamic) VALUES ('cron_lock', '0', 1);

phpBB/phpbb/avatar/driver/upload.php

@@ -19,9 +19,9 @@ namespace phpbb\avatar\driver;
 class upload extends \phpbb\avatar\driver\driver
 {
 	/**
-	 * @var \phpbb\filesystem\filesystem_interface
+	 * @var \phpbb\storage\storage
 	 */
-	protected $filesystem;
+	protected $storage;
 
 	/**
 	* @var \phpbb\event\dispatcher_interface
@@ -39,18 +39,18 @@ class upload extends \phpbb\avatar\driver\driver
 	* @param \phpbb\config\config $config phpBB configuration
 	* @param string $phpbb_root_path Path to the phpBB root
 	* @param string $php_ext PHP file extension
-	* @param \phpbb\filesystem\filesystem_interface $filesystem phpBB filesystem helper
+	* @param \phpbb\storage\storage phpBB avatar storage
 	* @param \phpbb\path_helper $path_helper phpBB path helper
 	* @param \phpbb\event\dispatcher_interface $dispatcher phpBB Event dispatcher object
 	* @param \phpbb\files\factory $files_factory File classes factory
 	* @param \phpbb\cache\driver\driver_interface $cache Cache driver
 	*/
-	public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\filesystem\filesystem_interface $filesystem, \phpbb\path_helper $path_helper, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\files\factory $files_factory, \phpbb\cache\driver\driver_interface $cache = null)
+	public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\storage\storage $storage, \phpbb\path_helper $path_helper, \phpbb\event\dispatcher_interface $dispatcher, \phpbb\files\factory $files_factory, \phpbb\cache\driver\driver_interface $cache = null)
 	{
 		$this->config = $config;
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->php_ext = $php_ext;
-		$this->filesystem = $filesystem;
+		$this->storage = $storage;
 		$this->path_helper = $path_helper;
 		$this->dispatcher = $dispatcher;
 		$this->files_factory = $files_factory;
@@ -116,7 +116,7 @@ class upload extends \phpbb\avatar\driver\driver
 
 		if (!empty($upload_file['name']))
 		{
-			$file = $upload->handle_upload('files.types.form', 'avatar_upload_file');
+			$file = $upload->handle_upload('files.types.form_storage', 'avatar_upload_file');
 		}
 		else if (!empty($this->config['allow_avatar_remote_upload']) && !empty($url))
 		{
@@ -156,7 +156,7 @@ class upload extends \phpbb\avatar\driver\driver
 				return false;
 			}
 
-			$file = $upload->handle_upload('files.types.remote', $url);
+			$file = $upload->handle_upload('files.types.remote_storage', $url);
 		}
 		else
 		{
@@ -169,26 +169,11 @@ class upload extends \phpbb\avatar\driver\driver
 		// If there was an error during upload, then abort operation
 		if (count($file->error))
 		{
-			$file->remove();
+			$file->remove($this->storage);
 			$error = $file->error;
 			return false;
 		}
 
-		// Calculate new destination
-		$destination = $this->config['avatar_path'];
-
-		// Adjust destination path (no trailing slash)
-		if (substr($destination, -1, 1) == '/' || substr($destination, -1, 1) == '\\')
-		{
-			$destination = substr($destination, 0, -1);
-		}
-
-		$destination = str_replace(array('../', '..\\', './', '.\\'), '', $destination);
-		if ($destination && ($destination[0] == '/' || $destination[0] == "\\"))
-		{
-			$destination = '';
-		}
-
 		$filedata = array(
 			'filename'			=> $file->get('filename'),
 			'filesize'			=> $file->get('filesize'),
@@ -203,7 +188,6 @@ class upload extends \phpbb\avatar\driver\driver
 		*
 		* @event core.avatar_driver_upload_move_file_before
 		* @var	array	filedata			Array containing uploaded file data
-		* @var	string	destination			Destination directory where the file is going to be moved
 		* @var	string	prefix				Prefix for the avatar filename
 		* @var	array	row					Array with avatar row data
 		* @var	array	error				Array of errors, if filled in by this event file will not be moved
@@ -212,7 +196,6 @@ class upload extends \phpbb\avatar\driver\driver
 		*/
 		$vars = array(
 			'filedata',
-			'destination',
 			'prefix',
 			'row',
 			'error',
@@ -224,14 +207,14 @@ class upload extends \phpbb\avatar\driver\driver
 		if (!count($error))
 		{
 			// Move file and overwrite any existing image
-			$file->move_file($destination, true);
+			$file->move_file($this->storage, true);
 		}
 
 		// If there was an error during move, then clean up leftovers
 		$error = array_merge($error, $file->error);
 		if (count($error))
 		{
-			$file->remove();
+			$file->remove($this->storage);
 			return false;
 		}
 
@@ -268,10 +251,9 @@ class upload extends \phpbb\avatar\driver\driver
 	{
 
 		$error = array();
-		$destination = $this->config['avatar_path'];
 		$prefix = $this->config['avatar_salt'] . '_';
 		$ext = substr(strrchr($row['avatar'], '.'), 1);
-		$filename = $this->phpbb_root_path . $destination . '/' . $prefix . $row['id'] . '.' . $ext;
+		$filename = $prefix . $row['id'] . '.' . $ext;
 
 		/**
 		* Before deleting an existing avatar
@@ -284,21 +266,20 @@ class upload extends \phpbb\avatar\driver\driver
 		* @since 3.1.6-RC1
 		*/
 		$vars = array(
-			'destination',
 			'prefix',
 			'row',
 			'error',
 		);
 		extract($this->dispatcher->trigger_event('core.avatar_driver_upload_delete_before', compact($vars)));
 
-		if (!count($error) && $this->filesystem->exists($filename))
+		if (!count($error) && $this->storage->exists($filename))
 		{
 			try
 			{
-				$this->filesystem->remove($filename);
+				$this->storage->delete($filename);
 				return true;
 			}
-			catch (\phpbb\filesystem\exception\filesystem_exception $e)
+			catch (\phpbb\storage\exception\exception $e)
 			{
 				// Fail is covered by return statement below
 			}
@@ -316,12 +297,12 @@ class upload extends \phpbb\avatar\driver\driver
 	}
 
 	/**
-	* Check if user is able to upload an avatar
+	* Check if user is able to upload an avatar to a temporary folder
 	*
 	* @return bool True if user can upload, false if not
 	*/
 	protected function can_upload()
 	{
-		return ($this->filesystem->exists($this->phpbb_root_path . $this->config['avatar_path']) && $this->filesystem->is_writable($this->phpbb_root_path . $this->config['avatar_path']) && (@ini_get('file_uploads') || strtolower(@ini_get('file_uploads')) == 'on'));
+		return (@ini_get('file_uploads') || strtolower(@ini_get('file_uploads')) == 'on');
 	}
 }

phpBB/phpbb/db/migration/data/v330/storage_avatar.php

@@ -0,0 +1,28 @@
+<?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\db\migration\data\v330;
+
+use \phpbb\storage\provider;
+
+class storage_avatar extends \phpbb\db\migration\migration
+{
+	public function update_data()
+	{
+		return array(
+			array('config.add', array('storage\\avatar\\adapter', provider\local::class)),
+			array('config.add', array('storage\\avatar\\config\\path', $this->config['avatar_path'])),
+			array('config.remove', array('avatar_path')),
+		);
+	}
+}

phpBB/phpbb/files/types/form_storage.php

@@ -0,0 +1,138 @@
+<?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 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_storage extends base
+{
+	/** @var factory Files factory */
+	protected $factory;
+
+	/** @var language */
+	protected $language;
+
+	/** @var IniGetWrapper */
+	protected $php_ini;
+
+	/** @var plupload */
+	protected $plupload;
+
+	/** @var request_interface */
+	protected $request;
+
+	/** @var \phpbb\files\upload */
+	protected $upload;
+
+	/**
+	 * Construct a form upload type
+	 *
+	 * @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, 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;
+	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function upload()
+	{
+		$args = func_get_args();
+		return $this->form_upload($args[0]);
+	}
+
+	/**
+	 * 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)
+	 *
+	 * @return filespec $file Object "filespec" is returned, all further operations can be done with this object
+	 * @access public
+	 */
+	protected function form_upload($form_name)
+	{
+		$upload = $this->request->file($form_name);
+		unset($upload['local_mode']);
+
+		$result = $this->plupload->handle_upload($form_name);
+		if (is_array($result))
+		{
+			$upload = array_merge($upload, $result);
+		}
+
+		/** @var filespec $file */
+		$file = $this->factory->get('filespec_storage')
+			->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->upload->error_prefix . 'EMPTY_FILEUPLOAD');
+			return $file;
+		}
+
+		// PHP Upload file size check
+		$file = $this->check_upload_size($file);
+		if (sizeof($file->error))
+		{
+			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);
+
+		return $file;
+	}
+}

phpBB/phpbb/files/types/remote_storage.php

@@ -0,0 +1,207 @@
+<?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 bantu\IniGetWrapper\IniGetWrapper;
+use phpbb\config\config;
+use phpbb\files\factory;
+use phpbb\files\filespec;
+use phpbb\language\language;
+use phpbb\request\request_interface;
+
+class remote_storage extends base
+{
+	/** @var config phpBB config */
+	protected $config;
+
+	/** @var factory Files factory */
+	protected $factory;
+
+	/** @var language */
+	protected $language;
+
+	/** @var IniGetWrapper */
+	protected $php_ini;
+
+	/** @var request_interface */
+	protected $request;
+
+	/** @var \phpbb\files\upload */
+	protected $upload;
+
+	/** @var string phpBB root path */
+	protected $phpbb_root_path;
+
+	/**
+	 * 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(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;
+		$this->request = $request;
+		$this->phpbb_root_path = $phpbb_root_path;
+	}
+
+	/**
+	 * {@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'));
+		}
+
+		$url = parse_url($upload_url);
+
+		$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 : '');
+
+		$remote_max_filesize = $this->get_max_file_size();
+
+		$guzzle_options = [
+			'timeout'			=> $this->upload->upload_timeout,
+			'connect_timeout'	=> $this->upload->upload_timeout,
+			'verify'			=> !empty($this->config['remote_upload_verify']) ? (bool) $this->config['remote_upload_verify'] : false,
+		];
+		$client = new \GuzzleHttp\Client($guzzle_options);
+
+		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 (\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
+			{
+				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->getBody()->getSize();
+		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() . '-');
+
+		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_storage')
+			->set_upload_ary($upload_ary)
+			->set_upload_namespace($this->upload);
+		$this->upload->common_checks($file);
+
+		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 = $this->php_ini->getString('upload_max_filesize');
+
+			if (!empty($max_file_size))
+			{
+				$unit = strtolower(substr($max_file_size, -1, 1));
+				$max_file_size = (int) $max_file_size;
+
+				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;
+	}
+}

phpBB/phpbb/storage/adapter/adapter_interface.php

@@ -22,6 +22,13 @@ interface adapter_interface
 	 */
 	public function configure($options);
 
+	/**
+	 * Dumps content into a file.
+	 *
+	 * @param array	options		Storage-specific options.
+	 */
+	public function configure($options);
+
 	/**
 	 * Dumps content into a file.
 	 *