From c2c4dfa03742f08745e2b6858aa6ad7f5fefd8ee Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 26 Mar 2026 17:28:20 +0100 Subject: [PATCH 1/2] gh-53584: Add reproducers for greedy opts issue Most of these are marked as expected fail for now, pending the fix. Signed-off-by: Stephen Finucane --- Lib/test/test_argparse.py | 108 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index e0c32976fd6f0d..63edb97b5e2a46 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -664,6 +664,17 @@ class TestOptionalsNargs3(ParserTestCase): ('-x a b c', NS(x=['a', 'b', 'c'])), ] + def test_does_not_steal_required_positional(self): + # https://github.com/python/cpython/issues/53584 + # Fixed integer nargs is never greedy: --foo takes exactly N args and + # leaves the remainder for positionals. + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs=2) + parser.add_argument('bar') + args = parser.parse_args(['--foo', 'a', 'b', 'c']) + self.assertEqual(args.foo, ['a', 'b']) + self.assertEqual(args.bar, 'c') + class TestOptionalsNargsOptional(ParserTestCase): """Tests specifying an Optional arg for an Optional""" @@ -687,6 +698,51 @@ class TestOptionalsNargsOptional(ParserTestCase): ('-z 2', NS(w=None, x=None, y='spam', z=2)), ] + @unittest.expectedFailure + def test_does_not_steal_required_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar') + args = parser.parse_args(['--foo', 'abc']) + self.assertIsNone(args.foo) + self.assertEqual(args.bar, 'abc') + + @unittest.expectedFailure + def test_does_not_steal_two_required_positionals(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar') + parser.add_argument('bax') + args = parser.parse_args(['--foo', 'a', 'b']) + self.assertIsNone(args.foo) + self.assertEqual(args.bar, 'a') + self.assertEqual(args.bax, 'b') + + @unittest.expectedFailure + def test_does_not_steal_two_required_positional_interspersed(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('--bar', nargs='?') + parser.add_argument('baz') + parser.add_argument('bax') + args = parser.parse_args(['--foo', 'a', '--bar', 'b']) + self.assertEqual(args.baz, 'a') + self.assertEqual(args.bax, 'b') + + def test_greedy_when_positional_also_optional(self): + # https://github.com/python/cpython/issues/53584 + # When the following positional is also optional, --foo taking the arg + # is acceptable (both are optional, no unambiguous correct answer). + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar', nargs='?') + args = parser.parse_args(['--foo', 'abc']) + self.assertEqual(args.foo, 'abc') + self.assertIsNone(args.bar) + class TestOptionalsNargsZeroOrMore(ParserTestCase): """Tests specifying args for an Optional that accepts zero or more""" @@ -706,6 +762,16 @@ class TestOptionalsNargsZeroOrMore(ParserTestCase): ('-y a b', NS(x=None, y=['a', 'b'])), ] + @unittest.expectedFailure + def test_does_not_steal_required_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='*') + parser.add_argument('bar') + args = parser.parse_args(['--foo', 'abc']) + self.assertEqual(args.foo, []) + self.assertEqual(args.bar, 'abc') + class TestOptionalsNargsOneOrMore(ParserTestCase): """Tests specifying args for an Optional that accepts one or more""" @@ -723,6 +789,29 @@ class TestOptionalsNargsOneOrMore(ParserTestCase): ('-y a b', NS(x=None, y=['a', 'b'])), ] + @unittest.expectedFailure + def test_does_not_steal_required_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='+') + parser.add_argument('bar') + args = parser.parse_args(['--foo', 'a', 'b']) + self.assertEqual(args.foo, ['a']) + self.assertEqual(args.bar, 'b') + + @unittest.expectedFailure + def test_does_not_steal_interspersed_required_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='+') + parser.add_argument('--bar', nargs='+') + parser.add_argument('baz') + parser.add_argument('bax') + args = parser.parse_args(['--foo', 'a', 'c', '--bar', 'b', 'd']) + self.assertEqual(args.foo, ['a']) + self.assertEqual(args.bar, ['b']) + self.assertEqual(args.baz, 'c') + self.assertEqual(args.bax, 'd') class TestOptionalsChoices(ParserTestCase): """Tests specifying the choices for an Optional""" @@ -6760,6 +6849,25 @@ def test_invalid_args(self): parser = ErrorRaisingArgumentParser(prog='PROG') self.assertRaises(ArgumentParserError, parser.parse_intermixed_args, ['a']) + @unittest.expectedFailure + def test_variable_nargs_option_before_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar') + args = parser.parse_intermixed_args(['--foo', 'abc']) + self.assertIsNone(args.foo) + self.assertEqual(args.bar, 'abc') + + def test_variable_nargs_option_after_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar') + args = parser.parse_intermixed_args(['abc', '--foo', 'xyz']) + self.assertEqual(args.foo, 'xyz') + self.assertEqual(args.bar, 'abc') + class TestIntermixedMessageContentError(TestCase): # case where Intermixed gives different error message From ec1411644f46fe374b50f03f4d1939cd7974cb5c Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 26 Mar 2026 17:36:22 +0100 Subject: [PATCH 2/2] gh-53584: Prevent variable-nargs options from stealing required positional args Options with nargs='?', nargs='*', or nargs='+' were greedily consuming argument strings that should have been reserved for required positional arguments. For example: parser.add_argument('--foo', nargs='?') parser.add_argument('bar') parser.parse_args(['--foo', 'abc']) # bar got nothing argparse works by pattern-matching, with the bulk of its logic defined in _parse_known_args(). That method encodes each argument string as a single character ('O' for a recognised option flag, 'A' for everything else, and '-' for strings following '--') which gives us a resulting string pattern (e.g. 'OAOA'). We then "consume" arguments using this string pattern, handling both positionals (via the consume_positionals() closure) and optionals (via consume_optional()). The bug arises in the latter. consume_optional() processes options by building a regex based on the option's nargs (e.g. '-*A?-*' for nargs='?') and then runs this regex against the remainder of the string pattern (i.e. anything following the option flag). This is done via _match_argument(). The regex will always stop at the next option flag ('O' token) but for non-fixed nargs values like '?' and '+' may greedily consume every positional ('A' token) up to that point. This fix works by manipulating the string pattern as part of our optional consumption. Any 'A' tokens that are required by remaining positionals are masked to 'O' to prevent the regex consuming them. Masking will only consider tokens up to the next option flag ('O') and it both accounts for what future options can absorb (to avoid masking more than is necessary) and ensures that at least the minimum arguments required for the optional are actually consumed. In addition, we also handle the parse_intermixed_args() case. In intermixed mode, positional-typed arguments already collected to the left of the current option are also accounted for, since they will satisfy positionals in the second-pass parse. Signed-off-by: Stephen Finucane --- Lib/argparse.py | 93 +++++++++++++++++++ Lib/test/test_argparse.py | 7 -- ...6-03-27-12-54-44.gh-issue-53584.4mA7Wp.rst | 3 + 3 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-03-27-12-54-44.gh-issue-53584.4mA7Wp.rst diff --git a/Lib/argparse.py b/Lib/argparse.py index d91707d9eec546..d83b167036a0d5 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -2199,6 +2199,17 @@ def take_action(action, argument_strings, option_string=None): if argument_values is not SUPPRESS: action(self, namespace, argument_values, option_string) + # returns the minimum number of arg strings an action requires; + # used by consume_optional() when computing positional reservations + def min_for_action(action): + nargs = action.nargs + if nargs is None or nargs in (ONE_OR_MORE, PARSER): + return 1 + elif isinstance(nargs, int): + return nargs + # OPTIONAL, ZERO_OR_MORE, REMAINDER contribute 0 + return 0 + # function to convert arg_strings into an optional action def consume_optional(start_index): @@ -2282,6 +2293,88 @@ def consume_optional(start_index): else: start = start_index + 1 selected_patterns = arg_strings_pattern[start:] + + # Reserve the minimum number of args required by the + # remaining positionals; replace excess 'A' tokens with 'O' + # so that match_argument cannot consume them. + # + # Note that the current option can only consume 'A' tokens + # up to the next 'O' (another option flag) in the pattern + # as the regex itself stops there. Only count tokens in + # that segment -- 'A' tokens in later segments belong to + # other options' territories and are handled when those + # options are processed. + first_o = selected_patterns.find('O') + if first_o == -1: + segment, rest = selected_patterns, '' + else: + segment = selected_patterns[:first_o] + rest = selected_patterns[first_o:] + segment_arg_count = segment.count('A') + + # Compute the minimum number of arg strings still required + # by the remaining positionals to avoid greedy consumption + min_pos = sum(min_for_action(pos) for pos in positionals) + if intermixed: + # In intermixed mode consume_positionals is never + # called during the main loop, so `positionals` never + # shrinks. Positional-typed tokens already collected in + # extras will satisfy the positional requirement in the + # second pass, so don't count them as still-unsatisfied + extras_arg_count = sum( + 1 for c in extras_pattern if c == 'A' + ) + min_pos = max(0, min_pos - extras_arg_count) + + if min_pos > 0 and segment_arg_count > 0: + # Compute the maximum positional args that future + # option segments can absorb, accounting for each + # future option's own minimum nargs requirement. + pattern_end = len(arg_strings_pattern) + later_option_string_indices = sorted( + i for i in option_string_indices + if i > start_index + ) + later_arg_capacity = 0 + for k, later_option_string_index in enumerate( + later_option_string_indices + ): + seg_start = later_option_string_index + 1 + seg_stop = ( + later_option_string_indices[k + 1] + if k + 1 < len(later_option_string_indices) + else pattern_end + ) + seg_arg_count = arg_strings_pattern[seg_start:seg_stop].count('A') + later_option_tuples = option_string_indices[later_option_string_index] + later_action = later_option_tuples[0][0] + later_explicit_arg = later_option_tuples[0][3] + if later_explicit_arg is not None or later_action is None: + # explicit arg (--opt=val) or unknown action: + # consumes no pattern 'A' tokens + later_min_arg_count = 0 + else: + later_min_arg_count = min_for_action(later_action) + later_arg_capacity += max( + 0, seg_arg_count - later_min_arg_count + ) + + reserved_arg_count = max(0, min_pos - later_arg_capacity) + max_arg_count = max( + min_for_action(action), + segment_arg_count - reserved_arg_count, + ) + if max_arg_count < segment_arg_count: + count = 0 + limited = [] + for c in segment: + if c == 'A': + count += 1 + if count > max_arg_count: + c = 'O' + limited.append(c) + selected_patterns = ''.join(limited) + rest + arg_count = match_argument(action, selected_patterns) stop = start + arg_count args = arg_strings[start:stop] diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index 63edb97b5e2a46..e94118117c548f 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -698,7 +698,6 @@ class TestOptionalsNargsOptional(ParserTestCase): ('-z 2', NS(w=None, x=None, y='spam', z=2)), ] - @unittest.expectedFailure def test_does_not_steal_required_positional(self): # https://github.com/python/cpython/issues/53584 parser = ErrorRaisingArgumentParser() @@ -708,7 +707,6 @@ def test_does_not_steal_required_positional(self): self.assertIsNone(args.foo) self.assertEqual(args.bar, 'abc') - @unittest.expectedFailure def test_does_not_steal_two_required_positionals(self): # https://github.com/python/cpython/issues/53584 parser = ErrorRaisingArgumentParser() @@ -720,7 +718,6 @@ def test_does_not_steal_two_required_positionals(self): self.assertEqual(args.bar, 'a') self.assertEqual(args.bax, 'b') - @unittest.expectedFailure def test_does_not_steal_two_required_positional_interspersed(self): # https://github.com/python/cpython/issues/53584 parser = ErrorRaisingArgumentParser() @@ -762,7 +759,6 @@ class TestOptionalsNargsZeroOrMore(ParserTestCase): ('-y a b', NS(x=None, y=['a', 'b'])), ] - @unittest.expectedFailure def test_does_not_steal_required_positional(self): # https://github.com/python/cpython/issues/53584 parser = ErrorRaisingArgumentParser() @@ -789,7 +785,6 @@ class TestOptionalsNargsOneOrMore(ParserTestCase): ('-y a b', NS(x=None, y=['a', 'b'])), ] - @unittest.expectedFailure def test_does_not_steal_required_positional(self): # https://github.com/python/cpython/issues/53584 parser = ErrorRaisingArgumentParser() @@ -799,7 +794,6 @@ def test_does_not_steal_required_positional(self): self.assertEqual(args.foo, ['a']) self.assertEqual(args.bar, 'b') - @unittest.expectedFailure def test_does_not_steal_interspersed_required_positional(self): # https://github.com/python/cpython/issues/53584 parser = ErrorRaisingArgumentParser() @@ -6849,7 +6843,6 @@ def test_invalid_args(self): parser = ErrorRaisingArgumentParser(prog='PROG') self.assertRaises(ArgumentParserError, parser.parse_intermixed_args, ['a']) - @unittest.expectedFailure def test_variable_nargs_option_before_positional(self): # https://github.com/python/cpython/issues/53584 parser = ErrorRaisingArgumentParser() diff --git a/Misc/NEWS.d/next/Library/2026-03-27-12-54-44.gh-issue-53584.4mA7Wp.rst b/Misc/NEWS.d/next/Library/2026-03-27-12-54-44.gh-issue-53584.4mA7Wp.rst new file mode 100644 index 00000000000000..b322b6abbd7428 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-03-27-12-54-44.gh-issue-53584.4mA7Wp.rst @@ -0,0 +1,3 @@ +Fix :mod:`argparse` options with variable :ref:`nargs ` (``'?'``, +``'*'``, ``'+'``) greedily consuming argument strings that should be +reserved for required positional arguments.