Skip to content

Check percent-c character lengths before index conversion #9052

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 1 commit into from
Oct 10, 2026
Merged

Check percent-c character lengths before index conversion #9052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 34 additions & 24 deletions crates/vm/src/cformat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,33 +142,39 @@ fn spec_format_bytes(
CFormatType::Character(CCharacterType::Character) => {
// CPython parity: bytes `%c` accepts a single byte or any object
// with `__index__` in range(256).
let type_error = || {
// A bytes-like argument that is not one byte long is named by
// its length rather than by its type.
let what = if let Some(b) = obj.downcast_ref::<PyBytes>() {
format!("a bytes object of length {}", b.as_bytes().len())
} else if let Some(ba) = obj.downcast_ref::<PyByteArray>() {
format!("a bytearray object of length {}", ba.borrow_buf().len())
} else {
obj.class().name().to_string()
};
vm.new_type_error(format!(
"%c requires an integer in range(256) or a single byte, not {what}"
))
};
if let Some(b) = obj.downcast_ref::<PyBytes>() {
if b.as_bytes().len() == 1 {
return Ok(spec.format_char(b.as_bytes()[0]));
}
return Err(type_error());
} else if let Some(ba) = obj.downcast_ref::<PyByteArray>() {
let buf = ba.borrow_buf();
if buf.len() == 1 {
return Ok(spec.format_char(buf[0]));
}
drop(buf);
return Err(type_error());
}
let int = if let Some(i) = obj.downcast_ref::<PyInt>() {
i.to_owned()
} else if let Some(int_result) = obj.try_index_opt(vm) {
int_result?
} else {
// A bytes-like argument that is not one byte long is named by
// its length rather than by its type.
let what = if let Some(b) = obj.downcast_ref::<PyBytes>() {
format!("a bytes object of length {}", b.as_bytes().len())
} else if let Some(ba) = obj.downcast_ref::<PyByteArray>() {
format!("a bytearray object of length {}", ba.borrow_buf().len())
} else {
obj.class().name().to_string()
};
return Err(vm.new_type_error(format!(
"%c requires an integer in range(256) or a single byte, not {what}"
)));
return Err(type_error());
};
let ch = int
.try_to_primitive::<u8>(vm)
Expand Down Expand Up @@ -238,25 +244,29 @@ fn spec_format_string(
CFormatType::Character(CCharacterType::Character) => {
// CPython parity: `%c` accepts a single-char str or any object with
// `__index__` (the latter via PyNumber_Index dispatch).
if let Some(s) = obj.downcast_ref::<PyStr>()
&& let Ok(ch) = s.as_wtf8().code_points().exactly_one()
{
return Ok(spec.format_char(ch));
}
let int = if let Some(i) = obj.downcast_ref::<PyInt>() {
i.to_owned()
} else if let Some(int_result) = obj.try_index_opt(vm) {
int_result?
} else {
let type_error = || {
// A string argument that is not one character long is named by
// its length rather than by its type.
let what = match obj.downcast_ref::<PyStr>() {
Some(s) => format!("a string of length {}", s.char_len()),
None => obj.class().name().to_string(),
};
return Err(vm.new_type_error(format!(
vm.new_type_error(format!(
"%c requires an int or a unicode character, not {what}"
)));
))
};
if let Some(s) = obj.downcast_ref::<PyStr>() {
if let Ok(ch) = s.as_wtf8().code_points().exactly_one() {
return Ok(spec.format_char(ch));
}
return Err(type_error());
}
let int = if let Some(i) = obj.downcast_ref::<PyInt>() {
i.to_owned()
} else if let Some(int_result) = obj.try_index_opt(vm) {
int_result?
} else {
return Err(type_error());
};
let ch = int
.as_bigint()
Expand Down
25 changes: 25 additions & 0 deletions extra_tests/snippets/builtin_str.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,31 @@ def __int__(self):
assert "%x" % PercentInt(3) == "7"
assert b"%o" % PercentInt(3) == b"7"


def test_percent_character_subclasses():

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Remove the added test logic

This adds a new test function and assertions to an existing test file, but the repository policy limits test-file changes to adding expected-failure decorators or removing them together with their TODO comments; keep the %c fix in Rust without adding this test logic.

AGENTS.md reference: AGENTS.md:L273-L279

Useful? React with 👍 / 👎.

for base, value, templates in (
(str, "ab", ("%c",)),
(bytes, b"ab", (b"%c", bytearray(b"%c"))),
(bytearray, b"ab", (b"%c", bytearray(b"%c"))),
):

class Indexable(base):
def __index__(self):
raise AssertionError("%c must check the character length first")

for template in templates:
assert template % Indexable(value[:1]) == template % value[:1]
for text in (value[:0], value):
with assert_raises(TypeError) as expected:
template % base(text)
with assert_raises(TypeError) as actual:
template % Indexable(text)
assert str(actual.exception) == str(expected.exception)
assert template % PercentIndex() == template % 7


test_percent_character_subclasses()

assert (
"%(first)s %(second)s" % {"second": "World!", "first": "Hello,"} == "Hello, World!"
)
Expand Down
Loading