diff --git a/crates/noa-app/src/app.rs b/crates/noa-app/src/app.rs index 92f71b4..dd07220 100644 --- a/crates/noa-app/src/app.rs +++ b/crates/noa-app/src/app.rs @@ -920,7 +920,7 @@ impl App { /// single terminal lock. One acquisition rather than three keeps the /// key-input path off the io thread's output-batch lock longer than /// necessary (input-latency under heavy pty output). - fn key_encode_modes(&self, window_id: WindowId) -> (bool, bool, u8) { + fn key_encode_modes(&self, window_id: WindowId) -> (bool, bool, u8, bool) { self.windows .get(&window_id) .and_then(WindowState::focused_surface) @@ -930,9 +930,10 @@ impl App { terminal.modes.app_cursor_keys(), terminal.modes.app_keypad(), terminal.kitty_keyboard_flags(), + terminal.modify_other_keys_2, ) }) - .unwrap_or((false, false, 0)) + .unwrap_or((false, false, 0, false)) } fn focus_reporting(&self, window_id: WindowId) -> bool { diff --git a/crates/noa-app/src/app/event_loop.rs b/crates/noa-app/src/app/event_loop.rs index 030d662..6bf4294 100644 --- a/crates/noa-app/src/app/event_loop.rs +++ b/crates/noa-app/src/app/event_loop.rs @@ -1076,7 +1076,8 @@ impl ApplicationHandler for App { ) { return; } - let (app_cursor_keys, app_keypad, kitty_flags) = self.key_encode_modes(window_id); + let (app_cursor_keys, app_keypad, kitty_flags, modify_other_keys) = + self.key_encode_modes(window_id); let unmodified_key = event.key_without_modifiers(); // On macOS, Option only acts as Alt when winit stripped its // composition per `macos-option-as-alt` — i.e. the delivered @@ -1095,6 +1096,7 @@ impl ApplicationHandler for App { app_cursor_keys, app_keypad, kitty_flags, + modify_other_keys, pressed, event.repeat, ); diff --git a/crates/noa-app/src/app/session_restore.rs b/crates/noa-app/src/app/session_restore.rs index 132a064..81af895 100644 --- a/crates/noa-app/src/app/session_restore.rs +++ b/crates/noa-app/src/app/session_restore.rs @@ -43,10 +43,14 @@ impl App { .first() .and_then(|id| self.windows.get(id)) .map(|state| capture_window_frame(&state.window)); - let focused_tab = self - .focused - .and_then(|focused| tabs.iter().position(|id| *id == focused)) - .unwrap_or(0); + // App-wide focus only identifies the tab of the focused group; a + // background window's selected tab comes from its native tab group. + let focused_tab = saved_focused_tab(tabs, self.focused, |id| { + self.windows + .get(&id) + .and_then(|state| crate::macos_window::native_tab_is_selected(&state.window)) + .unwrap_or(false) + }); let tab_sessions = tabs .iter() .filter_map(|id| { @@ -225,6 +229,18 @@ impl App { restored_groups.push(tab_ids); } + // Re-select each group's saved tab before restoring app-wide focus, so + // background windows come back showing the tab they were left on. + #[cfg(target_os = "macos")] + for (group, saved) in restored_groups.iter().zip(&state.windows) { + if saved.focused_tab == 0 || saved.focused_tab >= group.len() { + continue; + } + if let Some(state) = group.first().and_then(|id| self.windows.get(id)) { + state.window.select_tab_at_index(saved.focused_tab); + } + } + if let Some(focused_window) = state.focused_window && let (Some(group), Some(saved)) = ( restored_groups.get(focused_window), @@ -508,6 +524,20 @@ fn orientation_from_session(orientation: session::Orientation) -> SplitOrientati } } +/// Index of the tab to record as a group's selected tab: the app-focused +/// window when it belongs to this group, otherwise the natively selected tab, +/// otherwise the first. +fn saved_focused_tab( + tabs: &[WindowId], + focused: Option, + is_selected: impl Fn(WindowId) -> bool, +) -> usize { + focused + .and_then(|focused| tabs.iter().position(|id| *id == focused)) + .or_else(|| tabs.iter().position(|id| is_selected(*id))) + .unwrap_or(0) +} + /// Read a window's logical-pixel frame (scale-independent) for persistence. /// The position may be unavailable on some platforms; the size always is. fn capture_window_frame(window: &Window) -> session::WindowFrame { @@ -529,6 +559,18 @@ fn capture_window_frame(window: &Window) -> session::WindowFrame { mod tests { use super::*; + #[test] + fn saved_focused_tab_prefers_app_focus_then_native_selection() { + let tabs: Vec = (0..3).map(|i| WindowId::from(i as u64)).collect(); + // Focused window in this group wins. + assert_eq!(saved_focused_tab(&tabs, Some(tabs[2]), |_| false), 2); + // Focus elsewhere: the natively selected tab is recorded. + let other = WindowId::from(99u64); + assert_eq!(saved_focused_tab(&tabs, Some(other), |id| id == tabs[1]), 1); + // Nothing selected: first tab. + assert_eq!(saved_focused_tab(&tabs, None, |_| false), 0); + } + #[test] fn split_rebuild_keeps_remote_identity_on_the_original_leaf() { let remote = session::RemotePane { diff --git a/crates/noa-app/src/input/key.rs b/crates/noa-app/src/input/key.rs index 75e75b7..142259a 100644 --- a/crates/noa-app/src/input/key.rs +++ b/crates/noa-app/src/input/key.rs @@ -24,6 +24,7 @@ pub fn encode_key( app_cursor_keys, false, 0, + false, true, false, ) @@ -39,6 +40,11 @@ pub fn encode_key( /// physical press produces with no modifiers held — used by the Kitty encoder /// to report the unshifted base key code (Shift+1 must report `1`, not `!`). /// +/// `modify_other_keys` mirrors `Terminal::modify_other_keys_2` (xterm +/// `CSI > 4 ; 2 m`): Character keys pressed with Ctrl/Alt/Super are reported +/// as `CSI 27 ; mods ; codepoint ~` instead of the legacy C0/ESC forms. The +/// Kitty protocol, when active, still takes precedence. +/// /// `alt_sends_esc` says whether Alt held with this press should ESC-prefix the /// produced text. On macOS the Option key composes characters unless /// `macos-option-as-alt` claims it, so the caller decides per event; on other @@ -54,6 +60,7 @@ pub fn encode_key_with_modes( app_cursor_keys: bool, app_keypad: bool, kitty_flags: u8, + modify_other_keys: bool, pressed: bool, repeat: bool, ) -> Option> { @@ -87,6 +94,16 @@ pub fn encode_key_with_modes( return None; } + // xterm modifyOtherKeys level 2: a Character key with Ctrl/Alt/Super + // reports its codepoint after Shift/layout translation plus the modifier + // value, so Ctrl+I is distinguishable from Tab. Shift alone (and Option + // composing text on macOS) stays on the legacy path. + if modify_other_keys + && let Some(bytes) = modify_other_keys_bytes(logical_key, mods, alt_sends_esc) + { + return Some(bytes); + } + // Ctrl+key -> the corresponding C0 control byte. Checked before the // general text path since terminals expect Ctrl+A..Z (and the classic // xterm symbol/digit mappings, e.g. Ctrl+Space=NUL, Ctrl+[=ESC) to send @@ -99,12 +116,16 @@ pub fn encode_key_with_modes( if let (Some(c), None) = (chars.next(), chars.next()) && let Some(byte) = ctrl_c0_byte(c) { - return Some(vec![byte]); + // Alt still ESC-prefixes the control byte (Ctrl+Alt+A -> + // ESC 0x01) when it isn't composing text. + return Some(alt_esc_prefixed(vec![byte], mods, alt_sends_esc)); } } // winit can report Space as a named key; Ctrl+Space is NUL // (emacs set-mark and friends). - Key::Named(NamedKey::Space) => return Some(vec![0x00]), + Key::Named(NamedKey::Space) => { + return Some(alt_esc_prefixed(vec![0x00], mods, alt_sends_esc)); + } _ => {} } } @@ -165,6 +186,7 @@ pub(crate) fn encode_enter_key(kitty_flags: u8) -> Vec { false, false, kitty_flags, + false, true, false, ) @@ -189,6 +211,55 @@ fn ctrl_c0_byte(c: char) -> Option { Some(byte) } +/// ESC-prefix `bytes` for Alt, but only when Alt is acting as a modifier +/// (`alt_sends_esc`) rather than composing text via macOS Option. +fn alt_esc_prefixed(mut bytes: Vec, mods: ModifiersState, alt_sends_esc: bool) -> Vec { + if mods.alt_key() && alt_sends_esc { + bytes.insert(0, 0x1b); + } + bytes +} + +/// xterm `modifyOtherKeys=2` encoding: `CSI 27 ; ; ~` for +/// a Character (or Space) key pressed with Ctrl, Alt-as-modifier, or Super. +/// The logical key preserves Shift and keyboard-layout translation, so +/// Ctrl+Shift+1 on a US layout reports `33` (`!`) with the Shift bit. +fn modify_other_keys_bytes( + logical_key: &Key, + mods: ModifiersState, + alt_sends_esc: bool, +) -> Option> { + let alt = mods.alt_key() && alt_sends_esc; + if !(mods.control_key() || alt || mods.super_key()) { + return None; + } + let codepoint = match logical_key { + Key::Character(s) => { + let mut chars = s.chars(); + match (chars.next(), chars.next()) { + (Some(c), None) => c as u32, + _ => return None, + } + } + Key::Named(NamedKey::Space) => u32::from(' '), + _ => return None, + }; + let mut value = 1; + if mods.shift_key() { + value += 1; + } + if alt { + value += 2; + } + if mods.control_key() { + value += 4; + } + if mods.super_key() { + value += 8; + } + Some(format!("\x1b[27;{value};{codepoint}~").into_bytes()) +} + fn alt_prefixed(mut bytes: Vec, mods: ModifiersState) -> Vec { if mods.alt_key() { bytes.insert(0, 0x1b); diff --git a/crates/noa-app/src/input/tests.rs b/crates/noa-app/src/input/tests.rs index c554b8a..48e02c8 100644 --- a/crates/noa-app/src/input/tests.rs +++ b/crates/noa-app/src/input/tests.rs @@ -70,6 +70,7 @@ fn bench_encode_key_with_modes() { false, false, kitty_flags, + false, true, false, )); @@ -113,7 +114,9 @@ fn alt_printable_uses_escape_prefix() { } #[test] -fn ctrl_letter_takes_priority_over_alt_prefix() { +fn ctrl_alt_letter_sends_esc_prefixed_c0_byte() { + // Ctrl wins the byte (ETX, not text), and Alt-as-modifier still adds + // the ESC prefix (xterm metaSendsEscape / Ghostty legacy encoding). let key = Key::Character("c".into()); assert_eq!( encode_key( @@ -122,7 +125,7 @@ fn ctrl_letter_takes_priority_over_alt_prefix() { ModifiersState::CONTROL | ModifiersState::ALT, false ), - Some(vec![0x03]) + Some(vec![0x1b, 0x03]) ); } @@ -458,6 +461,7 @@ fn composed_option_text_passes_through_without_esc() { false, false, 0, + false, true, false, ), @@ -514,6 +518,7 @@ fn application_keypad_uses_ss3_for_numpad_digits_and_enter() { false, true, 0, + false, true, false, ), @@ -530,6 +535,7 @@ fn application_keypad_uses_ss3_for_numpad_digits_and_enter() { false, true, 0, + false, true, false, ), @@ -550,6 +556,7 @@ fn numeric_keypad_uses_text_or_standard_enter() { false, false, 0, + false, true, false, ), @@ -566,6 +573,7 @@ fn numeric_keypad_uses_text_or_standard_enter() { false, false, 0, + false, true, false, ), @@ -883,7 +891,7 @@ fn kitty_press( flags: u8, ) -> Option> { encode_key_with_modes( - logical, None, None, text, mods, true, false, false, flags, true, false, + logical, None, None, text, mods, true, false, false, flags, false, true, false, ) } @@ -1065,6 +1073,7 @@ fn kitty_shifted_symbol_reports_unshifted_base_key() { false, false, KITTY_REPORT_ALTERNATE_KEYS, + false, true, false, ), @@ -1111,6 +1120,7 @@ fn kitty_event_types_report_release_and_repeat() { false, KITTY_REPORT_EVENT_TYPES, false, + false, // released false, ), @@ -1128,6 +1138,7 @@ fn kitty_event_types_report_release_and_repeat() { false, false, KITTY_REPORT_EVENT_TYPES, + false, true, true, // repeat ), @@ -1151,6 +1162,7 @@ fn kitty_release_of_text_key_without_report_all_is_dropped() { KITTY_REPORT_EVENT_TYPES, false, false, + false, ), None ); @@ -1171,6 +1183,7 @@ fn kitty_modifier_key_alone_reported_only_with_report_all() { false, false, KITTY_REPORT_ALL_KEYS, + false, true, false, ), @@ -1188,6 +1201,7 @@ fn kitty_modifier_key_alone_reported_only_with_report_all() { false, false, KITTY_DISAMBIGUATE, + false, true, false, ), @@ -1219,14 +1233,14 @@ fn legacy_release_sends_nothing() { for (logical, text, mods) in cases { assert!( encode_key_with_modes( - &logical, None, None, text, mods, true, false, false, 0, true, false + &logical, None, None, text, mods, true, false, false, 0, false, true, false ) .is_some(), "press {logical:?} should still send" ); assert_eq!( encode_key_with_modes( - &logical, None, None, text, mods, true, false, false, 0, false, false + &logical, None, None, text, mods, true, false, false, 0, false, false, false ), None, "release {logical:?} should send nothing" @@ -1250,6 +1264,7 @@ fn kitty_event_types_repeat_legacy_keys_but_drop_their_release() { false, false, flags, + false, true, true, // repeat ), @@ -1266,6 +1281,7 @@ fn kitty_event_types_repeat_legacy_keys_but_drop_their_release() { false, false, flags, + false, true, true, // repeat ), @@ -1285,6 +1301,7 @@ fn kitty_event_types_repeat_legacy_keys_but_drop_their_release() { flags, false, false, + false, ), None ); @@ -1301,6 +1318,7 @@ fn kitty_event_types_repeat_legacy_keys_but_drop_their_release() { flags, false, false, + false, ), None ); @@ -1316,6 +1334,7 @@ fn kitty_event_types_repeat_legacy_keys_but_drop_their_release() { false, false, flags, + false, true, true, ), @@ -1336,6 +1355,7 @@ fn kitty_keypad_uses_dedicated_codes_under_report_all() { false, false, KITTY_REPORT_ALL_KEYS, + false, true, false, ), @@ -1358,3 +1378,182 @@ fn encode_enter_key_follows_kitty_flags() { b"\x1b[13u".to_vec() ); } + +#[test] +fn ctrl_alt_letter_keeps_the_alt_esc_prefix() { + // Ctrl+Alt+A with Alt acting as a modifier: ESC then the C0 byte, not a + // bare Ctrl+A. + let bytes = encode_key_with_modes( + &Key::Character("a".into()), + Some(&Key::Character("a".into())), + None, + None, + ModifiersState::CONTROL | ModifiersState::ALT, + true, + false, + false, + 0, + false, + true, + false, + ); + assert_eq!(bytes, Some(vec![0x1b, 0x01])); + // Option composing text (alt_sends_esc = false): the C0 byte alone. + let bytes = encode_key_with_modes( + &Key::Character("a".into()), + Some(&Key::Character("a".into())), + None, + None, + ModifiersState::CONTROL | ModifiersState::ALT, + false, + false, + false, + 0, + false, + true, + false, + ); + assert_eq!(bytes, Some(vec![0x01])); +} + +#[test] +fn modify_other_keys_2_reports_modified_characters() { + let enc = |key: &str, mods: ModifiersState| { + encode_key_with_modes( + &Key::Character(key.into()), + Some(&Key::Character(key.to_ascii_lowercase().into())), + None, + Some(key), + mods, + true, + false, + false, + 0, + true, + true, + false, + ) + }; + // Ctrl+I is distinguishable from Tab. + assert_eq!( + enc("i", ModifiersState::CONTROL), + Some(b"\x1b[27;5;105~".to_vec()) + ); + // Shift is reported in both the modifier value and the resulting character. + assert_eq!( + enc("A", ModifiersState::CONTROL | ModifiersState::SHIFT), + Some(b"\x1b[27;6;65~".to_vec()) + ); + assert_eq!( + enc("x", ModifiersState::ALT), + Some(b"\x1b[27;3;120~".to_vec()) + ); + // Shift alone and unmodified keys stay legacy text. + assert_eq!(enc("A", ModifiersState::SHIFT), Some(b"A".to_vec())); + assert_eq!(enc("a", ModifiersState::empty()), Some(b"a".to_vec())); +} + +#[test] +fn modify_other_keys_2_preserves_shifted_symbols_and_layout_characters() { + for (logical, unmodified, ctrl_expected, alt_expected) in [ + ("!", "1", "\x1b[27;6;33~", "\x1b[27;4;33~"), + ("?", "/", "\x1b[27;6;63~", "\x1b[27;4;63~"), + ("+", ";", "\x1b[27;6;43~", "\x1b[27;4;43~"), + ("£", "3", "\x1b[27;6;163~", "\x1b[27;4;163~"), + ("Ä", "ä", "\x1b[27;6;196~", "\x1b[27;4;196~"), + ] { + for (mods, expected) in [ + ( + ModifiersState::CONTROL | ModifiersState::SHIFT, + ctrl_expected, + ), + (ModifiersState::ALT | ModifiersState::SHIFT, alt_expected), + ] { + let bytes = encode_key_with_modes( + &Key::Character(logical.into()), + Some(&Key::Character(unmodified.into())), + None, + Some(logical), + mods, + true, + false, + false, + 0, + true, + true, + false, + ); + assert_eq!( + bytes, + Some(expected.as_bytes().to_vec()), + "{logical}, {mods:?}" + ); + } + } +} + +#[test] +fn modify_other_keys_full_reset_restores_legacy_ctrl_bytes() { + let mut terminal = noa_grid::Terminal::new(noa_core::GridSize::new(80, 24)); + let mut stream = noa_vt::Stream::new(); + let encode_ctrl_c = |terminal: &noa_grid::Terminal| { + encode_key_with_modes( + &Key::Character("c".into()), + Some(&Key::Character("c".into())), + None, + None, + ModifiersState::CONTROL, + true, + terminal.modes.app_cursor_keys(), + terminal.modes.app_keypad(), + terminal.kitty_keyboard_flags(), + terminal.modify_other_keys_2, + true, + false, + ) + }; + + stream.feed(b"\x1b[>4;2m", &mut terminal); + assert_eq!(encode_ctrl_c(&terminal), Some(b"\x1b[27;5;99~".to_vec())); + + stream.feed(b"\x1bc", &mut terminal); + assert_eq!(encode_ctrl_c(&terminal), Some(vec![0x03])); +} + +#[test] +fn modify_other_keys_off_keeps_legacy_ctrl_bytes() { + let bytes = encode_key_with_modes( + &Key::Character("i".into()), + Some(&Key::Character("i".into())), + None, + None, + ModifiersState::CONTROL, + true, + false, + false, + 0, + false, + true, + false, + ); + assert_eq!(bytes, Some(vec![0x09])); +} + +#[test] +fn kitty_flags_take_precedence_over_modify_other_keys() { + let bytes = encode_key_with_modes( + &Key::Character("i".into()), + Some(&Key::Character("i".into())), + None, + None, + ModifiersState::CONTROL, + true, + false, + false, + 1, + true, + true, + false, + ); + assert_eq!(bytes, Some(b"\x1b[105;5u".to_vec())); +} diff --git a/crates/noa-app/src/macos_window.rs b/crates/noa-app/src/macos_window.rs index eccae31..91fa84d 100644 --- a/crates/noa-app/src/macos_window.rs +++ b/crates/noa-app/src/macos_window.rs @@ -108,6 +108,44 @@ pub(crate) fn set_native_tab_title(window: &Window, title: &str) { } } +/// Whether `window` is the selected (front) tab of its native tab group. A +/// window with no tab group is trivially selected. `None` when the live +/// NSWindow can't be reached, or on non-macOS platforms. +#[allow(unused_variables)] +pub(crate) fn native_tab_is_selected(window: &Window) -> Option { + #[cfg(target_os = "macos")] + { + use objc2::msg_send; + use objc2::runtime::AnyObject; + use raw_window_handle::{HasWindowHandle, RawWindowHandle}; + + // SAFETY: called from session capture on winit's main (window-owning) + // thread; the NSWindow is live and owned by winit; every pointer is + // nil-checked and only compared, never dereferenced beyond messaging. + unsafe { + let handle = window.window_handle().ok()?; + let RawWindowHandle::AppKit(appkit) = handle.as_raw() else { + return None; + }; + let ns_view = appkit.ns_view.as_ptr().cast::(); + let ns_window: *mut AnyObject = msg_send![ns_view, window]; + if ns_window.is_null() { + return None; + } + let tab_group: *mut AnyObject = msg_send![ns_window, tabGroup]; + if tab_group.is_null() { + return Some(true); + } + let selected: *mut AnyObject = msg_send![tab_group, selectedWindow]; + Some(selected == ns_window) + } + } + #[cfg(not(target_os = "macos"))] + { + None + } +} + /// Toggle AppKit's native fullscreen Space for a normal terminal window. /// Returns `false` only when the live NSWindow cannot be reached. pub(crate) fn toggle_native_fullscreen(window: &Window) -> bool { diff --git a/crates/noa-grid/src/terminal/handler.rs b/crates/noa-grid/src/terminal/handler.rs index a60b92f..e5c4df8 100644 --- a/crates/noa-grid/src/terminal/handler.rs +++ b/crates/noa-grid/src/terminal/handler.rs @@ -476,6 +476,9 @@ impl Handler for Terminal { let scrollback_limit = self.primary.scrollback_limit_bytes(); self.primary = crate::screen::Screen::new(self.size.cols, self.size.rows); self.primary.set_scrollback_limit_bytes(scrollback_limit); + // The fresh screen starts from `Cursor::default()`; the user's + // `cursor-style` (DECSCUSR 0's target) must survive RIS. + self.primary.cursor.style = self.default_cursor_style; self.alt = None; self.active_is_alt = false; self.screen_generation = self.screen_generation.wrapping_add(1); @@ -495,6 +498,7 @@ impl Handler for Terminal { self.pending_agent_status = Some(None); self.pending_bell = false; self.kitty_keyboard.reset(); + self.modify_other_keys_2 = false; self.kitty_images.clear(); self.clear_selection(); self.clear_search(); @@ -504,6 +508,8 @@ impl Handler for Terminal { // DECTCEM on, DECOM off — tracked bits only; screen content untouched. self.modes.set(25, false, true); self.modes.set(6, false, false); + // IRM (ANSI mode 4) off — DECSTR returns to replace mode. + self.modes.set(4, true, false); self.charset = crate::charset::CharsetState::default(); let last_row = self.size.rows.saturating_sub(1); let screen = self.active_mut(); diff --git a/crates/noa-grid/src/tests/terminal_state.rs b/crates/noa-grid/src/tests/terminal_state.rs index 3281c9c..8541cff 100644 --- a/crates/noa-grid/src/tests/terminal_state.rs +++ b/crates/noa-grid/src/tests/terminal_state.rs @@ -1312,3 +1312,26 @@ fn cursor_is_at_prompt_follows_the_nearest_row_tagging_mark() { "alternate screen is always false regardless of marks" ); } + +#[test] +fn decstr_clears_insert_mode() { + let mut t = Terminal::new(GridSize::new(20, 4)); + let mut s = Stream::new(); + s.feed(b"ABCDE\x1b[4h", &mut t); + assert!(t.modes.insert_mode()); + s.feed(b"\x1b[!p", &mut t); + assert!(!t.modes.insert_mode(), "DECSTR returns to replace mode"); + s.feed(b"\rZ", &mut t); + assert_eq!(row_text(&t, 0, 6).trim_end(), "ZBCDE"); +} + +#[test] +fn ris_keeps_the_configured_default_cursor_style() { + let mut t = Terminal::new(GridSize::new(80, 24)); + t.set_default_cursor_style(CursorStyle::SteadyBar); + let mut s = Stream::new(); + s.feed(b"\x1b[2 q", &mut t); + assert_eq!(t.primary.cursor.style, CursorStyle::SteadyBlock); + s.feed(b"\x1bc", &mut t); + assert_eq!(t.primary.cursor.style, CursorStyle::SteadyBar); +} diff --git a/crates/noa-ipc/src/client.rs b/crates/noa-ipc/src/client.rs index 91622bf..141ff97 100644 --- a/crates/noa-ipc/src/client.rs +++ b/crates/noa-ipc/src/client.rs @@ -33,7 +33,11 @@ const CONNECT_TIMEOUT: Duration = Duration::from_secs(5); const CONTROL_REQUEST_TIMEOUT: Duration = Duration::from_secs(5); const ATTACH_SEED_TIMEOUT: Duration = Duration::from_secs(10); const MAX_WS_MESSAGE_SIZE: usize = 1024 * 1024; -const MAX_WS_FRAME_SIZE: usize = 256 * 1024; +// The server sends a capped `getText` response (up to ~1 MiB after JSON +// wrapping) as a single unfragmented text frame, so the frame bound must be +// as large as the message bound or a default-sized scrollback read would be +// rejected on receipt. +pub(crate) const MAX_WS_FRAME_SIZE: usize = MAX_WS_MESSAGE_SIZE; const ATTACH_PATH: &str = "/attach"; /// A connected JSON-RPC control client. Reconnect/backoff belongs to diff --git a/crates/noa-ipc/src/protocol.rs b/crates/noa-ipc/src/protocol.rs index 04aebbf..200101a 100644 --- a/crates/noa-ipc/src/protocol.rs +++ b/crates/noa-ipc/src/protocol.rs @@ -108,7 +108,9 @@ mod hex_color { let s = s .strip_prefix('#') .ok_or_else(|| D::Error::custom("expected #rrggbb"))?; - if s.len() != 6 { + // Length alone doesn't make the string ASCII; slicing a non-ASCII + // payload by byte offset would panic instead of failing to parse. + if s.len() != 6 || !s.bytes().all(|b| b.is_ascii_hexdigit()) { return Err(D::Error::custom("expected #rrggbb")); } let byte = |i: usize| -> Result { @@ -584,3 +586,21 @@ mod cap_grid_rows_tests { assert_boundary_fits_in_cap(4096); } } + +#[cfg(test)] +mod hex_color_tests { + use super::*; + + #[test] + fn hex_color_rejects_non_ascii_without_panicking() { + // Six bytes but not six ASCII hex digits: must be an Err, not a + // byte-offset slice panic inside a multibyte char. + for input in ["\"#あabc\"", "\"#12345\"", "\"#gggggg\"", "\"123456\""] { + assert!(serde_json::from_str::(input).is_err(), "{input}"); + } + assert_eq!( + serde_json::from_str::("\"#0A1b2C\"").unwrap(), + SpanColor::Hex((0x0a, 0x1b, 0x2c)) + ); + } +} diff --git a/crates/noa-ipc/src/server.rs b/crates/noa-ipc/src/server.rs index c15f838..aed49b6 100644 --- a/crates/noa-ipc/src/server.rs +++ b/crates/noa-ipc/src/server.rs @@ -1533,6 +1533,23 @@ fn handle_unsubscribe( mod tests { use super::*; + #[test] + fn default_get_text_response_fits_the_client_frame_bound() { + // A default-sized text made entirely of characters JSON must escape + // grows past the raw byte count once wrapped; the capped response + // must still be accepted by the client's frame limit. + let text = "\"".repeat(DEFAULT_TEXT_MAX_BYTES); + let result = serde_json::to_value(GetTextResult { + pane_id: WireId(1), + text, + truncated: false, + }) + .unwrap(); + let response = capped_get_text_response(Value::from(1), result); + assert!(response.len() <= crate::client::MAX_WS_FRAME_SIZE); + assert!(response.len() <= MAX_WS_TEXT_PAYLOAD_SIZE); + } + #[test] fn raw_output_drain_yields_to_input_after_a_bounded_byte_turn() { let (sender, receiver) = crate::attach::output_channel();