From 68f97584d0e472857f07f8421001e007d5983164 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 19:00:52 +0100 Subject: Make sure tag name contains at least one letter. With only ascii and numbers being allowed to go through, possible values still included things like `$()` which don't match anything in `REGEX_NON_ALPHABET` from tags.py resulting in an error. --- bot/converters.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bot/converters.py b/bot/converters.py index cca57a02d..d73ab73f1 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -175,6 +175,12 @@ class TagNameConverter(Converter): "Rejecting the request.") raise BadArgument("Are you insane? That's way too long!") + # The tag name is ascii but does not contain any letters. + elif not any(character.isalpha() for character in tag_name): + log.warning(f"{ctx.author} tried to request a tag name without letters. " + "Rejecting the request.") + raise BadArgument("Tag names must contain at least one letter.") + return tag_name -- cgit v1.2.3 From b4a52aded317572d51a0747ed8d74b3fc84c9428 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 19:13:48 +0100 Subject: Pass error handler tag fallback through TagNameConverter. The tag fallback didn't convert tags, resulting in possible invalid tag names being passed to the `tags_get_command`. This makes sure they're valid and ignores the risen exception if they are not. --- bot/cogs/error_handler.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 0abb7e521..3486a746c 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -20,6 +20,7 @@ from sentry_sdk import push_scope from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels +from bot.converters import TagNameConverter from bot.decorators import InChannelCheckFailure log = logging.getLogger(__name__) @@ -88,8 +89,11 @@ class ErrorHandler(Cog): return # Return to not raise the exception - with contextlib.suppress(ResponseCodeError): - await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) + with contextlib.suppress(BadArgument, ResponseCodeError): + await ctx.invoke( + tags_get_command, + tag_name=await TagNameConverter.convert(ctx, ctx.invoked_with) + ) return elif isinstance(e, BadArgument): await ctx.send(f"Bad argument: {e}\n") -- cgit v1.2.3 From 432311ce720a1aea23c3ed7b422615a2e304b070 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 19:22:28 +0100 Subject: Remove number check on tags. This case is already covered by checking if at least one letter is included. --- bot/converters.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index d73ab73f1..745ce5b5d 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -141,14 +141,6 @@ class TagNameConverter(Converter): @staticmethod async def convert(ctx: Context, tag_name: str) -> str: """Lowercase & strip whitespace from proposed tag_name & ensure it's valid.""" - def is_number(value: str) -> bool: - """Check to see if the input string is numeric.""" - try: - float(value) - except ValueError: - return False - return True - tag_name = tag_name.lower().strip() # The tag name has at least one invalid character. @@ -163,12 +155,6 @@ class TagNameConverter(Converter): "Rejecting the request.") raise BadArgument("Tag names should not be empty, or filled with whitespace.") - # The tag name is a number of some kind, we don't allow that. - elif is_number(tag_name): - log.warning(f"{ctx.author} tried to create a tag with a digit as its name. " - "Rejecting the request.") - raise BadArgument("Tag names can't be numbers.") - # The tag name is longer than 127 characters. elif len(tag_name) > 127: log.warning(f"{ctx.author} tried to request a tag name with over 127 characters. " -- cgit v1.2.3 From c0d2f51f4e4a57da23f1387e8cf6b1a9e8c02e73 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 19:24:52 +0100 Subject: Adjust tests for new converter behavior. --- tests/bot/test_converters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index b2b78d9dd..1e5ca62ae 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -68,7 +68,7 @@ class ConverterTests(unittest.TestCase): ('👋', "Don't be ridiculous, you can't use that character!"), ('', "Tag names should not be empty, or filled with whitespace."), (' ', "Tag names should not be empty, or filled with whitespace."), - ('42', "Tag names can't be numbers."), + ('42', "Tag names must contain at least one letter."), ('x' * 128, "Are you insane? That's way too long!"), ) -- cgit v1.2.3 From 5ae3949899bf8f5d637f7f5025311342012f6db6 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 20:36:36 +0100 Subject: Remove logging from tag converters. --- bot/converters.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 745ce5b5d..1945e1da3 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -145,26 +145,18 @@ class TagNameConverter(Converter): # The tag name has at least one invalid character. if ascii(tag_name)[1:-1] != tag_name: - log.warning(f"{ctx.author} tried to put an invalid character in a tag name. " - "Rejecting the request.") raise BadArgument("Don't be ridiculous, you can't use that character!") # The tag name is either empty, or consists of nothing but whitespace. elif not tag_name: - log.warning(f"{ctx.author} tried to create a tag with a name consisting only of whitespace. " - "Rejecting the request.") raise BadArgument("Tag names should not be empty, or filled with whitespace.") # The tag name is longer than 127 characters. elif len(tag_name) > 127: - log.warning(f"{ctx.author} tried to request a tag name with over 127 characters. " - "Rejecting the request.") raise BadArgument("Are you insane? That's way too long!") # The tag name is ascii but does not contain any letters. elif not any(character.isalpha() for character in tag_name): - log.warning(f"{ctx.author} tried to request a tag name without letters. " - "Rejecting the request.") raise BadArgument("Tag names must contain at least one letter.") return tag_name @@ -184,8 +176,6 @@ class TagContentConverter(Converter): # The tag contents should not be empty, or filled with whitespace. if not tag_content: - log.warning(f"{ctx.author} tried to create a tag containing only whitespace. " - "Rejecting the request.") raise BadArgument("Tag contents should not be empty, or filled with whitespace.") return tag_content -- cgit v1.2.3 From 97b07a8be526b62db0b1b072b4d9773bff7a8db1 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 21:58:08 +0100 Subject: Log invalid tag names in the error handler tag fallback. --- bot/cogs/error_handler.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 3486a746c..fff1f8c9f 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -89,12 +89,20 @@ class ErrorHandler(Cog): return # Return to not raise the exception - with contextlib.suppress(BadArgument, ResponseCodeError): - await ctx.invoke( - tags_get_command, - tag_name=await TagNameConverter.convert(ctx, ctx.invoked_with) + try: + tag_name = await TagNameConverter.convert(ctx, ctx.invoked_with) + except BadArgument: + log.debug( + f"{ctx.author} tried to use an invalid command " + f"and the fallback tag failed validation in TagNameConverter." ) - return + else: + with contextlib.suppress(ResponseCodeError): + await ctx.invoke( + tags_get_command, + tag_name=tag_name + ) + return elif isinstance(e, BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) -- cgit v1.2.3 From 848a613dee4bcbb00226fb98fc6501403dc0d7c7 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Thu, 27 Feb 2020 18:53:44 +0100 Subject: Remove unnecessary newlines from call. --- bot/cogs/error_handler.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index fff1f8c9f..0d4604430 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -98,10 +98,7 @@ class ErrorHandler(Cog): ) else: with contextlib.suppress(ResponseCodeError): - await ctx.invoke( - tags_get_command, - tag_name=tag_name - ) + await ctx.invoke(tags_get_command, tag_name=tag_name) return elif isinstance(e, BadArgument): await ctx.send(f"Bad argument: {e}\n") -- cgit v1.2.3