From a068da095301398cdf7812b9c88c93d304899733 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Mon, 18 Dec 2023 17:00:01 -0800 Subject: [PATCH] [BACKEND] Validate override variable names --- src/ytdl_sub/config/overrides.py | 15 ++++++-- .../entries/variables/override_variables.py | 13 +++++++ .../validators/source_variable_validator.py | 13 ++++--- .../validators/string_formatter_validators.py | 34 ------------------- tests/unit/config/test_preset.py | 27 +++++++++++++++ tests/unit/plugins/test_regex_capture.py | 1 - 6 files changed, 61 insertions(+), 42 deletions(-) delete mode 100644 tests/unit/plugins/test_regex_capture.py diff --git a/src/ytdl_sub/config/overrides.py b/src/ytdl_sub/config/overrides.py index a1ef2f81b..340413f6e 100644 --- a/src/ytdl_sub/config/overrides.py +++ b/src/ytdl_sub/config/overrides.py @@ -11,6 +11,7 @@ from ytdl_sub.entries.variables.override_variables import OverrideVariables from ytdl_sub.script.parser import parse from ytdl_sub.script.script import Script +from ytdl_sub.utils.exceptions import InvalidVariableNameException from ytdl_sub.utils.exceptions import ValidationException from ytdl_sub.utils.script import ScriptUtils from ytdl_sub.utils.scriptable import Scriptable @@ -86,16 +87,26 @@ def ensure_variable_name_valid(self, name: str) -> None: """ Ensures the variable name does not collide with any entry variables or built-in functions. """ + if not OverrideVariables.is_valid_name(name): + override_type = "function" if name.startswith("%") else "variable" + raise self._validation_exception( + f"Override {override_type} with name {name} is invalid. Names must be" + " lower_snake_cased and begin with a letter.", + exception_class=InvalidVariableNameException, + ) + if OverrideVariables.is_entry_variable_name(name): raise self._validation_exception( f"Override variable with name {name} cannot be used since it is a" - " built-in ytdl-sub entry variable name." + " built-in ytdl-sub entry variable name.", + exception_class=InvalidVariableNameException, ) if OverrideVariables.is_function_name(name): raise self._validation_exception( f"Override function definition with name {name} cannot be used since it is" - " a built-in ytdl-sub function name." + " a built-in ytdl-sub function name.", + exception_class=InvalidVariableNameException, ) def initial_variables( diff --git a/src/ytdl_sub/entries/variables/override_variables.py b/src/ytdl_sub/entries/variables/override_variables.py index b16bf68ad..a798dd185 100644 --- a/src/ytdl_sub/entries/variables/override_variables.py +++ b/src/ytdl_sub/entries/variables/override_variables.py @@ -1,6 +1,7 @@ from ytdl_sub.entries.script.function_scripts import CUSTOM_FUNCTION_SCRIPTS from ytdl_sub.entries.script.variable_scripts import VARIABLE_SCRIPTS from ytdl_sub.script.functions import Functions +from ytdl_sub.script.utils.name_validation import is_valid_name SUBSCRIPTION_NAME = "subscription_name" SUBSCRIPTION_VALUE = "subscription_value" @@ -81,3 +82,15 @@ def is_function_name(cls, name: str) -> bool: if name.startswith("%"): return name in CUSTOM_FUNCTION_SCRIPTS or Functions.is_built_in(name[1:]) return False + + @classmethod + def is_valid_name(cls, name: str) -> bool: + """ + Returns + ------- + True if the override name itself is valid. False otherwise. + """ + if name.startswith("%"): + return is_valid_name(name=name[1:]) + + return is_valid_name(name=name) diff --git a/src/ytdl_sub/validators/source_variable_validator.py b/src/ytdl_sub/validators/source_variable_validator.py index 0050779d1..ba17c8956 100644 --- a/src/ytdl_sub/validators/source_variable_validator.py +++ b/src/ytdl_sub/validators/source_variable_validator.py @@ -1,5 +1,5 @@ +from ytdl_sub.script.utils.name_validation import is_valid_name from ytdl_sub.utils.exceptions import InvalidVariableNameException -from ytdl_sub.validators.string_formatter_validators import is_valid_source_variable_name from ytdl_sub.validators.validators import ListValidator from ytdl_sub.validators.validators import StringValidator @@ -9,10 +9,13 @@ class SourceVariableNameValidator(StringValidator): def __init__(self, name, value): super().__init__(name, value) - try: - _ = is_valid_source_variable_name(self.value, raise_exception=True) - except InvalidVariableNameException as exc: - raise self._validation_exception(exc) from exc + + if not is_valid_name(value): + raise self._validation_exception( + f"Variable with name {name} is invalid. Names must be" + " lower_snake_cased and begin with a letter.", + exception_class=InvalidVariableNameException, + ) class SourceVariableNameListValidator(ListValidator[SourceVariableNameValidator]): diff --git a/src/ytdl_sub/validators/string_formatter_validators.py b/src/ytdl_sub/validators/string_formatter_validators.py index 59caabe3f..7957169d4 100644 --- a/src/ytdl_sub/validators/string_formatter_validators.py +++ b/src/ytdl_sub/validators/string_formatter_validators.py @@ -1,4 +1,3 @@ -import re from typing import Dict from typing import Set from typing import Union @@ -9,7 +8,6 @@ from ytdl_sub.script.script import Script from ytdl_sub.script.utils.exceptions import UserException from ytdl_sub.script.utils.exceptions import VariableDoesNotExist -from ytdl_sub.utils.exceptions import InvalidVariableNameException from ytdl_sub.utils.exceptions import StringFormattingVariableNotFoundException from ytdl_sub.validators.validators import DictValidator from ytdl_sub.validators.validators import ListValidator @@ -17,38 +15,6 @@ from ytdl_sub.validators.validators import StringValidator from ytdl_sub.validators.validators import Validator -_fields_validator = re.compile(r"{([a-z][a-z0-9_]*?)}") - -_fields_validator_exception_message: str = ( - "{variable_names} must start with a lowercase letter, should only contain lowercase letters, " - "numbers, underscores, and have a single open and close bracket." -) - - -def is_valid_source_variable_name(input_str: str, raise_exception: bool = False) -> bool: - """ - Parameters - ---------- - input_str - String to see if it can be a source variable - raise_exception - Raise InvalidVariableNameException False. - - Returns - ------- - True if it is. False otherwise. - - Raises - ------ - InvalidVariableNameException - If raise_exception and output is False - """ - # Add brackets around it to pretend its a StringFormatter, see if it captures - is_source_variable_name = len(re.findall(_fields_validator, f"{{{input_str}}}")) > 0 - if not is_source_variable_name and raise_exception: - raise InvalidVariableNameException(_fields_validator_exception_message) - return is_source_variable_name - class StringFormatterValidator(StringValidator): """ diff --git a/tests/unit/config/test_preset.py b/tests/unit/config/test_preset.py index 239a46c65..20c64214d 100644 --- a/tests/unit/config/test_preset.py +++ b/tests/unit/config/test_preset.py @@ -334,6 +334,33 @@ def test_preset_error_override_added_variable_collides_with_override( }, ) + @pytest.mark.parametrize( + "name", ["!ack", "*asfsaf", "1234352", "--234asdf", "___asdf", "1asdfasdfasd"] + ) + @pytest.mark.parametrize("is_function", [True, False]) + def test_preset_error_overrides_invalid_variable_name( + self, config_file, youtube_video, output_options, name: str, is_function: bool + ): + name_type = "function" if is_function else "variable" + name = f"%{name}" if is_function else name + + with pytest.raises( + ValidationException, + match=re.escape( + f"Override {name_type} with name {name} is invalid." + " Names must be lower_snake_cased and begin with a letter." + ), + ): + _ = Preset( + config=config_file, + name="test", + value={ + "download": youtube_video, + "output_options": output_options, + "overrides": {name: "ack"}, + }, + ) + def test_preset_error_added_url_variable_cannot_resolve(self, config_file, output_options): with pytest.raises( ValidationException, diff --git a/tests/unit/plugins/test_regex_capture.py b/tests/unit/plugins/test_regex_capture.py deleted file mode 100644 index 8b1378917..000000000 --- a/tests/unit/plugins/test_regex_capture.py +++ /dev/null @@ -1 +0,0 @@ -