From 872439b8f4158ce36a7fb680c97927754d069d71 Mon Sep 17 00:00:00 2001
From: Marc Alexander <admin@m-a-styles.de>
Date: Sat, 29 Jul 2023 23:10:51 +0200
Subject: [PATCH] [ticket/9687] Properly display error messages in ACP

PHPBB3-9687
---
 phpBB/includes/acp/acp_ban.php |  9 ++++++-
 phpBB/phpbb/ban/manager.php    | 47 +++++++++++++++++++---------------
 phpBB/phpbb/ban/type/email.php |  2 +-
 phpBB/phpbb/ban/type/ip.php    |  2 +-
 phpBB/phpbb/ban/type/user.php  |  2 +-
 phpBB/phpbb/session.php        |  2 +-
 6 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/phpBB/includes/acp/acp_ban.php b/phpBB/includes/acp/acp_ban.php
index b41f42fd6a..5f8261d978 100644
--- a/phpBB/includes/acp/acp_ban.php
+++ b/phpBB/includes/acp/acp_ban.php
@@ -103,7 +103,14 @@ class acp_ban
 				$ban_end = $ban_manager->get_ban_end($ban_start, $ban_length, $ban_length_other);
 
 				$ban = explode("\n", $ban);
-				$ban_manager->ban($mode, $ban, $ban_start, $ban_end, $ban_reason, $ban_give_reason);
+				try
+				{
+					$ban_manager->ban($mode, $ban, $ban_start, $ban_end, $ban_reason, $ban_give_reason);
+				}
+				catch (\phpbb\exception\exception_interface $exception)
+				{
+					trigger_error($language->lang_array($exception->getMessage(), $exception->get_parameters()), E_USER_WARNING);
+				}
 
 				/**
 				* Use this event to perform actions after the ban has been performed
diff --git a/phpBB/phpbb/ban/manager.php b/phpBB/phpbb/ban/manager.php
index 52cae3670a..f51cd371c8 100644
--- a/phpBB/phpbb/ban/manager.php
+++ b/phpBB/phpbb/ban/manager.php
@@ -79,14 +79,14 @@ class manager
 	{
 		if ($start > $end && $end->getTimestamp() !== 0)
 		{
-			throw new invalid_length_exception(); // TODO
+			throw new invalid_length_exception('LENGTH_BAN_INVALID');
 		}
 
 		/** @var type_interface $ban_mode */
 		$ban_mode = $this->find_type($mode);
 		if ($ban_mode === false)
 		{
-			throw new type_not_found_exception(); // TODO
+			throw new type_not_found_exception();
 		}
 
 		if (!empty($this->user))
@@ -154,31 +154,36 @@ class manager
 		$ban_mode = $this->find_type($mode);
 		if ($ban_mode === false)
 		{
-			throw new type_not_found_exception(); // TODO
+			throw new type_not_found_exception();
 		}
 		$this->tidy();
 
 		$sql_ids = array_map('intval', $items);
-		$sql = 'SELECT ban_item
-			FROM ' . $this->bans_table . '
-			WHERE ' . $this->db->sql_in_set('ban_id', $sql_ids); // TODO (what if empty?)
-		$result = $this->db->sql_query($sql);
 
-		$unbanned_items = [];
-		while ($row = $this->db->sql_fetchrow($result))
+		if (count($sql_ids))
 		{
-			$unbanned_items[] = $row['ban_item'];
+			$sql = 'SELECT ban_item
+				FROM ' . $this->bans_table . '
+				WHERE ' . $this->db->sql_in_set('ban_id', $sql_ids);
+			$result = $this->db->sql_query($sql);
+
+			$unbanned_items = [];
+			while ($row = $this->db->sql_fetchrow($result))
+			{
+				$unbanned_items[] = $row['ban_item'];
+			}
+			$this->db->sql_freeresult($result);
+
+			$sql = 'DELETE FROM ' . $this->bans_table . '
+				WHERE ' . $this->db->sql_in_set('ban_id', $sql_ids);
+			$this->db->sql_query($sql);
+
+			$unban_data = [
+				'items' => $unbanned_items,
+			];
+			$unbanned_users = $ban_mode->after_unban($unban_data);
+			// @todo: add logging for unbanned users
 		}
-		$this->db->sql_freeresult($result);
-
-		$sql = 'DELETE FROM ' . $this->bans_table . '
-			WHERE ' . $this->db->sql_in_set('ban_id', $sql_ids);
-		$this->db->sql_query($sql);
-
-		$unban_data = [
-			'items'		=> $unbanned_items,
-		];
-		$unbanned_users = $ban_mode->after_unban($unban_data);
 
 		$this->cache->destroy(self::CACHE_KEY_INFO);
 		$this->cache->destroy(self::CACHE_KEY_USERS);
@@ -389,7 +394,7 @@ class manager
 				}
 				else
 				{
-					throw new invalid_length_exception();
+					throw new invalid_length_exception('LENGTH_BAN_INVALID');
 				}
 			}
 		}
diff --git a/phpBB/phpbb/ban/type/email.php b/phpBB/phpbb/ban/type/email.php
index e4c6350b76..25feef313c 100644
--- a/phpBB/phpbb/ban/type/email.php
+++ b/phpBB/phpbb/ban/type/email.php
@@ -56,7 +56,7 @@ class email extends base
 
 		if (empty($ban_items))
 		{
-			throw new no_valid_emails_exception(); // TODO
+			throw new no_valid_emails_exception('NO_EMAILS_DEFINED');
 		}
 
 		return $ban_items;
diff --git a/phpBB/phpbb/ban/type/ip.php b/phpBB/phpbb/ban/type/ip.php
index d6466c2407..7f8c6ee81d 100644
--- a/phpBB/phpbb/ban/type/ip.php
+++ b/phpBB/phpbb/ban/type/ip.php
@@ -85,7 +85,7 @@ class ip extends base
 
 		if (empty($ban_items))
 		{
-			throw new no_valid_ips_exception(); // TODO
+			throw new no_valid_ips_exception('NO_IPS_DEFINED');
 		}
 
 		return $ban_items;
diff --git a/phpBB/phpbb/ban/type/user.php b/phpBB/phpbb/ban/type/user.php
index cb5de0f8dd..ea79cffc35 100644
--- a/phpBB/phpbb/ban/type/user.php
+++ b/phpBB/phpbb/ban/type/user.php
@@ -147,7 +147,7 @@ class user extends base
 
 		if (empty($ban_items))
 		{
-			throw new no_valid_users_exception(); // TODO
+			throw new no_valid_users_exception('NO_USER_SPECIFIED');
 		}
 
 		return $ban_items;
diff --git a/phpBB/phpbb/session.php b/phpBB/phpbb/session.php
index 813a156726..bf7402cb6a 100644
--- a/phpBB/phpbb/session.php
+++ b/phpBB/phpbb/session.php
@@ -1136,7 +1136,7 @@ class session
 	*/
 	function check_ban($user_id = false, $user_ips = false, $user_email = false, $return = false)
 	{
-		global $db, $phpbb_container, $phpbb_dispatcher;
+		global $phpbb_container, $phpbb_dispatcher;
 
 		if (defined('IN_CHECK_BAN') || defined('SKIP_CHECK_BAN'))
 		{