Allow lone-surrogate keyword keys in f(**d) (#8409)

* Allow lone-surrogate keyword keys in f(**d)

`f(**{'\udc81': 2})` raised `TypeError: keywords must be strings` even
though the key is a valid `str`, because `KwArgs` stored keys as Rust
`String` (strict UTF-8) and `collect_ex_args` narrowed each key to
`PyUtf8Str` (valid UTF-8 required). CPython only checks that the key is a
`str` (`Py_TPFLAGS_UNICODE_SUBCLASS`), never encoding validity.

Change `KwArgs<T>`'s key type from `String` to `Wtf8Buf` (the WTF-8
storage `PyStr` already uses) and relax the keyword-key downcast from
`PyUtf8Str` to `PyStr` in `collect_ex_args`, `from_vectorcall`/
`from_vectorcall_owned` (which previously panicked on surrogate keys),
the `functools.partial` keyword merge, and the C-API `dict_to_kwargs`.

`Wtf8Buf` borrows only as `Wtf8`, so inherent `get`/`contains_key`/
`swap_remove`/`shift_remove(&str)` on `KwArgs` restore the `&str` lookup
interface `String: Borrow<str>` used to provide (via the zero-cost
`Wtf8::new` cast), and a generic `FromIterator<(K: Into<Wtf8Buf>, T)>`
keeps construction sites unchanged. WTF-8 awareness stays localized to
`function/argument.rs`.

Fixes #8228

Assisted-by: Claude Code:claude-opus-4-8

* Avoid cloning kwargs keys in _ast, borrow via as_ref instead

`new_str` only needs a `&Wtf8`, so pass `key.as_ref()` rather than
cloning the key into an owned `Wtf8Buf`. The key is reused afterwards
(error message, `intern_str`), so a borrow is the right fit.

Assisted-by: Claude Code:claude-opus-4-8
This commit is contained in:
박효종 authored and GitHub committed 2026-07-30 22:13:22 +09:00
1 parent 1e34770094
commit 04c3ecfdcd
14 files changed
+102 -48

No files matched your search

+3 -1
View File
@@ -25,9 +25,11 @@ fn dict_to_kwargs(vm: &VirtualMachine, dict: &Py<PyDict>) -> PyResult<KwArgs> {
dict.items_vec()
.into_iter()
.map(|(key, value)| {
// `to_string()` would replace lone surrogates with U+FFFD; keep the
// raw WTF-8 so surrogate keys round-trip (issue #8228).
let key = key
.downcast_ref::<PyStr>()
.map(|s| s.to_string())
.map(|s| s.as_wtf8().to_owned())
.ok_or_else(|| vm.new_type_error("keywords must be strings"))?;
Ok((key, value))
})
+4 -4
View File
@@ -566,7 +566,7 @@ pub(crate) mod _asyncio {
let args = if let Some(ctx) = context {
FuncArgs::new(
vec![callback, future_arg],
KwArgs::new(core::iter::once(("context".to_owned(), ctx)).collect()),
KwArgs::new(core::iter::once((Wtf8Buf::from("context"), ctx)).collect()),
)
} else {
FuncArgs::new(vec![callback, future_arg], KwArgs::default())
@@ -1498,7 +1498,7 @@ pub(crate) mod _asyncio {
let args = if let Some(ctx) = context {
FuncArgs::new(
vec![callback, task_arg],
KwArgs::new(core::iter::once(("context".to_owned(), ctx)).collect()),
KwArgs::new(core::iter::once((Wtf8Buf::from("context"), ctx)).collect()),
)
} else {
FuncArgs::new(vec![callback, task_arg], KwArgs::default())
@@ -1527,7 +1527,7 @@ pub(crate) mod _asyncio {
let cancel_args = if let Some(ref m) = msg_value {
FuncArgs::new(
vec![],
KwArgs::new(core::iter::once(("msg".to_owned(), m.clone())).collect()),
KwArgs::new(core::iter::once((Wtf8Buf::from("msg"), m.clone())).collect()),
)
} else {
FuncArgs::new(vec![], KwArgs::default())
@@ -2213,7 +2213,7 @@ pub(crate) mod _asyncio {
let cancel_args = if let Some(ref m) = cancel_msg {
FuncArgs::new(
vec![],
KwArgs::new(core::iter::once(("msg".to_owned(), m.clone())).collect()),
KwArgs::new(core::iter::once((Wtf8Buf::from("msg"), m.clone())).collect()),
)
} else {
FuncArgs::new(vec![], KwArgs::default())
+9 -2
View File
@@ -337,8 +337,13 @@ impl PyFunction {
let mut posonly_passed_as_kwarg = Vec::new();
// Handle keyword arguments
for (name, value) in func_args.kwargs {
// Parameter names are plain identifiers, so a non-UTF-8 (surrogate) key
// can never match one and just falls through to **kwargs / the error path.
let name_str = name.as_str().ok();
// Check if we have a parameter with this name:
if let Some(pos) = arg_pos(code.posonlyarg_count as usize..total_args, &name) {
if let Some(pos) =
name_str.and_then(|s| arg_pos(code.posonlyarg_count as usize..total_args, s))
{
let slot = &mut fastlocals[pos];
if slot.is_some() {
return Err(vm.new_type_error(format!(
@@ -350,7 +355,9 @@ impl PyFunction {
*slot = Some(value);
} else if let Some(kwargs) = kwargs.as_ref() {
kwargs.set_item(&name, value, vm)?;
} else if arg_pos(0..code.posonlyarg_count as usize, &name).is_some() {
} else if name_str
.is_some_and(|s| arg_pos(0..code.posonlyarg_count as usize, s).is_some())
{
posonly_passed_as_kwarg.push(name);
} else {
return Err(vm.new_type_error(format!(
+3
View File
@@ -184,6 +184,9 @@ pub(crate) fn get_jit_args<'a>(
for (name, value) in &func_args.kwargs {
let arg_pos =
|args: &[&PyStrInterned], name: &str| args.iter().position(|arg| arg.as_str() == name);
// Parameter names are plain identifiers, so a non-UTF-8 (surrogate) key
// can never match one.
let name = name.as_str().map_err(|_| ArgsError::NotAKeywordArg)?;
if let Some(arg_idx) = arg_pos(arg_names.args, name) {
if jit_args.is_set(arg_idx) {
return Err(ArgsError::ArgPassedMultipleTimes);
+4 -2
View File
@@ -7112,11 +7112,13 @@ impl ExecutingFrame<'_> {
let func_str = Self::object_function_str(callable, vm);
Self::iterate_mapping_keys(vm, &kw_obj, &func_str, |key| {
// `PyStr`, not `PyUtf8Str`: CPython only checks that the key is a
// `str`, not that it is valid UTF-8, so surrogate keys are accepted.
let key_str = key
.downcast_ref::<PyUtf8Str>()
.downcast_ref::<PyStr>()
.ok_or_else(|| vm.new_type_error("keywords must be strings"))?;
let value = kw_obj.get_item(&*key, vm)?;
kwargs.insert(key_str.as_str().to_owned(), value);
kwargs.insert(key_str.as_wtf8().to_owned(), value);
Ok(())
})?
};
+45 -15
View File
@@ -1,6 +1,7 @@
use crate::{
AsObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
builtins::{PyBaseExceptionRef, PyTupleRef, PyTypeRef},
common::wtf8::{Wtf8, Wtf8Buf},
convert::ToPyObject,
object::{Traverse, TraverseFn},
};
@@ -155,10 +156,12 @@ impl FuncArgs {
.iter()
.zip(&args[nargs..nargs + names.len()])
.map(|(name, val)| {
// `PyStr`, not `PyUtf8Str`: a surrogate key is a valid str and
// must survive as WTF-8 rather than panic.
let key = name
.downcast_ref::<crate::builtins::PyUtf8Str>()
.downcast_ref::<crate::builtins::PyStr>()
.expect("kwnames must be strings")
.as_str()
.as_wtf8()
.to_owned();
(key, val.clone())
})
@@ -187,9 +190,9 @@ impl FuncArgs {
.zip(args.drain(nargs..nargs + kw_count))
.map(|(name, val)| {
let key = name
.downcast_ref::<crate::builtins::PyUtf8Str>()
.downcast_ref::<crate::builtins::PyStr>()
.expect("kwnames must be strings")
.as_str()
.as_wtf8()
.to_owned();
(key, val)
})
@@ -268,7 +271,7 @@ impl FuncArgs {
self.kwargs.swap_remove(name)
}
pub fn remaining_keywords(&mut self) -> impl Iterator<Item = (String, PyObjectRef)> + '_ {
pub fn remaining_keywords(&mut self) -> impl Iterator<Item = (Wtf8Buf, PyObjectRef)> + '_ {
self.kwargs.drain(..)
}
@@ -406,8 +409,12 @@ impl<T: TryFromObject> FromArgOptional for T {
/// KwArgs is only for functions that accept arbitrary keyword arguments. For
/// functions that accept only *specific* named arguments, a rust struct with
/// an appropriate FromArgs implementation must be created.
// Keys are stored as `Wtf8Buf`, not `String`, so that a lone-surrogate keyword
// name coming through `f(**d)` is preserved instead of being rejected (see
// issue #8228). `PyStr` is WTF-8 backed, and CPython only requires that a
// keyword key be a `str`, not that it be valid UTF-8.
#[derive(Clone, Debug)]
pub struct KwArgs<T = PyObjectRef>(IndexMap<String, T>);
pub struct KwArgs<T = PyObjectRef>(IndexMap<Wtf8Buf, T>);
impl<T> Default for KwArgs<T> {
fn default() -> Self {
@@ -416,7 +423,7 @@ impl<T> Default for KwArgs<T> {
}
impl<T> Deref for KwArgs<T> {
type Target = IndexMap<String, T>;
type Target = IndexMap<Wtf8Buf, T>;
fn deref(&self) -> &Self::Target {
&self.0
@@ -440,24 +447,47 @@ where
impl<T> KwArgs<T> {
#[must_use]
pub const fn new(map: IndexMap<String, T>) -> Self {
pub const fn new(map: IndexMap<Wtf8Buf, T>) -> Self {
Self(map)
}
// `String` keys accepted `&str` lookups for free via `Borrow<str>`; `Wtf8Buf`
// borrows only as `Wtf8`, so these inherent methods restore the `&str` interface
// via the zero-cost `Wtf8::new` cast, keeping every call site unchanged.
#[must_use]
pub fn get(&self, name: &str) -> Option<&T> {
self.0.get(Wtf8::new(name))
}
#[must_use]
pub fn contains_key(&self, name: &str) -> bool {
self.0.contains_key(Wtf8::new(name))
}
pub fn swap_remove(&mut self, name: &str) -> Option<T> {
self.0.swap_remove(Wtf8::new(name))
}
pub fn shift_remove(&mut self, name: &str) -> Option<T> {
self.0.shift_remove(Wtf8::new(name))
}
pub fn pop_kwarg(&mut self, name: &str) -> Option<T> {
self.swap_remove(name)
}
}
impl<T> FromIterator<(String, T)> for KwArgs<T> {
fn from_iter<I: IntoIterator<Item = (String, T)>>(iter: I) -> Self {
Self(iter.into_iter().collect())
// Accept any key that converts into `Wtf8Buf` (notably `String`), so existing
// call sites that build kwargs from string literals keep compiling unchanged.
impl<K: Into<Wtf8Buf>, T> FromIterator<(K, T)> for KwArgs<T> {
fn from_iter<I: IntoIterator<Item = (K, T)>>(iter: I) -> Self {
Self(iter.into_iter().map(|(k, v)| (k.into(), v)).collect())
}
}
impl<'a, T> IntoIterator for &'a KwArgs<T> {
type Item = (&'a String, &'a T);
type IntoIter = indexmap::map::Iter<'a, String, T>;
type Item = (&'a Wtf8Buf, &'a T);
type IntoIter = indexmap::map::Iter<'a, Wtf8Buf, T>;
fn into_iter(self) -> Self::IntoIter {
self.0.iter()
@@ -465,8 +495,8 @@ impl<'a, T> IntoIterator for &'a KwArgs<T> {
}
impl<T> IntoIterator for KwArgs<T> {
type Item = (String, T);
type IntoIter = indexmap::map::IntoIter<String, T>;
type Item = (Wtf8Buf, T);
type IntoIter = indexmap::map::IntoIter<Wtf8Buf, T>;
fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
+7 -6
View File
@@ -8,8 +8,9 @@ use super::{
pub(crate) mod _ast {
use crate::{
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
builtins::{PyDictRef, PySet, PyStr, PyTupleRef, PyType, PyTypeRef, PyUtf8Str},
builtins::{PyDictRef, PySet, PyStr, PyTupleRef, PyType, PyTypeRef},
class::{PyClassImpl, StaticType},
common::wtf8::Wtf8Buf,
function::{ArgIterable, FuncArgs, KwArgs, PyMethodDef, PyMethodFlags},
stdlib::_ast::repr,
types::{Constructor, Initializer},
@@ -229,7 +230,7 @@ pub(crate) mod _ast {
ast_replace_set_update(&expecting, attributes.as_ref(), vm)?;
for (key, _value) in &args.kwargs {
let key_obj: PyObjectRef = vm.ctx.new_str(key.as_str()).into();
let key_obj: PyObjectRef = vm.ctx.new_str(key.as_ref()).into();
if !ast_replace_set_discard(&expecting, &key_obj, vm)? {
return Err(vm.new_type_error(format!(
"{}.__replace__ got an unexpected keyword argument '{}'.",
@@ -290,11 +291,11 @@ pub(crate) mod _ast {
.into_iter()
.map(|(key, value)| {
let key = key
.downcast::<PyUtf8Str>()
.downcast::<PyStr>()
.map_err(|_| vm.new_type_error("keywords must be strings"))?;
Ok((key.as_str().to_owned(), value))
Ok((key.as_wtf8().to_owned(), value))
})
.collect::<PyResult<IndexMap<String, PyObjectRef>>>()?;
.collect::<PyResult<IndexMap<Wtf8Buf, PyObjectRef>>>()?;
let result = type_obj.call(FuncArgs::new(vec![], KwArgs::new(kwargs)), vm)?;
Ok(result)
}
@@ -418,7 +419,7 @@ pub(crate) mod _ast {
ast_replace_set_discard(&remaining_fields, &name, vm)?;
}
for (key, value) in args.kwargs {
let key_obj: PyObjectRef = vm.ctx.new_str(key.as_str()).into();
let key_obj: PyObjectRef = vm.ctx.new_str(key.as_ref()).into();
let contains = fields_seq.contains(&key_obj, vm)?;
if contains {
if !ast_replace_set_discard(&remaining_fields, &key_obj, vm)? {
+5 -4
View File
@@ -1,5 +1,6 @@
use super::base::{CDATA_BUFFER_METHODS, PyCData, PyCField, StgInfo, StgInfoFlags};
use crate::builtins::{PyList, PyStr, PyTuple, PyType, PyTypeRef, PyUtf8Str};
use crate::common::wtf8::Wtf8Buf;
use crate::convert::ToPyObject;
use crate::function::{FuncArgs, OptionalArg, PySetterValue};
use crate::protocol::{BufferDescriptor, PyBuffer, PyNumberMethods};
@@ -712,7 +713,7 @@ impl PyCStructure {
self_obj: &Py<Self>,
type_obj: &Py<PyType>,
args: &[PyObjectRef],
kwargs: &indexmap::IndexMap<String, PyObjectRef>,
kwargs: &indexmap::IndexMap<Wtf8Buf, PyObjectRef>,
index: usize,
vm: &VirtualMachine,
) -> PyResult<usize> {
@@ -746,7 +747,7 @@ impl PyCStructure {
&& let Some(name) = tuple.first()
&& let Some(name_str) = name.downcast_ref::<PyUtf8Str>()
{
let field_name = name_str.as_str().to_owned();
let field_name = name_str.as_wtf8().to_owned();
// Check for duplicate in kwargs
if kwargs.contains_key(&field_name) {
return Err(
@@ -784,9 +785,9 @@ impl Initializer for PyCStructure {
}
// 2. Process keyword arguments
for (key, value) in &args.kwargs {
for (key, value) in args.kwargs {
zelf.as_object()
.set_attr(vm.ctx.intern_str(key.as_str()), value.clone(), vm)?;
.set_attr(vm.ctx.intern_str(key), value, vm)?;
}
Ok(())
+5 -4
View File
@@ -1,6 +1,7 @@
use super::base::{CDATA_BUFFER_METHODS, StgInfoFlags};
use super::{PyCData, PyCField, StgInfo};
use crate::builtins::{PyList, PyStr, PyTuple, PyType, PyTypeRef, PyUtf8Str};
use crate::common::wtf8::Wtf8Buf;
use crate::convert::ToPyObject;
use crate::function::{ArgBytesLike, FuncArgs, OptionalArg, PySetterValue};
use crate::protocol::{BufferDescriptor, PyBuffer};
@@ -581,7 +582,7 @@ impl PyCUnion {
self_obj: &Py<Self>,
type_obj: &Py<PyType>,
args: &[PyObjectRef],
kwargs: &indexmap::IndexMap<String, PyObjectRef>,
kwargs: &indexmap::IndexMap<Wtf8Buf, PyObjectRef>,
index: usize,
vm: &VirtualMachine,
) -> PyResult<usize> {
@@ -617,7 +618,7 @@ impl PyCUnion {
&& let Some(name) = tuple.first()
&& let Some(name_str) = name.downcast_ref::<PyUtf8Str>()
{
let field_name = name_str.as_str().to_owned();
let field_name = name_str.as_wtf8().to_owned();
// Check for duplicate in kwargs
if kwargs.contains_key(&field_name) {
return Err(
@@ -655,9 +656,9 @@ impl Initializer for PyCUnion {
}
// 2. Process keyword arguments
for (key, value) in &args.kwargs {
for (key, value) in args.kwargs {
zelf.as_object()
.set_attr(vm.ctx.intern_str(key.as_str()), value.clone(), vm)?;
.set_attr(vm.ctx.intern_str(key), value, vm)?;
}
Ok(())
+3 -2
View File
@@ -373,7 +373,7 @@ mod _functools {
// Add new keywords
for (key, value) in args.kwargs {
final_keywords.set_item(vm.ctx.intern_str(key.as_str()), value, vm)?;
final_keywords.set_item(vm.ctx.intern_str(key), value, vm)?;
}
Ok(Self {
@@ -436,10 +436,11 @@ mod _functools {
// Add keywords from self.keywords
for (key, value) in &*keywords {
// `expect_str()` would panic on surrogate keys; keep them as WTF-8.
let key_str = key
.downcast_ref::<crate::builtins::PyStr>()
.ok_or_else(|| vm.new_type_error("keywords must be strings"))?;
final_kwargs.insert(key_str.expect_str().to_owned(), value);
final_kwargs.insert(key_str.as_wtf8().to_owned(), value);
}
// Add keywords from args.kwargs (these override self.keywords)
+1 -1
View File
@@ -610,7 +610,7 @@ mod _operator {
}
for (key, value) in kwargs {
result.push_str(", ");
result.push_str(key);
result.push_wtf8(key);
result.push_char('=');
result.push_wtf8(value.repr(vm)?.as_wtf8());
}
+2 -2
View File
@@ -353,9 +353,9 @@ pub(crate) mod decl {
// typealias(name, value, *, type_params=())
// name and value are positional-or-keyword; type_params is keyword-only.
// Reject unexpected keyword arguments
// Reject unexpected keyword arguments.
for key in args.kwargs.keys() {
if key != "name" && key != "value" && key != "type_params" {
if !matches!(key.as_str(), Ok("name" | "value" | "type_params")) {
return Err(vm.new_type_error(format!(
"typealias() got an unexpected keyword argument '{key}'"
)));
+2 -1
View File
@@ -1,4 +1,5 @@
use crate::common::lock::LazyLock;
use crate::common::wtf8::Wtf8;
use crate::{
AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, atomic_func,
builtins::{PyBaseExceptionRef, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef},
@@ -276,7 +277,7 @@ pub trait PyStructSequence: StaticType + PyClassImpl + Sized + 'static {
// Check for unexpected keyword arguments
if !kwargs.is_empty() {
let names: Vec<&str> = kwargs.keys().map(|k| k.as_str()).collect();
let names: Vec<&Wtf8> = kwargs.keys().map(|k| k.as_ref()).collect();
return Err(vm.new_type_error(format!("Got unexpected field name(s): {names:?}")));
}
+9 -4
View File
@@ -119,9 +119,12 @@ pub fn py_to_js(vm: &VirtualMachine, py_obj: PyObjectRef) -> JsValue {
if let Some(ref kwargs) = kwargs {
for pair in object_entries(kwargs) {
let (key, val) = pair?;
py_func_args
.kwargs
.insert(js_sys::JsString::from(key).into(), js_to_py(vm, val));
py_func_args.kwargs.insert(
// JS strings coming in are UTF-16; go through Rust `String`
// (kwargs keys are now WTF-8, so convert String -> Wtf8Buf).
String::from(js_sys::JsString::from(key)).into(),
js_to_py(vm, val),
);
}
}
let result = py_obj.call(py_func_args, vm);
@@ -229,7 +232,9 @@ pub fn js_to_py(vm: &VirtualMachine, js_val: JsValue) -> PyObjectRef {
move |args: FuncArgs, vm: &VirtualMachine| -> PyResult {
let this = Object::new();
for (k, v) in args.kwargs {
Reflect::set(&this, &k.into(), &py_to_js(vm, v))
// WTF-8 -> JS string: lone surrogates in the key become U+FFFD
// (wasm-bindgen only accepts Rust `String`); acceptable at this boundary.
Reflect::set(&this, &k.to_string().into(), &py_to_js(vm, v))
.expect("property to be settable");
}
let js_args = args