From 9797adaa0bd084af84141933776162689a2cf7cf Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 21 Jul 2026 16:27:43 -0400 Subject: [PATCH] fix(vterm): harden terminal cell and input invariants Reject C0/C1 controls before terminal text reaches screen cells, and preserve the released button code in SGR mouse reports. Remove dead screen branches, keep logical-line allocation saturating, and clear round-trip input state when pruning externally removed terminal buffers. --- src/editor.rs | 4 ++-- src/terminal/input.rs | 6 +++--- src/terminal/screen.rs | 32 +++++++++++++++++++------------- src/terminal/session.rs | 3 ++- tests/vterm_stage1_acceptance.rs | 12 +++++++++++- 5 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/editor.rs b/src/editor.rs index 09fa0ac..a8590ae 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -529,8 +529,8 @@ impl EditorState { supervisor.tick(); let mut manager = self.terminal_manager.borrow_mut(); manager.tick(&mut supervisor); - let core = self.core.borrow(); - manager.prune(&core, &mut supervisor); + let mut core = self.core.borrow_mut(); + manager.prune(&mut core, &mut supervisor); } self.lua_host .run_hook("process.after-tick", mlua::MultiValue::new()); diff --git a/src/terminal/input.rs b/src/terminal/input.rs index dfa3e71..35fada3 100644 --- a/src/terminal/input.rs +++ b/src/terminal/input.rs @@ -122,7 +122,7 @@ pub fn encode_mouse( } let (mut code, release) = match kind { MouseKind::Down(button) => (button_code(button), false), - MouseKind::Up(_) => (3, true), + MouseKind::Up(button) => (button_code(button), true), MouseKind::Drag(button) => (button_code(button) + 32, false), MouseKind::Move => (35, false), MouseKind::ScrollUp => (64, false), @@ -317,12 +317,12 @@ mod tests { ); assert_eq!( encode_mouse( - MouseKind::Up(MouseButton::Left), + MouseKind::Up(MouseButton::Right), CellCoord::new(4, 9), Modifiers::NONE, m ), - Some(b"\x1b[<3;10;5m".to_vec()) + Some(b"\x1b[<2;10;5m".to_vec()) ); assert_eq!( encode_mouse( diff --git a/src/terminal/screen.rs b/src/terminal/screen.rs index ab5c31f..fe4939f 100644 --- a/src/terminal/screen.rs +++ b/src/terminal/screen.rs @@ -234,12 +234,12 @@ impl TerminalScreen { None } AnsiEvent::LineFeed | AnsiEvent::Index => { - self.line_feed(false); + self.line_feed(); None } AnsiEvent::NextLine => { self.cursor.col = 0; - self.line_feed(false); + self.line_feed(); None } AnsiEvent::ReverseIndex => { @@ -455,7 +455,7 @@ impl TerminalScreen { if needs_newline { self.cursor.pending_wrap = false; self.cursor.col = 0; - self.line_feed(false); + self.line_feed(); } self.write_text(annotation); self.cursor.pending_wrap = false; @@ -561,6 +561,9 @@ impl TerminalScreen { fn write_text(&mut self, text: &str) { for ch in text.chars() { let ch = self.map_character(ch); + if ch.is_control() { + continue; + } if self.try_extend_previous_grapheme(ch) { continue; } @@ -624,9 +627,6 @@ impl TerminalScreen { } if width == 2 && lead + 1 >= cols { self.active_mut().rows[row].cells[lead] = blank(self.style); - if old_width == 2 && lead + 1 < cols { - self.active_mut().rows[row].cells[lead + 1] = blank(self.style); - } self.cursor.row = row; self.cursor.col = cols - 1; self.cursor.pending_wrap = true; @@ -751,13 +751,11 @@ impl TerminalScreen { self.changed(); } - fn line_feed(&mut self, soft: bool) { + fn line_feed(&mut self) { self.cursor.pending_wrap = false; let row = self.cursor.row; - if !soft { - self.active_mut().rows[row].soft_wrapped = false; - self.break_chain_after(row); - } + self.active_mut().rows[row].soft_wrapped = false; + self.break_chain_after(row); if row == self.scroll_bottom { self.scroll_up_internal(1, None); } else if row + 1 < self.size.rows as usize { @@ -1330,7 +1328,7 @@ impl TerminalScreen { } while rows.len() < size.rows as usize { let id = self.next_line_id; - self.next_line_id += 1; + self.next_line_id = self.next_line_id.saturating_add(1); rows.push(TerminalRow::new(new_cols, id, self.style)); } let split = rows.len().saturating_sub(size.rows as usize); @@ -1396,7 +1394,7 @@ impl Grid { *next_id, Style::default(), )); - *next_id += 1; + *next_id = next_id.saturating_add(1); } Self { rows, @@ -1616,6 +1614,14 @@ mod tests { assert!(matches!(s.snapshot().cells[1].glyph, Glyph::Char('x'))); } + #[test] + fn control_characters_never_enter_cells() { + let mut s = screen(2, 4); + let before = s.snapshot(); + s.apply_event(AnsiEvent::Text("\u{9b}\n\0".into())); + assert_eq!(s.snapshot(), before); + } + #[test] fn alternate_screen_preserves_main_and_has_no_history() { let mut s = screen(2, 4); diff --git a/src/terminal/session.rs b/src/terminal/session.rs index 8e0e672..2c1876b 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -473,7 +473,7 @@ impl TerminalManager { } /// Tear down sessions whose identity buffers were removed by any path. - pub fn prune(&mut self, core: &EditorCore, supervisor: &mut ProcessSupervisor) { + pub fn prune(&mut self, core: &mut EditorCore, supervisor: &mut ProcessSupervisor) { let removed: Vec = { let registry = core.registry.borrow(); self.sessions @@ -483,6 +483,7 @@ impl TerminalManager { .collect() }; for buffer_id in removed { + core.set_round_trip_input(buffer_id, false); let Some(session) = self.sessions.remove(&buffer_id) else { continue; }; diff --git a/tests/vterm_stage1_acceptance.rs b/tests/vterm_stage1_acceptance.rs index 2ec28a3..b01d5b5 100644 --- a/tests/vterm_stage1_acceptance.rs +++ b/tests/vterm_stage1_acceptance.rs @@ -2,11 +2,13 @@ use std::time::{Duration, Instant}; +use pmacs::ansi::AnsiEvent; use pmacs::buffer::{Buffer, BufferError, BufferId, EditOp}; -use pmacs::cell::Glyph; +use pmacs::cell::{CellSize, Glyph}; use pmacs::editor::EditorState; use pmacs::process::ProcessState; use pmacs::rope::Range; +use pmacs::terminal::screen::TerminalScreen; use pmacs::terminal::{TerminalProcessState, TerminalSpec}; fn rope_bytes(buffer: &Buffer) -> Vec { @@ -48,6 +50,14 @@ fn tick_until( panic!("terminal condition did not settle before {timeout:?}"); } +#[test] +fn terminal_cells_reject_child_control_characters() { + let mut screen = TerminalScreen::new(CellSize::new(2, 4), 0).expect("valid screen"); + let before = screen.snapshot(); + screen.apply_event(AnsiEvent::Text("\u{9b}\n\0".into())); + assert_eq!(screen.snapshot(), before); +} + #[test] fn spawn_failure_is_transactional() { let mut state = EditorState::new();