From 8a8dacca14234ba9cb730c5cdedd2a2225d3e924 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 22 Apr 2026 13:37:38 +0200 Subject: [PATCH 1/3] Demonstrate that unknown escape sequences leak as literal text The escape interpreter errors on anything outside the handful of sequences it understands (SGR, EL, OSC 8 hyperlinks), and view.go then renders the unparsed bytes as text cells. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/escape_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/pkg/gocui/escape_test.go b/pkg/gocui/escape_test.go index 382d27bad..3d9ab5b36 100644 --- a/pkg/gocui/escape_test.go +++ b/pkg/gocui/escape_test.go @@ -150,6 +150,29 @@ func TestParseOneColours(t *testing.T) { } } +func TestParseOneIgnoresUnknownSequences(t *testing.T) { + // These are the kinds of sequences ConPTY emits as session-init on Windows. A text-mode + // interpreter can't do anything meaningful with them, but it must silently consume them + // instead of leaking them into the view as literal text. + scenarios := []string{ + "\x1b[?9001h", // DEC private-mode set (?-prefix) + "\x1b[?25l", // hide cursor + "\x1b[?25h", // show cursor + "\x1b[2;J", // erase display (unusual 2;J variant) + "\x1b[H", // cursor home — final byte immediately after [ + "\x1b[5;1;H", // cursor position with multiple params + "\x1bc", // RIS — single-char ESC sequence + } + + for _, input := range scenarios { + ei := newEscapeInterpreter(OutputNormal) + /* EXPECTED: + parseEscRunes(t, ei, input) + ACTUAL: */ + parseEscRunesExpectingError(t, ei, input) + } +} + func parseEscRunes(t *testing.T, ei *escapeInterpreter, runes string) { t.Helper() for _, b := range []byte(runes) { @@ -158,3 +181,13 @@ func parseEscRunes(t *testing.T, ei *escapeInterpreter, runes string) { assert.NoError(t, err) } } + +func parseEscRunesExpectingError(t *testing.T, ei *escapeInterpreter, runes string) { + t.Helper() + for _, b := range []byte(runes) { + if _, err := ei.parseOne([]byte{b}); err != nil { + return + } + } + t.Errorf("expected a parse error for %q, got none", runes) +} From 31ed34a4214adbb68e5612063b036eebeb97de86 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 24 Apr 2026 12:09:10 +0200 Subject: [PATCH 2/3] Silently consume unrecognized escape sequences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A text-mode escape interpreter can't do anything meaningful with cursor positioning, DEC private modes, or terminal resets — but it must still consume them, not print them as literal text. Before this change, any sequence outside SGR / EL / OSC-8 errored out of parseOne, and view.go rendered the unparsed bytes as visible cells. On Windows this would show up as junk at the start of main-panel output once we add PTY support using ConPTY, because ConPTY's session-init stream is full of such sequences. Three additions to the state machine: - stateEscape: a single byte in 0x30–0x7E after ESC (e.g. ESC c = RIS) is a complete Fs/Fp sequence per ECMA-48; consume and reset. - stateCSI: accept the DEC private-mode prefix bytes (<, =, >, ?), and accept a CSI final byte (0x40–0x7E) immediately after [ as the end of a zero-param sequence. - stateParams: accept any CSI final byte we don't implement as the end of the sequence rather than a parse error. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/escape.go | 25 +++++++++++++++++++++++++ pkg/gocui/escape_test.go | 13 ------------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index cb557f088..da55830aa 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -151,6 +151,12 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { characterEquals(ch, '+'): ei.state = stateCharacterSetDesignation return true, nil + case len(ch) == 1 && ch[0] >= 0x30 && ch[0] <= 0x7E: + // Single-byte ESC sequence (e.g. ESC c = RIS). We don't + // interpret these, but we must consume them so they don't + // leak into the view as literal text. + ei.state = stateNone + return true, nil default: return false, errNotCSI } @@ -166,6 +172,19 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.csiParam = append(ei.csiParam, "0") case characterEquals(ch, 'K'): // fall through + case len(ch) == 1 && ch[0] >= 0x3C && ch[0] <= 0x3F: + // Private-mode prefix byte (<, =, >, ?). We don't interpret + // DEC private-mode sequences, but must consume them so they + // don't leak into the view as literal text. Seed an empty + // param so the subsequent digits land on a valid slot. + ei.csiParam = append(ei.csiParam, "") + ei.state = stateParams + return true, nil + case len(ch) == 1 && ch[0] >= 0x40 && ch[0] <= 0x7E: + // Valid CSI final byte we don't implement — swallow. + ei.state = stateNone + ei.csiParam = nil + return true, nil default: return false, errCSIParseError } @@ -203,6 +222,12 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.instruction = noInstruction{} } + ei.state = stateNone + ei.csiParam = nil + return true, nil + case len(ch) == 1 && ch[0] >= 0x40 && ch[0] <= 0x7E: + // Valid CSI final byte we don't implement — swallow the + // whole sequence rather than printing it as text. ei.state = stateNone ei.csiParam = nil return true, nil diff --git a/pkg/gocui/escape_test.go b/pkg/gocui/escape_test.go index 3d9ab5b36..d41f009d2 100644 --- a/pkg/gocui/escape_test.go +++ b/pkg/gocui/escape_test.go @@ -166,10 +166,7 @@ func TestParseOneIgnoresUnknownSequences(t *testing.T) { for _, input := range scenarios { ei := newEscapeInterpreter(OutputNormal) - /* EXPECTED: parseEscRunes(t, ei, input) - ACTUAL: */ - parseEscRunesExpectingError(t, ei, input) } } @@ -181,13 +178,3 @@ func parseEscRunes(t *testing.T, ei *escapeInterpreter, runes string) { assert.NoError(t, err) } } - -func parseEscRunesExpectingError(t *testing.T, ei *escapeInterpreter, runes string) { - t.Helper() - for _, b := range []byte(runes) { - if _, err := ei.parseOne([]byte{b}); err != nil { - return - } - } - t.Errorf("expected a parse error for %q, got none", runes) -} From 06d2450459013498c06e48d9395713f0195b9510 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 24 Apr 2026 13:44:54 +0200 Subject: [PATCH 3/3] Stop leaking other malformed and unimplemented escape sequences After the previous commit, the escape interpreter still had five paths that returned an error from parseOne, which view.go handles by rendering whatever bytes it had accumulated as literal cells. Each of these is a case where silently consuming the sequence is strictly better than leaking garbage. - ';' as the first CSI byte: '\x1b[;5H' is a valid sequence (row defaults to 1) but we errored on the leading ';'. - Intermediate bytes in CSI ('\x1b[0 q' = DECSCUSR): the sequence ends in a final byte we don't implement, so consume and drop. - Malformed SGR params (empty slot like '\x1b[1;;m'): if outputCSI fails mid-parse, reset state instead of re-emitting the sequence. - OSC 8 that isn't actually OSC 8 ('\x1b]8x...'): treat as an OSC we don't understand and skip to its terminator rather than error- resetting mid-sequence, which used to leave the rest of the OSC body to be printed as text. - The sanity-check overflow paths (too many params, param too long) now switch to a 'discard until final byte' state rather than returning the accumulated bytes. A new stateCSIDiscard centralizes the 'consume bytes until the CSI final' behavior used by both the intermediate-byte and overflow paths. errCSITooLong and errOSCParseError are gone with their only callers. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/gocui/escape.go | 62 +++++++++++++++++++++++++++++++++------- pkg/gocui/escape_test.go | 34 +++++++++++++++------- 2 files changed, 75 insertions(+), 21 deletions(-) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index da55830aa..726fd4de7 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -42,6 +42,7 @@ const ( stateCharacterSetDesignation stateCSI stateParams + stateCSIDiscard stateOSC stateOSCWaitForParams stateOSCParams @@ -66,8 +67,6 @@ const ( var ( errNotCSI = errors.New("Not a CSI escape sequence") errCSIParseError = errors.New("CSI escape sequence parsing error") - errCSITooLong = errors.New("CSI escape sequence is too long") - errOSCParseError = errors.New("OSC escape sequence parsing error") ) // characters in case of error will output the non-parsed characters as a string. @@ -120,12 +119,13 @@ func (ei *escapeInterpreter) instructionRead() { // is part of an escape sequence, and as such should not be printed verbatim. Otherwise, it's not an // escape sequence. func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { - // Sanity checks - if len(ei.csiParam) > 20 { - return false, errCSITooLong - } - if len(ei.csiParam) > 0 && len(ei.csiParam[len(ei.csiParam)-1]) > 255 { - return false, errCSITooLong + // Sanity checks: if a sequence has grown absurdly long, stop + // accumulating state and just swallow bytes until its final byte — + // much better than leaking the accumulated garbage into the view. + if len(ei.csiParam) > 20 || (len(ei.csiParam) > 0 && len(ei.csiParam[len(ei.csiParam)-1]) > 255) { + ei.state = stateCSIDiscard + ei.csiParam = nil + return true, nil } ei.curch = string(ch) @@ -172,6 +172,11 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.csiParam = append(ei.csiParam, "0") case characterEquals(ch, 'K'): // fall through + case characterEquals(ch, ';'): + // Empty first param ([;Xm ≡ [0;Xm). Seed a slot for the + // empty param; stateParams will append the next one when it + // re-reads this ';' via the fallthrough. + ei.csiParam = append(ei.csiParam, "") case len(ch) == 1 && ch[0] >= 0x3C && ch[0] <= 0x3F: // Private-mode prefix byte (<, =, >, ?). We don't interpret // DEC private-mode sequences, but must consume them so they @@ -180,6 +185,13 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.csiParam = append(ei.csiParam, "") ei.state = stateParams return true, nil + case len(ch) == 1 && ch[0] >= 0x20 && ch[0] <= 0x2F: + // CSI intermediate byte. A sequence with intermediates is + // one we don't implement; consume the rest until the final + // byte. + ei.state = stateCSIDiscard + ei.csiParam = nil + return true, nil case len(ch) == 1 && ch[0] >= 0x40 && ch[0] <= 0x7E: // Valid CSI final byte we don't implement — swallow. ei.state = stateNone @@ -199,10 +211,16 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.csiParam = append(ei.csiParam, "") return true, nil case characterEquals(ch, 'm'): + // outputCSI applies params left-to-right and mutates as it + // goes, so on failure some leading params may already have + // taken effect (e.g. `[1;;m` would leave AttrBold set before + // hitting the empty param). Snapshot the colors beforehand + // and restore them on error so a malformed SGR is truly a + // no-op rather than a partial apply. + savedFg, savedBg := ei.curFgColor, ei.curBgColor if err := ei.outputCSI(); err != nil { - return false, errCSIParseError + ei.curFgColor, ei.curBgColor = savedFg, savedBg } - ei.state = stateNone ei.csiParam = nil return true, nil @@ -225,6 +243,13 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.state = stateNone ei.csiParam = nil return true, nil + case len(ch) == 1 && ch[0] >= 0x20 && ch[0] <= 0x2F: + // CSI intermediate byte after params. The final byte will + // have a semantic we don't implement (e.g. `[0 q` = + // DECSCUSR); consume everything until it arrives. + ei.state = stateCSIDiscard + ei.csiParam = nil + return true, nil case len(ch) == 1 && ch[0] >= 0x40 && ch[0] <= 0x7E: // Valid CSI final byte we don't implement — swallow the // whole sequence rather than printing it as text. @@ -234,6 +259,15 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { default: return false, errCSIParseError } + case stateCSIDiscard: + // Consume the rest of a CSI sequence whose semantic we don't + // interpret (one with intermediate bytes, or one the sanity + // checks at the top of parseOne bailed out of). Any byte in the + // final-byte range ends it. + if len(ch) == 1 && ch[0] >= 0x40 && ch[0] <= 0x7E { + ei.state = stateNone + } + return true, nil case stateOSC: if characterEquals(ch, '8') { ei.state = stateOSCWaitForParams @@ -245,7 +279,13 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { return true, nil case stateOSCWaitForParams: if !characterEquals(ch, ';') { - return true, errOSCParseError + // Malformed OSC 8 (expected ';' after '8'). Rather than + // erroring — which would reset state mid-OSC and cause the + // rest of the sequence to leak as literal text — treat the + // whole OSC as one we don't understand and skip to its + // terminator. + ei.state = stateOSCSkipUnknown + return true, nil } ei.state = stateOSCParams diff --git a/pkg/gocui/escape_test.go b/pkg/gocui/escape_test.go index d41f009d2..cb8eb1a4b 100644 --- a/pkg/gocui/escape_test.go +++ b/pkg/gocui/escape_test.go @@ -1,6 +1,7 @@ package gocui import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -151,22 +152,35 @@ func TestParseOneColours(t *testing.T) { } func TestParseOneIgnoresUnknownSequences(t *testing.T) { - // These are the kinds of sequences ConPTY emits as session-init on Windows. A text-mode - // interpreter can't do anything meaningful with them, but it must silently consume them - // instead of leaking them into the view as literal text. + // Escape sequences the interpreter doesn't implement -- whether well-formed-but-unsupported + // (cursor movement, private modes, DECSCUSR, …) or outright malformed -- must be silently + // consumed rather than leaked into the view as literal text. scenarios := []string{ - "\x1b[?9001h", // DEC private-mode set (?-prefix) - "\x1b[?25l", // hide cursor - "\x1b[?25h", // show cursor - "\x1b[2;J", // erase display (unusual 2;J variant) - "\x1b[H", // cursor home — final byte immediately after [ - "\x1b[5;1;H", // cursor position with multiple params - "\x1bc", // RIS — single-char ESC sequence + "\x1b[?9001h", // DEC private-mode set (?-prefix) + "\x1b[?25l", // hide cursor + "\x1b[?25h", // show cursor + "\x1b[2;J", // erase display (unusual 2;J variant) + "\x1b[H", // cursor home — final byte immediately after [ + "\x1b[5;1;H", // cursor position with multiple params + "\x1bc", // RIS — single-char ESC sequence + "\x1b[;5H", // empty first param (';' immediately after '[') + "\x1b[ q", // intermediate byte with no params (DECSCUSR family) + "\x1b[0 q", // intermediate byte after a param + "\x1b[1;;m", // malformed SGR: empty middle param + "\x1b]8bogus\x07", // OSC 8 missing ';' + "\x1b[" + strings.Repeat("0", 300) + "m", // single param overflows length cap + "\x1b[" + strings.Repeat("1;", 25) + "1m", // too many params } for _, input := range scenarios { ei := newEscapeInterpreter(OutputNormal) parseEscRunes(t, ei, input) + // An unimplemented/malformed sequence must leave no trace: no + // pending instruction, no color change. + _, noop := ei.instruction.(noInstruction) + assert.True(t, noop, "input %q left a pending instruction", input) + assert.Equal(t, ColorDefault, ei.curFgColor, "input %q mutated fg color", input) + assert.Equal(t, ColorDefault, ei.curBgColor, "input %q mutated bg color", input) } }