diff --git a/crates/derive-impl/src/from_args.rs b/crates/derive-impl/src/from_args.rs index cb53dfa6e6f..a3f142ab7d7 100644 --- a/crates/derive-impl/src/from_args.rs +++ b/crates/derive-impl/src/from_args.rs @@ -602,6 +602,13 @@ 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. An unbounded one + // such as `PosArgs` is `usize::MAX`, so the sum saturates. + 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 +626,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..a66029b5993 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); + // 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)); + } + // 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,8 @@ macro_rules! tuple_from_py_func_args { $( let (start, end) = $T::arity().into_inner(); min += start; - max += end; + // An unbounded `end` is `usize::MAX`, so the sum saturates. + 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()