Skip to content

Commit 04c3ecf

Browse files
HyoJongParkHyoJongPark
authored
Allow lone-surrogate keyword keys in f(**d) (RustPython#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 RustPython#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
1 parent 1e34770 commit 04c3ecf

14 files changed

Lines changed: 102 additions & 48 deletions

File tree

‎crates/capi/src/abstract_.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@ fn dict_to_kwargs(vm: &VirtualMachine, dict: &Py<PyDict>) -> PyResult<KwArgs> {
2525
dict.items_vec()
2626
.into_iter()
2727
.map(|(key, value)| {
28+
// `to_string()` would replace lone surrogates with U+FFFD; keep the
29+
// raw WTF-8 so surrogate keys round-trip (issue #8228).
2830
let key = key
2931
.downcast_ref::<PyStr>()
30-
.map(|s| s.to_string())
32+
.map(|s| s.as_wtf8().to_owned())
3133
.ok_or_else(|| vm.new_type_error("keywords must be strings"))?;
3234
Ok((key, value))
3335
})

‎crates/stdlib/src/_asyncio.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ pub(crate) mod _asyncio {
566566
let args = if let Some(ctx) = context {
567567
FuncArgs::new(
568568
vec![callback, future_arg],
569-
KwArgs::new(core::iter::once(("context".to_owned(), ctx)).collect()),
569+
KwArgs::new(core::iter::once((Wtf8Buf::from("context"), ctx)).collect()),
570570
)
571571
} else {
572572
FuncArgs::new(vec![callback, future_arg], KwArgs::default())
@@ -1498,7 +1498,7 @@ pub(crate) mod _asyncio {
14981498
let args = if let Some(ctx) = context {
14991499
FuncArgs::new(
15001500
vec![callback, task_arg],
1501-
KwArgs::new(core::iter::once(("context".to_owned(), ctx)).collect()),
1501+
KwArgs::new(core::iter::once((Wtf8Buf::from("context"), ctx)).collect()),
15021502
)
15031503
} else {
15041504
FuncArgs::new(vec![callback, task_arg], KwArgs::default())
@@ -1527,7 +1527,7 @@ pub(crate) mod _asyncio {
15271527
let cancel_args = if let Some(ref m) = msg_value {
15281528
FuncArgs::new(
15291529
vec![],
1530-
KwArgs::new(core::iter::once(("msg".to_owned(), m.clone())).collect()),
1530+
KwArgs::new(core::iter::once((Wtf8Buf::from("msg"), m.clone())).collect()),
15311531
)
15321532
} else {
15331533
FuncArgs::new(vec![], KwArgs::default())
@@ -2213,7 +2213,7 @@ pub(crate) mod _asyncio {
22132213
let cancel_args = if let Some(ref m) = cancel_msg {
22142214
FuncArgs::new(
22152215
vec![],
2216-
KwArgs::new(core::iter::once(("msg".to_owned(), m.clone())).collect()),
2216+
KwArgs::new(core::iter::once((Wtf8Buf::from("msg"), m.clone())).collect()),
22172217
)
22182218
} else {
22192219
FuncArgs::new(vec![], KwArgs::default())

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,8 +337,13 @@ impl PyFunction {
337337
let mut posonly_passed_as_kwarg = Vec::new();
338338
// Handle keyword arguments
339339
for (name, value) in func_args.kwargs {
340+
// Parameter names are plain identifiers, so a non-UTF-8 (surrogate) key
341+
// can never match one and just falls through to **kwargs / the error path.
342+
let name_str = name.as_str().ok();
340343
// Check if we have a parameter with this name:
341-
if let Some(pos) = arg_pos(code.posonlyarg_count as usize..total_args, &name) {
344+
if let Some(pos) =
345+
name_str.and_then(|s| arg_pos(code.posonlyarg_count as usize..total_args, s))
346+
{
342347
let slot = &mut fastlocals[pos];
343348
if slot.is_some() {
344349
return Err(vm.new_type_error(format!(
@@ -350,7 +355,9 @@ impl PyFunction {
350355
*slot = Some(value);
351356
} else if let Some(kwargs) = kwargs.as_ref() {
352357
kwargs.set_item(&name, value, vm)?;
353-
} else if arg_pos(0..code.posonlyarg_count as usize, &name).is_some() {
358+
} else if name_str
359+
.is_some_and(|s| arg_pos(0..code.posonlyarg_count as usize, s).is_some())
360+
{
354361
posonly_passed_as_kwarg.push(name);
355362
} else {
356363
return Err(vm.new_type_error(format!(

‎crates/vm/src/builtins/function/jit.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,9 @@ pub(crate) fn get_jit_args<'a>(
184184
for (name, value) in &func_args.kwargs {
185185
let arg_pos =
186186
|args: &[&PyStrInterned], name: &str| args.iter().position(|arg| arg.as_str() == name);
187+
// Parameter names are plain identifiers, so a non-UTF-8 (surrogate) key
188+
// can never match one.
189+
let name = name.as_str().map_err(|_| ArgsError::NotAKeywordArg)?;
187190
if let Some(arg_idx) = arg_pos(arg_names.args, name) {
188191
if jit_args.is_set(arg_idx) {
189192
return Err(ArgsError::ArgPassedMultipleTimes);

‎crates/vm/src/frame.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7112,11 +7112,13 @@ impl ExecutingFrame<'_> {
71127112
let func_str = Self::object_function_str(callable, vm);
71137113

71147114
Self::iterate_mapping_keys(vm, &kw_obj, &func_str, |key| {
7115+
// `PyStr`, not `PyUtf8Str`: CPython only checks that the key is a
7116+
// `str`, not that it is valid UTF-8, so surrogate keys are accepted.
71157117
let key_str = key
7116-
.downcast_ref::<PyUtf8Str>()
7118+
.downcast_ref::<PyStr>()
71177119
.ok_or_else(|| vm.new_type_error("keywords must be strings"))?;
71187120
let value = kw_obj.get_item(&*key, vm)?;
7119-
kwargs.insert(key_str.as_str().to_owned(), value);
7121+
kwargs.insert(key_str.as_wtf8().to_owned(), value);
71207122
Ok(())
71217123
})?
71227124
};

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

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::{
22
AsObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
33
builtins::{PyBaseExceptionRef, PyTupleRef, PyTypeRef},
4+
common::wtf8::{Wtf8, Wtf8Buf},
45
convert::ToPyObject,
56
object::{Traverse, TraverseFn},
67
};
@@ -155,10 +156,12 @@ impl FuncArgs {
155156
.iter()
156157
.zip(&args[nargs..nargs + names.len()])
157158
.map(|(name, val)| {
159+
// `PyStr`, not `PyUtf8Str`: a surrogate key is a valid str and
160+
// must survive as WTF-8 rather than panic.
158161
let key = name
159-
.downcast_ref::<crate::builtins::PyUtf8Str>()
162+
.downcast_ref::<crate::builtins::PyStr>()
160163
.expect("kwnames must be strings")
161-
.as_str()
164+
.as_wtf8()
162165
.to_owned();
163166
(key, val.clone())
164167
})
@@ -187,9 +190,9 @@ impl FuncArgs {
187190
.zip(args.drain(nargs..nargs + kw_count))
188191
.map(|(name, val)| {
189192
let key = name
190-
.downcast_ref::<crate::builtins::PyUtf8Str>()
193+
.downcast_ref::<crate::builtins::PyStr>()
191194
.expect("kwnames must be strings")
192-
.as_str()
195+
.as_wtf8()
193196
.to_owned();
194197
(key, val)
195198
})
@@ -268,7 +271,7 @@ impl FuncArgs {
268271
self.kwargs.swap_remove(name)
269272
}
270273

271-
pub fn remaining_keywords(&mut self) -> impl Iterator<Item = (String, PyObjectRef)> + '_ {
274+
pub fn remaining_keywords(&mut self) -> impl Iterator<Item = (Wtf8Buf, PyObjectRef)> + '_ {
272275
self.kwargs.drain(..)
273276
}
274277

@@ -406,8 +409,12 @@ impl<T: TryFromObject> FromArgOptional for T {
406409
/// KwArgs is only for functions that accept arbitrary keyword arguments. For
407410
/// functions that accept only *specific* named arguments, a rust struct with
408411
/// an appropriate FromArgs implementation must be created.
412+
// Keys are stored as `Wtf8Buf`, not `String`, so that a lone-surrogate keyword
413+
// name coming through `f(**d)` is preserved instead of being rejected (see
414+
// issue #8228). `PyStr` is WTF-8 backed, and CPython only requires that a
415+
// keyword key be a `str`, not that it be valid UTF-8.
409416
#[derive(Clone, Debug)]
410-
pub struct KwArgs<T = PyObjectRef>(IndexMap<String, T>);
417+
pub struct KwArgs<T = PyObjectRef>(IndexMap<Wtf8Buf, T>);
411418

412419
impl<T> Default for KwArgs<T> {
413420
fn default() -> Self {
@@ -416,7 +423,7 @@ impl<T> Default for KwArgs<T> {
416423
}
417424

418425
impl<T> Deref for KwArgs<T> {
419-
type Target = IndexMap<String, T>;
426+
type Target = IndexMap<Wtf8Buf, T>;
420427

421428
fn deref(&self) -> &Self::Target {
422429
&self.0
@@ -440,33 +447,56 @@ where
440447

441448
impl<T> KwArgs<T> {
442449
#[must_use]
443-
pub const fn new(map: IndexMap<String, T>) -> Self {
450+
pub const fn new(map: IndexMap<Wtf8Buf, T>) -> Self {
444451
Self(map)
445452
}
446453

454+
// `String` keys accepted `&str` lookups for free via `Borrow<str>`; `Wtf8Buf`
455+
// borrows only as `Wtf8`, so these inherent methods restore the `&str` interface
456+
// via the zero-cost `Wtf8::new` cast, keeping every call site unchanged.
457+
#[must_use]
458+
pub fn get(&self, name: &str) -> Option<&T> {
459+
self.0.get(Wtf8::new(name))
460+
}
461+
462+
#[must_use]
463+
pub fn contains_key(&self, name: &str) -> bool {
464+
self.0.contains_key(Wtf8::new(name))
465+
}
466+
467+
pub fn swap_remove(&mut self, name: &str) -> Option<T> {
468+
self.0.swap_remove(Wtf8::new(name))
469+
}
470+
471+
pub fn shift_remove(&mut self, name: &str) -> Option<T> {
472+
self.0.shift_remove(Wtf8::new(name))
473+
}
474+
447475
pub fn pop_kwarg(&mut self, name: &str) -> Option<T> {
448476
self.swap_remove(name)
449477
}
450478
}
451479

452-
impl<T> FromIterator<(String, T)> for KwArgs<T> {
453-
fn from_iter<I: IntoIterator<Item = (String, T)>>(iter: I) -> Self {
454-
Self(iter.into_iter().collect())
480+
// Accept any key that converts into `Wtf8Buf` (notably `String`), so existing
481+
// call sites that build kwargs from string literals keep compiling unchanged.
482+
impl<K: Into<Wtf8Buf>, T> FromIterator<(K, T)> for KwArgs<T> {
483+
fn from_iter<I: IntoIterator<Item = (K, T)>>(iter: I) -> Self {
484+
Self(iter.into_iter().map(|(k, v)| (k.into(), v)).collect())
455485
}
456486
}
457487

458488
impl<'a, T> IntoIterator for &'a KwArgs<T> {
459-
type Item = (&'a String, &'a T);
460-
type IntoIter = indexmap::map::Iter<'a, String, T>;
489+
type Item = (&'a Wtf8Buf, &'a T);
490+
type IntoIter = indexmap::map::Iter<'a, Wtf8Buf, T>;
461491

462492
fn into_iter(self) -> Self::IntoIter {
463493
self.0.iter()
464494
}
465495
}
466496

467497
impl<T> IntoIterator for KwArgs<T> {
468-
type Item = (String, T);
469-
type IntoIter = indexmap::map::IntoIter<String, T>;
498+
type Item = (Wtf8Buf, T);
499+
type IntoIter = indexmap::map::IntoIter<Wtf8Buf, T>;
470500

471501
fn into_iter(self) -> Self::IntoIter {
472502
self.0.into_iter()

‎crates/vm/src/stdlib/_ast/python.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ use super::{
88
pub(crate) mod _ast {
99
use crate::{
1010
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
11-
builtins::{PyDictRef, PySet, PyStr, PyTupleRef, PyType, PyTypeRef, PyUtf8Str},
11+
builtins::{PyDictRef, PySet, PyStr, PyTupleRef, PyType, PyTypeRef},
1212
class::{PyClassImpl, StaticType},
13+
common::wtf8::Wtf8Buf,
1314
function::{ArgIterable, FuncArgs, KwArgs, PyMethodDef, PyMethodFlags},
1415
stdlib::_ast::repr,
1516
types::{Constructor, Initializer},
@@ -229,7 +230,7 @@ pub(crate) mod _ast {
229230
ast_replace_set_update(&expecting, attributes.as_ref(), vm)?;
230231

231232
for (key, _value) in &args.kwargs {
232-
let key_obj: PyObjectRef = vm.ctx.new_str(key.as_str()).into();
233+
let key_obj: PyObjectRef = vm.ctx.new_str(key.as_ref()).into();
233234
if !ast_replace_set_discard(&expecting, &key_obj, vm)? {
234235
return Err(vm.new_type_error(format!(
235236
"{}.__replace__ got an unexpected keyword argument '{}'.",
@@ -290,11 +291,11 @@ pub(crate) mod _ast {
290291
.into_iter()
291292
.map(|(key, value)| {
292293
let key = key
293-
.downcast::<PyUtf8Str>()
294+
.downcast::<PyStr>()
294295
.map_err(|_| vm.new_type_error("keywords must be strings"))?;
295-
Ok((key.as_str().to_owned(), value))
296+
Ok((key.as_wtf8().to_owned(), value))
296297
})
297-
.collect::<PyResult<IndexMap<String, PyObjectRef>>>()?;
298+
.collect::<PyResult<IndexMap<Wtf8Buf, PyObjectRef>>>()?;
298299
let result = type_obj.call(FuncArgs::new(vec![], KwArgs::new(kwargs)), vm)?;
299300
Ok(result)
300301
}
@@ -418,7 +419,7 @@ pub(crate) mod _ast {
418419
ast_replace_set_discard(&remaining_fields, &name, vm)?;
419420
}
420421
for (key, value) in args.kwargs {
421-
let key_obj: PyObjectRef = vm.ctx.new_str(key.as_str()).into();
422+
let key_obj: PyObjectRef = vm.ctx.new_str(key.as_ref()).into();
422423
let contains = fields_seq.contains(&key_obj, vm)?;
423424
if contains {
424425
if !ast_replace_set_discard(&remaining_fields, &key_obj, vm)? {

‎crates/vm/src/stdlib/_ctypes/structure.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use super::base::{CDATA_BUFFER_METHODS, PyCData, PyCField, StgInfo, StgInfoFlags};
22
use crate::builtins::{PyList, PyStr, PyTuple, PyType, PyTypeRef, PyUtf8Str};
3+
use crate::common::wtf8::Wtf8Buf;
34
use crate::convert::ToPyObject;
45
use crate::function::{FuncArgs, OptionalArg, PySetterValue};
56
use crate::protocol::{BufferDescriptor, PyBuffer, PyNumberMethods};
@@ -712,7 +713,7 @@ impl PyCStructure {
712713
self_obj: &Py<Self>,
713714
type_obj: &Py<PyType>,
714715
args: &[PyObjectRef],
715-
kwargs: &indexmap::IndexMap<String, PyObjectRef>,
716+
kwargs: &indexmap::IndexMap<Wtf8Buf, PyObjectRef>,
716717
index: usize,
717718
vm: &VirtualMachine,
718719
) -> PyResult<usize> {
@@ -746,7 +747,7 @@ impl PyCStructure {
746747
&& let Some(name) = tuple.first()
747748
&& let Some(name_str) = name.downcast_ref::<PyUtf8Str>()
748749
{
749-
let field_name = name_str.as_str().to_owned();
750+
let field_name = name_str.as_wtf8().to_owned();
750751
// Check for duplicate in kwargs
751752
if kwargs.contains_key(&field_name) {
752753
return Err(
@@ -784,9 +785,9 @@ impl Initializer for PyCStructure {
784785
}
785786

786787
// 2. Process keyword arguments
787-
for (key, value) in &args.kwargs {
788+
for (key, value) in args.kwargs {
788789
zelf.as_object()
789-
.set_attr(vm.ctx.intern_str(key.as_str()), value.clone(), vm)?;
790+
.set_attr(vm.ctx.intern_str(key), value, vm)?;
790791
}
791792

792793
Ok(())

‎crates/vm/src/stdlib/_ctypes/union.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use super::base::{CDATA_BUFFER_METHODS, StgInfoFlags};
22
use super::{PyCData, PyCField, StgInfo};
33
use crate::builtins::{PyList, PyStr, PyTuple, PyType, PyTypeRef, PyUtf8Str};
4+
use crate::common::wtf8::Wtf8Buf;
45
use crate::convert::ToPyObject;
56
use crate::function::{ArgBytesLike, FuncArgs, OptionalArg, PySetterValue};
67
use crate::protocol::{BufferDescriptor, PyBuffer};
@@ -581,7 +582,7 @@ impl PyCUnion {
581582
self_obj: &Py<Self>,
582583
type_obj: &Py<PyType>,
583584
args: &[PyObjectRef],
584-
kwargs: &indexmap::IndexMap<String, PyObjectRef>,
585+
kwargs: &indexmap::IndexMap<Wtf8Buf, PyObjectRef>,
585586
index: usize,
586587
vm: &VirtualMachine,
587588
) -> PyResult<usize> {
@@ -617,7 +618,7 @@ impl PyCUnion {
617618
&& let Some(name) = tuple.first()
618619
&& let Some(name_str) = name.downcast_ref::<PyUtf8Str>()
619620
{
620-
let field_name = name_str.as_str().to_owned();
621+
let field_name = name_str.as_wtf8().to_owned();
621622
// Check for duplicate in kwargs
622623
if kwargs.contains_key(&field_name) {
623624
return Err(
@@ -655,9 +656,9 @@ impl Initializer for PyCUnion {
655656
}
656657

657658
// 2. Process keyword arguments
658-
for (key, value) in &args.kwargs {
659+
for (key, value) in args.kwargs {
659660
zelf.as_object()
660-
.set_attr(vm.ctx.intern_str(key.as_str()), value.clone(), vm)?;
661+
.set_attr(vm.ctx.intern_str(key), value, vm)?;
661662
}
662663

663664
Ok(())

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ mod _functools {
373373

374374
// Add new keywords
375375
for (key, value) in args.kwargs {
376-
final_keywords.set_item(vm.ctx.intern_str(key.as_str()), value, vm)?;
376+
final_keywords.set_item(vm.ctx.intern_str(key), value, vm)?;
377377
}
378378

379379
Ok(Self {
@@ -436,10 +436,11 @@ mod _functools {
436436

437437
// Add keywords from self.keywords
438438
for (key, value) in &*keywords {
439+
// `expect_str()` would panic on surrogate keys; keep them as WTF-8.
439440
let key_str = key
440441
.downcast_ref::<crate::builtins::PyStr>()
441442
.ok_or_else(|| vm.new_type_error("keywords must be strings"))?;
442-
final_kwargs.insert(key_str.expect_str().to_owned(), value);
443+
final_kwargs.insert(key_str.as_wtf8().to_owned(), value);
443444
}
444445

445446
// Add keywords from args.kwargs (these override self.keywords)

0 commit comments

Comments
 (0)