fix(rest): buffer split UTF-8 codepoints across PTY frames - #1200
fix(rest): buffer split UTF-8 codepoints across PTY frames#1200kishore280 wants to merge 4 commits into
Conversation
A multi-byte codepoint straddling two WS binary frames was decoded independently per frame via from_utf8_lossy, turning the incomplete tail into U+FFFD before any consumer saw it. Box-drawing characters, spinners, and emoji were the common casualties under a full-screen TUI redraw. Buffer the incomplete trailing bytes per channel (stdout/stderr) and prepend them to the next frame instead of decoding each frame in isolation.
The existing regression test only covered a 3-byte codepoint (Euro sign). Add coverage for a 4-byte codepoint (emoji) split after 1, 2, or 3 bytes, since that is a distinct case from the 3-byte one.
📦 BoxLite review — couldn't completepowered by BoxLite |
📝 WalkthroughWalkthroughThe REST WebSocket output path now preserves incomplete UTF-8 sequences across binary frames for stdout and stderr. It flushes pending bytes at termination and handles malformed sequences with replacement characters. Tests cover multibyte, TUI, isolated-channel, and terminal-flush cases. ChangesStreaming UTF-8 output
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WebSocket
participant WebSocketPump
participant UTF8Buffers
participant ExecResult
WebSocket->>WebSocketPump: receive stdout or stderr bytes
WebSocketPump->>UTF8Buffers: append channel bytes
UTF8Buffers-->>WebSocketPump: emit decoded fragments
WebSocketPump->>ExecResult: flush pending bytes at termination
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/boxlite/src/rest/litebox.rs (1)
849-855: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFlush pending bytes before terminal returns.
An Exit frame returns without decoding
stdout_pendingorstderr_pending. For example, a final stdout frame containing only0xE2is silently lost. TheProbeResult::Terminalreturn path has the same omission.Before every terminal return, flush each pending buffer with lossy decoding so incomplete final bytes remain observable as U+FFFD instead of disappearing.
🤖 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 `@src/boxlite/src/rest/litebox.rs` around lines 849 - 855, Update the terminal return paths for ControlFrame::Exit and ProbeResult::Terminal to flush stdout_pending and stderr_pending using lossy decoding before sending or returning the final result. Preserve any already-decoded output and ensure incomplete trailing bytes are emitted as U+FFFD rather than discarded.
🧹 Nitpick comments (1)
src/boxlite/src/rest/litebox.rs (1)
1620-1795: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd stderr and channel-isolation coverage.
These tests only send channel
0x01and only readstdout_rx. Add a test that splits UTF-8 on channel0x02and readsstderr_rx. Also interleave incomplete stdout and stderr sequences to prove that the two pending buffers never combine bytes across channels.🤖 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 `@src/boxlite/src/rest/litebox.rs` around lines 1620 - 1795, Add tests alongside ws_stdout_utf8_split_across_frames and its related cases that send UTF-8 fragments with channel prefix 0x02 and assert the reassembled text arrives through stderr_rx. Add an interleaved case with incomplete stdout and stderr byte sequences, completing each on separate frames, and assert stdout_rx and stderr_rx each contain only their own decoded character without cross-channel byte combination.
🤖 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 `@src/boxlite/src/rest/litebox.rs`:
- Around line 653-670: Update decode_utf8_streaming to handle malformed UTF-8 by
consuming the valid prefix and malformed sequence, appending U+FFFD, then
continuing to decode subsequent bytes in the same payload. Preserve valid bytes
after the malformed sequence and retain only an incomplete trailing sequence in
pending; do not clear the entire buffer when error_len() is Some.
---
Outside diff comments:
In `@src/boxlite/src/rest/litebox.rs`:
- Around line 849-855: Update the terminal return paths for ControlFrame::Exit
and ProbeResult::Terminal to flush stdout_pending and stderr_pending using lossy
decoding before sending or returning the final result. Preserve any
already-decoded output and ensure incomplete trailing bytes are emitted as
U+FFFD rather than discarded.
---
Nitpick comments:
In `@src/boxlite/src/rest/litebox.rs`:
- Around line 1620-1795: Add tests alongside ws_stdout_utf8_split_across_frames
and its related cases that send UTF-8 fragments with channel prefix 0x02 and
assert the reassembled text arrives through stderr_rx. Add an interleaved case
with incomplete stdout and stderr byte sequences, completing each on separate
frames, and assert stdout_rx and stderr_rx each contain only their own decoded
character without cross-channel byte combination.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: de40bcd1-d6e7-4593-9b93-c38f232fb618
📒 Files selected for processing (1)
src/boxlite/src/rest/litebox.rs
Summary
A multi-byte UTF-8 character split across two WS stdout/stderr frames was decoded per-frame, turning the incomplete half into U+FFFD. Buffer the incomplete tail per channel, flush it on terminal return, and preserve valid bytes after a genuinely malformed sequence.
Call graph
Before
After
Fixes #1155
Changes
decode_utf8_streaming, buffering an incomplete trailing UTF-8 sequence instead of lossily replacing it; on genuinely malformed bytes, only the bad sequence becomes U+FFFD and decoding continues on the rest of the buffer.flush_pending, called on both terminal-return paths so a partial sequence still in the buffer when the exec ends isn't silently lost.stdout_pending,stderr_pending) persist across reconnects and never mix bytes between channels.How to verify