From bcdc605436ebe137173c531844a739eda6ee41ae Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Sun, 17 Nov 2019 05:10:11 +0300 Subject: [PATCH] Fix ESC escapes not ignoring invalid intermediates Previously, `ESC` escapes would ignore invalid intermediates and still execute the specified actions, leading to false positives. If there's an unexpected, intermediate specified now, the escape will be dropped. This also fixes an issue with `CSI Ps c` not dropping the escape with invalid intermediates. --- CHANGELOG.md | 2 + alacritty_terminal/src/ansi.rs | 248 +++++++++++++++++------------ alacritty_terminal/src/term/mod.rs | 1 + 3 files changed, 149 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68389f0..2bedd1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Mouse protocols/encodings not being mutually exclusive within themselves - Escape `CSI Ps M` deleting lines above cursor when at the bottom of the viewport - Cell reset not clearing underline, strikeout and foreground color +- Escape `CSI Ps c` honored with a wrong `Ps` +- Ignore `ESC` escapes with invalid intermediates ### Removed diff --git a/alacritty_terminal/src/ansi.rs b/alacritty_terminal/src/ansi.rs index a889862..c238854 100644 --- a/alacritty_terminal/src/ansi.rs +++ b/alacritty_terminal/src/ansi.rs @@ -943,7 +943,9 @@ where ('B', None) | ('e', None) => { handler.move_down(Line(arg_or_default!(idx: 0, default: 1) as usize)) }, - ('c', None) => handler.identify_terminal(writer), + ('c', None) if arg_or_default!(idx: 0, default: 0) == 0 => { + handler.identify_terminal(writer) + }, ('C', None) | ('a', None) => { handler.move_forward(Column(arg_or_default!(idx: 0, default: 1) as usize)) }, @@ -1104,8 +1106,8 @@ where } macro_rules! configure_charset { - ($charset:path) => {{ - let index: CharsetIndex = match intermediates.first().cloned() { + ($charset:path, $intermediate:expr) => {{ + let index: CharsetIndex = match $intermediate { Some(b'(') => CharsetIndex::G0, Some(b')') => CharsetIndex::G1, Some(b'*') => CharsetIndex::G2, @@ -1119,29 +1121,27 @@ where }}; } - match byte { - b'B' => configure_charset!(StandardCharset::Ascii), - b'D' => self.handler.linefeed(), - b'E' => { + match (byte, intermediates.get(0)) { + (b'B', intermediate) => configure_charset!(StandardCharset::Ascii, intermediate), + (b'D', None) => self.handler.linefeed(), + (b'E', None) => { self.handler.linefeed(); self.handler.carriage_return(); }, - b'H' => self.handler.set_horizontal_tabstop(), - b'M' => self.handler.reverse_index(), - b'Z' => self.handler.identify_terminal(self.writer), - b'c' => self.handler.reset_state(), - b'0' => configure_charset!(StandardCharset::SpecialCharacterAndLineDrawing), - b'7' => self.handler.save_cursor_position(), - b'8' => { - if !intermediates.is_empty() && intermediates[0] == b'#' { - self.handler.decaln(); - } else { - self.handler.restore_cursor_position(); - } + (b'H', None) => self.handler.set_horizontal_tabstop(), + (b'M', None) => self.handler.reverse_index(), + (b'Z', None) => self.handler.identify_terminal(self.writer), + (b'c', None) => self.handler.reset_state(), + (b'0', intermediate) => { + configure_charset!(StandardCharset::SpecialCharacterAndLineDrawing, intermediate) }, - b'=' => self.handler.set_keypad_application_mode(), - b'>' => self.handler.unset_keypad_application_mode(), - b'\\' => (), // String terminator, do nothing (parser handles as string terminator) + (b'7', None) => self.handler.save_cursor_position(), + (b'8', Some(b'#')) => self.handler.decaln(), + (b'8', None) => self.handler.restore_cursor_position(), + (b'=', None) => self.handler.set_keypad_application_mode(), + (b'>', None) => self.handler.unset_keypad_application_mode(), + // String terminator, do nothing (parser handles as string terminator) + (b'\\', None) => (), _ => unhandled!(), } } @@ -1446,66 +1446,142 @@ mod tests { use crate::term::color::Rgb; use std::io; - /// The /dev/null of `io::Write` - struct Void; - - impl io::Write for Void { - fn write(&mut self, bytes: &[u8]) -> io::Result { - Ok(bytes.len()) - } - - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } - } - - #[derive(Default)] - struct AttrHandler { + struct MockHandler { + index: CharsetIndex, + charset: StandardCharset, attr: Option, + identity_reported: bool, } - impl Handler for AttrHandler { + impl Handler for MockHandler { fn terminal_attribute(&mut self, attr: Attr) { self.attr = Some(attr); } + + fn configure_charset(&mut self, index: CharsetIndex, charset: StandardCharset) { + self.index = index; + self.charset = charset; + } + + fn set_active_charset(&mut self, index: CharsetIndex) { + self.index = index; + } + + fn identify_terminal(&mut self, _: &mut W) { + self.identity_reported = true; + } + + fn reset_state(&mut self) { + *self = Self::default(); + } } - impl TermInfo for AttrHandler { + impl TermInfo for MockHandler { fn lines(&self) -> Line { - Line(24) + Line(200) } fn cols(&self) -> Column { - Column(80) + Column(90) + } + } + + impl Default for MockHandler { + fn default() -> MockHandler { + MockHandler { + index: CharsetIndex::G0, + charset: StandardCharset::Ascii, + attr: None, + identity_reported: false, + } } } #[test] fn parse_control_attribute() { - static BYTES: &[u8] = &[0x1b, 0x5b, 0x31, 0x6d]; + static BYTES: &[u8] = &[0x1b, b'[', b'1', b'm']; let mut parser = Processor::new(); - let mut handler = AttrHandler::default(); + let mut handler = MockHandler::default(); for byte in &BYTES[..] { - parser.advance(&mut handler, *byte, &mut Void); + parser.advance(&mut handler, *byte, &mut io::sink()); } assert_eq!(handler.attr, Some(Attr::Bold)); } + #[test] + fn parse_terminal_identity_csi() { + let bytes: &[u8] = &[0x1b, b'[', b'1', b'c']; + + let mut parser = Processor::new(); + let mut handler = MockHandler::default(); + + for byte in &bytes[..] { + parser.advance(&mut handler, *byte, &mut io::sink()); + } + + assert!(!handler.identity_reported); + handler.reset_state(); + + let bytes: &[u8] = &[0x1b, b'[', b'c']; + + for byte in &bytes[..] { + parser.advance(&mut handler, *byte, &mut io::sink()); + } + + assert!(handler.identity_reported); + handler.reset_state(); + + let bytes: &[u8] = &[0x1b, b'[', b'0', b'c']; + + for byte in &bytes[..] { + parser.advance(&mut handler, *byte, &mut io::sink()); + } + + assert!(handler.identity_reported); + } + + #[test] + fn parse_terminal_identity_esc() { + let bytes: &[u8] = &[0x1b, b'Z']; + + let mut parser = Processor::new(); + let mut handler = MockHandler::default(); + + for byte in &bytes[..] { + parser.advance(&mut handler, *byte, &mut io::sink()); + } + + assert!(handler.identity_reported); + handler.reset_state(); + + let bytes: &[u8] = &[0x1b, b'#', b'Z']; + + let mut parser = Processor::new(); + let mut handler = MockHandler::default(); + + for byte in &bytes[..] { + parser.advance(&mut handler, *byte, &mut io::sink()); + } + + assert!(!handler.identity_reported); + handler.reset_state(); + } + #[test] fn parse_truecolor_attr() { static BYTES: &[u8] = &[ - 0x1b, 0x5b, 0x33, 0x38, 0x3b, 0x32, 0x3b, 0x31, 0x32, 0x38, 0x3b, 0x36, 0x36, 0x3b, - 0x32, 0x35, 0x35, 0x6d, + 0x1b, b'[', b'3', b'8', b';', b'2', b';', b'1', b'2', b'8', b';', b'6', b'6', b';', + b'2', b'5', b'5', b'm', ]; let mut parser = Processor::new(); - let mut handler = AttrHandler::default(); + let mut handler = MockHandler::default(); for byte in &BYTES[..] { - parser.advance(&mut handler, *byte, &mut Void); + parser.advance(&mut handler, *byte, &mut io::sink()); } let spec = Rgb { r: 128, g: 66, b: 255 }; @@ -1517,58 +1593,26 @@ mod tests { #[test] fn parse_zsh_startup() { static BYTES: &[u8] = &[ - 0x1b, 0x5b, 0x31, 0x6d, 0x1b, 0x5b, 0x37, 0x6d, 0x25, 0x1b, 0x5b, 0x32, 0x37, 0x6d, - 0x1b, 0x5b, 0x31, 0x6d, 0x1b, 0x5b, 0x30, 0x6d, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, - 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, - 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, - 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, - 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, - 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, - 0x20, 0x20, 0x20, 0x0d, 0x20, 0x0d, 0x0d, 0x1b, 0x5b, 0x30, 0x6d, 0x1b, 0x5b, 0x32, - 0x37, 0x6d, 0x1b, 0x5b, 0x32, 0x34, 0x6d, 0x1b, 0x5b, 0x4a, 0x6a, 0x77, 0x69, 0x6c, - 0x6d, 0x40, 0x6a, 0x77, 0x69, 0x6c, 0x6d, 0x2d, 0x64, 0x65, 0x73, 0x6b, 0x20, 0x1b, - 0x5b, 0x30, 0x31, 0x3b, 0x33, 0x32, 0x6d, 0xe2, 0x9e, 0x9c, 0x20, 0x1b, 0x5b, 0x30, - 0x31, 0x3b, 0x33, 0x32, 0x6d, 0x20, 0x1b, 0x5b, 0x33, 0x36, 0x6d, 0x7e, 0x2f, 0x63, - 0x6f, 0x64, 0x65, + 0x1b, b'[', b'1', b'm', 0x1b, b'[', b'7', b'm', b'%', 0x1b, b'[', b'2', b'7', b'm', + 0x1b, b'[', b'1', b'm', 0x1b, b'[', b'0', b'm', b' ', b' ', b' ', b' ', b' ', b' ', + b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', + b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', + b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', + b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', + b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', b' ', + b' ', b' ', b' ', b'\r', b' ', b'\r', b'\r', 0x1b, b'[', b'0', b'm', 0x1b, b'[', b'2', + b'7', b'm', 0x1b, b'[', b'2', b'4', b'm', 0x1b, b'[', b'J', b'j', b'w', b'i', b'l', + b'm', b'@', b'j', b'w', b'i', b'l', b'm', b'-', b'd', b'e', b's', b'k', b' ', 0x1b, + b'[', b'0', b'1', b';', b'3', b'2', b'm', 0xe2, 0x9e, 0x9c, b' ', 0x1b, b'[', b'0', + b'1', b';', b'3', b'2', b'm', b' ', 0x1b, b'[', b'3', b'6', b'm', b'~', b'/', b'c', + b'o', b'd', b'e', ]; - let mut handler = AttrHandler::default(); + let mut handler = MockHandler::default(); let mut parser = Processor::new(); for byte in &BYTES[..] { - parser.advance(&mut handler, *byte, &mut Void); - } - } - - struct CharsetHandler { - index: CharsetIndex, - charset: StandardCharset, - } - - impl Default for CharsetHandler { - fn default() -> CharsetHandler { - CharsetHandler { index: CharsetIndex::G0, charset: StandardCharset::Ascii } - } - } - - impl Handler for CharsetHandler { - fn configure_charset(&mut self, index: CharsetIndex, charset: StandardCharset) { - self.index = index; - self.charset = charset; - } - - fn set_active_charset(&mut self, index: CharsetIndex) { - self.index = index; - } - } - - impl TermInfo for CharsetHandler { - fn lines(&self) -> Line { - Line(200) - } - - fn cols(&self) -> Column { - Column(90) + parser.advance(&mut handler, *byte, &mut io::sink()); } } @@ -1576,10 +1620,10 @@ mod tests { fn parse_designate_g0_as_line_drawing() { static BYTES: &[u8] = &[0x1b, b'(', b'0']; let mut parser = Processor::new(); - let mut handler = CharsetHandler::default(); + let mut handler = MockHandler::default(); for byte in &BYTES[..] { - parser.advance(&mut handler, *byte, &mut Void); + parser.advance(&mut handler, *byte, &mut io::sink()); } assert_eq!(handler.index, CharsetIndex::G0); @@ -1588,19 +1632,19 @@ mod tests { #[test] fn parse_designate_g1_as_line_drawing_and_invoke() { - static BYTES: &[u8] = &[0x1b, 0x29, 0x30, 0x0e]; + static BYTES: &[u8] = &[0x1b, b')', b'0', 0x0e]; let mut parser = Processor::new(); - let mut handler = CharsetHandler::default(); + let mut handler = MockHandler::default(); for byte in &BYTES[..3] { - parser.advance(&mut handler, *byte, &mut Void); + parser.advance(&mut handler, *byte, &mut io::sink()); } assert_eq!(handler.index, CharsetIndex::G1); assert_eq!(handler.charset, StandardCharset::SpecialCharacterAndLineDrawing); - let mut handler = CharsetHandler::default(); - parser.advance(&mut handler, BYTES[3], &mut Void); + let mut handler = MockHandler::default(); + parser.advance(&mut handler, BYTES[3], &mut io::sink()); assert_eq!(handler.index, CharsetIndex::G1); } diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index ce63c40..8d69fb3 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -1397,6 +1397,7 @@ impl ansi::Handler for Term { #[inline] fn identify_terminal(&mut self, writer: &mut W) { + trace!("Reporting terminal identity"); let _ = writer.write_all(b"\x1b[?6c"); }