Skip to content
Merged
Changes from 1 commit
Commits
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
Next Next commit
mbcs_codec
  • Loading branch information
youknowone committed Dec 7, 2025
commit e3c7ca5dfa18db8f6500869d4b62f815b4c53f46
193 changes: 193 additions & 0 deletions crates/vm/src/stdlib/codecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,203 @@ mod _codecs {
}};
}

#[cfg(windows)]
#[derive(FromArgs)]
struct MbcsEncodeArgs {
#[pyarg(positional)]
s: PyStrRef,
#[pyarg(positional, optional)]
errors: Option<PyStrRef>,
}

#[cfg(windows)]
#[pyfunction]
fn mbcs_encode(args: MbcsEncodeArgs, vm: &VirtualMachine) -> PyResult<(Vec<u8>, usize)> {
use std::os::windows::ffi::OsStrExt;
use windows_sys::Win32::Globalization::{
CP_ACP, WC_NO_BEST_FIT_CHARS, WideCharToMultiByte,
};

let errors = args.errors.as_ref().map(|s| s.as_str()).unwrap_or("strict");
let s = match args.s.to_str() {
Some(s) => s,
None => {
// String contains surrogates - not encodable with mbcs
return Err(vm.new_unicode_encode_error(
"'mbcs' codec can't encode character: surrogates not allowed".to_string(),
));
}
};
let char_len = args.s.char_len();

if s.is_empty() {
return Ok((Vec::new(), char_len));
}

// Convert UTF-8 string to UTF-16
let wide: Vec<u16> = std::ffi::OsStr::new(s).encode_wide().collect();

// Get the required buffer size
let size = unsafe {
WideCharToMultiByte(
CP_ACP,
WC_NO_BEST_FIT_CHARS,
wide.as_ptr(),
wide.len() as i32,
std::ptr::null_mut(),
0,
std::ptr::null(),
std::ptr::null_mut(),
)
};

if size == 0 {
let err = std::io::Error::last_os_error();
return Err(vm.new_os_error(format!("mbcs_encode failed: {}", err)));
}

let mut buffer = vec![0u8; size as usize];
let mut used_default_char: i32 = 0;

let result = unsafe {
WideCharToMultiByte(
CP_ACP,
WC_NO_BEST_FIT_CHARS,
wide.as_ptr(),
wide.len() as i32,
buffer.as_mut_ptr().cast(),
size,
std::ptr::null(),
if errors == "strict" {
&mut used_default_char
} else {
std::ptr::null_mut()
},
)
};

if result == 0 {
let err = std::io::Error::last_os_error();
return Err(vm.new_os_error(format!("mbcs_encode failed: {err}")));
}

if errors == "strict" && used_default_char != 0 {
return Err(vm.new_unicode_encode_error(
"'mbcs' codec can't encode characters: invalid character",
));
}

buffer.truncate(result as usize);
Ok((buffer, char_len))
}
Comment on lines +237 to +316

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Search for tests related to mbcs codec
rg -n "mbcs" --type=rs --type=py -i

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Look for error handler validation patterns in the codebase
rg -n "errors.*strict|ignore|replace" --type=rs | head -30

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check the broader context around codecs.rs to understand how other encode functions handle errors
rg -n "errors.*as_ref|errors.*unwrap_or" --type=rs -B2 -A2

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check the codecs.rs file structure to see if there are other encode functions
cat -n crates/vm/src/stdlib/codecs.rs | head -100

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check the entire mbcs_encode function and surrounding context
sed -n '237,310p' crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check if there are other encode functions to see how they handle errors
grep -n "fn.*encode" crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's see what other encoding functions look like to understand error handling patterns
sed -n '1,150p' crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's also check if there are any other error-related constants being used
rg -n "CP_ACP|WC_NO_BEST_FIT_CHARS" --type=rs -B5 -A5

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Look for any test files that test this function
find . -name "*.rs" -type f | xargs grep -l "mbcs" 2>/dev/null

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check if there are Python stdlib tests we can reference
find . -name "*.py" -type f | xargs grep -l "mbcs" 2>/dev/null | head -10

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's check the entire file to understand the error handling context
wc -l crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check the full codecs.rs file for other encode functions and their error handling
sed -n '100,250p' crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's check if there are any comments or documentation about error handling in this file
grep -n "error\|Error\|ERROR" crates/vm/src/stdlib/codecs.rs | head -20

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's understand what the "errors" parameter should control by checking Python documentation
# and looking at the actual Windows API behavior
# Check if this function validates the errors parameter properly
sed -n '237,250p' crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check what MbcsEncodeArgs structure looks like
rg -n "struct MbcsEncodeArgs" --type=rs -A10

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's look at the broader context of the file
sed -n '1,50p' crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Find the MbcsEncodeArgs definition
grep -n "MbcsEncodeArgs" crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Get the definition of MbcsEncodeArgs
sed -n '210,236p' crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's check if there's a utf_8_encode or similar function to see how errors are handled there
sed -n '50,150p' crates/vm/src/stdlib/codecs.rs | grep -A20 "fn utf"

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's look for any utf_8_encode function in the file
rg -n "fn utf_8_encode" --type=rs -A30

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Let's understand the complete behavior by looking at lines around mbcs_encode
sed -n '230,330p' crates/vm/src/stdlib/codecs.rs

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check commit history for mbcs_encode changes
git log --all -p --follow -S "mbcs_encode" -- crates/vm/src/stdlib/codecs.rs | head -100

Repository: RustPython/RustPython

Length of output: 186


🏁 Script executed:

# Check the git blame to understand the history
git blame -L237,310 crates/vm/src/stdlib/codecs.rs | head -20

Repository: RustPython/RustPython

Length of output: 186


Implement error handling for non-strict error modes in mbcs_encode.

The current implementation only properly handles "strict" error mode. Non-strict modes ("ignore", "replace", "xmlcharrefreplace", "backslashreplace") lack proper implementation:

  • Invalid error mode names are silently treated as non-strict (no validation at line 245)
  • Only "strict" mode tracks unmappable characters via used_default_char (lines 294-302)
  • Non-strict modes don't implement their specified behavior: "ignore" should skip unmappable characters, "replace" should substitute them, and other modes require custom handling
  • The WC_NO_BEST_FIT_CHARS flag is always set regardless of error mode, but the code doesn't explicitly handle the consequences in non-strict modes

This causes non-strict error modes to exhibit undefined behavior dependent on the Windows MBCS implementation rather than Python's codec specification.

🤖 Prompt for AI Agents
In crates/vm/src/stdlib/codecs.rs around lines 237 to 308, the mbcs_encode
currently validates only "strict" and doesn't implement behavior for non-strict
modes: validate the errors argument against allowed modes
("strict","ignore","replace","xmlcharrefreplace","backslashreplace") and return
a ValueError for invalid names; always request WideCharToMultiByte to report
default-char usage (track used_default_char) and, after conversion, if
used_default_char!=0 handle unmappable characters according to the selected
mode: for "strict" raise a UnicodeEncodeError as now; for "ignore" remove the
bytes corresponding to unmappable input characters (skip them from the output
and adjust returned char count appropriately); for "replace" substitute
unmappable characters with a single replacement byte (e.g. b'?' or the system
best-fit char provided by the conversion) while preserving char length
semantics; for "backslashreplace" and "xmlcharrefreplace" post-process the
original UTF-8 input to replace each unmappable character with the corresponding
ASCII escape or XML character reference and then re-encode that sequence to
bytes; ensure WC_NO_BEST_FIT_CHARS is cleared or set appropriately per mode so
Windows does/doesn't do best-fit mapping, and update buffer
construction/truncation and returned char length to reflect any removals or
insertions.


#[cfg(not(windows))]
#[pyfunction]
fn mbcs_encode(args: FuncArgs, vm: &VirtualMachine) -> PyResult {
delegate_pycodecs!(mbcs_encode, args, vm)
}

#[cfg(windows)]
#[derive(FromArgs)]
struct MbcsDecodeArgs {
#[pyarg(positional)]
data: ArgBytesLike,
#[pyarg(positional, optional)]
errors: Option<PyStrRef>,
#[pyarg(positional, default = false)]
#[allow(dead_code)]
r#final: bool,
}

#[cfg(windows)]
#[pyfunction]
fn mbcs_decode(args: MbcsDecodeArgs, vm: &VirtualMachine) -> PyResult<(String, usize)> {
use windows_sys::Win32::Globalization::{
CP_ACP, MB_ERR_INVALID_CHARS, MultiByteToWideChar,
};

let _errors = args.errors.as_ref().map(|s| s.as_str()).unwrap_or("strict");
let data = args.data.borrow_buf();
let len = data.len();

if data.is_empty() {
return Ok((String::new(), 0));
}

// Get the required buffer size for UTF-16
let size = unsafe {
MultiByteToWideChar(
CP_ACP,
MB_ERR_INVALID_CHARS,
data.as_ptr().cast(),
len as i32,
std::ptr::null_mut(),
0,
)
};

if size == 0 {
// Try without MB_ERR_INVALID_CHARS for non-strict mode (replacement behavior)
let size = unsafe {
MultiByteToWideChar(
CP_ACP,
0,
data.as_ptr().cast(),
len as i32,
std::ptr::null_mut(),
0,
)
};
if size == 0 {
let err = std::io::Error::last_os_error();
return Err(vm.new_os_error(format!("mbcs_decode failed: {}", err)));
}

let mut buffer = vec![0u16; size as usize];
let result = unsafe {
MultiByteToWideChar(
CP_ACP,
0,
data.as_ptr().cast(),
len as i32,
buffer.as_mut_ptr(),
size,
)
};
if result == 0 {
let err = std::io::Error::last_os_error();
return Err(vm.new_os_error(format!("mbcs_decode failed: {}", err)));
}
buffer.truncate(result as usize);
let s = String::from_utf16(&buffer)
.map_err(|e| vm.new_unicode_decode_error(format!("mbcs_decode failed: {}", e)))?;
return Ok((s, len));
}

// Strict mode succeeded - no invalid characters
let mut buffer = vec![0u16; size as usize];
let result = unsafe {
MultiByteToWideChar(
CP_ACP,
MB_ERR_INVALID_CHARS,
data.as_ptr().cast(),
len as i32,
buffer.as_mut_ptr(),
size,
)
};
if result == 0 {
let err = std::io::Error::last_os_error();
return Err(vm.new_os_error(format!("mbcs_decode failed: {}", err)));
}
buffer.truncate(result as usize);
let s = String::from_utf16(&buffer)
.map_err(|e| vm.new_unicode_decode_error(format!("mbcs_decode failed: {}", e)))?;

Ok((s, len))
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#[cfg(not(windows))]
#[pyfunction]
fn mbcs_decode(args: FuncArgs, vm: &VirtualMachine) -> PyResult {
delegate_pycodecs!(mbcs_decode, args, vm)
Expand Down