From 1d238bc9a365d9dd1297b4e11fe775cd656aebd0 Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Wed, 22 Jul 2026 18:59:01 +0300 Subject: refactor: replace pane mouse and clipboard internals replaces contributions from othavi0 --- src/pane/osc.rs | 471 +++++++++++++++------------------------------------ src/pane/terminal.rs | 42 ++++- 2 files changed, 173 insertions(+), 340 deletions(-) (limited to 'src/pane') diff --git a/src/pane/osc.rs b/src/pane/osc.rs index 22e76ff1..812c8edd 100644 --- a/src/pane/osc.rs +++ b/src/pane/osc.rs @@ -314,95 +314,133 @@ fn parse_default_color_set_events(body: &[u8]) -> Vec { .collect() } -/// 256 KiB of base64 ≈ 192 KiB of text — enough for real source-file copies -/// while still bounding memory against stream garbage. -const OSC52_MAX_PAYLOAD_BYTES: usize = 256 * 1024; +pub(super) fn parse_reported_cwd(value: &[u8]) -> Option { + let value = std::str::from_utf8(value).ok()?.trim(); + if value.starts_with("file://") { + return parse_file_uri_cwd(value); + } + let path = value.trim_matches('"'); + (!path.is_empty()).then(|| PathBuf::from(path)) +} + +/// Collects complete OSC bodies from a raw byte stream. Consumers receive only +/// bodies, keeping the framing state machine independent from OSC commands. +#[derive(Debug, Default)] +struct OscStreamCollector { + state: OscStreamState, + body: Vec, +} #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -enum Osc52ForwarderState { +enum OscStreamState { #[default] Ground, Escape, - OscBody, - OscEscape, + Body, + BodyEscape, + IgnoringString, + IgnoringStringEscape, + Discarding, + DiscardingEscape, } -/// Reconstructs OSC 52 clipboard-write sequences from raw PTY bytes so the -/// main loop can re-emit them. `libghostty-vt` drops `.clipboard_contents`, -/// so child clipboard writes never reach the host terminal unless we forward -/// them ourselves. -#[derive(Debug, Default)] -pub(super) struct Osc52Forwarder { - state: Osc52ForwarderState, - body: Vec, - pending: Vec>, -} +impl OscStreamCollector { + const MAX_BODY_BYTES: usize = 4096; -impl Osc52Forwarder { - pub(super) fn observe(&mut self, bytes: &[u8]) { + fn observe(&mut self, bytes: &[u8], mut receive: impl FnMut(&[u8])) { for &byte in bytes { match self.state { - Osc52ForwarderState::Ground => { + OscStreamState::Ground => { if byte == 0x1b { - self.state = Osc52ForwarderState::Escape; + self.state = OscStreamState::Escape; } } - Osc52ForwarderState::Escape => { - if byte == b']' { + OscStreamState::Escape => match byte { + b']' => { self.body.clear(); - self.state = Osc52ForwarderState::OscBody; - } else if byte == 0x1b { - self.state = Osc52ForwarderState::Escape; - } else { - self.state = Osc52ForwarderState::Ground; + self.state = OscStreamState::Body; } - } - Osc52ForwarderState::OscBody => match byte { + 0x1b => self.state = OscStreamState::Escape, + byte if is_ignored_string_intro(byte) => { + self.state = OscStreamState::IgnoringString; + } + _ => self.state = OscStreamState::Ground, + }, + OscStreamState::Body => match byte { + 0x07 => self.finish(&mut receive), + 0x1b => self.state = OscStreamState::BodyEscape, + _ => self.push(byte), + }, + OscStreamState::BodyEscape => match byte { + b'\\' => self.finish(&mut receive), 0x07 => { - self.finalize(); - self.state = Osc52ForwarderState::Ground; + self.push(0x1b); + if matches!(self.state, OscStreamState::Body) { + self.finish(&mut receive); + } else { + self.state = OscStreamState::Ground; + } + } + 0x1b => { + self.push(0x1b); + self.state = match self.state { + OscStreamState::Body => OscStreamState::BodyEscape, + OscStreamState::Discarding => OscStreamState::DiscardingEscape, + state => state, + }; + } + _ => { + self.push(0x1b); + if matches!(self.state, OscStreamState::Body) { + self.push(byte); + } } - 0x1b => self.state = Osc52ForwarderState::OscEscape, - _ => self.body.push(byte), }, - Osc52ForwarderState::OscEscape => { + OscStreamState::IgnoringString => { + if byte == 0x1b { + self.state = OscStreamState::IgnoringStringEscape; + } + } + OscStreamState::IgnoringStringEscape => { if byte == b'\\' { - self.finalize(); - self.state = Osc52ForwarderState::Ground; - } else { - self.body.push(0x1b); - self.body.push(byte); - self.state = Osc52ForwarderState::OscBody; + self.state = OscStreamState::Ground; + } else if byte != 0x1b { + self.state = OscStreamState::IgnoringString; + } + } + OscStreamState::Discarding => { + if byte == 0x07 { + self.state = OscStreamState::Ground; + } else if byte == 0x1b { + self.state = OscStreamState::DiscardingEscape; + } + } + OscStreamState::DiscardingEscape => { + if byte == b'\\' { + self.state = OscStreamState::Ground; + } else if byte != 0x1b { + self.state = OscStreamState::Discarding; } } - } - - if self.body.len() > OSC52_MAX_PAYLOAD_BYTES { - self.body.clear(); - self.state = Osc52ForwarderState::Ground; } } } - fn finalize(&mut self) { - if let Some(content) = parse_osc52_clipboard_write(&self.body) { - self.pending.push(content); + fn push(&mut self, byte: u8) { + self.body.push(byte); + if self.body.len() > Self::MAX_BODY_BYTES { + self.body.clear(); + self.state = OscStreamState::Discarding; + } else { + self.state = OscStreamState::Body; } - self.body.clear(); - } - - pub(super) fn drain_pending(&mut self) -> Vec> { - std::mem::take(&mut self.pending) } -} -pub(super) fn parse_reported_cwd(value: &[u8]) -> Option { - let value = std::str::from_utf8(value).ok()?.trim(); - if value.starts_with("file://") { - return parse_file_uri_cwd(value); + fn finish(&mut self, receive: &mut impl FnMut(&[u8])) { + receive(&self.body); + self.body.clear(); + self.state = OscStreamState::Ground; } - let path = value.trim_matches('"'); - (!path.is_empty()).then(|| PathBuf::from(path)) } /// Maximum retained string length for agent OSC title and progress payloads. @@ -419,8 +457,7 @@ const AGENT_OSC_MAX_CHARS: usize = 256; /// as-is after sanitization. E.g. `"4;3;"` or `"4;0;"`. #[derive(Debug, Default)] pub(super) struct AgentOscStateTracker { - state: Osc52ForwarderState, - body: Vec, + collector: OscStreamCollector, latest_title: Option, terminal_title: Option, latest_progress: Option, @@ -428,71 +465,29 @@ pub(super) struct AgentOscStateTracker { impl AgentOscStateTracker { pub(super) fn observe(&mut self, bytes: &[u8]) { - for &byte in bytes { - match self.state { - Osc52ForwarderState::Ground => { - if byte == 0x1b { - self.state = Osc52ForwarderState::Escape; - } - } - Osc52ForwarderState::Escape => { - if byte == b']' { - self.body.clear(); - self.state = Osc52ForwarderState::OscBody; - } else if byte == 0x1b { - self.state = Osc52ForwarderState::Escape; - } else { - self.state = Osc52ForwarderState::Ground; - } - } - Osc52ForwarderState::OscBody => match byte { - 0x07 => { - self.finalize(); - self.state = Osc52ForwarderState::Ground; - } - 0x1b => self.state = Osc52ForwarderState::OscEscape, - _ => self.body.push(byte), - }, - Osc52ForwarderState::OscEscape => { - if byte == b'\\' { - self.finalize(); - self.state = Osc52ForwarderState::Ground; - } else { - self.body.push(0x1b); - self.body.push(byte); - self.state = Osc52ForwarderState::OscBody; - } - } - } - - if self.body.len() > 4096 { - self.body.clear(); - self.state = Osc52ForwarderState::Ground; - } - } - } - - fn finalize(&mut self) { - if let Some((command, payload)) = parse_agent_osc_body(&self.body) { + let (collector, latest_title, terminal_title, latest_progress) = ( + &mut self.collector, + &mut self.latest_title, + &mut self.terminal_title, + &mut self.latest_progress, + ); + collector.observe(bytes, |body| { + let Some((command, payload)) = parse_agent_osc_body(body) else { + return; + }; match command { b"0" | b"2" => { - let title = if payload.is_empty() { - None - } else { - let title = sanitize_agent_osc_string(payload, AGENT_OSC_MAX_CHARS); - (!title.is_empty()).then_some(title) - }; - self.latest_title.clone_from(&title); - self.terminal_title = title; + let title = sanitize_agent_osc_string(payload, AGENT_OSC_MAX_CHARS); + *terminal_title = (!title.is_empty()).then_some(title.clone()); + *latest_title = (!title.is_empty()).then_some(title); } b"9" => { - self.latest_progress = + *latest_progress = Some(sanitize_agent_osc_string(payload, AGENT_OSC_MAX_CHARS)); } _ => {} } - } - self.body.clear(); + }); } pub(super) fn terminal_title(&self) -> Option<&str> { @@ -549,8 +544,7 @@ fn sanitize_agent_osc_string(payload: &[u8], max_chars: usize) -> String { #[derive(Debug)] pub(super) struct OscDebugTracker { enabled: bool, - state: Osc52ForwarderState, - body: Vec, + collector: OscStreamCollector, pending: Vec, } @@ -564,8 +558,7 @@ impl OscDebugTracker { pub(super) fn from_env() -> Self { Self { enabled: osc_debug_enabled_from_env(), - state: Osc52ForwarderState::Ground, - body: Vec::new(), + collector: OscStreamCollector::default(), pending: Vec::new(), } } @@ -574,56 +567,12 @@ impl OscDebugTracker { if !self.enabled { return; } - - for &byte in bytes { - match self.state { - Osc52ForwarderState::Ground => { - if byte == 0x1b { - self.state = Osc52ForwarderState::Escape; - } - } - Osc52ForwarderState::Escape => { - if byte == b']' { - self.body.clear(); - self.state = Osc52ForwarderState::OscBody; - } else if byte == 0x1b { - self.state = Osc52ForwarderState::Escape; - } else { - self.state = Osc52ForwarderState::Ground; - } - } - Osc52ForwarderState::OscBody => match byte { - 0x07 => { - self.finalize(); - self.state = Osc52ForwarderState::Ground; - } - 0x1b => self.state = Osc52ForwarderState::OscEscape, - _ => self.body.push(byte), - }, - Osc52ForwarderState::OscEscape => { - if byte == b'\\' { - self.finalize(); - self.state = Osc52ForwarderState::Ground; - } else { - self.body.push(0x1b); - self.body.push(byte); - self.state = Osc52ForwarderState::OscBody; - } - } - } - - if self.body.len() > 4096 { - self.body.clear(); - self.state = Osc52ForwarderState::Ground; + let (collector, pending) = (&mut self.collector, &mut self.pending); + collector.observe(bytes, |body| { + if let Some(event) = parse_osc_debug_event(body) { + pending.push(event); } - } - } - - fn finalize(&mut self) { - if let Some(event) = parse_osc_debug_event(&self.body) { - self.pending.push(event); - } - self.body.clear(); + }); } pub(super) fn drain_pending(&mut self) -> Vec { @@ -733,22 +682,6 @@ fn hex_value(byte: u8) -> Option { } } -/// Accepts `52;c;` and `52;;`. -/// Queries (`?`) are rejected because herdr has no reply path. -/// The payload must decode as base64 before it is forwarded. -fn parse_osc52_clipboard_write(body: &[u8]) -> Option> { - use base64::Engine; - - let rest = body.strip_prefix(b"52;")?; - let sep = rest.iter().position(|b| *b == b';')?; - let selector = &rest[..sep]; - let data = &rest[sep + 1..]; - if !(selector.is_empty() || selector == b"c") || data == b"?" { - return None; - } - base64::engine::general_purpose::STANDARD.decode(data).ok() -} - fn foreground_job_is_shell(job: &crate::platform::ForegroundJob, shell_pid: u32) -> bool { job.processes.iter().any(|process| process.pid == shell_pid) } @@ -983,12 +916,25 @@ mod tests { fn enabled_osc_debug_tracker() -> OscDebugTracker { OscDebugTracker { enabled: true, - state: Osc52ForwarderState::Ground, - body: Vec::new(), + collector: OscStreamCollector::default(), pending: Vec::new(), } } + #[test] + fn osc_stream_collector_ignores_strings_and_preserves_escaped_bytes() { + let mut collector = OscStreamCollector::default(); + let mut bodies = Vec::new(); + + collector.observe( + b"\x1bPignored\x1b]0;not-osc\x07\x1b\\\x1b]9;a\x1b", + |body| bodies.push(body.to_vec()), + ); + collector.observe(b"\x1b\\\x1b]2;b\x1b\x07", |body| bodies.push(body.to_vec())); + + assert_eq!(bodies, vec![b"9;a\x1b".to_vec(), b"2;b\x1b".to_vec()]); + } + #[test] fn default_color_tracker_detects_split_osc_11_sequences() { let mut tracker = DefaultColorOscTracker::default(); @@ -1399,151 +1345,6 @@ mod tests { ); } - #[test] - fn osc52_forwarder_detects_write_with_bel() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;aGVsbG8=\x07"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hello".to_vec()]); - } - - #[test] - fn osc52_forwarder_detects_write_with_st() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;aGVsbG8=\x1b\\"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hello".to_vec()]); - } - - #[test] - fn osc52_forwarder_detects_empty_selector_form() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;;aGVsbG8=\x07"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hello".to_vec()]); - } - - #[test] - fn osc52_forwarder_accepts_clear_clipboard() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;\x07"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![Vec::::new()]); - } - - #[test] - fn osc52_forwarder_ignores_query() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;?\x07"); - assert!(fw.drain_pending().is_empty()); - } - - #[test] - fn osc52_forwarder_ignores_empty_selector_query() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;;?\x07"); - assert!(fw.drain_pending().is_empty()); - } - - #[test] - fn osc52_forwarder_ignores_other_kinds() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;p;aGk=\x07"); - fw.observe(b"\x1b]52;s;aGk=\x07"); - fw.observe(b"\x1b]52;q;aGk=\x07"); - fw.observe(b"\x1b]52;0;aGk=\x07"); - fw.observe(b"\x1b]52;7;aGk=\x07"); - assert!(fw.drain_pending().is_empty()); - } - - #[test] - fn osc52_forwarder_ignores_invalid_base64() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;%%%\x07"); - fw.observe(b"\x1b]52;c;aGVs\x1b[bG8=\x07"); - assert!(fw.drain_pending().is_empty()); - } - - #[test] - fn osc52_forwarder_ignores_non_osc52() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]11;?\x07"); - fw.observe(b"\x1b]0;title\x07"); - fw.observe(b"\x1b]8;;https://example.com\x1b\\"); - assert!(fw.drain_pending().is_empty()); - } - - #[test] - fn osc52_forwarder_handles_split_sequence_mid_payload() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;aGVs"); - assert!(fw.drain_pending().is_empty()); - fw.observe(b"bG8gd29y"); - assert!(fw.drain_pending().is_empty()); - fw.observe(b"bGQ=\x07"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hello world".to_vec()]); - } - - #[test] - fn osc52_forwarder_handles_split_before_bel() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;aGk="); - assert!(fw.drain_pending().is_empty()); - fw.observe(b"\x07"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hi".to_vec()]); - } - - #[test] - fn osc52_forwarder_handles_split_between_esc_and_backslash() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;aGk=\x1b"); - assert!(fw.drain_pending().is_empty()); - fw.observe(b"\\"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hi".to_vec()]); - } - - #[test] - fn osc52_forwarder_payload_size_limit() { - let mut fw = Osc52Forwarder::default(); - let mut huge = Vec::with_capacity(OSC52_MAX_PAYLOAD_BYTES + 32); - huge.extend_from_slice(b"\x1b]52;c;"); - huge.extend(std::iter::repeat_n(b'A', OSC52_MAX_PAYLOAD_BYTES + 16)); - huge.push(0x07); - fw.observe(&huge); - assert!(fw.drain_pending().is_empty()); - - fw.observe(b"\x1b]52;c;aGk=\x07"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hi".to_vec()]); - } - - #[test] - fn osc52_forwarder_recovers_after_garbage() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x01\x02random\x7fbytes\x1b]52;c;aGk=\x07tail"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hi".to_vec()]); - } - - #[test] - fn osc52_forwarder_multiple_in_one_chunk() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;aGk=\x07\x1b]52;c;Ynll\x07"); - let pending = fw.drain_pending(); - assert_eq!(pending, vec![b"hi".to_vec(), b"bye".to_vec()]); - } - - #[test] - fn osc52_forwarder_drain_clears_pending() { - let mut fw = Osc52Forwarder::default(); - fw.observe(b"\x1b]52;c;aGk=\x07"); - assert_eq!(fw.drain_pending(), vec![b"hi".to_vec()]); - assert!(fw.drain_pending().is_empty()); - } - #[test] fn droid_scrollback_compat_matches_process_name_and_cmdline() { let name_only = crate::platform::ForegroundJob { diff --git a/src/pane/terminal.rs b/src/pane/terminal.rs index 2c83f8f2..261f7025 100644 --- a/src/pane/terminal.rs +++ b/src/pane/terminal.rs @@ -31,7 +31,7 @@ use super::{ maybe_filter_primary_screen_scrollback_clear, parse_reported_cwd, restore_host_terminal_theme_if_needed, write_host_terminal_theme_selective, AgentOscStateTracker, DefaultColorEvent, DefaultColorEventTracker, DefaultColorOscTracker, - DefaultColorQuery, DefaultColorTrackedEvent, Osc52Forwarder, OscDebugTracker, + DefaultColorQuery, DefaultColorTrackedEvent, OscDebugTracker, }, xtgettcap::{XtgettcapQueryTracker, XtgettcapResponse}, }; @@ -161,7 +161,6 @@ pub(crate) struct GhosttyPaneCore { pub default_color_event_tracker: DefaultColorEventTracker, pub child_default_foreground_changed: bool, pub child_default_background_changed: bool, - pub osc52_forwarder: Osc52Forwarder, pub osc_debug_tracker: OscDebugTracker, pub agent_osc_state: AgentOscStateTracker, pub xtgettcap_query_tracker: XtgettcapQueryTracker, @@ -915,7 +914,6 @@ impl GhosttyPaneTerminal { default_color_event_tracker: DefaultColorEventTracker::default(), child_default_foreground_changed: false, child_default_background_changed: false, - osc52_forwarder: Osc52Forwarder::default(), osc_debug_tracker: OscDebugTracker::default(), agent_osc_state: AgentOscStateTracker::default(), xtgettcap_query_tracker: XtgettcapQueryTracker::default(), @@ -1049,6 +1047,9 @@ impl GhosttyPaneTerminal { }; let _ = core.terminal.take_pwd_changes(); + // Restored history may have exercised terminal callbacks before this live PTY write. + // Those writes must not be delivered as live pane output. + let _ = core.terminal.take_clipboard_writes(); let default_color_observation = core.default_color_tracker.observe(bytes); if shell_pid > 0 && default_color_observation { if let Some(owner_pgid) = current_transient_default_color_owner(shell_pid) { @@ -1060,8 +1061,6 @@ impl GhosttyPaneTerminal { } } - core.osc52_forwarder.observe(bytes); - let clipboard_writes = core.osc52_forwarder.drain_pending(); core.osc_debug_tracker.observe(bytes); for event in core.osc_debug_tracker.drain_pending() { debug!( @@ -1116,6 +1115,7 @@ impl GhosttyPaneTerminal { xtgettcap_responses, &mut terminal_responses, ); + let clipboard_writes = core.terminal.take_clipboard_writes(); let reported_cwd = core .terminal .take_pwd_changes() @@ -3221,6 +3221,38 @@ mod tests { ); } + #[test] + fn process_pty_bytes_surfaces_clipboard_writes_without_other_results() { + let (tx, _rx) = mpsc::channel(4); + let terminal = crate::ghostty::Terminal::new(80, 24, 100).unwrap(); + let pane = GhosttyPaneTerminal::new(terminal, tx.clone()).unwrap(); + + let result = pane.process_pty_bytes( + PaneId::from_raw(1), + 0, + b"output\x1b]52;c;Y2xpcGJvYXJk\x07", + &tx, + ); + + assert!(result.request_render); + assert_eq!(result.render_delay, None); + assert_eq!(result.clipboard_writes, vec![b"clipboard".to_vec()]); + assert_eq!(result.reported_cwd, None); + assert!(result.terminal_responses.is_empty()); + } + + #[test] + fn seeded_history_clipboard_write_does_not_leak_into_live_output() { + let (tx, _rx) = mpsc::channel(4); + let terminal = crate::ghostty::Terminal::new(80, 24, 100).unwrap(); + let pane = GhosttyPaneTerminal::new(terminal, tx.clone()).unwrap(); + pane.seed_history_ansi("\x1b]52;c;c3RhbGU=\x07"); + + let result = pane.process_pty_bytes(PaneId::from_raw(1), 0, b"live output", &tx); + + assert!(result.clipboard_writes.is_empty()); + } + #[test] fn seeded_history_pwd_does_not_leak_into_live_output() { let (tx, _rx) = mpsc::channel(4); -- cgit v1.2.3