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) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-04-24 13:44:54 +02:00
parent 31ed34a421
commit 06d2450459
2 changed files with 75 additions and 21 deletions

View file

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

View file

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