Merge pull request #4894 from rubencm/ticket/15276

[ticket/15276] Use storage in avatars * github.com:phpbb/phpbb: (34 commits) [ticket/15276] Changed annotation [ticket/15276] Remove unused code [ticket/15276] Revert some changes [ticket/15276] Use IniGetWrapper [ticket/15276] Add missing dependency [ticket/15276] Remove unused dependency [ticket/15276] Add missing properties [ticket/15276] Use InitGetWrapper [ticket/15276] Fix comments [ticket/15276] Fix code and add phpdoc [ticket/15276] Use stream_copy_to_stream [ticket/15276] Fix typo [ticket/15276] Use mimetype guesser [ticket/15276] Update file_info to get size of images [ticket/15276] Update [ticket/15276] Remove avatar_path [ticket/15276] Remove avatar_path from acp [ticket/15276] Use finfo to get mimetype [ticket/15276] Update file_info [ticket/15276] Fix code style ...
phpbb · Sep 8, 2017 · 9b2c45d · 9b2c45d
2 parents b1fe0f9 + 443c503
commit 9b2c45d
Show file tree

Hide file tree

Showing 23 changed files with 668 additions and 143 deletions.
diff --git a/phpBB/config/default/container/services_avatar.yml b/phpBB/config/default/container/services_avatar.yml
@@ -61,11 +61,11 @@ services:
             - '@config'
             - '%core.root_path%'
             - '%core.php_ext%'
-            - '@filesystem'
+            - '@storage.avatar'
             - '@path_helper'
             - '@dispatcher'
             - '@files.factory'
-            - '@cache.driver'
+            - '@php_ini'
         calls:
             - [set_name, [avatar.driver.upload]]
         tags:

diff --git a/phpBB/config/default/container/services_storage.yml b/phpBB/config/default/container/services_storage.yml
@@ -1,4 +1,14 @@
 services:
+
+# Storages
+    storage.avatar:
+        class: phpbb\storage\storage
+        arguments:
+            - '@storage.adapter.factory'
+            - 'avatar'
+        tags:
+            - { name: storage }
+
 # Factory
     storage.adapter.factory:
         class: phpbb\storage\adapter_factory
@@ -28,6 +38,8 @@ services:
         shared: false
         arguments:
             - '@filesystem'
+            - '@upload_imagesize'
+            - '@mimetype.guesser'
             - '%core.root_path%'
         tags:
             - { name: storage.adapter }

diff --git a/phpBB/develop/adjust_avatars.php b/phpBB/develop/adjust_avatars.php
@@ -19,7 +19,7 @@
 $user->setup();
 
 $echos = 0;
- 
+
 if (!isset($config['avatar_salt']))
 {
 	$cache->purge();
@@ -30,6 +30,11 @@
 	die('database not up to date');
 }
 
+if (!isset($config['storage\\avatar\\config\\path']) || $config['storage\\avatar\\config\\path'] !== 'phpbb\\storage\\provider\\local')
+{
+	die('use local provider');
+}
+
 // let's start with the users using a group_avatar.
 $sql = 'SELECT group_id, group_avatar
 	FROM ' . GROUPS_TABLE . '
@@ -46,16 +51,16 @@
 {
 	$new_avatar_name = adjust_avatar($row['group_avatar'], 'g' . $row['group_id']);
 	$group_avatars[] = $new_avatar_name;
-	
+
 	// failure is probably due to the avatar name already being adjusted
 	if ($new_avatar_name !== false)
 	{
 		$sql = 'UPDATE ' . USERS_TABLE . "
 			SET user_avatar = '" . $db->sql_escape($new_avatar_name) . "'
-			WHERE user_avatar = '" . $db->sql_escape($row['group_avatar']) . "' 
+			WHERE user_avatar = '" . $db->sql_escape($row['group_avatar']) . "'
 			AND user_avatar_type = " . AVATAR_UPLOAD;
 		$db->sql_query($sql);
-		
+
 		$sql = 'UPDATE ' . GROUPS_TABLE . "
 			SET group_avatar = '" . $db->sql_escape($new_avatar_name) . "'
 			WHERE group_id = {$row['group_id']}";
@@ -80,8 +85,8 @@
 $db->sql_freeresult($result);
 
 $sql = 'SELECT user_id, username, user_avatar, user_avatar_type
-	FROM ' . USERS_TABLE . ' 
-	WHERE user_avatar_type = ' . AVATAR_UPLOAD . ' 
+	FROM ' . USERS_TABLE . '
+	WHERE user_avatar_type = ' . AVATAR_UPLOAD . '
 	AND ' . $db->sql_in_set('user_avatar', $group_avatars, true, true);
 $result = $db->sql_query($sql);
 
@@ -108,7 +113,7 @@
 		$db->sql_query($sql);
 		echo '<br /> Failed updating user ' . $row['user_id'] . "\n";
 	}
-	
+
 	if ($echos > 200)
 	{
 		echo '<br />' . "\n";
@@ -131,8 +136,8 @@
 function adjust_avatar($old_name, $midfix)
 {
 	global $config, $phpbb_root_path;
-	
-	$avatar_path = $phpbb_root_path . $config['avatar_path'];
+
+	$avatar_path = $phpbb_root_path . $config['storage\\avatar\\config\\path'];
 	$extension = strtolower(substr(strrchr($old_name, '.'), 1));
 	$new_name = $config['avatar_salt'] . '_' . $midfix . '.' . $extension;
 

diff --git a/phpBB/docs/coding-guidelines.html b/phpBB/docs/coding-guidelines.html
@@ -264,7 +264,6 @@ <h4>PHPBB_USE_BOARD_URL_PATH</h4>
 	<li>{T_SUPER_TEMPLATE_PATH} - styles/xxx/template</li>
 	<li>{T_IMAGES_PATH} - images/</li>
 	<li>{T_SMILIES_PATH} - $config['smilies_path']/</li>
-	<li>{T_AVATAR_PATH} - $config['avatar_path']/</li>
 	<li>{T_AVATAR_GALLERY_PATH} - $config['avatar_gallery_path']/</li>
 	<li>{T_ICONS_PATH} - $config['icons_path']/</li>
 	<li>{T_RANKS_PATH} - $config['ranks_path']/</li>

diff --git a/phpBB/includes/acp/acp_main.php b/phpBB/includes/acp/acp_main.php
@@ -502,26 +502,8 @@ function main($id, $mode)
 
 		$upload_dir_size = get_formatted_filesize($config['upload_dir_size']);
 
-		$avatar_dir_size = 0;
-
-		if ($avatar_dir = @opendir($phpbb_root_path . $config['avatar_path']))
-		{
-			while (($file = readdir($avatar_dir)) !== false)
-			{
-				if ($file[0] != '.' && $file != 'CVS' && strpos($file, 'index.') === false)
-				{
-					$avatar_dir_size += filesize($phpbb_root_path . $config['avatar_path'] . '/' . $file);
-				}
-			}
-			closedir($avatar_dir);
-
-			$avatar_dir_size = get_formatted_filesize($avatar_dir_size);
-		}
-		else
-		{
-			// Couldn't open Avatar dir.
-			$avatar_dir_size = $user->lang['NOT_AVAILABLE'];
-		}
+		// Couldn't open Avatar dir.
+		$avatar_dir_size = $user->lang['NOT_AVAILABLE'];
 
 		if ($posts_per_day > $total_posts)
 		{

diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
@@ -4434,7 +4434,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id =
 		'T_SUPER_TEMPLATE_PATH'	=> "{$web_path}styles/" . rawurlencode($user->style['style_path']) . '/template',
 		'T_IMAGES_PATH'			=> "{$web_path}images/",
 		'T_SMILIES_PATH'		=> "{$web_path}{$config['smilies_path']}/",
-		'T_AVATAR_PATH'			=> "{$web_path}{$config['avatar_path']}/",
 		'T_AVATAR_GALLERY_PATH'	=> "{$web_path}{$config['avatar_gallery_path']}/",
 		'T_ICONS_PATH'			=> "{$web_path}{$config['icons_path']}/",
 		'T_RANKS_PATH'			=> "{$web_path}{$config['ranks_path']}/",
@@ -4452,7 +4451,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id =
 		'T_SUPER_TEMPLATE_NAME'	=> rawurlencode((isset($user->style['style_parent_tree']) && $user->style['style_parent_tree']) ? $user->style['style_parent_tree'] : $user->style['style_path']),
 		'T_IMAGES'				=> 'images',
 		'T_SMILIES'				=> $config['smilies_path'],
-		'T_AVATAR'				=> $config['avatar_path'],
 		'T_AVATAR_GALLERY'		=> $config['avatar_gallery_path'],
 		'T_ICONS'				=> $config['icons_path'],
 		'T_RANKS'				=> $config['ranks_path'],

diff --git a/phpBB/includes/functions_acp.php b/phpBB/includes/functions_acp.php
@@ -88,7 +88,6 @@ function adm_page_header($page_title)
 
 		'T_IMAGES_PATH'			=> "{$phpbb_root_path}images/",
 		'T_SMILIES_PATH'		=> "{$phpbb_root_path}{$config['smilies_path']}/",
-		'T_AVATAR_PATH'			=> "{$phpbb_root_path}{$config['avatar_path']}/",
 		'T_AVATAR_GALLERY_PATH'	=> "{$phpbb_root_path}{$config['avatar_gallery_path']}/",
 		'T_ICONS_PATH'			=> "{$phpbb_root_path}{$config['icons_path']}/",
 		'T_RANKS_PATH'			=> "{$phpbb_root_path}{$config['ranks_path']}/",

diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php
@@ -25,30 +25,27 @@
 */
 function send_avatar_to_browser($file, $browser)
 {
-	global $config, $phpbb_root_path;
+	global $config, $phpbb_container;
 
-	$prefix = $config['avatar_salt'] . '_';
-	$image_dir = $config['avatar_path'];
+	$storage = $phpbb_container->get('storage.avatar');
 
-	// Adjust image_dir path (no trailing slash)
-	if (substr($image_dir, -1, 1) == '/' || substr($image_dir, -1, 1) == '\\')
-	{
-		$image_dir = substr($image_dir, 0, -1) . '/';
-	}
-	$image_dir = str_replace(array('../', '..\\', './', '.\\'), '', $image_dir);
+	$prefix = $config['avatar_salt'] . '_';
+	$file_path = $prefix . $file;
 
-	if ($image_dir && ($image_dir[0] == '/' || $image_dir[0] == '\\'))
+	if ($storage->exists($file_path) && !headers_sent())
 	{
-		$image_dir = '';
-	}
-	$file_path = $phpbb_root_path . $image_dir . '/' . $prefix . $file;
+		$file_info = $storage->file_info($file_path);
 
-	if ((@file_exists($file_path) && @is_readable($file_path)) && !headers_sent())
-	{
 		header('Cache-Control: public');
 
-		$image_data = @getimagesize($file_path);
-		header('Content-Type: ' . image_type_to_mime_type($image_data[2]));
+		try
+		{
+			header('Content-Type: ' . $file_info->mimetype);
+		}
+		catch (\phpbb\storage\exception\exception $e)
+		{
+			// Just don't send this header
+		}
 
 		if ((strpos(strtolower($browser), 'msie') !== false) && !phpbb_is_greater_ie_version($browser, 7))
 		{
@@ -69,24 +66,26 @@ function send_avatar_to_browser($file, $browser)
 			header('Expires: ' . gmdate('D, d M Y H:i:s', time() + 31536000) . ' GMT');
 		}
 
-		$size = @filesize($file_path);
-		if ($size)
+		try
 		{
-			header("Content-Length: $size");
+			header('Content-Length: ' . $file_info->size);
 		}
-
-		if (@readfile($file_path) == false)
+		catch (\phpbb\storage\exception\exception $e)
 		{
-			$fp = @fopen($file_path, 'rb');
+			// Just don't send this header
+		}
 
-			if ($fp !== false)
-			{
-				while (!feof($fp))
-				{
-					echo fread($fp, 8192);
-				}
-				fclose($fp);
-			}
+		try
+		{
+			$fp = $storage->read_stream($file_path);
+			$output = fopen('php://output', 'w+b');
+			stream_copy_to_stream($fp, $output);
+			fclose($fp);
+			fclose($output);
+		}
+		catch (\Exception $e)
+		{
+			// Send nothing
 		}
 
 		flush();

diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php
@@ -2166,7 +2166,9 @@ function phpbb_style_is_active($style_id)
 */
 function avatar_delete($mode, $row, $clean_db = false)
 {
-	global $phpbb_root_path, $config;
+	global $config, $phpbb_container;
+
+	$storage = $phpbb_container->get('storage.avatar');
 
 	// Check if the users avatar is actually *not* a group avatar
 	if ($mode == 'user')
@@ -2183,11 +2185,16 @@ function avatar_delete($mode, $row, $clean_db = false)
 	}
 	$filename = get_avatar_filename($row[$mode . '_avatar']);
 
-	if (file_exists($phpbb_root_path . $config['avatar_path'] . '/' . $filename))
+	try
 	{
-		@unlink($phpbb_root_path . $config['avatar_path'] . '/' . $filename);
+		$storage->delete($filename);
+
 		return true;
 	}
+	catch (\phpbb\storage\exception\exception $e)
+	{
+		// Fail is covered by return statement below
+	}
 
 	return false;
 }
@@ -2507,22 +2514,29 @@ function group_create(&$group_id, $type, $name, $desc, $group_attributes, $allow
 */
 function group_correct_avatar($group_id, $old_entry)
 {
-	global $config, $db, $phpbb_root_path;
+	global $config, $db, $phpbb_container;
+
+	$storage = $phpbb_container->get('storage.avatar');
 
 	$group_id		= (int) $group_id;
 	$ext 			= substr(strrchr($old_entry, '.'), 1);
 	$old_filename 	= get_avatar_filename($old_entry);
 	$new_filename 	= $config['avatar_salt'] . "_g$group_id.$ext";
 	$new_entry 		= 'g' . $group_id . '_' . substr(time(), -5) . ".$ext";
 
-	$avatar_path = $phpbb_root_path . $config['avatar_path'];
-	if (@rename($avatar_path . '/'. $old_filename, $avatar_path . '/' . $new_filename))
+	try
 	{
+		$this->storage->rename($old_filename, $new_filename);
+
 		$sql = 'UPDATE ' . GROUPS_TABLE . '
 			SET group_avatar = \'' . $db->sql_escape($new_entry) . "'
 			WHERE group_id = $group_id";
 		$db->sql_query($sql);
 	}
+	catch (\phpbb\storage\exception\exception $e)
+	{
+		// If rename fail, dont execute the query
+	}
 }
 
 

diff --git a/phpBB/install/convert/convertor.php b/phpBB/install/convert/convertor.php
@@ -281,7 +281,7 @@ function convert_data($converter)
 				$bad_folders = array();
 
 				$local_paths = array(
-					'avatar_path'			=> path($config['avatar_path']),
+					'avatar_path'			=> path($config['storage\\avatar\\config\\path']),
 					'avatar_gallery_path'	=> path($config['avatar_gallery_path']),
 					'icons_path'			=> path($config['icons_path']),
 					'ranks_path'			=> path($config['ranks_path']),

diff --git a/phpBB/install/schemas/schema_data.sql b/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\provider', '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);