From 47a714c16be9382c20abd1604023762049307d78 Mon Sep 17 00:00:00 2001 From: arpitjain099 Date: Fri, 17 Jul 2026 05:55:20 +0900 Subject: [PATCH] fix: prevent crashes when unparsing count, extend, and append actions Three action unparsers raised on valid input reachable through the public get_effective_command_line_invocation(): - _unparse_count_action multiplied the flag body by the stored value. When a count option was not given and its default was None, the value was None and the multiplication raised TypeError. A default of 0 (or a value equal to a nonzero default) produced a bare prefix character like "-" instead of omitting the unused flag. - _unparse_extend_action passed the stored values straight into " ".join, so a typed option (for example type=int) raised TypeError, and unlike the append unparser it never quoted values, so entries containing spaces broke the round trip. - _unparse_append_action indexed values[0] to detect nested lists, which raised IndexError when an append option with default=[] was not given. Each case now emits the correct effective invocation or nothing when the option was not supplied. Added regression tests covering all three. Signed-off-by: arpitjain099 --- reverse_argparse/reverse_argparse.py | 11 ++++++- test/test_reverse_argparse.py | 46 ++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/reverse_argparse/reverse_argparse.py b/reverse_argparse/reverse_argparse.py index fd89047..44b9538 100644 --- a/reverse_argparse/reverse_argparse.py +++ b/reverse_argparse/reverse_argparse.py @@ -400,6 +400,8 @@ def _unparse_append_action(self, action: Action) -> None: flag = self._get_option_string(action) if not isinstance(values, list): values = [values] + if not values: + return result = [] if isinstance(values[0], list): for entry in values: @@ -433,7 +435,11 @@ def _unparse_count_action(self, action: Action) -> None: action: The :class:`_CountAction` in question. """ value = getattr(self._namespace, action.dest) + if value is None: + return count = value if action.default is None else (value - action.default) + if count <= 0: + return flag = self._get_option_string(action, prefer_short=True) if ( len(flag) == SHORT_OPTION_LENGTH @@ -494,7 +500,10 @@ def _unparse_extend_action(self, action: Action) -> None: values = getattr(self._namespace, action.dest) if values is not None: self._append_list_of_args( - [self._get_option_string(action), *values] + [ + self._get_option_string(action), + *(quote_arg_if_necessary(str(value)) for value in values), + ] ) def _unparse_boolean_optional_action(self, action: Action) -> None: diff --git a/test/test_reverse_argparse.py b/test/test_reverse_argparse.py index da4fb60..02f6f98 100644 --- a/test/test_reverse_argparse.py +++ b/test/test_reverse_argparse.py @@ -503,6 +503,7 @@ def test__unparse_store_false_action( "--foo bar baz --foo bif", [" --foo bar baz", " --foo bif"], ), + (["--foo"], {"action": "append", "default": []}, "", []), ], ) def test__unparse_append_action( @@ -556,10 +557,16 @@ def test__unparse_append_const_action(args: str, expected: str | None) -> None: "-vv", " -vv", ), + (["--verbose", "-v"], {"action": "count"}, "", None), + (["--verbose", "-v"], {"action": "count", "default": 0}, "", None), + (["--verbose", "-v"], {"action": "count", "default": 2}, "", None), ], ) def test__unparse_count_action( - add_args: list[str], add_kwargs: dict[str, Any], args: str, expected: str + add_args: list[str], + add_kwargs: dict[str, Any], + args: str, + expected: str | None, ) -> None: """Ensure ``count`` actions are handled appropriately.""" parser = ArgumentParser() @@ -567,7 +574,7 @@ def test__unparse_count_action( namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_count_action(action) - assert unparser._args[1:] == [expected] + assert unparser._args[1:] == ([expected] if expected is not None else []) @pytest.mark.parametrize( @@ -645,13 +652,40 @@ def test__unparse_sub_parsers_action_nested() -> None: assert result == pretty -def test__unparse_extend_action() -> None: +@pytest.mark.parametrize( + ("add_args", "add_kwargs", "args", "expected"), + [ + ( + ["--foo"], + {"action": "extend", "nargs": "*"}, + "--foo bar --foo baz bif", + " --foo bar baz bif", + ), + ( + ["--nums"], + {"action": "extend", "nargs": "+", "type": int}, + "--nums 1 2", + " --nums 1 2", + ), + ( + ["--words"], + {"action": "extend", "nargs": "+"}, + "--words 'a b' c", + " --words 'a b' c", + ), + ], +) +def test__unparse_extend_action( + add_args: list[str], + add_kwargs: dict[str, Any], + args: str, + expected: str, +) -> None: """Ensure ``extend`` actions are handled appropriately.""" parser = ArgumentParser() - action = parser.add_argument("--foo", action="extend", nargs="*") - namespace = parser.parse_args(shlex.split("--foo bar --foo baz bif")) + action = parser.add_argument(*add_args, **add_kwargs) + namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) - expected = " --foo bar baz bif" unparser._unparse_extend_action(action) assert unparser._args[1:] == [expected]