Fix malformed Unicode error constructors - #8497
Open
youknowone wants to merge 2 commits into
Open
Conversation
Replace message-only Unicode decode and encode errors with fully initialized exceptions, preserve source objects and failure ranges, and remove the obsolete constructors. Assisted-by: Codex:gpt-5
Contributor
📝 WalkthroughWalkthroughChangesUnicode error handling
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Assisted-by: Codex:gpt-5
Contributor
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] lib: cpython/Lib/codecs.py dependencies:
dependent tests: (161 tests)
Legend:
|
youknowone
marked this pull request as ready for review
August 12, 2026 01:39
Member
Author
|
@devyubin Could you please review this patch? |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/vm/src/stdlib/nt.rs`:
- Around line 52-70: Update utf8_from_bytes to decode Windows paths using
validated WTF-8 with surrogatepass semantics, allowing valid surrogate byte
sequences such as ED A0 80 to be preserved. Keep malformed-byte validation and
the existing error ranges and reasons unchanged, so _path_splitroot_ex and
_path_normpath continue returning Unicode decode errors for invalid input.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: fdefb466-99b4-4c36-98e1-7116a2e94fe8
⛔ Files ignored due to path filters (1)
Lib/test/test_codeccallbacks.pyis excluded by!Lib/**
📒 Files selected for processing (13)
crates/capi/src/pyerrors.rscrates/host_env/src/posix.rscrates/stdlib/src/array.rscrates/stdlib/src/csv.rscrates/stdlib/src/socket.rscrates/stdlib/src/tkinter.rscrates/vm/src/codecs.rscrates/vm/src/function/fspath.rscrates/vm/src/stdlib/_codecs.rscrates/vm/src/stdlib/nt.rscrates/vm/src/stdlib/os.rscrates/vm/src/stdlib/posix.rscrates/vm/src/vm/vm_new.rs
Comment on lines
+52
to
+70
| fn utf8_from_bytes<'a>(bytes: &'a [u8], vm: &VirtualMachine) -> PyResult<&'a str> { | ||
| core::str::from_utf8(bytes).map_err(|err| { | ||
| let reason = match err.error_len() { | ||
| None => "unexpected end of data", | ||
| Some(_) => match bytes[err.valid_up_to()] { | ||
| 0xc2..=0xf4 => "invalid continuation byte", | ||
| _ => "invalid start byte", | ||
| }, | ||
| }; | ||
| vm.new_unicode_decode_error( | ||
| vm.ctx.new_str("utf-8"), | ||
| vm.ctx.new_bytes(bytes.to_vec()), | ||
| err.valid_up_to(), | ||
| err.error_len() | ||
| .map_or(bytes.len(), |len| err.valid_up_to() + len), | ||
| vm.ctx.new_str(reason), | ||
| ) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline crates/vm/src/stdlib/nt.rs --items all --type function
rg -n -C 4 'fn utf8_from_bytes|from_utf8|Wtf8Buf|_path_splitroot_ex|_path_normpath' \
crates/vm/src/stdlib/nt.rs
rg -n -C 3 --glob '*.rs' '_path_splitroot_ex|_path_normpath|surrogatepass|Wtf8Buf' cratesRepository: RustPython/RustPython
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- nt.rs relevant implementation ---'
sed -n '45,75p;680,750p;900,940p' crates/vm/src/stdlib/nt.rs
printf '%s\n' '--- WTF-8 constructors and validation ---'
rg -n -C 5 'from_bytes|is_utf8|encode_wide|decode_utf16|Wtf8Buf' crates/wtf8/src/lib.rs crates/host/src crates/common/src 2>/dev/null | head -n 240
printf '%s\n' '--- Windows path argument and test coverage ---'
rg -n -C 5 '_path_splitroot_ex|_path_normpath|splitroot|normpath|invalid continuation byte|invalid start byte' --glob '*.rs' --glob '*.py' --glob '*.toml' . | head -n 320
printf '%s\n' '--- relevant package metadata ---'
rg -n 'rustpython-wtf8|rustpython_common|windows' crates/vm/Cargo.toml crates/wtf8/Cargo.toml Cargo.tomlRepository: RustPython/RustPython
Length of output: 19375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '45,75p;680,750p;900,940p' crates/vm/src/stdlib/nt.rs
rg -n -C 5 'from_bytes|is_utf8|encode_wide|decode_utf16|Wtf8Buf' crates/wtf8/src/lib.rs crates/host/src crates/common/src 2>/dev/null | head -n 240
rg -n -C 5 '_path_splitroot_ex|_path_normpath|splitroot|normpath|invalid continuation byte|invalid start byte' --glob '*.rs' --glob '*.py' --glob '*.toml' . | head -n 320
rg -n 'rustpython-wtf8|rustpython_common|windows' crates/vm/Cargo.toml crates/wtf8/Cargo.toml Cargo.tomlRepository: RustPython/RustPython
Length of output: 19295
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact WTF-8 validator ---'
sed -n '1,220p' crates/wtf8/src/lib.rs
rg -n -C 8 'fn from_bytes|from_bytes\(' crates/wtf8 crates | head -n 180
printf '%s\n' '--- path API tests and fixtures ---'
rg -n -C 8 '_path_splitroot_ex|_path_normpath|splitroot|normpath' . \
-g '*.rs' -g '*.py' -g '*.toml' -g '*.md' | head -n 360
printf '%s\n' '--- CPython Windows filesystem decoding references ---'
curl -L --fail --silent --show-error \
https://raw.githubusercontent.com/python/cpython/main/Modules/posixmodule.c |
rg -n -C 6 'DecodeFSDefault|splitroot|normpath|surrogatepass' | head -n 220Repository: RustPython/RustPython
Length of output: 45547
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' crates/wtf8/src/lib.rs
rg -n -C 8 'fn from_bytes|from_bytes\(' crates/wtf8 crates | head -n 180
rg -n -C 8 '_path_splitroot_ex|_path_normpath|splitroot|normpath' . \
-g '*.rs' -g '*.py' -g '*.toml' -g '*.md' | head -n 360
curl -L --fail --silent --show-error \
https://raw.githubusercontent.com/python/cpython/main/Modules/posixmodule.c |
rg -n -C 6 'DecodeFSDefault|splitroot|normpath|surrogatepass' | head -n 220Repository: RustPython/RustPython
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- WTF-8 surrogate decoder ---'
sed -n '700,805p' crates/wtf8/src/lib.rs
printf '%s\n' '--- CPython filesystem codec policy ---'
curl -L --fail --silent --show-error \
https://raw.githubusercontent.com/python/cpython/main/Objects/unicodeobject.c |
rg -n -C 8 'filesystem|surrogatepass|PyUnicode_DecodeFSDefault|_PyUnicode_DecodeUTF8Stateful' |
head -n 260
printf '%s\n' '--- standalone WTF-8 behavior probe ---'
python3 - <<'PY'
def strict_utf8(data):
try:
return ("ok", data.decode("utf-8"))
except UnicodeDecodeError as e:
return ("error", e.start, e.end, e.reason)
def wtf8_valid(data):
i = 0
while i < len(data):
try:
data[i:].decode("utf-8")
return True
except UnicodeDecodeError as e:
pos = i + e.start
# RustPython's Wtf8::from_bytes accepts exactly a 3-byte
# UTF-8 encoding of one UTF-16 surrogate at the failure point.
if pos + 3 > len(data):
return False
b0, b1, b2 = data[pos:pos + 3]
if not (b0 == 0xED and 0xA0 <= b1 <= 0xBF and 0x80 <= b2 <= 0xBF):
return False
i = pos + 3
return True
cases = {
"ascii": b"a",
"valid_utf8": "é".encode(),
"lone_high_surrogate": b"\xed\xa0\x80",
"lone_low_surrogate": b"\xed\xb0\x80",
"paired_surrogates": b"\xed\xa0\x80\xed\xb0\x80",
"bad_continuation": b"\xc2A",
"truncated": b"\xe2\x82",
"overlong": b"\xc0\x80",
}
for name, data in cases.items():
print(name, strict_utf8(data), "wtf8_valid=", wtf8_valid(data))
PYRepository: RustPython/RustPython
Length of output: 12979
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '700,805p' crates/wtf8/src/lib.rs
curl -L --fail --silent --show-error \
https://raw.githubusercontent.com/python/cpython/main/Objects/unicodeobject.c |
rg -n -C 8 'filesystem|surrogatepass|PyUnicode_DecodeFSDefault|_PyUnicode_DecodeUTF8Stateful' |
head -n 260
python3 - <<'PY'
def strict_utf8(data):
try:
return ("ok", data.decode("utf-8"))
except UnicodeDecodeError as e:
return ("error", e.start, e.end, e.reason)
def wtf8_valid(data):
i = 0
while i < len(data):
try:
data[i:].decode("utf-8")
return True
except UnicodeDecodeError as e:
pos = i + e.start
if pos + 3 > len(data):
return False
b0, b1, b2 = data[pos:pos + 3]
if not (b0 == 0xED and 0xA0 <= b1 <= 0xBF and 0x80 <= b2 <= 0xBF):
return False
i = pos + 3
return True
cases = {
"ascii": b"a",
"valid_utf8": "é".encode(),
"lone_high_surrogate": b"\xed\xa0\x80",
"lone_low_surrogate": b"\xed\xb0\x80",
"paired_surrogates": b"\xed\xa0\x80\xed\xb0\x80",
"bad_continuation": b"\xc2A",
"truncated": b"\xe2\x82",
"overlong": b"\xc0\x80",
}
for name, data in cases.items():
print(name, strict_utf8(data), "wtf8_valid=", wtf8_valid(data))
PYRepository: RustPython/RustPython
Length of output: 12867
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- surrogate validation bounds ---'
sed -n '785,845p' crates/wtf8/src/lib.rs
printf '%s\n' '--- Windows filesystem defaults in CPython ---'
curl -L --fail --silent --show-error \
https://raw.githubusercontent.com/python/cpython/main/Python/initconfig.c |
rg -n -C 8 'filesystem_errors|surrogatepass|utf-8' | head -n 240
printf '%s\n' '--- current helper error mapping ---'
nl -ba crates/vm/src/stdlib/nt.rs | sed -n '52,72p'Repository: RustPython/RustPython
Length of output: 8670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '785,845p' crates/wtf8/src/lib.rs
curl -L --fail --silent --show-error \
https://raw.githubusercontent.com/python/cpython/main/Python/initconfig.c |
rg -n -C 8 'filesystem_errors|surrogatepass|utf-8' | head -n 240
nl -ba crates/vm/src/stdlib/nt.rs | sed -n '52,72p'Repository: RustPython/RustPython
Length of output: 8549
Decode Windows byte paths with WTF-8 semantics.
utf8_from_bytes rejects valid WTF-8 such as b"\xED\xA0\x80". Both _path_splitroot_ex and _path_normpath therefore fail before preserving the surrogate. Use validated WTF-8 decoding with Windows surrogatepass behavior, while retaining the current malformed-byte ranges and reasons.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/vm/src/stdlib/nt.rs` around lines 52 - 70, Update utf8_from_bytes to
decode Windows paths using validated WTF-8 with surrogatepass semantics,
allowing valid surrogate byte sequences such as ED A0 80 to be preserved. Keep
malformed-byte validation and the existing error ranges and reasons unchanged,
so _path_splitroot_ex and _path_normpath continue returning Unicode decode
errors for invalid input.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8352.
Summary
UnicodeDecodeErrorandUnicodeEncodeErrorconstruction with fully initialized exceptionsnew_unicode_decode_errorRoot cause and impact
The old macro-generated helpers bypassed Unicode error initialization and created one-argument exceptions. Those exceptions had no
encoding,object,start,end, orreasonattributes, and their string representation could be empty. All affected paths now produce structurally valid exceptions that error handlers and user code can inspect.Validation
cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capicargo check -p rustpython-stdlib --features tkintercargo checkfromcrates/capimbcs/oem,ntpath decoding, and the original CSV reproductionrustfmt,cspell, merge-conflict and patch checks)The standalone C-API
cargo testreaches the link step but cannot run in this checkout because its Windows configuration references the unavailable placeholder librarypythonXY.lib; itscargo checkpasses.Summary by CodeRabbit