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

@@ -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
 

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<usize> {
-            Ok(bytes.len())
-        }
-
-        fn flush(&mut self) -> io::Result<()> {
-            Ok(())
-        }
-    }
-
-    #[derive(Default)]
-    struct AttrHandler {
+    struct MockHandler {
+        index: CharsetIndex,
+        charset: StandardCharset,
         attr: Option<Attr>,
+        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<W: io::Write>(&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);
     }

alacritty_terminal/src/term/mod.rs

@@ -1397,6 +1397,7 @@ impl<T: EventListener> ansi::Handler for Term<T> {
 
     #[inline]
     fn identify_terminal<W: io::Write>(&mut self, writer: &mut W) {
+        trace!("Reporting terminal identity");
         let _ = writer.write_all(b"\x1b[?6c");
     }