Skip to content

Commit a61c2a9

Browse files
committed
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
1 parent 398dd70 commit a61c2a9

5 files changed

Lines changed: 58 additions & 5 deletions

File tree

‎crates/derive-impl/src/from_args.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,12 @@ pub(crate) fn impl_from_args(input: DeriveInput) -> Result<TokenStream> {
602602
.map(ArgAttribute::try_from)
603603
.collect::<Result<Vec<_>>>()?;
604604
let (min_arity, max_arity) = compute_arity_bounds(&field_attrs);
605+
// A flattened field adds its own type's upper bound.
606+
let flattened = struct_fields
607+
.iter()
608+
.zip(&field_attrs)
609+
.filter(|(_, attr)| attr.kind == ParameterKind::Flatten)
610+
.map(|(field, _)| &field.ty);
605611

606612
let mut params = Vec::new();
607613
for field in &struct_fields {
@@ -619,7 +625,9 @@ pub(crate) fn impl_from_args(input: DeriveInput) -> Result<TokenStream> {
619625
Some(&[#(#params),*]);
620626

621627
fn arity() -> ::std::ops::RangeInclusive<usize> {
622-
#min_arity..=#max_arity
628+
#min_arity..=#max_arity #(
629+
.saturating_add(*<#flattened as ::rustpython_vm::function::FromArgs>::arity().end())
630+
)*
623631
}
624632

625633
fn from_args(

‎crates/stdlib/src/csv.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,10 @@ mod _csv {
383383
Param::var_keyword("fmtparams"),
384384
]);
385385

386+
fn arity() -> core::ops::RangeInclusive<usize> {
387+
0..=1
388+
}
389+
386390
fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result<Self, ArgumentError> {
387391
let res = Self {
388392
dialect: args.take_positional_keyword("dialect"),

‎crates/vm/src/function/argument.rs‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,12 @@ impl FuncArgs {
307307
let arity = arity.start().saturating_sub(instance)..=arity.end().saturating_sub(instance);
308308
let num_given = self.args.len().saturating_sub(instance);
309309

310+
// Like CPython, too many positional arguments is reported before any
311+
// argument is converted, so a conversion cannot run user code first.
312+
if num_given > *arity.end() {
313+
return Err(ArgumentError::TooManyArgs.into_exception(&arity, num_given, callee, vm));
314+
}
315+
310316
// A keyword that fills a parameter already given by position is
311317
// reported before leftover-keyword and conversion errors.
312318
if let Some(params) = T::PARAMS
@@ -581,10 +587,11 @@ impl ArgumentError {
581587
pub trait FromArgs: Sized {
582588
/// The range of positional arguments permitted by the function signature.
583589
///
584-
/// Returns an empty range if not applicable.
590+
/// The default has no upper bound, so a type that takes positional
591+
/// arguments itself is never rejected before `from_args` runs.
585592
#[must_use]
586593
fn arity() -> RangeInclusive<usize> {
587-
0..=0
594+
0..=usize::MAX
588595
}
589596

590597
/// Parameters this type contributes to a text signature.
@@ -799,6 +806,10 @@ where
799806
{
800807
const PARAMS: Option<&'static [Param]> = Some(&[Param::var_keyword(Name::NAME)]);
801808

809+
fn arity() -> RangeInclusive<usize> {
810+
0..=0
811+
}
812+
802813
fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result<Self, ArgumentError> {
803814
let mut kwargs = KwArgsMap::default();
804815
for (name, value) in args.remaining_keywords() {
@@ -989,6 +1000,10 @@ where
9891000
impl FromArgs for () {
9901001
const PARAMS: Option<&'static [Param]> = Some(&[]);
9911002

1003+
fn arity() -> RangeInclusive<usize> {
1004+
0..=0
1005+
}
1006+
9921007
fn from_args(_vm: &VirtualMachine, _args: &mut FuncArgs) -> Result<Self, ArgumentError> {
9931008
Ok(())
9941009
}
@@ -1020,7 +1035,7 @@ macro_rules! tuple_from_py_func_args {
10201035
$(
10211036
let (start, end) = $T::arity().into_inner();
10221037
min += start;
1023-
max += end;
1038+
max = usize::saturating_add(max, end);
10241039
)+
10251040
min..=max
10261041
}

‎crates/vm/src/stdlib/os.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ impl<'fd, KW: DirFdKeyword> DirFd<'fd, 1, KW> {
9797
impl<const AVAILABLE: usize, KW: DirFdKeyword> FromArgs for DirFd<'_, AVAILABLE, KW> {
9898
const PARAMS: Option<&'static [Param]> = Some(KW::PARAMS);
9999

100+
fn arity() -> core::ops::RangeInclusive<usize> {
101+
0..=0
102+
}
103+
100104
fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result<Self, ArgumentError> {
101105
let fd = match args.take_keyword(KW::NAME) {
102106
Some(o) if vm.is_none(&o) => Ok(DEFAULT_DIR_FD),
@@ -290,6 +294,10 @@ pub(super) mod _os {
290294
impl FromArgs for OpenDirFd {
291295
const PARAMS: Option<&'static [Param]> = Some(DefaultDirFd::PARAMS);
292296

297+
fn arity() -> core::ops::RangeInclusive<usize> {
298+
0..=0
299+
}
300+
293301
fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result<Self, ArgumentError> {
294302
let fd = match args.take_keyword("dir_fd") {
295303
Some(o) if vm.is_none(&o) => AT_FDCWD,

‎extra_tests/snippets/vm_argument_errors.py‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import typing
1111
import weakref
1212

13-
from testutils import assert_raises
13+
from testutils import TestFailingIter, assert_raises
1414

1515

1616
def check(exc_type, message, fn, *args, **kwargs):
@@ -150,6 +150,23 @@ def test_a_keyword_check_of_its_own_says_the_same_thing():
150150
)
151151

152152

153+
def test_too_many_arguments_are_counted_before_any_is_converted():
154+
class FailingIndex:
155+
def __index__(self):
156+
raise RuntimeError
157+
158+
check(
159+
TypeError,
160+
"filter expected 2 arguments, got 3",
161+
filter,
162+
None,
163+
TestFailingIter(),
164+
0,
165+
)
166+
with assert_raises(TypeError):
167+
hex(FailingIndex(), 1)
168+
169+
153170
test_the_message_names_the_function()
154171
test_a_method_does_not_count_its_instance()
155172
test_one_argument_is_singular()
@@ -158,3 +175,4 @@ def test_a_keyword_check_of_its_own_says_the_same_thing():
158175
test_a_type_is_named_by_the_type_it_builds()
159176
test_a_slot_that_counts_its_own_arguments_says_the_same_thing()
160177
test_a_keyword_check_of_its_own_says_the_same_thing()
178+
test_too_many_arguments_are_counted_before_any_is_converted()

0 commit comments

Comments
 (0)