Fixed an issue preventing bans being added when a ban already existed

Fixes #1094

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 This is the rolling changelog for TShock for Terraria. Use past tense when adding new entries; sign your name off when you add or change something. This should primarily be things like user changes, not necessarily codebase changes unless it's really relevant or large.
 
+## TShock 4.3.10
+
+* API: Added crash protection around malicious and/or invalid packets (@Wolfje)
+* Fixed an issue adding a ban on a player who has previously been banned (@Wolfje)
+
 ## TShock 4.3.8
 * API: Update to Terraria 1.3.0.8 (@Patrikkk)
 * **API: Added a crash reporter which collects memory dumps on Windows** (@Wolfje)

TShockAPI/DB/BanManager.cs

@@ -23,10 +23,17 @@ using MySql.Data.MySqlClient;
 
 namespace TShockAPI.DB
 {
+    /// <summary>
+    /// Class that manages bans.
+    /// </summary>
 	public class BanManager
 	{
 		private IDbConnection database;
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TShockAPI.DB.BanManager"/> class.
+        /// </summary>
+        /// <param name="db">A valid connection to the TShock database</param>
 		public BanManager(IDbConnection db)
 		{
 			database = db;
@@ -150,11 +157,33 @@ namespace TShockAPI.DB
 			return null;
 		}
 
+        /// <summary>
+        /// Adds a ban.
+        /// </summary>
+        /// <returns><c>true</c>, if ban was added, <c>false</c> otherwise.</returns>
+        /// <param name="ip">Ip.</param>
+        /// <param name="name">Name.</param>
+        /// <param name="uuid">UUID.</param>
+        /// <param name="reason">Reason.</param>
+        /// <param name="exceptions">If set to <c>true</c> enable throwing exceptions.</param>
+        /// <param name="banner">Banner.</param>
+        /// <param name="expiration">Expiration date.</param>
 		public bool AddBan(string ip, string name = "", string uuid = "", string reason = "", bool exceptions = false, string banner = "", string expiration = "")
 		{
 			try
 			{
+                /*
+                 * If the ban already exists, update its entry with the new date
+                 * and expiration details.
+                 */
+                if (GetBanByIp(ip) != null)
+                {
+                    return database.Query("UPDATE Bans SET Date = @0, Expiration = @1 WHERE IP = @2", DateTime.UtcNow.ToString("s"), expiration) == 1;
+                }
+                else
+                {
                     return database.Query("INSERT INTO Bans (IP, Name, UUID, Reason, BanningUser, Date, Expiration) VALUES (@0, @1, @2, @3, @4, @5, @6);", ip, name, uuid, reason, banner, DateTime.UtcNow.ToString("s"), expiration) != 0;
+                }
 			}
 			catch (Exception ex)
 			{
@@ -165,6 +194,14 @@ namespace TShockAPI.DB
 			return false;
 		}
 
+        /// <summary>
+        /// Removes a ban.
+        /// </summary>
+        /// <returns><c>true</c>, if ban was removed, <c>false</c> otherwise.</returns>
+        /// <param name="match">Match.</param>
+        /// <param name="byName">If set to <c>true</c> by name.</param>
+        /// <param name="casesensitive">If set to <c>true</c> casesensitive.</param>
+        /// <param name="exceptions">If set to <c>true</c> exceptions.</param>
 		public bool RemoveBan(string match, bool byName = false, bool casesensitive = true, bool exceptions = false)
 		{
 			try
@@ -184,6 +221,10 @@ namespace TShockAPI.DB
 			return false;
 		}
 
+        /// <summary>
+        /// Clears bans.
+        /// </summary>
+        /// <returns><c>true</c>, if bans were cleared, <c>false</c> otherwise.</returns>
 		public bool ClearBans()
 		{
 			try
@@ -198,22 +239,63 @@ namespace TShockAPI.DB
 		}
 	}
 
+    /// <summary>
+    /// Model class that represents a ban entry in the TShock database.
+    /// </summary>
 	public class Ban
 	{
+        /// <summary>
+        /// Gets or sets the IP address of the ban entry.
+        /// </summary>
+        /// <value>The IP Address</value>
 		public string IP { get; set; }
 
+        /// <summary>
+        /// Gets or sets the name.
+        /// </summary>
+        /// <value>The name.</value>
 		public string Name { get; set; }
 
+        /// <summary>
+        /// Gets or sets the Client UUID of the ban
+        /// </summary>
+        /// <value>The UUID</value>
 		public string UUID { get; set; }
 
+        /// <summary>
+        /// Gets or sets the ban reason.
+        /// </summary>
+        /// <value>The ban reason.</value>
 		public string Reason { get; set; }
 
+        /// <summary>
+        /// Gets or sets the name of the user who added this ban entry.
+        /// </summary>
+        /// <value>The banning user.</value>
         public string BanningUser { get; set; }
 
+        /// <summary>
+        /// Gets or sets the UTC date of when the ban is to take effect
+        /// </summary>
+        /// <value>The date, which must be in UTC</value>
         public string Date { get; set; }
 
+        /// <summary>
+        /// Gets or sets the expiration date, in which the ban shall be lifted
+        /// </summary>
+        /// <value>The expiration.</value>
         public string Expiration { get; set; }
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TShockAPI.DB.Ban"/> class.
+        /// </summary>
+        /// <param name="ip">Ip.</param>
+        /// <param name="name">Name.</param>
+        /// <param name="uuid">UUID.</param>
+        /// <param name="reason">Reason.</param>
+        /// <param name="banner">Banner.</param>
+        /// <param name="date">UTC ban date.</param>
+        /// <param name="exp">Expiration time</param>
 		public Ban(string ip, string name, string uuid, string reason, string banner, string date, string exp)
 		{
 			IP = ip;
@@ -225,6 +307,9 @@ namespace TShockAPI.DB
 		    Expiration = exp;
 		}
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TShockAPI.DB.Ban"/> class.
+        /// </summary>
 		public Ban()
 		{
 			IP = string.Empty;