⚠️ Potential issue | 🟠 Major

Reject unsupported console mode flags instead of silently ignoring them.
+, a, and x are accepted but ignored, which can lead to misleading downstream errors. Consider explicitly rejecting unsupported flags (or implement the semantics).

-            for c in mode_str.bytes() {
-                match c {
-                    b'+' | b'a' | b'b' | b'x' => {}
+            for c in mode_str.bytes() {
+                match c {
+                    b'b' => {}
+                    b'+' | b'a' | b'x' => {
+                        return Err(vm.new_value_error(format!(
+                            "unsupported console mode: {mode_str}"
+                        )));
+                    }
                     b'r' => {
                         if rwa {
                             return Err(
                                 vm.new_value_error("Must have exactly one of read or write mode")
                             );
                         }
                         rwa = true;
                         readable = true;
                     }
                     b'w' => {
                         if rwa {
                             return Err(
                                 vm.new_value_error("Must have exactly one of read or write mode")
                             );
                         }
                         rwa = true;
                         writable = true;
                     }
                     _ => {
                         return Err(vm.new_value_error(format!("invalid mode: {mode_str}")));
                     }
                 }
             }

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

In `@crates/vm/src/stdlib/io.rs` around lines 5991 - 6029, The mode-parsing loop
currently accepts '+'/'a'/'b'/'x' but silently ignores them; change the match in
the parsing loop (the code handling mode_str, rwa, readable, writable,
console_type) so that encountering unsupported flags ('+', 'a', 'x' — and any
other non-rc/w characters) returns an explicit vm.new_value_error (include the
offending flag and mode_str in the message) instead of falling through; leave
handling for 'r' and 'w' as-is and ensure the final rwa check remains to error
when neither read nor write is selected.

⚠️ Potential issue | 🟠 Major

🌐 Web query:

CPython _WindowsConsoleIO MultiByteToWideChar error handling

💡 Result:

In CPython’s _io._WindowsConsoleIO (Windows console stdout/stderr), the write path converts the UTF‑8 bytes you pass into UTF‑16 using:

• MultiByteToWideChar(CP_UTF8, 0, ...) (flags 0, i.e. not MB_ERR_INVALID_CHARS), then
• WriteConsoleW(...). (chromium.googlesource.com)

That choice drives the “error handling” you see:

• With dwFlags=0, Windows’ UTF‑8 decoder generally does not fail on ill‑formed input; on Windows Vista+ it will replace illegal sequences with U+FFFD rather than returning an error. (learn.microsoft.com)
• If you want strict failure on invalid UTF‑8, you must pass MB_ERR_INVALID_CHARS and then handle MultiByteToWideChar returning 0 (typically with GetLastError()==ERROR_NO_UNICODE_TRANSLATION). (learn.microsoft.com)

This is consistent with how CPython handles strict code-page decoding elsewhere: it uses MB_ERR_INVALID_CHARS, but only reliably raises when the decode is “final” (so the decoder knows it’s not waiting for more bytes). (bugs.python.org)

Citations:

• 1: https://chromium.googlesource.com/external/github.com/python/cpython/%2B/8435a2278f964f48d36edbc5092be5ebecfcb120/Modules/_io/winconsoleio.c
• 2: https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-multibytetowidechar?utm_source=openai
• 3: https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-multibytetowidechar?utm_source=openai
• 4: https://bugs.python.org/issue45120?utm_source=openai

🌐 Web query:

Windows MultiByteToWideChar return 0 error handling best practices

💡 Result:

Best practices when MultiByteToWideChar() returns 0

1. Treat return value 0 as failure and call GetLastError() immediately

   • On failure, MultiByteToWideChar() returns 0; details are in GetLastError() (common codes: ERROR_INSUFFICIENT_BUFFER, ERROR_INVALID_FLAGS, ERROR_INVALID_PARAMETER, ERROR_NO_UNICODE_TRANSLATION). [1]

2. Use the “two-pass” pattern (query required size, then convert)

   • First call with cchWideChar = 0 to get the required UTF‑16 length (in wide chars, including NUL if it’s part of the input), allocate that many wchar_t, then call again to perform the conversion. [1]

3. Use strict validation for UTF-8 (and many other multibyte code pages): MB_ERR_INVALID_CHARS

   • For CP_UTF8, Microsoft restricts dwFlags to 0 or MB_ERR_INVALID_CHARS; other flags can yield ERROR_INVALID_FLAGS. [1]
   • With MB_ERR_INVALID_CHARS, invalid input makes the call fail (typically ERROR_NO_UNICODE_TRANSLATION), which is usually preferable to silently accepting/rewriting malformed data. [1][2]

4. Be explicit about input length / NUL-termination

   • cbMultiByte = 0 always fails. [1]
   • If you want a NUL-terminated UTF‑16 output, pass cbMultiByte = -1 (input must be NUL-terminated) or ensure your explicit byte count includes the terminator; otherwise the output may be not NUL-terminated. [1]

5. Expect “best-effort” decoding differences if you don’t use MB_ERR_INVALID_CHARS

   • Starting with Windows Vista, illegal sequences may be replaced (e.g., with U+FFFD for UTF‑8) when MB_ERR_INVALID_CHARS is not set; older systems had different behavior. [1]

Minimal error-handling template (C/C++)

int needed = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS,
                                 src, src_len, NULL, 0);
if (needed == 0) {
    DWORD err = GetLastError(); // e.g. ERROR_NO_UNICODE_TRANSLATION, etc.
    // handle/log err
}

wchar_t *dst = (wchar_t*)malloc((size_t)needed * sizeof(wchar_t));
int written = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS,
                                  src, src_len, dst, needed);
if (written == 0) {
    DWORD err = GetLastError();
    // handle/log err
}

Sources: Microsoft documentation for return values/flags/error codes and UTF‑8 flag restrictions [1]; discussion of preferring MB_ERR_INVALID_CHARS for strict UTF‑8 validation/security [2].

[1] (learn.microsoft.com)
[2] (stackoverflow.com)

Citations:

• 1: https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-multibytetowidechar?utm_source=openai
• 2: https://stackoverflow.com/questions/22824537/should-the-mb-err-invalid-chars-flag-be-used-in-utf-8-conversions-with-multibyte?utm_source=openai

🏁 Script executed:

cd crates/vm/src/stdlib && head -n 6700 io.rs | tail -n 100

Repository: RustPython/RustPython

Length of output: 3286

🏁 Script executed:

cd crates/vm/src/stdlib && sed -n '6620,6680p' io.rs

Repository: RustPython/RustPython

Length of output: 1951

Propagate MultiByteToWideChar conversion failures in write().

When MultiByteToWideChar returns 0 on the first call, it indicates a Windows error (system-level failure) that must be propagated rather than silently converted to Ok(0). This approach:

• Violates Windows API error-handling best practices (0 indicates failure; call GetLastError())
• Is semantically incorrect (success should not be reported for a failed conversion)
• Creates asymmetric error handling (first call returns Ok(0), second call returns Err)
• Hides real errors from callers, preventing proper error diagnosis

-            if wlen == 0 {
-                return Ok(0);
-            }
+            if wlen == 0 {
+                return Err(std::io::Error::last_os_error().into_pyexception(vm));
+            }

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

In `@crates/vm/src/stdlib/io.rs` around lines 6646 - 6673, The initial
MultiByteToWideChar call in write() currently treats wlen == 0 as Ok(0); change
this to propagate the Windows error the same way the second call does: when the
first MultiByteToWideChar returns 0, return
Err(std::io::Error::last_os_error().into_pyexception(vm)) so callers receive the
OS error instead of a successful 0 length; update the branch handling the first
wlen check around the MultiByteToWideChar invocation accordingly (refer to
MultiByteToWideChar and the surrounding write() logic).

⚠️ Potential issue | 🟠 Major

Respect closefd for fd‑backed consoles.

The initializer forces closefd to false when constructed from an fd, which diverges from io.open(closefd=...) semantics and can leak handles when users pass a console fd explicitly. Consider honoring the passed closefd (or document the deviation if intentional).

-            // When opened by fd, never close the fd (user owns it)
-            zelf.closefd.store(false);
+            // When opened by fd, honor closefd to match io.open semantics
+            zelf.closefd.store(args.closefd);

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

In `@crates/vm/src/stdlib/io.rs` around lines 6111 - 6114, The code currently
forces fd-backed consoles to never close the fd by unconditionally calling
zelf.closefd.store(false); instead honor the closefd parameter passed to the
console initializer (or constructor) so io.open(closefd=...) semantics are
respected: locate the initializer that sets zelf.closefd (the branch that
handles "opened by fd") and change it to store the provided closefd value (or
leave the existing value) rather than overriding it; ensure any callers that
construct from an fd accept and forward a closefd boolean so explicit user
requests to close the fd are honored.

⚠️ Potential issue | 🟡 Minor

Avoid using 0 as a sentinel in the small buffer.

copy_from_buf treats 0 as “no data”, which drops valid UTF‑8 \0 bytes and can permanently stall buffered data. Track an explicit length (or use a small VecDeque) instead of sentinel bytes.

In `@crates/vm/src/stdlib/io.rs` around lines 6298 - 6307, In copy_from_buf the
implementation uses a 0 byte as a sentinel (buf[0] != 0) which incorrectly drops
valid NUL bytes and can stall data; change the buffering to track an explicit
length or index instead of sentinel bytes (e.g., maintain a separate usize field
like "len" or use a small VecDeque) and update the logic in copy_from_buf and
any producers/consumers that mutate the SMALLBUF so they push/pop based on
length rather than shifting until a 0; locate and modify the copy_from_buf
function and any associated SMALLBUF management code to read from buf[0..len],
shift or pop properly, and decrement len (or use VecDeque::pop_front) so NUL
bytes are preserved and buffer state is explicit.