Skip to content

Align socket receive and control argument binding #9668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 3 commits into from
Nov 7, 2026
+108 −63
Merged

Align socket receive and control argument binding #9668

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b807b94
Align socket receive and control argument binding
youknowdot Nov 3, 2026
194c8ed
Preserve setsockopt arity error messages
youknowdot Nov 4, 2026
358945c
Restore typed socket arguments with legacy conversion ordering
codex Nov 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 108 additions & 63 deletions crates/stdlib/src/socket.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ mod _socket {
},
convert::{IntoPyException, ToPyObject, TryFromBorrowedObject, TryFromObject},
function::{
ArgBytesLike, ArgIndex, ArgIntoFloat, ArgMemoryBuffer, ArgStrOrBytesLike, FsPath,
FuncArgs, OptionalArg,
ArgBytesLike, ArgIndex, ArgIntoFloat, ArgMemoryBuffer, ArgSpec, ArgStrOrBytesLike,
FsPath, FuncArgs, OptionalArg,
},
types::{Constructor, DefaultConstructor, Destructor, Initializer, Representable},
utils::ToCString,
Expand Down Expand Up @@ -1894,6 +1894,47 @@ mod _socket {
addr: Option<PyObjectRef>,
}

struct ReceiveIntoArgs {
buffer: ArgMemoryBuffer,
nbytes: usize,
flags: i32,
}

impl ReceiveIntoArgs {
fn parse(
args: &FuncArgs,
name: &'static str,
too_small: &'static str,
vm: &VirtualMachine,
) -> PyResult<Self> {
let (buffer, nbytes, flags) = ArgSpec {
fname: name,
keywords: &["buffer", "nbytes", "flags"],
required: 1,
max_positional: 3,
}
.bind(args, vm, |slots| -> PyResult<_> {
let buffer = slots.required_as::<ArgMemoryBuffer>(vm)?;
let nbytes = slots.optional_as::<isize>(vm)?.unwrap_or(0);
let flags = slots.optional_as::<i32>(vm)?.unwrap_or(0);
Ok((buffer, nbytes, flags))
})?;
let nbytes = usize::try_from(nbytes)
.map_err(|_| vm.new_value_error(format!("negative buffersize in {name}")))?;
let nbytes = if nbytes == 0 { buffer.len() } else { nbytes };
if nbytes > buffer.len() {
return Err(vm.new_value_error(too_small));
}
// Keep only the export alive during conversions and I/O, not a
// payload lock: index callbacks may write to this same buffer.
Ok(Self {
buffer,
nbytes,
flags,
})
}
}

impl Initializer for PySocket {
type Args = SocketInitArgs;

Expand Down Expand Up @@ -2163,6 +2204,9 @@ mod _socket {
let bufsize = bufsize
.to_usize()
.ok_or_else(|| vm.new_value_error("negative buffersize in recv"))?;
if bufsize == 0 {
return Ok(Vec::new());
}
let mut buffer = alloc_recv_scratch(bufsize, flags, vm)?;
let n = zelf.sock_op(vm, SockWaitKind::Read, || {
zelf.sock_snapshot()?
Expand All @@ -2177,27 +2221,23 @@ mod _socket {
#[pymethod]
fn recv_into(
zelf: &Py<Self>,
buf: ArgMemoryBuffer,
nbytes: OptionalArg<isize>,
flags: OptionalArg<i32>,
args: FuncArgs,

@youknowone youknowone Nov 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check this change is necessary after #9563

@youknowone youknowone Nov 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked after #9563. This boundary still needs legacy argument parsing. CPython 3.15's recv_into uses PyArg_ParseTupleAndKeywords("w*|ni:recv_into"), whose conversion loop precedes leftover duplicate/unknown-keyword checks.

For example, recv_into(bytearray(1), IndexRaises(), nbytes=1) must propagate an exception from IndexRaises.__index__() before reporting the duplicate nbytes. Restoring ordinary typed parameters or a derived FromArgs struct would let #9563's Clinic validation reject the duplicate before that callback. I checked this counterexample on the current PR binary and existing CPython 3.12.14; the 3.15 parsing order was checked against the source above.

The safe internal typing is already restored in 358945c: ReceiveIntoArgs::parse uses ArgSpec::bind and typed ArgSlots conversions for ArgMemoryBuffer, isize, and i32, without the old temporary Vec/Cell accumulation. The outer FuncArgs retains the original argument owners through conversion and I/O.

A custom typed FromArgs wrapper is possible, but would still need to opt out of Clinic validation, use this legacy parser, and retain those owners. It would move the boundary rather than eliminate the requirement. Leaving the current boundary unchanged; no new wrapper or common converter is needed here.

vm: &VirtualMachine,
) -> Result<usize, IoOrPyException> {
let flags = flags.unwrap_or(0);

// Handle nbytes parameter
let read_len = if let OptionalArg::Present(nbytes) = nbytes {
let nbytes = nbytes
.to_usize()
.ok_or_else(|| vm.new_value_error("negative buffersize in recv_into"))?;
if nbytes > buf.len() {
return Err(vm
.new_value_error("buffer too small for requested bytes")
.into());
}
if nbytes == 0 { buf.len() } else { nbytes }
} else {
buf.len()
};
let ReceiveIntoArgs {
buffer: buf,
nbytes: read_len,
flags,
} = ReceiveIntoArgs::parse(
&args,
"recv_into",
"buffer too small for requested bytes",
vm,
)?;
// Like recv(), an empty receive does not enter the socket wait.
if read_len == 0 {
return Ok(0);
}

let mut scratch = alloc_recv_scratch(read_len, flags, vm)?;
let n = zelf.sock_op(vm, SockWaitKind::Read, || {
Expand Down Expand Up @@ -2239,26 +2279,19 @@ mod _socket {
#[pymethod]
fn recvfrom_into(
zelf: &Py<Self>,
buf: ArgMemoryBuffer,
nbytes: OptionalArg<isize>,
flags: OptionalArg<i32>,
args: FuncArgs,
vm: &VirtualMachine,
) -> Result<(usize, PyObjectRef), IoOrPyException> {
let read_len = match nbytes {
OptionalArg::Present(i) => {
let i = i.to_usize().ok_or_else(|| {
vm.new_value_error("negative buffersize in recvfrom_into")
})?;
if i > buf.len() {
return Err(vm
.new_value_error("nbytes is greater than the length of the buffer")
.into());
}
if i == 0 { buf.len() } else { i }
}
OptionalArg::Missing => buf.len(),
};
let flags = flags.unwrap_or(0);
let ReceiveIntoArgs {
buffer: buf,
nbytes: read_len,
flags,
} = ReceiveIntoArgs::parse(
&args,
"recvfrom_into",
"nbytes is greater than the length of the buffer",
vm,
)?;
let mut scratch = alloc_recv_scratch(read_len, flags, vm)?;
let (n, addr) = zelf.sock_op(vm, SockWaitKind::Read, || {
zelf.sock_snapshot()?
Expand Down Expand Up @@ -2323,20 +2356,34 @@ mod _socket {
.new_type_error("sendto() takes no keyword arguments")
.into());
}
let n = args.args.len();
let (bytes, flags, address) = match n {
2 => (args.args[0].clone(), 0, args.args[1].clone()),
3 => {
let flags = args.args[1].try_index(vm)?.try_to_primitive::<i32>(vm)?;
(args.args[0].clone(), flags, args.args[2].clone())
}
_ => {
return Err(vm
.new_type_error(format!("sendto() takes 2 or 3 arguments ({n} given)"))
.into());
}
// This overload's diagnostic cannot be expressed by the binder's
// ordinary minimum/maximum arity messages.
let nargs = args.args.len();
if !(2..=3).contains(&nargs) {
return Err(vm
.new_type_error(format!("sendto() takes 2 or 3 arguments ({nargs} given)"))
.into());
}
#[expect(
clippy::redundant_clone,
reason = "Keep original arguments alive through callbacks and the socket operation"
)]
let (bytes, flags_or_address, address): (
ArgBytesLike,
PyObjectRef,
OptionalArg<PyObjectRef>,
) = args.clone().bind_for(vm, "sendto")?;
// Export before converting flags, but defer borrowing its data
// until all Python callbacks, including address conversion, finish.
let (flags, address) = match address {
OptionalArg::Missing => (0, flags_or_address),
OptionalArg::Present(address) => (
flags_or_address
.try_index(vm)?
.try_to_primitive::<i32>(vm)?,
address,
),
};
let bytes = ArgBytesLike::try_from_object(vm, bytes)?;
let addr = zelf.extract_address(address, "sendto", vm)?;
let buf = bytes.borrow_buf_unlocked(vm)?;
let buf = &*buf;
Expand Down Expand Up @@ -2807,27 +2854,25 @@ mod _socket {
.new_type_error("setsockopt() takes no keyword arguments")
.into());
}
let nargs = args.args.len();
if nargs < 3 {
return Err(vm
.new_type_error(format!(
"setsockopt() takes at least 3 arguments ({nargs} given)"
))
.into());
}
if nargs > 4 {
let argc = args.args.len();
if !(3..=4).contains(&argc) {
let (bound, limit) = if argc < 3 { ("least", 3) } else { ("most", 4) };
return Err(vm
.new_type_error(format!(
"setsockopt() takes at most 4 arguments ({nargs} given)"
"setsockopt() takes at {bound} {limit} arguments ({argc} given)"
))
.into());
}
#[expect(
clippy::redundant_clone,
reason = "Keep original arguments alive through callbacks and the socket operation"
)]
let (level, name, value, optlen): (
i32,
i32,
Option<PyObjectRef>,
OptionalArg<ArgIndex>,
) = args.bind(vm)?;
) = args.clone().bind_for(vm, "setsockopt")?;
let optlen = match optlen {
OptionalArg::Present(index) => {
OptionalArg::Present(index.as_unsigned_bitfield::<32>(vm)? as u32)
Expand Down
Loading