Repository navigation
Read console stdin with ReadConsoleW on Windows so non-ASCII input survives (REPL, prompt(), bun -) - #38140
Read console stdin with ReadConsoleW on Windows so non-ASCII input survives (REPL, prompt(), bun -)#38140robobun wants to merge 6 commits into
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
|
Updated 10:57 AM PT - Aug 13th, 2026
✅ @robobun, your commit f91439296e73bc3c59b76c9d9e7be28c389e3605 passed in 🧪 To try this PR locally: bunx bun-pr 38140That installs a local version of the PR into your bun-38140 --bun |
|
Status: ready for a maintainer. Reproduced on Windows Server 2019 with bun 1.4.0-canary.1+9a543cc18 by giving bun a real console and typing into it (the harness is Review: the two line comments on |
There was a problem hiding this comment.
Thanks — f914392 addresses the pending_lead ordering nit (now cleared only after read_units() succeeds), and 0be7d61 already handled the mid_line Ctrl+Z tracking. I have no further findings.
Given this adds a new Windows console-input subsystem in the core bun_sys::read() path (process-global Mutex-guarded buffer held across a blocking ReadConsoleW, conhost-version-dependent Ctrl+Z / line-length semantics, plus a hand-rolled FFI console driver in the test fixture), a human look is still worthwhile.
What was reviewed: the fd == Fd::stdin() dispatch keeps ordinary file reads off the GetConsoleMode probe; BYTES_CAPACITY = 3 * UNITS_CAPACITY covers the UTF-8 worst case so copy_utf16_into_utf8 cannot short-write; the OPERATION_ABORTED retry mirrors the existing ReadFile path; and the surrogate-hold-back / mid_line state now survives an error return.
Extended reasoning...
Overview
This PR adds src/sys/windows/console_stdin.rs, a new module that intercepts bun_sys::read() on Windows when the fd is the process stdin and GetConsoleMode succeeds on it, replacing the ReadFile path with ReadConsoleW + in-process UTF-16→UTF-8 transcoding. The dispatch is a six-line insertion in src/sys/lib.rs (windows_impl::read); src/sys/windows/mod.rs gains only the pub(crate) mod console_stdin; line. Three new test files exercise prompt()/alert()/confirm(), the REPL in raw mode, and bun - against a real headless console driven via WriteConsoleInputW over bun:ffi.
Security risks
None identified. The new code is read-only console input; the unsafe block around ReadConsoleW has a SAFETY comment and its buffer is a fixed [u16; 8193] owned by the State struct. out.len() as DWORD cannot overflow (8192). The transcode destination is sized for the 3-bytes-per-unit worst case, so copy_utf16_into_utf8 cannot write past bytes. No user-controlled sizes reach an allocation.
Level of scrutiny
Moderate-to-high. The change is small in line count but sits directly in the bun_sys::read() hot path for HANDLE-backed fds on Windows, introduces process-global mutable state guarded by a bun_core::Mutex that is held across a blocking syscall (acceptable here — concurrent readers of the one stdin must serialize anyway, and the comment says so), and encodes conhost behaviour that differs between Windows 10/Server 2019 and Windows 11 (line-length capping vs multi-chunk delivery, Ctrl+Z semantics). The test fixture is itself ~190 lines of raw Win32 FFI (CreateProcessW, job objects, INPUT_RECORD construction). This is not a mechanical change; a maintainer familiar with the Windows lanes should confirm the design (global buffer, 8192-unit chunk, fd == Fd::stdin() gating) and that CI on windows-x64 is green.
Other factors
Two prior nits from earlier runs of this reviewer were both addressed in follow-up commits: mid_line tracking so the Ctrl+Z→EOF rule only fires at start-of-line (0be7d61), and reordering pending_lead clearing to after the fallible read_units() call (f914392). All comment-cop bot threads about long comments were resolved (7b675ab, 7172e07). The PR description is unusually thorough — every design constant is backed by a measurement — and the tests are structured to fail on the released build (USE_SYSTEM_BUN=1) and pass on the debug build. Nonetheless the guidelines exclude auto-approval for changes of this scope in a core syscall wrapper.
Problem
bun replsilently drops them,prompt()returns one junk byte per character (typinga中b😀intoprompt()returned"a b"on one run anda+ U+FFFD +bon another; the junk byte varies per run and the emoji's two UTF-16 units come back as two NULs), and a script typed intobun -gets the same junk in its source.한국어came back as three junk bytes. This is the Windows half of bun repl: support more language characters #27556; repl: keep multi-byte UTF-8 input in the interactive line editor #31876 fixed the POSIX line editor.bun_sys::read()for HANDLE-backed fds (src/sys/lib.rs,windows_impl::read) callsReadFile. On a console handle that is the ANSI console API: conhost converts the typed UTF-16 through the console input code page, and its CP_UTF8 conversion (bun sets code page 65001 at startup insrc/bun_core/output.rs) does not produce multi-byte UTF-8 on Windows 10 / Server 2019. Every native stdin reader ends up in this one function: the REPL'sread_byte(File::stdin().read, 256-byte buffer),prompt()through the 4 KiBBufferedStdin,alert()/confirm()/bun init/bun update -ithrough one-byteStdinReaderreads,bun publish's OTP prompt, andbun -throughread_to_end_into.process.stdin,Bun.stdinandfs.readSync(0)go through libuv (fd 0 is a Uv-kind fd) and were never affected; they are not touched.Fix
read()hands the process stdin to a newbun_sys::windows::console_stdinmodule whenGetConsoleModesucceeds on it. The module reads 8192 UTF-16 units at a time withReadConsoleWinto a process-wide buffer, transcodes the chunk withbun_core::strings::copy_utf16_into_utf8, and servesread()calls of any size from the transcoded bytes. Pipes and files still take theReadFilepath, so redirected stdin stays byte-exact. The dispatch is keyed onfd == Fd::stdin(), so ordinary file reads do not pay for aGetConsoleModecall; stdin is the only console handle bun reads this way.prompt()accepted 4094 characters withReadFile(its 4 KiB buffer), a first version of this change that requestedbuf.len() / 3units cut that to 1022, and with 8192 unitsprompt()takes 8190 andbun -keeps the 8190 it had (itsReadFilerequests were 8 KiB). A 5000-character line is part of the test.read()go straight back to the console rather than report end of input.ReadFile, measured on Server 2019 before the change: in line-input mode a line that starts with Ctrl+Z is end of input (prompt()returnsnull,bun -runs the script), Ctrl+Z elsewhere in a line is an ordinary character, and end of input is not sticky (the next line is readable).ReadConsoleWitself has no Ctrl+Z rule, so it is applied here, to chunks that start a line (a newer console delivers a line longer than one chunk over several reads, and the continuation chunks are not checked); raw mode (the REPL) gets byte 0x1A as before. Ctrl+C / Ctrl+Break cancel a console read withERROR_OPERATION_ABORTED, which is retried exactly like theReadFilepath does; a Ctrl+C probe againstprompt()and the REPL behaves identically before and after (details below).test/js/bun/windows/console-stdin.test.ts.console-stdin-driver-fixture.tsgives bun a real headless console (AllocConsole, the child gets the console as its stdio) and types into it withWriteConsoleInputW; the program under test writes what it read to a file. Three cases:prompt()/alert()/confirm()with the Ctrl+Z lines above and the 5000-character line; the REPL in raw mode, where the emoji's high surrogate is typed only after the REPL has consumed everything before it (the driver waits for the console input queue to drain) so it arrives in a read of its own; andbun -ended with a Ctrl+Z line. On windows-x64 all three fail with bun 1.4.0-canary.1 (USE_SYSTEM_BUN=1, output below) and pass withbun bd test; in CI the file runs and passes on both the Windows Server 2019 x64 and the Windows 11 aarch64 test lanes (the old and the new console), and is skipped on other platforms, so the Linux gate cannot exercise it. A temporarily instrumented build confirmed the REPL case reaches the surrogate hold-back with a one-unit chunk.cargo check -p bun_sysforx86_64-pc-windows-msvcandaarch64-pc-windows-msvc, clippy on the Windows target (nothing reported in the new code),test/internal/source-lints/, and the test typecheck for the new files.cargo test -p bun_syscannot link (bun_core needs the C++ objects), so the module has no Rust unit tests; the console cases cover the large-buffer, one-byte, split-surrogate, raw-mode, long-line and end-of-input paths.Background
ReadConsoleWreturns the typed text as UTF-16.ReadFileon a console handle is routed to the ANSI variant, which converts that text to bytes using the console input code page; that conversion is what misbehaves for code page 65001 on the conhost versions in question, which is why Unicode-aware programs read consoles withReadConsoleWand convert themselves.ENABLE_LINE_INPUTset (the default, used byprompt()and the CLI prompts) a read returns one line at a time after Enter, terminated by\r\n, and on the older conhost the requested count is the line-length limit; newer consoles return a longer line over several reads. With the flag cleared (the REPL's raw mode) a read returns whatever keys are available. The mode is applied when the program reads, not when the user types, which is what lets the test queue keystrokes before launching the child.Fdis either Uv-kind (a CRT fd owned by libuv, such as fd 0 used byprocess.stdinandfs) or System-kind (a raw HANDLE). The cached process stdin behindFile::stdin()andOutput's stdin readers is System-kind, which is why these readers reach theReadFilebranch ofread().Measurements behind the Ctrl+Z and Ctrl+C rules and the fail-before output (Windows Server 2019, typed through the same console harness)
prompt()with the released build (ReadFile) and with this change (ReadConsoleW) agree on every row:^ZEnternull^ZabcEnternullabc^ZEnter"abc\x1a"ab^ZcdEnter"ab\x1acd"^Zwithout EnterLine length in line-input mode: released build,
prompt()4094 characters,bun -first line about 8190; this change,prompt()8190,bun -8190 (a 9000-character line is cut at 8190 by the console in both cases).Ctrl+C sent with
GenerateConsoleCtrlEventwhile blocked, released build and this change identical:prompt()with no SIGINT listener exits withSTATUS_CONTROL_C_EXIT;prompt()with aprocess.on("SIGINT")listener keeps blocking in the read (the listener cannot run while the main thread is blocked, same as before); the REPL exits withSTATUS_CONTROL_C_EXIT(pre-existing Windows REPL behaviour, reported separately).New test against bun 1.4.0-canary.1+9a543cc18 (
USE_SYSTEM_BUN=1) on windows-x64: