fix(ext/node): rewrite node:tty on top of uv compat#32777
fix(ext/node): rewrite node:tty on top of uv compat#32777nathanwhit merged 21 commits intodenoland:mainfrom
Conversation
kajukitli
left a comment
There was a problem hiding this comment.
This PR introduces libuv stream wrapping infrastructure with multiple critical memory safety issues. The primary concerns are use-after-free vulnerabilities stemming from raw pointers stored in callback data that can outlive their referents when GC occurs, and improper lifetime management between Rust structs and libuv handles. The most severe issue is that LibUvStreamWrap stores raw pointers to its internal fields in callback data passed to libuv, but these pointers can become dangling if the GarbageCollected object is freed while async operations are pending.
[CRITICAL] ext/node/ops/stream_wrap.rs:147: LibUvStreamWrap stores a raw pointer stream: *const uv_stream_t without any ownership tracking. If the underlying stream is freed (e.g., closed by uv_close) while this GarbageCollected object still holds the pointer, subsequent method calls will dereference a dangling pointer causing use-after-free.
Suggestion: Track whether the stream handle is still valid (similar to HandleWrap's state tracking) and set the stream pointer to null when the handle is closed.
[HIGH] ext/node/ops/stream_wrap.rs:302: CallbackData contains bytes_read: *const Cell<u64> pointing to LibUvStreamWrap's internal field. If LibUvStreamWrap is garbage collected while reads are pending, the on_uv_read callback will dereference freed memory at line 262.
Suggestion: Store bytes_read in a shared ownership structure (Arc) or ensure CallbackData prevents GC of the parent LibUvStreamWrap via v8::Global reference.
[HIGH] ext/node/ops/stream_wrap.rs:579: In write_buffer(), after partial uv_try_write(), a raw pointer to ArrayBuffer data is passed to async uv_write. If the ArrayBuffer is GC'd before the async write completes, this causes use-after-free. WriteCallbackData doesn't hold a reference to the buffer.
Suggestion: Store a v8::Global reference to the buffer in WriteCallbackData, or copy the remaining data to owned memory.
[HIGH] ext/node/ops/stream_wrap.rs:152: js_handle field uses UnsafeCell but is not traced in GarbageCollected::trace implementation. The referenced JS object can be collected while still being referenced by this field.
Suggestion: Add js_handle to the GarbageCollected::trace implementation to prevent premature collection.
[MEDIUM] ext/node/ops/handle_wrap.rs:153: Handle enum stores raw pointer *const uv_handle_t that can become dangling if the handle is closed/freed. Methods like has_ref(), uv_ref(), uv_unref() dereference this pointer without validity checks.
Suggestion: Track handle validity state and check before dereferencing the raw pointer.
[MEDIUM] ext/node/ops/stream_wrap.rs:243: Pattern of reconstructing Global from raw pointer, cloning, then leaking back is error-prone. If an exception occurs between from_raw() and into_raw(), the Global will be dropped and invalidate the stored pointer.
Suggestion: Consider a safer pattern that doesn't require this clone-and-leak approach, or wrap in a helper that guarantees re-leaking.
[LOW] ext/node/ops/stream_wrap.rs:189: In on_uv_alloc, allocation failure (null from std::alloc::alloc) sets buffer to null, but libuv may not handle this gracefully and could still call on_uv_read with invalid data.
Suggestion: Consider aborting on OOM as is common practice, or verify libuv's handling of null buffers.
…ndestructible Two fixes for issues introduced by the TTY rewrite (#32777): 1. The TLA stall detection in `poll_event_loop_inner` didn't consider `has_uv_alive_handles`, so active native TTY handles (e.g. stdin reading via readline) couldn't prevent the "Top-level await promise never resolved" error. This broke any code using `await` with readline/stdin when output was piped through a PassThrough stream (like @inquirer/prompts does). 2. TTY `process.stdout`/`process.stderr` were not made indestructible at the process level. Libraries like mute-stream call `destroy()` on stdout between prompts. Without the `dummyDestroy` override and `_isStdio` flag, the underlying TTY handle would be closed, breaking subsequent I/O. This matches Node.js's behavior in `lib/internal/bootstrap/switches/is_main_thread.js`. Fixes #32782 Fixes #30747 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ing, and read blocking Three fixes for Windows TTY regressions introduced in the node:tty rewrite (denoland#32777): 1. Restore saved console mode for NORMAL: uv_tty_set_mode(NORMAL) was setting a hardcoded subset of flags (ECHO|LINE|PROCESSED = 0x0007), losing ENABLE_QUICK_EDIT_MODE, ENABLE_INSERT_MODE, and ENABLE_EXTENDED_FLAGS that Windows sets by default. Now restores the original mode saved at init time, matching libuv behavior. 2. Use WriteConsoleW for TTY output: WriteFile writes raw bytes interpreted by the console's active code page, garbling non-ASCII output (box-drawing chars, Unicode) when the code page isn't UTF-8. Now converts UTF-8 to UTF-16 and calls WriteConsoleW, matching libuv. 3. Re-check events before each read: the Windows read loop evaluated GetNumberOfConsoleInputEvents once and looped with a stale boolean, so ReadFile could block the event loop after consuming all available events. Now re-checks before each ReadFile call. Fixes denoland#32996 Fixes denoland#32997 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…de, encoding, raw + line mode) (#32999) ## Summary Rewrites the Windows TTY read/write paths to match libuv's behavior, fixing regressions introduced in the `node:tty` rewrite (#32777, landed in v2.7.6). ### Console mode restoration `uv_tty_set_mode(NORMAL)` was replacing the console mode with a hardcoded subset of flags (`ECHO|LINE|PROCESSED` = 0x0007), losing `ENABLE_QUICK_EDIT_MODE`, `ENABLE_INSERT_MODE`, and `ENABLE_EXTENDED_FLAGS` that Windows sets by default. This broke interactive input after the first raw-mode cycle (e.g. vite create's multi-step prompts). Now restores the original mode saved at init time, matching libuv's behavior. ### TTY output encoding `tty_try_write` used `WriteFile` which writes raw bytes interpreted according to the console's active code page. On non-UTF-8 code pages (common on Windows), this garbled Unicode output -- box-drawing characters, accented text, CJK, etc. Now converts UTF-8 to UTF-16 and uses `WriteConsoleW`, matching libuv's approach. ### Raw mode reading rewrite Replaced `ReadFile`-based console reading with `ReadConsoleInputW` for raw mode, matching libuv's `uv_process_tty_read_raw_req` approach. `ReadFile` on a console handle only consumes KEY_DOWN events that produce characters, blocking on KEY_UP/FOCUS/MOUSE events and freezing the event loop. The new implementation: - Reads individual `INPUT_RECORD` structs via `ReadConsoleInputW` - Filters non-key events, KEY_UP events (except Alt composition), and numpad keys during Alt-code entry - Encodes Unicode characters to UTF-8 with surrogate pair support - Maps function keys (arrows, F1-F12, Home/End/etc.) to VT100/xterm escape sequences (ported from libuv's `get_vt100_fn_key`) - Handles key repeat counts and Alt-prefix for escape sequences ### Line mode reading rewrite Replaced blocking `ReadFile` with `ReadConsoleW` on a worker thread, matching libuv's `uv_tty_line_read_thread` + `QueueUserWorkItem` approach. `ReadFile`/`ReadConsoleW` block until Enter is pressed, so they must run off the event loop thread. The worker thread reads UTF-16 via `ReadConsoleW`, converts to UTF-8, and wakes the event loop when the line is complete. ### Async console input notification Uses `RegisterWaitForSingleObject` (matching libuv's `uv__tty_queue_read_raw`) to register a one-shot thread pool wait on the console input handle. When input becomes available, the callback wakes the tokio event loop. Without this, tokio would park after the first keystroke and never re-poll the TTY. ### Input gating Only allocates buffers and calls `read_cb` when there is actually data to process, preventing spurious `read_cb(nread=0)` calls on every poll cycle. Fixes #32996 Fixes #32997 Fixes #32639 Fixes #33002 Fixes #32992 --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixed #30075
A decent chunk of this is written by claude, mostly uv_tty.