From a61c2a96f4bb7ffb2c3d042f0f8f6bfee9d7a158 Mon Sep 17 00:00:00 2001 From: Jeongseop Lim Date: Tue, 20 Oct 2026 14:53:12 +0900 Subject: [PATCH 1/2] Count positional arguments before converting any FuncArgs::bind_for rejected surplus positional arguments only after FromArgs::from_args had converted every declared parameter. A conversion can run user code (__iter__ for PyIter, __index__ for ArgIndex), so filter(None, it, 0), hex(x, 1) and pairwise(it, 0) raised whatever that code raised instead of the TypeError CPython reports before it touches any argument. Check the count against FromArgs::arity() before from_args runs. For that bound to be safe: - The default arity() has no upper bound, since a manual impl may take positional arguments itself. KwArgs, (), DirFd, OpenDirFd and csv's FormatOptions declare the bound they actually have. - A derived struct adds the upper bound of each flattened field. - A tuple sums upper bounds with saturating_add. Assisted-by: Claude Code:claude-opus-5-5 --- crates/derive-impl/src/from_args.rs | 10 +++++++++- crates/stdlib/src/csv.rs | 4 ++++ crates/vm/src/function/argument.rs | 21 ++++++++++++++++++--- crates/vm/src/stdlib/os.rs | 8 ++++++++ extra_tests/snippets/vm_argument_errors.py | 20 +++++++++++++++++++- 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/crates/derive-impl/src/from_args.rs b/crates/derive-impl/src/from_args.rs index cb53dfa6e6f..a14f5d00a23 100644 --- a/crates/derive-impl/src/from_args.rs +++ b/crates/derive-impl/src/from_args.rs @@ -602,6 +602,12 @@ pub(crate) fn impl_from_args(input: DeriveInput) -> Result { .map(ArgAttribute::try_from) .collect::>>()?; let (min_arity, max_arity) = compute_arity_bounds(&field_attrs); + // A flattened field adds its own type's upper bound. + let flattened = struct_fields + .iter() + .zip(&field_attrs) + .filter(|(_, attr)| attr.kind == ParameterKind::Flatten) + .map(|(field, _)| &field.ty); let mut params = Vec::new(); for field in &struct_fields { @@ -619,7 +625,9 @@ pub(crate) fn impl_from_args(input: DeriveInput) -> Result { Some(&[#(#params),*]); fn arity() -> ::std::ops::RangeInclusive { - #min_arity..=#max_arity + #min_arity..=#max_arity #( + .saturating_add(*<#flattened as ::rustpython_vm::function::FromArgs>::arity().end()) + )* } fn from_args( diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index c7b3d9a014f..bf094d37f06 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -383,6 +383,10 @@ mod _csv { Param::var_keyword("fmtparams"), ]); + fn arity() -> core::ops::RangeInclusive { + 0..=1 + } + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { let res = Self { dialect: args.take_positional_keyword("dialect"), diff --git a/crates/vm/src/function/argument.rs b/crates/vm/src/function/argument.rs index 0df326c71ff..ff1ff98fdfd 100644 --- a/crates/vm/src/function/argument.rs +++ b/crates/vm/src/function/argument.rs @@ -307,6 +307,12 @@ impl FuncArgs { let arity = arity.start().saturating_sub(instance)..=arity.end().saturating_sub(instance); let num_given = self.args.len().saturating_sub(instance); + // Like CPython, too many positional arguments is reported before any + // argument is converted, so a conversion cannot run user code first. + if num_given > *arity.end() { + return Err(ArgumentError::TooManyArgs.into_exception(&arity, num_given, callee, vm)); + } + // A keyword that fills a parameter already given by position is // reported before leftover-keyword and conversion errors. if let Some(params) = T::PARAMS @@ -581,10 +587,11 @@ impl ArgumentError { pub trait FromArgs: Sized { /// The range of positional arguments permitted by the function signature. /// - /// Returns an empty range if not applicable. + /// The default has no upper bound, so a type that takes positional + /// arguments itself is never rejected before `from_args` runs. #[must_use] fn arity() -> RangeInclusive { - 0..=0 + 0..=usize::MAX } /// Parameters this type contributes to a text signature. @@ -799,6 +806,10 @@ where { const PARAMS: Option<&'static [Param]> = Some(&[Param::var_keyword(Name::NAME)]); + fn arity() -> RangeInclusive { + 0..=0 + } + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { let mut kwargs = KwArgsMap::default(); for (name, value) in args.remaining_keywords() { @@ -989,6 +1000,10 @@ where impl FromArgs for () { const PARAMS: Option<&'static [Param]> = Some(&[]); + fn arity() -> RangeInclusive { + 0..=0 + } + fn from_args(_vm: &VirtualMachine, _args: &mut FuncArgs) -> Result { Ok(()) } @@ -1020,7 +1035,7 @@ macro_rules! tuple_from_py_func_args { $( let (start, end) = $T::arity().into_inner(); min += start; - max += end; + max = usize::saturating_add(max, end); )+ min..=max } diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 3a69305d2d0..4152bd74351 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -97,6 +97,10 @@ impl<'fd, KW: DirFdKeyword> DirFd<'fd, 1, KW> { impl FromArgs for DirFd<'_, AVAILABLE, KW> { const PARAMS: Option<&'static [Param]> = Some(KW::PARAMS); + fn arity() -> core::ops::RangeInclusive { + 0..=0 + } + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { let fd = match args.take_keyword(KW::NAME) { Some(o) if vm.is_none(&o) => Ok(DEFAULT_DIR_FD), @@ -290,6 +294,10 @@ pub(super) mod _os { impl FromArgs for OpenDirFd { const PARAMS: Option<&'static [Param]> = Some(DefaultDirFd::PARAMS); + fn arity() -> core::ops::RangeInclusive { + 0..=0 + } + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { let fd = match args.take_keyword("dir_fd") { Some(o) if vm.is_none(&o) => AT_FDCWD, diff --git a/extra_tests/snippets/vm_argument_errors.py b/extra_tests/snippets/vm_argument_errors.py index 0fe6ff7fdce..d5925ac1e4e 100644 --- a/extra_tests/snippets/vm_argument_errors.py +++ b/extra_tests/snippets/vm_argument_errors.py @@ -10,7 +10,7 @@ import typing import weakref -from testutils import assert_raises +from testutils import TestFailingIter, assert_raises def check(exc_type, message, fn, *args, **kwargs): @@ -150,6 +150,23 @@ def test_a_keyword_check_of_its_own_says_the_same_thing(): ) +def test_too_many_arguments_are_counted_before_any_is_converted(): + class FailingIndex: + def __index__(self): + raise RuntimeError + + check( + TypeError, + "filter expected 2 arguments, got 3", + filter, + None, + TestFailingIter(), + 0, + ) + with assert_raises(TypeError): + hex(FailingIndex(), 1) + + test_the_message_names_the_function() test_a_method_does_not_count_its_instance() test_one_argument_is_singular() @@ -158,3 +175,4 @@ def test_a_keyword_check_of_its_own_says_the_same_thing(): test_a_type_is_named_by_the_type_it_builds() test_a_slot_that_counts_its_own_arguments_says_the_same_thing() test_a_keyword_check_of_its_own_says_the_same_thing() +test_too_many_arguments_are_counted_before_any_is_converted() From ad79873a2e86051d854118bae7cbfbf9439a7942 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 22 Oct 2026 21:02:01 +0900 Subject: [PATCH 2/2] Note why arity upper bounds saturate Comment that an unbounded upper bound is usize::MAX, which is why the tuple and derived arity() sums use saturating_add, and drop the CPython mention from the positional count check comment. Assisted-by: Claude Code:claude-opus-5-5 --- crates/derive-impl/src/from_args.rs | 3 ++- crates/vm/src/function/argument.rs | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/derive-impl/src/from_args.rs b/crates/derive-impl/src/from_args.rs index a14f5d00a23..a3f142ab7d7 100644 --- a/crates/derive-impl/src/from_args.rs +++ b/crates/derive-impl/src/from_args.rs @@ -602,7 +602,8 @@ pub(crate) fn impl_from_args(input: DeriveInput) -> Result { .map(ArgAttribute::try_from) .collect::>>()?; let (min_arity, max_arity) = compute_arity_bounds(&field_attrs); - // A flattened field adds its own type's upper bound. + // A flattened field adds its own type's upper bound. An unbounded one + // such as `PosArgs` is `usize::MAX`, so the sum saturates. let flattened = struct_fields .iter() .zip(&field_attrs) diff --git a/crates/vm/src/function/argument.rs b/crates/vm/src/function/argument.rs index ff1ff98fdfd..a66029b5993 100644 --- a/crates/vm/src/function/argument.rs +++ b/crates/vm/src/function/argument.rs @@ -307,8 +307,8 @@ impl FuncArgs { let arity = arity.start().saturating_sub(instance)..=arity.end().saturating_sub(instance); let num_given = self.args.len().saturating_sub(instance); - // Like CPython, too many positional arguments is reported before any - // argument is converted, so a conversion cannot run user code first. + // Too many positional arguments are reported before any argument is + // converted, so a conversion cannot run user code first. if num_given > *arity.end() { return Err(ArgumentError::TooManyArgs.into_exception(&arity, num_given, callee, vm)); } @@ -1035,6 +1035,7 @@ macro_rules! tuple_from_py_func_args { $( let (start, end) = $T::arity().into_inner(); min += start; + // An unbounded `end` is `usize::MAX`, so the sum saturates. max = usize::saturating_add(max, end); )+ min..=max