Check escape sequence bytes before counting them (#4929)

The parser decided how many bytes a key used before checking them, so a
key that arrived in the same read was lost or misread.

- Recognize F1-F8 and modified Delete, PageUp and PageDown when more input
  follows them, as with a paste or tmux send-keys. They required the
  sequence to be the whole buffer.
- After \e[ and a digit, do not consume a byte that cannot continue the
  sequence. ALT-[, 2 and Enter lost the Enter.
- Report Home and End for \e[7, \e[4 and \e[8 only before ~, $, ^ or @.
  ALT-[, 4 and x fired End and lost the x.
- Look for the end of a mouse report within its frame. A later M swallowed
  the keys before it, and repeated \e[< took quadratic time.

CHANGELOG.md

@@ -5,6 +5,7 @@ CHANGELOG
 ------
 - Fixed `--gap-line` cutting a grapheme cluster when filling the last cells of the line (#4920)
 - Fixed an escape sequence fzf does not recognize being partly consumed, which leaked the rest into the query (#4926)
+- Fixed a key lost when it arrived in the same read as an escape sequence
 
 0.74.4
 ------

src/tui/light.go

@@ -400,6 +400,12 @@ func stringEnd(buffer []byte) int {
 	return 0
 }
 
+// rxvtKeyEnd reports whether the byte ends a key such as \e[7~ for Home.
+// rxvt marks modifiers with the final byte: $ for SHIFT, ^ for CTRL, @ for both.
+func rxvtKeyEnd(b byte) bool {
+	return b == '~' || b == '$' || b == '^' || b == '@'
+}
+
 // csiIntroducer reports whether the byte after ESC starts a sequence that is
 // framed by a final byte. The parser routes CSI and SS3 through the same
 // parameterized subcases, so both are framed by csiEnd.
@@ -747,6 +753,11 @@ func (r *LightRenderer) parseEscSequence(sz *int) Event {
 			if len(r.buffer) < 4 {
 				return Event{Invalid, 0, nil}
 			}
+			// A byte that cannot continue the sequence, such as Enter after
+			// ALT-[ and a digit, is a key of its own
+			if csiEnd(r.buffer) < 0 {
+				break
+			}
 			*sz = 4
 			switch r.buffer[2] {
 			case '2':
@@ -779,7 +790,7 @@ func (r *LightRenderer) parseEscSequence(sz *int) Event {
 				if r.buffer[3] == '~' {
 					return Event{Delete, 0, nil}
 				}
-				if len(r.buffer) == 7 && r.buffer[6] == '~' && r.buffer[4] == '1' {
+				if len(r.buffer) > 6 && r.buffer[6] == '~' && r.buffer[4] == '1' {
 					*sz = 7
 					switch r.buffer[5] {
 					case '0':
@@ -798,7 +809,7 @@ func (r *LightRenderer) parseEscSequence(sz *int) Event {
 						return Event{CtrlAltShiftDelete, 0, nil}
 					}
 				}
-				if len(r.buffer) == 6 && r.buffer[5] == '~' {
+				if len(r.buffer) > 5 && r.buffer[5] == '~' {
 					*sz = 6
 					switch r.buffer[4] {
 					case '2':
@@ -821,12 +832,14 @@ func (r *LightRenderer) parseEscSequence(sz *int) Event {
 				}
 				return Event{Invalid, 0, nil}
 			case '4':
-				return Event{End, 0, nil}
+				if rxvtKeyEnd(r.buffer[3]) {
+					return Event{End, 0, nil}
+				}
 			case '5':
 				if r.buffer[3] == '~' {
 					return Event{PageUp, 0, nil}
 				}
-				if len(r.buffer) == 7 && r.buffer[6] == '~' && r.buffer[4] == '1' {
+				if len(r.buffer) > 6 && r.buffer[6] == '~' && r.buffer[4] == '1' {
 					*sz = 7
 					switch r.buffer[5] {
 					case '0':
@@ -845,7 +858,7 @@ func (r *LightRenderer) parseEscSequence(sz *int) Event {
 						return Event{CtrlAltShiftPageUp, 0, nil}
 					}
 				}
-				if len(r.buffer) == 6 && r.buffer[5] == '~' {
+				if len(r.buffer) > 5 && r.buffer[5] == '~' {
 					*sz = 6
 					switch r.buffer[4] {
 					case '2':
@@ -871,7 +884,7 @@ func (r *LightRenderer) parseEscSequence(sz *int) Event {
 				if r.buffer[3] == '~' {
 					return Event{PageDown, 0, nil}
 				}
-				if len(r.buffer) == 7 && r.buffer[6] == '~' && r.buffer[4] == '1' {
+				if len(r.buffer) > 6 && r.buffer[6] == '~' && r.buffer[4] == '1' {
 					*sz = 7
 					switch r.buffer[5] {
 					case '0':
@@ -890,7 +903,7 @@ func (r *LightRenderer) parseEscSequence(sz *int) Event {
 						return Event{CtrlAltShiftPageDown, 0, nil}
 					}
 				}
-				if len(r.buffer) == 6 && r.buffer[5] == '~' {
+				if len(r.buffer) > 5 && r.buffer[5] == '~' {
 					*sz = 6
 					switch r.buffer[4] {
 					case '2':
@@ -913,15 +926,19 @@ func (r *LightRenderer) parseEscSequence(sz *int) Event {
 				}
 				return Event{Invalid, 0, nil}
 			case '7':
-				return Event{Home, 0, nil}
+				if rxvtKeyEnd(r.buffer[3]) {
+					return Event{Home, 0, nil}
+				}
 			case '8':
-				return Event{End, 0, nil}
+				if rxvtKeyEnd(r.buffer[3]) {
+					return Event{End, 0, nil}
+				}
 			case '1':
 				switch r.buffer[3] {
 				case '~':
 					return Event{Home, 0, nil}
 				case '1', '2', '3', '4', '5', '7', '8', '9':
-					if len(r.buffer) == 5 && r.buffer[4] == '~' {
+					if len(r.buffer) > 4 && r.buffer[4] == '~' {
 						*sz = 5
 						switch r.buffer[3] {
 						case '1':
@@ -1162,11 +1179,13 @@ func (r *LightRenderer) mouseSequence(sz *int) Event {
 		return Event{Invalid, 0, nil}
 	}
 
-	rest := r.buffer[*sz:]
-	end := bytes.IndexAny(rest, "mM")
-	if end == -1 {
+	// Not past the frame, where M or m can be a key typed later
+	frame := csiEnd(r.buffer)
+	if frame <= 0 || r.buffer[frame-1] != 'm' && r.buffer[frame-1] != 'M' {
 		return Event{Invalid, 0, nil}
 	}
+	rest := r.buffer[*sz:]
+	end := frame - 1 - *sz
 
 	elems := strings.SplitN(string(rest[:end]), ";", 3)
 	if len(elems) != 3 {

src/tui/light_csi_test.go

@@ -231,3 +231,44 @@ func TestCsiEnd(t *testing.T) {
 		}
 	}
 }
+
+// A key that follows a sequence in the same read must not be consumed with it
+func TestKeyAfterSequence(t *testing.T) {
+	for _, c := range []struct {
+		sequence string
+		mouse    bool
+		event    EventType
+		size     int
+	}{
+		// Recognized when more input follows, as from a paste or tmux send-keys
+		{"\x1b[15~x", false, F5, 5},
+		{"\x1b[3;5~x", false, CtrlDelete, 6},
+		{"\x1b[5;13~x", false, CtrlAltPageUp, 7},
+		{"\x1b[6;2~\x1b[A", false, ShiftPageDown, 6},
+
+		// ALT-[ or ALT-O and a digit, then a key that cannot continue a sequence
+		{"\x1b[2\r", false, Alt, 2},
+		{"\x1b[12\x03", false, Alt, 2},
+		{"\x1bO2\a", false, Alt, 2},
+		{"\x1b[4x", false, Alt, 2},  // not End
+		{"\x1b[7\r", false, Alt, 2}, // not Home
+		{"\x1b[7~x", false, Home, 4},
+		{"\x1b[8^x", false, End, 4}, // rxvt CTRL-END
+
+		// A mouse report ends at its own final byte
+		{"\x1b[<0;5;3Mx", true, Mouse, 9},
+		{"\x1b[<0;1;\x1b[AM", true, Invalid, 3},
+		{"\x1b[<0;1;1xM", true, Invalid, 9},
+	} {
+		r := &LightRenderer{buffer: []byte(c.sequence), mouse: c.mouse}
+		sz := 1
+		event := r.escSequence(&sz)
+		if event.Type != c.event {
+			t.Errorf("escSequence(%q) = %s, want %s",
+				c.sequence, event.Type.String(), c.event.String())
+		}
+		if sz != c.size {
+			t.Errorf("escSequence(%q) consumed %d bytes, want %d", c.sequence, sz, c.size)
+		}
+	}
+}

src/tui/light_test.go

@@ -66,7 +66,6 @@ func TestLightRenderer(t *testing.T) {
 	assertEscSequence("\x1b[1;1R", "!Invalid")
 	assertEscSequence("\x1b[", "!Invalid")
 	assertEscSequence("\x1b[1", "!Invalid")
-	assertEscSequence("\x1b[3;3~1", "!Invalid")
 	assertEscSequence("\x1b[13", "!Invalid")
 	assertEscSequence("\x1b[1;3", "!Invalid")
 	assertEscSequence("\x1b[1;10", "!Invalid")
@@ -200,6 +199,7 @@ func TestLightRenderer(t *testing.T) {
 	assertEscSequence("\x1b[1;3H", "alt-home")
 	assertEscSequence("\x1b[1;3F", "alt-end")
 	assertEscSequence("\x1b[3;3~", "alt-delete")
+	assertEscSequence("\x1b[3;3~1", "alt-delete") // then 1
 	assertEscSequence("\x1b[5;3~", "alt-page-up")
 	assertEscSequence("\x1b[6;3~", "alt-page-down")