Repository navigation
Conversation
handle_text logged inbound CDP messages with &text[..text.len().min(300)], which panics when the 300-byte cutoff lands inside a multi-byte UTF-8 character (e.g. a non-ASCII tab title in a Target.targetInfoChanged event). The panic lands on the detached mux-cdp-reader thread, so it is swallowed and the CDP session silently stalls. Only reachable with CMUX_MUX_CDP_DEBUG set. Truncate on a char boundary instead. Adds two unit tests for the helper.
|
@lollol-jr is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaced unsafe raw byte slicing used for truncating logged CDP text payloads with a new helper function that truncates on UTF-8 character boundaries, preventing panics on multibyte characters. Added unit tests covering multibyte boundary handling and short-input behavior. ChangesTruncation Fix and Tests
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 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 |
Greptile SummaryFixes a panic in the
Confidence Score: 5/5The change is a one-file, debug-path-only fix with no impact on normal execution; the new helper is straightforward and the tests directly exercise the boundary that caused the panic. The manual walk-back loop is correct and equivalent to the standard library's floor_char_boundary, the early-return fast path avoids any work on short strings, and the two tests pin both the panicking case and the trivial case. The fix is isolated to the CMUX_MUX_CDP_DEBUG logging branch, so production data flow is unchanged. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["handle_text(inner, text)"] --> B{"cdp_debug?"}
B -->|No| D["parse JSON / dispatch event"]
B -->|Yes| C["truncate_for_log(text, 300)"]
C --> E{"text.len() <= max?"}
E -->|Yes| F["return text unchanged"]
E -->|No| G["end = max, walk back until is_char_boundary(end)"]
G --> H["return text slice to end"]
F --> I["eprintln cdp message"]
H --> I
I --> D
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["handle_text(inner, text)"] --> B{"cdp_debug?"}
B -->|No| D["parse JSON / dispatch event"]
B -->|Yes| C["truncate_for_log(text, 300)"]
C --> E{"text.len() <= max?"}
E -->|Yes| F["return text unchanged"]
E -->|No| G["end = max, walk back until is_char_boundary(end)"]
G --> H["return text slice to end"]
F --> I["eprintln cdp message"]
H --> I
I --> D
Reviews (1): Last reviewed commit: "mux: fix panic when CDP debug log trunca..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn truncate_for_log(text: &str, max: usize) -> &str { | ||
| if text.len() <= max { | ||
| return text; | ||
| } | ||
| let mut end = max; | ||
| while end > 0 && !text.is_char_boundary(end) { | ||
| end -= 1; | ||
| } | ||
| &text[..end] | ||
| } |
There was a problem hiding this comment.
str::floor_char_boundary which does exactly this and avoids the custom loop. If the crate's MSRV is 1.72 or later, the whole body collapses to one line and the intent is immediately obvious from the standard-library name.
| fn truncate_for_log(text: &str, max: usize) -> &str { | |
| if text.len() <= max { | |
| return text; | |
| } | |
| let mut end = max; | |
| while end > 0 && !text.is_char_boundary(end) { | |
| end -= 1; | |
| } | |
| &text[..end] | |
| } | |
| fn truncate_for_log(text: &str, max: usize) -> &str { | |
| &text[..text.floor_char_boundary(max.min(text.len()))] | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
teamleaderleo
added
ready-to-land
|
The UTF-8 truncation bug is still present in the current cmux-tui CDP client, but the old |
This branch has not been deployed
3 participants
handle_textinmux-cdplogs every inbound CDP message underCMUX_MUX_CDP_DEBUG:&text[..300]slices by byte offset. When the cutoff falls inside a multi-byte UTF-8 character it panics (byte index 300 is not a char boundary). CDP payloads carry arbitrary page text, so aTarget.targetInfoChangedfor a tab whose title crosses offset 300 with a non-ASCII character is enough to hit it. The panic happens on the detachedmux-cdp-readerthread, so it's swallowed: the reader stops, pendingcall()s time out, and the session goes quiet with no error. It only triggers with the debug flag set — i.e. while debugging the browser.Truncate on a char boundary instead. Two unit tests cover the helper; the multi-byte one panics against the old byte slice and passes with the fix.
Verified in
mux/:cargo test -p mux-cdp— 4 unit + 3 integration tests passcargo fmt --check— cleancargo clippy -p mux-cdp --all-targets --locked -- -D warnings— cleanThe change is confined to
mux-cdp, which doesn't depend on ghostty, so I ran crate-scoped checks rather than the full--workspacebuild (that needs theghosttysubmodule). The same slice is present in the open #7378.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Prevented a panic in
mux-cdpdebug logging by truncating CDP messages on UTF‑8 character boundaries. This keeps the reader thread running whenCMUX_MUX_CDP_DEBUGis set and avoids silent session stalls.&text[..min(300)]with a safetruncate_for_log(text, 300)that never splits multi-byte characters.Written for commit 6470411. Summary will update on new commits.
Summary by CodeRabbit