Skip to content

Commit cc51913

Browse files
jseop-limjseop-lim
authored
Convert an __index__ key before refusing item assignment or deletion (#9516)
* Convert an index key before refusing item assignment When a type has a sequence table but no ass_item slot, such as tuple, bytes, str or any heap type, item assignment and deletion raised TypeError without looking at the key. An index key is converted first, so an exception from its __index__ propagates and an int too large for an index raises IndexError. Only then is the operation refused. Do that conversion in set_item() and del_item(). Deletion keeps the "doesn't support" wording for index keys only, and uses "does not support" for any other key. Assisted-by: Claude Code:claude-opus-5-5 * Read the type name after converting the deletion key del_item() held the read guard on a heap type's name while the key's __index__ ran, so an __index__ that assigned the type's __name__ deadlocked a threaded build. Convert the key first and read the name afterwards, which also names the renamed type as CPython does. Keep the snippet's original docstring. Assisted-by: Claude Code:claude-opus-5-5
1 parent 860df6f commit cc51913

2 files changed

Lines changed: 77 additions & 1 deletion

File tree

‎crates/vm/src/protocol/object.rs‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,20 @@ use crate::{
1515
types::{Constructor, PyComparisonOp},
1616
};
1717

18+
/// Converts an index key the way a sequence does before it refuses the
19+
/// operation, so an error from `__index__` or the size check wins.
20+
fn sequence_index_key(key: &PyObject, vm: &VirtualMachine) -> PyResult<Option<isize>> {
21+
let Some(index) = key.try_index_opt(vm) else {
22+
return Ok(None);
23+
};
24+
index?.try_to_primitive(vm).map(Some).map_err(|_| {
25+
vm.new_index_error(format!(
26+
"cannot fit '{}' into an index-sized integer",
27+
key.class().name()
28+
))
29+
})
30+
}
31+
1832
// RustPython doesn't need these items
1933
// PyObject *Py_NotImplemented
2034
// Py_RETURN_NOTIMPLEMENTED
@@ -812,6 +826,10 @@ impl PyObject {
812826
return f(seq, i, Some(value), vm);
813827
}
814828

829+
// Every heap type carries a sequence table.
830+
if seq.slots().has_any() || self.class().heaptype_ext().is_some() {
831+
sequence_index_key(&needle.to_pyobject(vm), vm)?;
832+
}
815833
Err(vm.new_type_error(format!(
816834
"'{}' object does not support item assignment",
817835
self.class().slot_name()
@@ -836,8 +854,11 @@ impl PyObject {
836854

837855
// A type carrying a sequence table turns the deletion down in
838856
// PySequence_DelItem's words instead; every heap type carries one.
857+
let index_key = (seq.slots().has_any() || self.class().heaptype_ext().is_some())
858+
&& sequence_index_key(&needle.to_pyobject(vm), vm)?.is_some();
859+
// Read the name only now: the key's __index__ may have renamed the type.
839860
let name = self.class().slot_name();
840-
let msg = if seq.slots().has_any() || self.class().heaptype_ext().is_some() {
861+
let msg = if index_key {
841862
format!("'{name}' object doesn't support item deletion")
842863
} else {
843864
format!("'{name}' object does not support item deletion")

‎extra_tests/snippets/protocol_index_bad.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Test that indexing ops don't hang when an object with a mutating
22
__index__ is used."""
33

4+
import operator
45
from array import array
56

67
from testutils import assert_raises
@@ -33,3 +34,57 @@ def run_delslice():
3334
for e in instances:
3435
run_setslice()
3536
run_delslice()
37+
38+
39+
# An exception from __index__ propagates before the operation is refused.
40+
class RaisingIndex:
41+
def __index__(self):
42+
raise ZeroDivisionError
43+
44+
45+
class Plain:
46+
pass
47+
48+
49+
t = ()
50+
with assert_raises(ZeroDivisionError):
51+
t[RaisingIndex()] = 0
52+
with assert_raises(ZeroDivisionError):
53+
del t[RaisingIndex()]
54+
with assert_raises(ZeroDivisionError):
55+
operator.setitem(b"", RaisingIndex(), 0)
56+
with assert_raises(ZeroDivisionError):
57+
operator.delitem("", RaisingIndex())
58+
with assert_raises(ZeroDivisionError):
59+
Plain()[RaisingIndex()] = 0
60+
with assert_raises(ZeroDivisionError):
61+
del Plain()[RaisingIndex()]
62+
with assert_raises(IndexError):
63+
t[2**70] = 0
64+
with assert_raises(IndexError):
65+
del t[2**70]
66+
67+
with assert_raises(TypeError) as cm:
68+
del t[0]
69+
assert str(cm.exception) == "'tuple' object doesn't support item deletion"
70+
with assert_raises(TypeError) as cm:
71+
del t["a"]
72+
assert str(cm.exception) == "'tuple' object does not support item deletion"
73+
with assert_raises(TypeError) as cm:
74+
del object()[RaisingIndex()]
75+
assert str(cm.exception) == "'object' object does not support item deletion"
76+
77+
78+
class Renamed:
79+
pass
80+
81+
82+
class RenamingIndex:
83+
def __index__(self):
84+
Renamed.__name__ = "Other"
85+
return 0
86+
87+
88+
with assert_raises(TypeError) as cm:
89+
del Renamed()[RenamingIndex()]
90+
assert str(cm.exception) == "'Other' object doesn't support item deletion"

0 commit comments

Comments
 (0)