Skip to content

Commit

Permalink
pythongh-65865: Raise early errors for invalid help strings in argparse
Browse files Browse the repository at this point in the history
  • Loading branch information
serhiy-storchaka committed Oct 2, 2024
1 parent 2c050d4 commit 877a91c
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 7 deletions.
31 changes: 24 additions & 7 deletions Lib/argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):

Expand Down Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions Lib/test/test_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
@@ -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`.

0 comments on commit 877a91c

Please sign in to comment.