From eb2d268ac7480b5e2b4ffb9a644cad7ac75ae954 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 12 Oct 2024 13:10:50 +0300 Subject: gh-65865: Raise early errors for invalid help strings in argparse (GH-124899) --- Lib/argparse.py | 31 +++++++++++++++++----- Lib/test/test_argparse.py | 31 ++++++++++++++++++++++ .../2024-10-02-16-35-07.gh-issue-65865.S2D4wq.rst | 3 +++ 3 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-10-02-16-35-07.gh-issue-65865.S2D4wq.rst diff --git a/Lib/argparse.py b/Lib/argparse.py index 2d8a7ef..208c182 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -588,17 +588,20 @@ class HelpFormatter(object): return result def _expand_help(self, action): + help_string = self._get_help_string(action) + if '%' not in help_string: + return help_string params = dict(vars(action), prog=self._prog) for name in list(params): - if params[name] is SUPPRESS: + value = params[name] + if value is SUPPRESS: del params[name] - for name in list(params): - if hasattr(params[name], '__name__'): - params[name] = params[name].__name__ + elif hasattr(value, '__name__'): + params[name] = value.__name__ if params.get('choices') is not None: choices_str = ', '.join([str(c) for c in params['choices']]) params['choices'] = choices_str - return self._get_help_string(action) % params + return help_string % params def _iter_indented_subactions(self, action): try: @@ -1180,9 +1183,13 @@ class _SubParsersAction(Action): help = kwargs.pop('help') choice_action = self._ChoicesPseudoAction(name, aliases, help) self._choices_actions.append(choice_action) + else: + choice_action = None # create the parser and add it to the map parser = self._parser_class(**kwargs) + if choice_action is not None: + parser._check_help(choice_action) self._name_parser_map[name] = parser # make parser available under aliases also @@ -1449,11 +1456,12 @@ class _ActionsContainer(object): # raise an error if the metavar does not match the type if hasattr(self, "_get_formatter"): + formatter = self._get_formatter() try: - self._get_formatter()._format_args(action, None) + formatter._format_args(action, None) except TypeError: raise ValueError("length of metavar tuple does not match nargs") - + self._check_help(action) return self._add_action(action) def add_argument_group(self, *args, **kwargs): @@ -1635,6 +1643,14 @@ class _ActionsContainer(object): if not action.option_strings: action.container._remove_action(action) + def _check_help(self, action): + if action.help and hasattr(self, "_get_formatter"): + formatter = self._get_formatter() + try: + formatter._expand_help(action) + except (ValueError, TypeError, KeyError) as exc: + raise ValueError('badly formed help string') from exc + class _ArgumentGroup(_ActionsContainer): @@ -1852,6 +1868,7 @@ class ArgumentParser(_AttributeHolder, _ActionsContainer): # create the parsers action and add it to the positionals list parsers_class = self._pop_action_class(kwargs, 'parsers') action = parsers_class(option_strings=[], **kwargs) + self._check_help(action) self._subparsers._add_action(action) # return the created parsers action diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index 1ebbc21..000b810 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -2623,6 +2623,29 @@ class TestAddSubparsers(TestCase): --foo foo help ''')) + def assert_bad_help(self, context_type, func, *args, **kwargs): + with self.assertRaisesRegex(ValueError, 'badly formed help string') as cm: + func(*args, **kwargs) + self.assertIsInstance(cm.exception.__context__, context_type) + + def test_invalid_subparsers_help(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + self.assert_bad_help(ValueError, parser.add_subparsers, help='%Y-%m-%d') + parser = ErrorRaisingArgumentParser(prog='PROG') + self.assert_bad_help(KeyError, parser.add_subparsers, help='%(spam)s') + parser = ErrorRaisingArgumentParser(prog='PROG') + self.assert_bad_help(TypeError, parser.add_subparsers, help='%(prog)d') + + def test_invalid_subparser_help(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + subparsers = parser.add_subparsers() + self.assert_bad_help(ValueError, subparsers.add_parser, '1', + help='%Y-%m-%d') + self.assert_bad_help(KeyError, subparsers.add_parser, '1', + help='%(spam)s') + self.assert_bad_help(TypeError, subparsers.add_parser, '1', + help='%(prog)d') + def test_subparser_title_help(self): parser = ErrorRaisingArgumentParser(prog='PROG', description='main description') @@ -5375,6 +5398,14 @@ class TestInvalidArgumentConstructors(TestCase): self.assertValueError('--foo', action="store-true", errmsg='unknown action') + def test_invalid_help(self): + self.assertValueError('--foo', help='%Y-%m-%d', + errmsg='badly formed help string') + self.assertValueError('--foo', help='%(spam)s', + errmsg='badly formed help string') + self.assertValueError('--foo', help='%(prog)d', + errmsg='badly formed help string') + def test_multiple_dest(self): parser = argparse.ArgumentParser() parser.add_argument(dest='foo') diff --git a/Misc/NEWS.d/next/Library/2024-10-02-16-35-07.gh-issue-65865.S2D4wq.rst b/Misc/NEWS.d/next/Library/2024-10-02-16-35-07.gh-issue-65865.S2D4wq.rst new file mode 100644 index 0000000..106a8b8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-10-02-16-35-07.gh-issue-65865.S2D4wq.rst @@ -0,0 +1,3 @@ +:mod:`argparse` now raises early error for invalid ``help`` arguments to +:meth:`~argparse.ArgumentParser.add_argument`, +:meth:`~argparse.ArgumentParser.add_subparsers` and :meth:`!add_parser`. -- cgit v0.12