Merge remote-tracking branch 'origin/master'

5 files changed, 163 insertions, 66 deletions

diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md
new file mode 100644
index 000000000..8b803acad
--- /dev/null
+++ b/.github/CONTRIBUTING.md
@@ -0,0 +1,42 @@
+# Contributing to one of our projects
+
+Our projects are open-source, and are deployed as commits are pushed to the `master` branch on each repository.
+We've created a set of guidelines here in order to keep everything clean and in working order. Please note that
+contributions may be rejected on the basis of a contributor failing to follow the guidelines.
+
+## Rules
+
+1. **No force-pushes** or modifying the Git history in any way.
+1. If you have direct access to the repository, **create a branch for your changes** and create a pull request for that branch.
+   If not, fork it and work on a separate branch there.
+    * Some repositories require this and will reject any direct pushes to `master`. Make this a habit!
+1. If someone is working on a pull request, **do not open your own pull request for the same task**. Instead, leave some comments
+   on the existing pull request. Communication is key, and there's no point in two separate implementations of the same thing.
+    * One option is to fork the other contributor's repository, and submit your changes to their branch with your 
+      own pull request. If you do this, we suggest following these guidelines when interacting with their repository 
+      as well.
+1. **Adhere to the prevailing code style**, which we enforce using [flake8](http://flake8.pycqa.org/en/latest/index.html).
+    * Additionally, run `flake8` against your code before you push it. Your commit will be rejected by the build server 
+      if it fails to lint. For an automatic way to do this, check out 
+      [our article on Git hooks](https://github.com/discord-python/site/wiki/Git-Hooks).
+1. **Don't fight the framework**. Every framework has its flaws, but the frameworks we've picked out have been carefully 
+    chosen for their particular merits. If you can avoid it, please resist reimplementing swathes of framework logic - the
+    work has already been done for you!
+1. **Work as a team** and cooperate where possible. Keep things friendly, and help each other out - these are shared
+    projects, and nobody likes to have their feet trodden on.
+1. **Internal projects are internal**. As a contributor, you have access to information that the rest of the server
+    does not. With this trust comes responsibility - do not release any information you have learned as a result of
+    your contributor position. We are very strict about announcing things at specific times, and many staff members
+    will not appreciate a disruption of the announcement schedule.
+
+Above all, the needs of our community should come before the wants of an individual. Work together, build solutions to
+problems and try to do so in a way that people can learn from easily. Abuse of our trust may result in the loss of your Contributor role, especially in relation to Rule 7.
+
+## Changes to this arrangement
+
+All projects evolve over time, and this contribution guide is no different. This document may also be subject to pull 
+requests or changes by contributors, where you believe you have something valuable to add or change.
+
+## Footnotes
+
+This document was inspired by the [Glowstone contribution guidelines](https://github.com/GlowstoneMC/Glowstone/blob/dev/docs/CONTRIBUTING.md).
diff --git a/Pipfile.lock b/Pipfile.lock
index 11a76eeae..c7c2cca1b 100644
--- a/Pipfile.lock
+++ b/Pipfile.lock
@@ -70,10 +70,10 @@
         },
         "dulwich": {
             "hashes": [
-                "sha256:91aad98f37a5494c6eabf08c78ed2b6e1b1ce6e7a5556406245d8763a352b99e"
+                "sha256:c51e10c260543240e0806052af046e1a78b98cbe1ac1ef3880a78d2269e09da4"
             ],
             "index": "pypi",
-            "version": "==0.19.0"
+            "version": "==0.19.2"
         },
         "idna": {
             "hashes": [
@@ -256,10 +256,10 @@
         },
         "dparse": {
             "hashes": [
-                "sha256:7c9f9175d8fd83aed6d31a16c1a3ba4c38189120f1df416b46029d940b4ef582",
-                "sha256:e4b479dd4d6078ba5f087b28447a50eee0caed57f135b0f5a5e5d5024390f41d"
+                "sha256:00a5fdfa900629e5159bf3600d44905b333f4059a3366f28e0dbd13eeab17b19",
+                "sha256:cef95156fa0adedaf042cd42f9990974bec76f25dfeca4dc01f381a243d5aa5b"
             ],
-            "version": "==0.2.1"
+            "version": "==0.4.1"
         },
         "flake8": {
             "hashes": [
@@ -360,10 +360,10 @@
         },
         "pbr": {
             "hashes": [
-                "sha256:8b9a7c3704657cb0831b3ded0a6b61377947c8235b76649bb7de4bfe2e6cfa56",
-                "sha256:cf66675e22ae91a4f20e4b8354f117d3e3d1de651513051d109cc39645fb3672"
+                "sha256:56b7a8ba7d64bf6135a9dfefb85a80d95924b3fde5ed6343a1a1d464a040dae3",
+                "sha256:de75cf1d510542c746beeff66b52241eb12c8f95f2ef846ee50ed5d72392caa4"
             ],
-            "version": "==4.0.0"
+            "version": "==4.0.1"
         },
         "pycodestyle": {
             "hashes": [
@@ -420,11 +420,11 @@
         },
         "safety": {
             "hashes": [
-                "sha256:9fb74211a0a0ab09541fe894293d66a558b6138a9fe8ebabc8cf56670e8a009c",
-                "sha256:ff0c4b76ad791d33825e36c41671ea45330d438921e5395903c0e87e576a377a"
+                "sha256:0bd2a26b872668767c6db8efecfc8869b547463bedff5e7cd7b52f037aa6f200",
+                "sha256:fc3fc55656f1c909d65311b49a38211c42c937f57a05393289fb3f17cadfa4a1"
             ],
             "index": "pypi",
-            "version": "==1.7.0"
+            "version": "==1.8.1"
         },
         "six": {
             "hashes": [
diff --git a/bot/__init__.py b/bot/__init__.py
index df76215e6..071ec6c98 100644
--- a/bot/__init__.py
+++ b/bot/__init__.py
@@ -137,7 +137,7 @@ def _get_word(self) -> str:
         args = ast.literal_eval(args)
 
         # Force args into container
-        if isinstance(args, str):
+        if not isinstance(args, tuple):
             args = (args,)
 
         # Type validate and format
diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py
index e4719e124..645835f53 100644
--- a/bot/cogs/tags.py
+++ b/bot/cogs/tags.py
@@ -1,7 +1,9 @@
 import logging
+import random
 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
@@ -16,6 +18,18 @@ class Tags:
     Save new tags and fetch existing tags.
     """
 
+    FAIL_TITLES = [
+        "Please don't do that.",
+        "You have to stop.",
+        "Do you mind?",
+        "In the future, don't do that.",
+        "That was a mistake.",
+        "You blew it.",
+        "You're bad at computers.",
+        "Are you trying to kill me?",
+        "Noooooo!!"
+    ]
+
     def __init__(self, bot: AutoShardedBot):
         self.bot = bot
         self.tag_cooldowns = {}
@@ -84,6 +98,59 @@ 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(value):
+            try:
+                float(value)
+            except ValueError:
+                return False
+            else:
+                return True
+
+        embed = Embed()
+        embed.colour = Colour.red()
+
+        # 'tag_name' has at least one invalid character.
+        if ascii(tag_name)[1:-1] != tag_name:
+            log.warning(f"{author} tried to put an invalid character in a tag name. "
+                        "Rejecting the request.")
+            embed.description = "Don't be ridiculous, you can't use that character!"
+
+        # '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.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.description = "Tag names can't be numbers."
+
+        # 'tag_name' is longer than 127 characters
+        elif len(tag_name) > 127:
+            log.warning(f"{author} tried to request a tag name with over 127 characters. "
+                        "Rejecting the request.")
+            embed.description = "Are you insane? That's way too long!"
+
+        else:
+            return None
+
+        embed.title = random.choice(Tags.FAIL_TITLES)
+        return embed
+
     @command(name="tags()", aliases=["tags"], hidden=True)
     async def info_command(self, ctx: Context):
         """
@@ -96,7 +163,7 @@ class Tags:
         return await ctx.invoke(self.bot.get_command("help"), "Tags")
 
     @command(name="tags.get()", aliases=["tags.get", "tags.show()", "tags.show", "get_tag"])
-    async def get_command(self, ctx: Context, tag_name=None):
+    async def get_command(self, ctx: Context, tag_name: str=None):
         """
         Get a list of all tags or a specified tag.
 
@@ -135,9 +202,17 @@ class Tags:
                         f"Cooldown ends in {time_left:.1f} seconds.")
             return
 
-        embed = Embed()
         tags = []
 
+        if tag_name is not None:
+            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
@@ -169,14 +244,14 @@ class Tags:
 
             if isinstance(tag_data, dict):
                 log.warning(f"{ctx.author} requested the tag '{tag_name}', but it could not be found.")
-                embed.description = f"Unknown tag: **{tag_name}**"
+                embed.description = f"**{tag_name}** is an unknown tag name. Please check the spelling and try again."
             else:
                 log.warning(f"{ctx.author} requested a list of all tags, but the tags database was empty!")
                 embed.description = "**There are no tags in the database!**"
 
             if tag_name:
                 embed.set_footer(text="To show a list of all tags, use bot.tags.get().")
-                embed.title = "Tag not found"
+                embed.title = "Tag not found."
 
         # Paginate if this is a list of all tags
         if tags:
@@ -202,58 +277,33 @@ class Tags:
         :param tag_content: The content of the tag.
         """
 
-        def is_number(string):
-            try:
-                float(string)
-            except ValueError:
-                return False
-            else:
-                return True
-
-        embed = Embed()
-        embed.colour = Colour.red()
+        validation = await self.validate(ctx.author, 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."
+        if validation is not None:
+            return await ctx.send(embed=validation)
 
-        # '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 = tag_name.lower().strip()
+        tag_content = tag_content.strip()
 
-        # '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."
+        embed = Embed()
+        embed.colour = Colour.red()
+        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:
-            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 +317,14 @@ class Tags:
         :param tag_name: The name of the tag to delete.
         """
 
+        validation = await self.validate(ctx.author, tag_name)
+
+        if validation is not None:
+            return await ctx.send(embed=validation)
+
+        tag_name = tag_name.lower().strip()
         embed = Embed()
         embed.colour = Colour.red()
-
         tag_data = await self.delete_tag_data(tag_name)
 
         if tag_data.get("success") is True:
diff --git a/tox.ini b/tox.ini
index a8869d3ba..fb2176741 100644
--- a/tox.ini
+++ b/tox.ini
@@ -2,5 +2,5 @@
 max-line-length=120
 application_import_names=bot
 exclude=.venv
-ignore=B311,W503,E226
+ignore=B311,W503,E226,S311
 import-order-style=pycharm