Improve tag validation (#40)

* Sort out tag name literal eval

* Fixed typo >.>

* Validate all tag commands

* Clean up validation

* Add default embed colours

* Sorted a docstring

* Typehinted validate method

* Explicitly return None

1 files changed, 97 insertions, 50 deletions

diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py
index e4719e124..3e08778b1 100644
--- a/bot/cogs/tags.py
+++ b/bot/cogs/tags.py
@@ -1,7 +1,8 @@
 import logging
 import time
+from typing import Optional
 
-from discord import Colour, Embed
+from discord import Colour, Embed, User
 from discord.ext.commands import AutoShardedBot, Context, command
 
 from bot.constants import ADMIN_ROLE, MODERATOR_ROLE, OWNER_ROLE, SITE_API_KEY, SITE_API_TAGS_URL, TAG_COOLDOWN
@@ -84,6 +85,67 @@ class Tags:
 
         return tag_data
 
+    @staticmethod
+    async def validate(author: User, tag_name: str, tag_content: str = None) -> Optional[Embed]:
+        """
+        Create an embed based on the validity of a tag's name and content
+
+        :param author: The user that called the command
+        :param tag_name: The name of the tag to validate.
+        :param tag_content: The tag's content, if any.
+        :return: A validation embed if invalid, otherwise None
+        """
+
+        def is_number(string):
+            try:
+                float(string)
+            except ValueError:
+                return False
+            else:
+                return True
+
+        embed = Embed()
+        embed.colour = Colour.red()
+
+        # Replace any special characters
+        raw_name = tag_name.translate(
+            {
+                0x8: "\\b",  # Backspace
+                0x9: "\\t",  # Horizontal tab
+                0xA: "\\n",  # Linefeed
+                0xB: "\\v",  # Vertical tab
+                0xC: "\\f",  # Form feed
+                0xD: "\\r"   # Carriage return
+            }
+        )
+
+        # 'tag_name' has at least one special character.
+        if tag_name != raw_name:
+            log.warning(f"{author} tried to put a special character in a tag name. "
+                        "Rejecting the request.")
+            embed.title = "Please don't do that"
+            embed.description = "Don't be ridiculous, special characters obviously aren't allowed in the tag name"
+
+        # 'tag_content' or 'tag_name' are either empty, or consist of nothing but whitespace
+        elif (tag_content is not None and not tag_content) or not tag_name:
+            log.warning(f"{author} tried to create a tag with a name consisting only of whitespace. "
+                        "Rejecting the request.")
+            embed.title = "Please don't do that"
+            embed.description = "Tags should not be empty, or filled with whitespace."
+
+        # 'tag_name' is a number of some kind, we don't allow that.
+        elif is_number(tag_name):
+            log.error("inside the is_number")
+            log.warning(f"{author} tried to create a tag with a digit as its name. "
+                        "Rejecting the request.")
+            embed.title = "Please don't do that"
+            embed.description = "Tag names can't be numbers."
+
+        else:
+            return None
+
+        return embed
+
     @command(name="tags()", aliases=["tags"], hidden=True)
     async def info_command(self, ctx: Context):
         """
@@ -135,9 +197,15 @@ class Tags:
                         f"Cooldown ends in {time_left:.1f} seconds.")
             return
 
-        embed = Embed()
         tags = []
+        tag_name = tag_name.lower().strip()
+        validation = await self.validate(ctx.author, tag_name)
+
+        if validation is not None:
+            return await ctx.send(embed=validation)
 
+        embed = Embed()
+        embed.colour = Colour.red()
         tag_data = await self.get_tag_data(tag_name)
 
         # If we found something, prepare that data
@@ -202,58 +270,32 @@ class Tags:
         :param tag_content: The content of the tag.
         """
 
-        def is_number(string):
-            try:
-                float(string)
-            except ValueError:
-                return False
-            else:
-                return True
+        tag_name = tag_name.lower().strip()
+        tag_content = tag_content.strip()
+        validation = await self.validate(ctx.author, tag_name, tag_content)
+
+        if validation is not None:
+            return await ctx.send(embed=validation)
 
         embed = Embed()
         embed.colour = Colour.red()
+        tag_data = await self.post_tag_data(tag_name, tag_content)
 
-        # Newline in 'tag_name'
-        if "\n" in tag_name:
-            log.warning(f"{ctx.author} tried to put a newline in a tag name. "
-                        "Rejecting the request.")
-            embed.title = "Please don't do that"
-            embed.description = "Don't be ridiculous. Newlines are obviously not allowed in the tag name."
-
-        # 'tag_name' or 'tag_content' consists of nothing but whitespace
-        elif not tag_content.strip() or not tag_name.strip():
-            log.warning(f"{ctx.author} tried to create a tag with a name consisting only of whitespace. "
-                        "Rejecting the request.")
-            embed.title = "Please don't do that"
-            embed.description = "Tags should not be empty, or filled with whitespace."
-
-        # 'tag_name' is a number of some kind, we don't allow that.
-        elif is_number(tag_name):
-            log.error("inside the is_number")
-            log.warning(f"{ctx.author} tried to create a tag with a digit as its name. "
-                        "Rejecting the request.")
-            embed.title = "Please don't do that"
-            embed.description = "Tag names can't be numbers."
-
+        if tag_data.get("success"):
+            log.debug(f"{ctx.author} successfully added the following tag to our database: \n"
+                      f"tag_name: {tag_name}\n"
+                      f"tag_content: '{tag_content}'")
+            embed.colour = Colour.blurple()
+            embed.title = "Tag successfully added"
+            embed.description = f"**{tag_name}** added to tag database."
         else:
-            tag_name = tag_name.lower()
-            tag_data = await self.post_tag_data(tag_name, tag_content)
-
-            if tag_data.get("success"):
-                log.debug(f"{ctx.author} successfully added the following tag to our database: \n"
-                          f"tag_name: {tag_name}\n"
-                          f"tag_content: '{tag_content}'")
-                embed.colour = Colour.blurple()
-                embed.title = "Tag successfully added"
-                embed.description = f"**{tag_name}** added to tag database."
-            else:
-                log.error("There was an unexpected database error when trying to add the following tag: \n"
-                          f"tag_name: {tag_name}\n"
-                          f"tag_content: '{tag_content}'\n"
-                          f"response: {tag_data}")
-                embed.title = "Database error"
-                embed.description = ("There was a problem adding the data to the tags database. "
-                                     "Please try again. If the problem persists, see the error logs.")
+            log.error("There was an unexpected database error when trying to add the following tag: \n"
+                      f"tag_name: {tag_name}\n"
+                      f"tag_content: '{tag_content}'\n"
+                      f"response: {tag_data}")
+            embed.title = "Database error"
+            embed.description = ("There was a problem adding the data to the tags database. "
+                                 "Please try again. If the problem persists, see the error logs.")
 
         return await ctx.send(embed=embed)
 
@@ -267,9 +309,14 @@ class Tags:
         :param tag_name: The name of the tag to delete.
         """
 
+        tag_name = tag_name.lower().strip()
+        validation = await self.validate(ctx.author, tag_name)
+
+        if validation is not None:
+            return await ctx.send(embed=validation)
+
         embed = Embed()
         embed.colour = Colour.red()
-
         tag_data = await self.delete_tag_data(tag_name)
 
         if tag_data.get("success") is True: