From 4aa281a7fe7be065353b5b30159b9f716c9b9d45 Mon Sep 17 00:00:00 2001 From: Josh Megnauth Date: Mon, 20 Jul 2026 15:21:20 -0400 Subject: [PATCH] Unify nul errors Part of #8245 to reduce the amount of work needed to review. I simplified the embedded nul errors by forwarding to the implementations in `vm::exceptions`. --- crates/stdlib/src/grp.rs | 2 +- crates/stdlib/src/multiprocessing.rs | 5 +-- crates/stdlib/src/openssl.rs | 8 ++--- crates/stdlib/src/ssl.rs | 5 +-- crates/vm/src/buffer.rs | 3 +- crates/vm/src/exceptions.rs | 49 ++++++++++++++++++++++++++-- crates/vm/src/function/fspath.rs | 2 +- crates/vm/src/stdlib/_codecs.rs | 8 ++--- crates/vm/src/stdlib/_io.rs | 8 ++--- crates/vm/src/stdlib/_winapi.rs | 4 +-- crates/vm/src/stdlib/nt.rs | 4 +-- crates/vm/src/stdlib/os.rs | 16 +++++---- crates/vm/src/stdlib/pwd.rs | 2 +- crates/vm/src/stdlib/time.rs | 11 +++---- crates/vm/src/stdlib/winsound.rs | 9 ++--- crates/vm/src/utils.rs | 4 +-- 16 files changed, 95 insertions(+), 45 deletions(-) diff --git a/crates/stdlib/src/grp.rs b/crates/stdlib/src/grp.rs index c2a231bef27..7e3dd8ef378 100644 --- a/crates/stdlib/src/grp.rs +++ b/crates/stdlib/src/grp.rs @@ -63,7 +63,7 @@ mod grp { fn getgrnam(name: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult { let gr_name = name.as_str(); if gr_name.contains('\0') { - return Err(exceptions::cstring_error(vm)); + return Err(exceptions::nul_char_error(vm)); } let group = host_grp::getgrnam(gr_name).map_err(|err| err.into_pyexception(vm))?; let group = group.ok_or_else(|| { diff --git a/crates/stdlib/src/multiprocessing.rs b/crates/stdlib/src/multiprocessing.rs index 1eee2ec04e4..9883219dfb1 100644 --- a/crates/stdlib/src/multiprocessing.rs +++ b/crates/stdlib/src/multiprocessing.rs @@ -359,6 +359,7 @@ mod _multiprocessing { use rustpython_host_env::multiprocessing::{ self as host_multiprocessing, SemError, TryAcquireStatus, WaitStatus, }; + use rustpython_vm::exceptions; /// Error type for sem_timedwait operations #[cfg(target_vendor = "apple")] @@ -811,7 +812,7 @@ mod _multiprocessing { let (handle, name) = SemHandle::create(&args.name, value, args.unlink).map_err(|err| { if err == SemError::InvalidInput && args.name.contains('\0') { - vm.new_value_error("embedded null character") + exceptions::nul_char_error(vm) } else { os_error(vm, err) } @@ -835,7 +836,7 @@ mod _multiprocessing { fn sem_unlink(name: String, vm: &VirtualMachine) -> PyResult<()> { host_multiprocessing::sem_unlink(&name).map_err(|err| { if err == SemError::InvalidInput && name.contains('\0') { - vm.new_value_error("embedded null character") + exceptions::nul_char_error(vm) } else { os_error(vm, err) } diff --git a/crates/stdlib/src/openssl.rs b/crates/stdlib/src/openssl.rs index 3397fff8ef0..8b7d1de0639 100644 --- a/crates/stdlib/src/openssl.rs +++ b/crates/stdlib/src/openssl.rs @@ -1041,7 +1041,7 @@ mod _ssl { fn set_ciphers(&self, cipherlist: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { let ciphers: &str = cipherlist.as_ref(); if ciphers.contains('\0') { - return Err(exceptions::cstring_error(vm)); + return Err(exceptions::nul_char_error(vm)); } self.builder() .set_cipher_list(ciphers) @@ -1097,12 +1097,12 @@ mod _ssl { Either::A(s) => { let s: &str = s.as_ref(); if s.contains('\0') { - return Err(exceptions::cstring_error(vm)); + return Err(exceptions::nul_char_error(vm)); } s.to_cstring(vm)? } Either::B(b) => std::ffi::CString::new(b.borrow_buf().to_vec()) - .map_err(|_| exceptions::cstring_error(vm))?, + .map_err(|_| exceptions::nul_char_error(vm))?, }; // Find the NID for the curve name using OBJ_sn2nid @@ -2038,7 +2038,7 @@ mod _ssl { )); } if hostname_str.contains('\0') { - return Err(vm.new_type_error("embedded null character")); + return Err(exceptions::nul_char_type_error(vm)); } let ip = hostname_str.parse::(); if ip.is_err() { diff --git a/crates/stdlib/src/ssl.rs b/crates/stdlib/src/ssl.rs index 7b7e2127f48..5e6f35943bd 100644 --- a/crates/stdlib/src/ssl.rs +++ b/crates/stdlib/src/ssl.rs @@ -70,6 +70,7 @@ mod _ssl { sync::atomic::{AtomicUsize, Ordering}, time::Duration, }; + use rustpython_vm::exceptions; use std::{ collections::{HashMap, hash_map::DefaultHasher}, io::BufRead, @@ -392,7 +393,7 @@ mod _ssl { // SNI will not be sent for IP addresses if hostname.contains('\0') { - return Err(vm.new_type_error("embedded null character")); + return Err(exceptions::nul_char_type_error(vm)); } if hostname.len() > 253 { @@ -1869,7 +1870,7 @@ mod _ssl { // Check for NULL bytes if hostname.contains('\0') { - return Err(vm.new_type_error("embedded null character")); + return Err(exceptions::nul_char_error(vm)); } Some(hostname.to_string()) diff --git a/crates/vm/src/buffer.rs b/crates/vm/src/buffer.rs index 84c0ac17f3b..ae5ce6b0065 100644 --- a/crates/vm/src/buffer.rs +++ b/crates/vm/src/buffer.rs @@ -3,6 +3,7 @@ use crate::{ builtins::{PyBaseExceptionRef, PyBytesRef, PyTuple, PyTupleRef, PyTypeRef}, common::{static_cell, str::wchar_t}, convert::ToPyObject, + exceptions, function::{ArgBytesLike, ArgIntoBool, ArgIntoFloat}, }; @@ -282,7 +283,7 @@ impl FormatCode { // Check for embedded null character if c == 0 { - return Err("embedded null character".to_owned()); + return Err(exceptions::NulError.to_string()); } // PEP3118: Handle extended format specifiers diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index cd1eb1adbcb..fe07a7e3c9e 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -15,6 +15,7 @@ use crate::{ suggestion::offer_suggestions, types::{Callable, Constructor, Initializer, Representable}, }; +use core::fmt::{self, Display, Formatter}; use crossbeam_utils::atomic::AtomicCell; use itertools::Itertools; #[cfg(feature = "host_env")] @@ -1188,20 +1189,62 @@ impl serde::Serialize for SerializeException<'_, '_> { } } -pub fn cstring_error(vm: &VirtualMachine) -> PyBaseExceptionRef { +#[derive(Debug)] +pub struct NulError; + +impl Display for NulError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "embedded null character") + } +} + +pub fn nul_char_error(vm: &VirtualMachine) -> PyBaseExceptionRef { vm.new_value_error("embedded null character") } +pub fn nul_char_type_error(vm: &VirtualMachine) -> PyBaseExceptionRef { + vm.new_type_error("embedded null character") +} + +pub fn nul_byte_error(vm: &VirtualMachine) -> PyBaseExceptionRef { + vm.new_value_error("embedded null byte") +} + impl ToPyException for alloc::ffi::NulError { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { - cstring_error(vm) + nul_char_error(vm) + } +} + +impl ToPyException for alloc::ffi::FromVecWithNulError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + nul_char_error(vm) + } +} + +impl ToPyException for core::ffi::FromBytesWithNulError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + nul_char_error(vm) + } +} + +impl ToPyException for NulError { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + nul_char_error(vm) } } #[cfg(windows)] impl ToPyException for widestring::error::ContainsNul { fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { - cstring_error(vm) + nul_char_error(vm) + } +} + +#[cfg(windows)] +impl ToPyException for widestring::error::MissingNulTerminator { + fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef { + vm.new_value_error(self.to_string()) } } diff --git a/crates/vm/src/function/fspath.rs b/crates/vm/src/function/fspath.rs index 5ec81ba63a6..053802285cd 100644 --- a/crates/vm/src/function/fspath.rs +++ b/crates/vm/src/function/fspath.rs @@ -40,7 +40,7 @@ impl FsPath { if !check_for_nul || memchr::memchr(b'\0', b).is_none() { Ok(()) } else { - Err(crate::exceptions::cstring_error(vm)) + Err(crate::exceptions::nul_char_error(vm)) } }; let match1 = |obj: PyObjectRef| { diff --git a/crates/vm/src/stdlib/_codecs.rs b/crates/vm/src/stdlib/_codecs.rs index a9402edc3a2..6052350159d 100644 --- a/crates/vm/src/stdlib/_codecs.rs +++ b/crates/vm/src/stdlib/_codecs.rs @@ -13,7 +13,7 @@ mod _codecs { AsObject, PyObjectRef, PyResult, VirtualMachine, builtins::{PyStrRef, PyUtf8StrRef}, codecs, - exceptions::cstring_error, + exceptions::nul_char_error, function::{ArgBytesLike, FuncArgs}, }; @@ -30,7 +30,7 @@ mod _codecs { #[pyfunction] fn lookup(encoding: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult { if encoding.as_str().contains('\0') { - return Err(cstring_error(vm)); + return Err(nul_char_error(vm)); } vm.state .codec_registry @@ -106,7 +106,7 @@ mod _codecs { #[pyfunction] fn lookup_error(name: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult { if name.as_str().contains('\0') { - return Err(cstring_error(vm)); + return Err(nul_char_error(vm)); } vm.state.codec_registry.lookup_error(name.as_str(), vm) } @@ -114,7 +114,7 @@ mod _codecs { #[pyfunction] fn _unregister_error(errors: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult { if errors.as_str().contains('\0') { - return Err(cstring_error(vm)); + return Err(nul_char_error(vm)); } vm.state .codec_registry diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index 4cffb909fa7..eb2fa6cfc4f 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -132,7 +132,7 @@ mod _io { }, common::wtf8::{Wtf8, Wtf8Buf}, convert::ToPyObject, - exceptions::cstring_error, + exceptions::nul_char_error, function::{ ArgBytesLike, ArgIterable, ArgMemoryBuffer, ArgSize, Either, FsPath, FuncArgs, IntoFuncArgs, OptionalArg, OptionalOption, PySetterValue, @@ -2855,7 +2855,7 @@ mod _io { fn validate_errors(errors: &PyRef, vm: &VirtualMachine) -> PyResult<()> { if errors.as_str().contains('\0') { - return Err(cstring_error(vm)); + return Err(nul_char_error(vm)); } vm.state .codec_registry @@ -2896,7 +2896,7 @@ mod _io { }, Some(enc) => { if enc.as_str().contains('\0') { - return Err(cstring_error(vm)); + return Err(nul_char_error(vm)); } enc } @@ -2915,7 +2915,7 @@ mod _io { }, }; if encoding.as_str().contains('\0') { - return Err(cstring_error(vm)); + return Err(nul_char_error(vm)); } Ok(encoding) } diff --git a/crates/vm/src/stdlib/_winapi.rs b/crates/vm/src/stdlib/_winapi.rs index e1b86f2ef54..0d54530d4b2 100644 --- a/crates/vm/src/stdlib/_winapi.rs +++ b/crates/vm/src/stdlib/_winapi.rs @@ -235,12 +235,12 @@ mod _winapi { if let Some(ref name) = args.name && name.as_bytes().contains(&0) { - return Err(crate::exceptions::cstring_error(vm)); + return Err(crate::exceptions::nul_char_error(vm)); } if let Some(ref cmd) = args.command_line && cmd.as_bytes().contains(&0) { - return Err(crate::exceptions::cstring_error(vm)); + return Err(crate::exceptions::nul_char_error(vm)); } let wcstring = |s: PyStrRef| s.as_wtf8().to_wide_cstring(); diff --git a/crates/vm/src/stdlib/nt.rs b/crates/vm/src/stdlib/nt.rs index 0d423541b87..acc9f4ee67e 100644 --- a/crates/vm/src/stdlib/nt.rs +++ b/crates/vm/src/stdlib/nt.rs @@ -9,7 +9,7 @@ pub(crate) mod module { Py, PyResult, TryFromObject, VirtualMachine, builtins::{PyBytes, PyDictRef, PyListRef, PyStr, PyStrRef, PyTupleRef}, convert::ToPyException, - exceptions::OSErrorBuilder, + exceptions::{self, OSErrorBuilder}, function::{ArgMapping, Either, OptionalArg}, host_env::{crt_fd, windows::ToWideString}, ospath::{OsPath, OsPathOrFd}, @@ -552,7 +552,7 @@ pub(crate) mod module { // Validate: no null characters in key or value if key_str.contains('\0') || value_str.contains('\0') { - return Err(vm.new_value_error("embedded null character")); + return Err(exceptions::nul_char_error(vm)); } // Validate: empty key or '=' in key after position 0 // (search from index 1 because on Windows starting '=' is allowed diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 5772cc46f5d..f885fb1db05 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -181,6 +181,8 @@ impl ToPyObject for crt_fd::Borrowed<'_> { #[pymodule(sub)] pub(super) mod _os { use super::{DirFd, DstDirFd, FollowSymlinks, RawMode, SrcDirFd, SupportFunc}; + #[cfg(not(windows))] + use crate::exceptions; use crate::host_env::fileutils::StatStruct; #[cfg(any(unix, windows))] use crate::utils::ToCString; @@ -531,7 +533,7 @@ pub(super) mod _os { let key = env_bytes_as_bytes(&key); let value = env_bytes_as_bytes(&value); if key.contains(&b'\0') || value.contains(&b'\0') { - return Err(vm.new_value_error("embedded null byte")); + return Err(exceptions::nul_byte_error(vm)); } if key.is_empty() || key.contains(&b'=') { return Err(vm.new_value_error("illegal environment variable name")); @@ -574,7 +576,7 @@ pub(super) mod _os { ) -> PyResult<()> { let key = env_bytes_as_bytes(&key); if key.contains(&b'\0') { - return Err(vm.new_value_error("embedded null byte")); + return Err(exceptions::nul_byte_error(vm)); } if key.is_empty() || key.contains(&b'=') { let x = vm.new_errno_error( @@ -817,7 +819,7 @@ pub(super) mod _os { FollowSymlinks(false), ) .map_err(|e| e.into_pyexception(vm))? - .ok_or_else(|| crate::exceptions::cstring_error(vm))?; + .ok_or_else(|| crate::exceptions::nul_char_error(vm))?; // On Windows, combine st_ino and st_ino_high into 128-bit value let ino: u128 = cfg_select! { windows => stat.st_ino as u128 | ((stat.st_ino_high as u128) << 64), @@ -1360,7 +1362,7 @@ pub(super) mod _os { ) -> PyResult { let stat = stat_inner(file.clone(), dir_fd, follow_symlinks) .map_err(|err| OSErrorBuilder::with_filename(&err, file, vm))? - .ok_or_else(|| crate::exceptions::cstring_error(vm))?; + .ok_or_else(|| crate::exceptions::nul_char_error(vm))?; Ok(StatResultData::from_stat(&stat, vm).to_pyobject(vm)) } @@ -1543,10 +1545,12 @@ pub(super) mod _os { #[cfg(unix)] { use std::os::unix::ffi::OsStrExt; + + use crate::convert::ToPyException; let src_cstr = alloc::ffi::CString::new(src.path.as_os_str().as_bytes()) - .map_err(|_| vm.new_value_error("embedded null byte"))?; + .map_err(|e| e.to_pyexception(vm))?; let dst_cstr = alloc::ffi::CString::new(dst.path.as_os_str().as_bytes()) - .map_err(|_| vm.new_value_error("embedded null byte"))?; + .map_err(|e| e.to_pyexception(vm))?; let follow = follow_symlinks.into_option().unwrap_or(true); if let Err(err) = diff --git a/crates/vm/src/stdlib/pwd.rs b/crates/vm/src/stdlib/pwd.rs index e2f987ce019..e181de240f6 100644 --- a/crates/vm/src/stdlib/pwd.rs +++ b/crates/vm/src/stdlib/pwd.rs @@ -52,7 +52,7 @@ mod pwd { fn getpwnam(name: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult { let pw_name = name.as_str(); if pw_name.contains('\0') { - return Err(exceptions::cstring_error(vm)); + return Err(exceptions::nul_char_error(vm)); } let user = host_pwd::getpwnam(name.as_str()); let user = user.ok_or_else(|| { diff --git a/crates/vm/src/stdlib/time.rs b/crates/vm/src/stdlib/time.rs index 5c77afb4f5c..05894cdc2a1 100644 --- a/crates/vm/src/stdlib/time.rs +++ b/crates/vm/src/stdlib/time.rs @@ -571,8 +571,8 @@ mod decl { for codepoint in format.as_wtf8().code_points() { if codepoint.to_u32() == 0 { if !ascii.is_empty() { - let part = host_time::strftime_ascii(&ascii, &tm) - .map_err(|_| vm.new_value_error("embedded null character"))?; + let part = + host_time::strftime_ascii(&ascii, &tm).map_err(|e| e.to_pyexception(vm))?; out.extend(part.chars()); ascii.clear(); } @@ -587,16 +587,15 @@ mod decl { } if !ascii.is_empty() { - let part = host_time::strftime_ascii(&ascii, &tm) - .map_err(|_| vm.new_value_error("embedded null character"))?; + let part = + host_time::strftime_ascii(&ascii, &tm).map_err(|e| e.to_pyexception(vm))?; out.extend(part.chars()); ascii.clear(); } out.push(codepoint); } if !ascii.is_empty() { - let part = host_time::strftime_ascii(&ascii, &tm) - .map_err(|_| vm.new_value_error("embedded null character"))?; + let part = host_time::strftime_ascii(&ascii, &tm).map_err(|e| e.to_pyexception(vm))?; out.extend(part.chars()); } Ok(out.to_pyobject(vm)) diff --git a/crates/vm/src/stdlib/winsound.rs b/crates/vm/src/stdlib/winsound.rs index 67d7c8a7ffe..95032ad8970 100644 --- a/crates/vm/src/stdlib/winsound.rs +++ b/crates/vm/src/stdlib/winsound.rs @@ -6,7 +6,8 @@ pub(crate) use winsound::module_def; #[pymodule] mod winsound { use crate::builtins::{PyBaseExceptionRef, PyBytes, PyStr}; - use crate::convert::{IntoPyException, TryFromBorrowedObject}; + use crate::convert::{IntoPyException, ToPyException, TryFromBorrowedObject}; + use crate::exceptions; use crate::host_env::windows::ToWideString; use crate::protocol::PyBuffer; use crate::{AsObject, PyObjectRef, PyResult, VirtualMachine}; @@ -142,12 +143,12 @@ mod winsound { // Check for embedded null characters if path.as_bytes().contains(&0) { - return Err(vm.new_value_error("embedded null character")); + return Err(exceptions::nul_char_error(vm)); } let wide = path.to_wide_with_nul(); - let wide_cstr = widestring::WideCStr::from_slice_truncate(&wide) - .map_err(|_| vm.new_value_error("embedded null character"))?; + let wide_cstr = + widestring::WideCStr::from_slice_truncate(&wide).map_err(|e| e.to_pyexception(vm))?; play_sound(PlaySoundSource::Name(wide_cstr), flags).map_err(map_play_err(vm)) } diff --git a/crates/vm/src/utils.rs b/crates/vm/src/utils.rs index b5117ddd8d1..d23cbb689a6 100644 --- a/crates/vm/src/utils.rs +++ b/crates/vm/src/utils.rs @@ -4,7 +4,7 @@ use crate::{ PyObjectRef, PyResult, VirtualMachine, builtins::{PyStr, PyUtf8Str}, convert::{ToPyException, ToPyObject}, - exceptions::cstring_error, + exceptions::nul_char_error, }; pub fn hash_iter<'a, I: IntoIterator>( @@ -26,7 +26,7 @@ pub trait ToCString: AsRef { } fn ensure_no_nul(&self, vm: &VirtualMachine) -> PyResult<()> { if self.as_ref().as_bytes().contains(&b'\0') { - Err(cstring_error(vm)) + Err(nul_char_error(vm)) } else { Ok(()) }