From 877a91ccbea2aafe61ad9ea8807fab87d35a89c5 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 2 Oct 2024 16:35:25 +0300 Subject: [PATCH] gh-65865: Raise early errors for invalid help strings in argparse --- Lib/argparse.py | 31 ++++++++++++++----- Lib/test/test_argparse.py | 29 +++++++++++++++++ ...4-10-02-16-35-07.gh-issue-65865.S2D4wq.rst | 3 ++ 3 files changed, 56 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 21299b69ecd74c..1ec42e884b2672 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -588,17 +588,20 @@ def _format_args(self, action, default_metavar): 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 @@ def add_parser(self, name, *, deprecated=False, **kwargs): 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 @@ def add_argument(self, *args, **kwargs): # 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): @@ -1631,6 +1639,14 @@ def _handle_conflict_resolve(self, action, conflicting_actions): 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): + raise ValueError('badly formed help string') + class _ArgumentGroup(_ActionsContainer): @@ -1848,6 +1864,7 @@ def add_subparsers(self, **kwargs): # 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 1bf812b36fc2c6..0b4ae669245456 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -2610,6 +2610,27 @@ def test_parser_command_help(self): --foo foo help ''')) + def test_invalid_subparsers_help(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + with self.assertRaisesRegex(ValueError, 'badly formed help string'): + parser.add_subparsers(help='%Y-%m-%d') + parser = ErrorRaisingArgumentParser(prog='PROG') + with self.assertRaisesRegex(ValueError, 'badly formed help string'): + parser.add_subparsers(help='%(spam)s') + parser = ErrorRaisingArgumentParser(prog='PROG') + with self.assertRaisesRegex(ValueError, 'badly formed help string'): + parser.add_subparsers(help='%(prog)d') + + def test_invalid_subparser_help(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + subparsers = parser.add_subparsers() + with self.assertRaisesRegex(ValueError, 'badly formed help string'): + subparsers.add_parser('1', help='%Y-%m-%d') + with self.assertRaisesRegex(ValueError, 'badly formed help string'): + subparsers.add_parser('1', help='%(spam)s') + with self.assertRaisesRegex(ValueError, 'badly formed help string'): + subparsers.add_parser('1', help='%(prog)d') + def test_subparser_title_help(self): parser = ErrorRaisingArgumentParser(prog='PROG', description='main description') @@ -5333,6 +5354,14 @@ def test_invalid_action(self): 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 00000000000000..106a8b81140520 --- /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`.