diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 1985e69d..0cc00aed 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -14,6 +14,7 @@ ### Fixed - Linux sound notifications now terminate and reap audio players that do not exit, preventing unavailable audio from leaving CPU-bound `mpg123` processes behind. (#1622) +- Oversized bracketed text pastes are now rejected with a client-local notification instead of disconnecting the client. (#1665) - Agent prompt waits now report `agent_prompt_stalled` after five seconds without an observed state change instead of waiting indefinitely after an ineffective submission. - `herdr config check` now reports unknown config keys with their full paths instead of treating ignored typos as valid configuration. (#1573) - Codex panes with customized static terminal titles now fall back to the live working footer instead of remaining idle, while OSC activity remains preferred. (#1563) diff --git a/src/raw_input.rs b/src/raw_input.rs index a6ca316a..191688c3 100644 --- a/src/raw_input.rs +++ b/src/raw_input.rs @@ -107,6 +107,21 @@ pub(crate) const GHOSTTY_COLOR_SCHEME_LIGHT_REPORT: &[u8] = b"\x1b[?997;2n"; const BRACKETED_PASTE_START: &[u8] = b"\x1b[200~"; const BRACKETED_PASTE_END: &[u8] = b"\x1b[201~"; +/// Returns whether `data` is exactly one complete bracketed-paste sequence. +/// +/// Client transport uses this to distinguish recoverable oversized interactive +/// pastes from generic oversized input, which remains a protocol violation. +pub(crate) fn is_complete_text_bracketed_paste(data: &[u8]) -> bool { + if !data.starts_with(BRACKETED_PASTE_START) { + return false; + } + let Some(end) = find_subsequence(data, BRACKETED_PASTE_END) else { + return false; + }; + end + BRACKETED_PASTE_END.len() == data.len() + && std::str::from_utf8(&data[BRACKETED_PASTE_START.len()..end]).is_ok() +} + #[derive(Debug)] pub enum RawInputEvent { Key(TerminalKey), @@ -1134,6 +1149,19 @@ mod tests { assert_eq!(consumed, 17); } + #[test] + fn complete_text_bracketed_paste_requires_one_exact_utf8_sequence() { + assert!(is_complete_text_bracketed_paste(b"\x1b[200~hello\x1b[201~")); + assert!(!is_complete_text_bracketed_paste(b"\x1b[200~hello")); + assert!(!is_complete_text_bracketed_paste( + b"\x1b[200~hello\x1b[201~rest" + )); + assert!(!is_complete_text_bracketed_paste( + b"\x1b[200~one\x1b[201~\x1b[200~two\x1b[201~" + )); + assert!(!is_complete_text_bracketed_paste(b"\x1b[200~\xff\x1b[201~")); + } + #[test] fn parses_sgr_mouse() { let (RawInputEvent::Mouse(mouse), consumed) = extract_one_event(b"\x1b[<0;20;10M").unwrap() diff --git a/src/server/client_transport.rs b/src/server/client_transport.rs index c4e43e0d..743a108f 100644 --- a/src/server/client_transport.rs +++ b/src/server/client_transport.rs @@ -300,6 +300,12 @@ pub(crate) enum ServerEvent { client_id: u64, events: Vec, }, + /// A fully decoded interactive paste exceeded the text-input limit. + ClientPasteRejected { + client_id: u64, + size: usize, + max: usize, + }, /// A client sent local clipboard image bytes to paste into a remote pane. ClientClipboardImage { client_id: u64, @@ -372,22 +378,30 @@ fn parse_client_keybindings( } } -fn input_events_within_limits(events: &[ClientInputEvent]) -> bool { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum InputEventLimit { + WithinLimits, + TooManyEvents, + PasteTooLarge { size: usize }, +} + +fn input_event_limit(events: &[ClientInputEvent]) -> InputEventLimit { if events.len() > MAX_INPUT_EVENT_BATCH { - return false; + return InputEventLimit::TooManyEvents; } let mut paste_bytes = 0usize; for event in events { if let ClientInputEvent::Paste { text } = event { paste_bytes = paste_bytes.saturating_add(text.len()); - if paste_bytes > MAX_INPUT_PAYLOAD { - return false; - } } } - true + if paste_bytes > MAX_INPUT_PAYLOAD { + InputEventLimit::PasteTooLarge { size: paste_bytes } + } else { + InputEventLimit::WithinLimits + } } #[cfg(windows)] @@ -650,32 +664,60 @@ fn client_read_loop( ClientMessage::Input { data } => { // Validate input size. if data.len() > MAX_INPUT_PAYLOAD { - warn!( - client_id, - size = data.len(), - "oversized input from client, closing" - ); - let _ = server_event_tx - .blocking_send(ServerEvent::ClientDisconnected { client_id }); - break; + if crate::raw_input::is_complete_text_bracketed_paste(&data) { + warn!( + client_id, + size = data.len(), + max = MAX_INPUT_PAYLOAD, + "oversized bracketed paste from client, rejecting" + ); + ServerEvent::ClientPasteRejected { + client_id, + size: data.len(), + max: MAX_INPUT_PAYLOAD, + } + } else { + warn!( + client_id, + size = data.len(), + "oversized input from client, closing" + ); + let _ = server_event_tx + .blocking_send(ServerEvent::ClientDisconnected { client_id }); + break; + } } else { ServerEvent::ClientInput { client_id, data } } } - ClientMessage::InputEvents { events } => { - if !input_events_within_limits(&events) { + ClientMessage::InputEvents { events } => match input_event_limit(&events) { + InputEventLimit::WithinLimits => { + ServerEvent::ClientInputEvents { client_id, events } + } + InputEventLimit::TooManyEvents => { warn!( client_id, count = events.len(), - "oversized input events from client, closing" + "oversized input event batch from client, closing" ); let _ = server_event_tx .blocking_send(ServerEvent::ClientDisconnected { client_id }); break; - } else { - ServerEvent::ClientInputEvents { client_id, events } } - } + InputEventLimit::PasteTooLarge { size } => { + warn!( + client_id, + size, + max = MAX_INPUT_PAYLOAD, + "oversized structured paste from client, rejecting" + ); + ServerEvent::ClientPasteRejected { + client_id, + size, + max: MAX_INPUT_PAYLOAD, + } + } + }, ClientMessage::ObserveTerminal { target } => { ServerEvent::ClientObserveTerminal { client_id, target } } @@ -799,6 +841,29 @@ mod tests { (client, server, TestSocketPath(path)) } + fn recv_server_event(receiver: &mut mpsc::Receiver, context: &str) -> ServerEvent { + let deadline = std::time::Instant::now() + Duration::from_secs(1); + loop { + match receiver.try_recv() { + Ok(event) => return event, + Err(mpsc::error::TryRecvError::Empty) if std::time::Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(1)); + } + Err(err) => panic!("{context}: {err}"), + } + } + } + + fn bracketed_paste_with_total_len(total_len: usize) -> Vec { + const DELIMITER_BYTES: usize = b"\x1b[200~".len() + b"\x1b[201~".len(); + assert!(total_len >= DELIMITER_BYTES); + let mut data = Vec::with_capacity(total_len); + data.extend_from_slice(b"\x1b[200~"); + data.resize(total_len - b"\x1b[201~".len(), b'x'); + data.extend_from_slice(b"\x1b[201~"); + data + } + fn test_queue_writer() -> (ClientWriter, Arc) { let queue = ClientWriterQueue::new(); ( @@ -1175,7 +1240,7 @@ new_tab = "ctrl+notakey" } #[test] - fn client_read_loop_rejects_oversized_input() { + fn client_read_loop_rejects_oversized_bracketed_paste_without_disconnect() { let (mut client_stream, server_stream, _path) = local_stream_pair("client-read-oversized"); let (server_event_tx, mut server_event_rx) = mpsc::channel(4); let should_quit = Arc::new(AtomicBool::new(false)); @@ -1184,21 +1249,122 @@ new_tab = "ctrl+notakey" client_read_loop(server_stream, 7, &server_event_tx, &read_quit) }); + protocol::write_message( + &mut client_stream, + &ClientMessage::Input { + data: bracketed_paste_with_total_len(MAX_INPUT_PAYLOAD), + }, + ) + .expect("write maximum-size bracketed paste"); + + match recv_server_event(&mut server_event_rx, "maximum-size paste event") { + ServerEvent::ClientInput { client_id, data } => { + assert_eq!(client_id, 7); + assert_eq!(data.len(), MAX_INPUT_PAYLOAD); + } + other => panic!("expected maximum-size ClientInput, got {other:?}"), + } + + protocol::write_message( + &mut client_stream, + &ClientMessage::Input { + data: bracketed_paste_with_total_len(MAX_INPUT_PAYLOAD + 1), + }, + ) + .expect("write oversized bracketed paste"); + + match recv_server_event(&mut server_event_rx, "oversized paste rejection") { + ServerEvent::ClientPasteRejected { + client_id, + size, + max, + } => { + assert_eq!(client_id, 7); + assert_eq!(size, MAX_INPUT_PAYLOAD + 1); + assert_eq!(max, MAX_INPUT_PAYLOAD); + } + ServerEvent::ClientDisconnected { .. } => { + panic!("oversized input must be rejected without disconnecting the client") + } + other => panic!("expected ClientPasteRejected, got {other:?}"), + } + + protocol::write_message( + &mut client_stream, + &ClientMessage::Input { + data: b"still connected".to_vec(), + }, + ) + .expect("write valid input after rejection"); + + match recv_server_event(&mut server_event_rx, "valid input after rejection") { + ServerEvent::ClientInput { client_id, data } => { + assert_eq!(client_id, 7); + assert_eq!(data, b"still connected"); + } + other => panic!("expected ClientInput after rejection, got {other:?}"), + } + + drop(client_stream); + should_quit.store(true, Ordering::Release); + handle + .join() + .expect("read thread join") + .expect("read thread result"); + } + + #[test] + fn client_read_loop_disconnects_oversized_non_paste_input() { + let (mut client_stream, server_stream, _path) = + local_stream_pair("client-read-oversized-non-paste"); + let (server_event_tx, mut server_event_rx) = mpsc::channel(4); + let should_quit = Arc::new(AtomicBool::new(false)); + let read_quit = should_quit.clone(); + let handle = std::thread::spawn(move || { + client_read_loop(server_stream, 7, &server_event_tx, &read_quit) + }); + protocol::write_message( &mut client_stream, &ClientMessage::Input { data: vec![b'x'; MAX_INPUT_PAYLOAD + 1], }, ) - .expect("write oversized input"); + .expect("write oversized non-paste input"); - match server_event_rx - .blocking_recv() - .expect("client disconnected event") - { - ServerEvent::ClientDisconnected { client_id } => assert_eq!(client_id, 7), - other => panic!("expected ClientDisconnected, got {other:?}"), - } + assert!(matches!( + recv_server_event(&mut server_event_rx, "oversized non-paste disconnect"), + ServerEvent::ClientDisconnected { client_id: 7 } + )); + + drop(client_stream); + should_quit.store(true, Ordering::Release); + handle + .join() + .expect("read thread join") + .expect("read thread result"); + } + + #[test] + fn client_read_loop_disconnects_marker_wrapped_invalid_utf8() { + let (mut client_stream, server_stream, _path) = + local_stream_pair("client-read-invalid-utf8-paste"); + let (server_event_tx, mut server_event_rx) = mpsc::channel(4); + let should_quit = Arc::new(AtomicBool::new(false)); + let read_quit = should_quit.clone(); + let handle = std::thread::spawn(move || { + client_read_loop(server_stream, 7, &server_event_tx, &read_quit) + }); + let mut data = bracketed_paste_with_total_len(MAX_INPUT_PAYLOAD + 1); + data[b"\x1b[200~".len()] = 0xff; + + protocol::write_message(&mut client_stream, &ClientMessage::Input { data }) + .expect("write marker-wrapped invalid UTF-8 input"); + + assert!(matches!( + recv_server_event(&mut server_event_rx, "invalid UTF-8 input disconnect"), + ServerEvent::ClientDisconnected { client_id: 7 } + )); drop(client_stream); should_quit.store(true, Ordering::Release); @@ -1302,22 +1468,77 @@ new_tab = "ctrl+notakey" client_read_loop(server_stream, 7, &server_event_tx, &read_quit) }); + let maximum = vec![ + ClientInputEvent::Paste { + text: "x".repeat(MAX_INPUT_PAYLOAD / 2), + }, + ClientInputEvent::Paste { + text: "y".repeat(MAX_INPUT_PAYLOAD - (MAX_INPUT_PAYLOAD / 2)), + }, + ]; protocol::write_message( &mut client_stream, &ClientMessage::InputEvents { - events: vec![ClientInputEvent::Paste { - text: "x".repeat(MAX_INPUT_PAYLOAD + 1), - }], + events: maximum.clone(), }, ) - .expect("write oversized paste event"); + .expect("write maximum-size structured paste"); - match server_event_rx - .blocking_recv() - .expect("client disconnected event") - { - ServerEvent::ClientDisconnected { client_id } => assert_eq!(client_id, 7), - other => panic!("expected ClientDisconnected, got {other:?}"), + match recv_server_event(&mut server_event_rx, "maximum-size structured paste") { + ServerEvent::ClientInputEvents { client_id, events } => { + assert_eq!(client_id, 7); + assert_eq!(events, maximum); + } + other => panic!("expected maximum-size ClientInputEvents, got {other:?}"), + } + + let oversized = vec![ + ClientInputEvent::FocusGained, + ClientInputEvent::Paste { + text: "x".repeat(MAX_INPUT_PAYLOAD / 2), + }, + ClientInputEvent::Paste { + text: "y".repeat(MAX_INPUT_PAYLOAD - (MAX_INPUT_PAYLOAD / 2) + 1), + }, + ClientInputEvent::FocusLost, + ClientInputEvent::Paste { + text: "tail".to_owned(), + }, + ]; + protocol::write_message( + &mut client_stream, + &ClientMessage::InputEvents { events: oversized }, + ) + .expect("write oversized structured paste"); + + match recv_server_event(&mut server_event_rx, "oversized structured paste rejection") { + ServerEvent::ClientPasteRejected { + client_id, + size, + max, + } => { + assert_eq!(client_id, 7); + assert_eq!(size, MAX_INPUT_PAYLOAD + 5); + assert_eq!(max, MAX_INPUT_PAYLOAD); + } + other => panic!("expected ClientPasteRejected, got {other:?}"), + } + + let valid = vec![ClientInputEvent::FocusGained]; + protocol::write_message( + &mut client_stream, + &ClientMessage::InputEvents { + events: valid.clone(), + }, + ) + .expect("write valid structured input after rejection"); + + match recv_server_event(&mut server_event_rx, "structured input after rejection") { + ServerEvent::ClientInputEvents { client_id, events } => { + assert_eq!(client_id, 7); + assert_eq!(events, valid); + } + other => panic!("expected ClientInputEvents after rejection, got {other:?}"), } drop(client_stream); diff --git a/src/server/headless.rs b/src/server/headless.rs index a2b8811a..a1c0e257 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -2797,6 +2797,23 @@ impl HeadlessServer { .collect(); self.handle_client_input_events(client_id, events) } + ServerEvent::ClientPasteRejected { + client_id, + size, + max, + } => { + self.send_to_client( + client_id, + ServerMessage::Notify { + kind: protocol::NotifyKind::Toast, + message: "Paste rejected".to_owned(), + body: Some(format!( + "Input message is {size} bytes; Herdr's limit is {max} bytes" + )), + }, + ); + false + } ServerEvent::ClientClipboardImage { client_id, extension, @@ -8694,6 +8711,78 @@ next_tab = "" ); } + #[test] + fn oversized_paste_rejection_notifies_only_the_sending_client() { + let mut server = test_headless_server(); + let (sender_writer, sender_control_rx, _sender_render_rx) = test_client_writer(); + let (foreground_writer, foreground_control_rx, _foreground_render_rx) = + test_client_writer(); + + server.clients.insert( + 1, + ClientConnection::new( + (120, 40), + crate::kitty_graphics::HostCellSize::default(), + crate::terminal_theme::TerminalTheme::default(), + None, + 1, + RenderEncoding::SemanticFrame, + Some(sender_writer), + ), + ); + server.clients.insert( + 2, + ClientConnection::new( + (80, 24), + crate::kitty_graphics::HostCellSize::default(), + crate::terminal_theme::TerminalTheme::default(), + None, + 2, + RenderEncoding::SemanticFrame, + Some(foreground_writer), + ), + ); + server.foreground_client_id = Some(2); + server.sync_foreground_client_state(); + + assert!( + !server.handle_server_event(ServerEvent::ClientPasteRejected { + client_id: 1, + size: 5_000_012, + max: 1_048_576, + }) + ); + + match read_server_message( + sender_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("sending client rejection notification"), + ) { + ServerMessage::Notify { + kind, + message, + body, + } => { + assert_eq!(kind, protocol::NotifyKind::Toast); + assert_eq!(message, "Paste rejected"); + assert_eq!( + body.as_deref(), + Some("Input message is 5000012 bytes; Herdr's limit is 1048576 bytes") + ); + } + other => panic!("expected paste rejection notification, got {other:?}"), + } + assert!( + foreground_control_rx + .recv_timeout(Duration::from_millis(50)) + .is_err(), + "foreground client must not receive another client's rejection" + ); + assert_eq!(server.foreground_client_id, Some(2)); + assert_eq!(server.clients.len(), 2); + assert!(server.app.state.toast.is_none()); + } + #[test] fn herdr_toast_delivery_keeps_toast_in_frame_without_client_notify() { let mut server = test_headless_server();