From 5bb3b15151aa7aa813cf3c2664d995310a92ca3b Mon Sep 17 00:00:00 2001 From: Noboru Saito Date: Thu, 14 Nov 2024 21:14:46 +0900 Subject: [PATCH] Improve escape sequence compatibility - Enhanced handling of invalid values in escape sequences to better match terminal and `less`` behavior. - Updated related functions to ensure compatibility. - Improved error handling and reporting for invalid escape sequences. --- oviewer/convert_es.go | 203 ++++++++++++++++++++++++------------- oviewer/convert_es_test.go | 66 ++++++++++-- oviewer/oviewer.go | 4 + 3 files changed, 194 insertions(+), 79 deletions(-) diff --git a/oviewer/convert_es.go b/oviewer/convert_es.go index 32402464..610840ca 100644 --- a/oviewer/convert_es.go +++ b/oviewer/convert_es.go @@ -1,6 +1,7 @@ package oviewer import ( + "errors" "fmt" "log" "strconv" @@ -77,7 +78,12 @@ func (es *escapeSequence) convert(st *parseState) bool { case ansiControlSequence: switch { case mainc == 'm': - st.style = csToStyle(st.style, es.parameter.String()) + style, err := csToStyle(st.style, es.parameter.String()) + if err != nil { + es.state = ansiText + return false + } + st.style = style case mainc == 'K': // CSI 0 K or CSI K maintains the style after the newline // (can change the background color of the line). @@ -90,6 +96,9 @@ func (es *escapeSequence) convert(st *parseState) bool { case mainc >= '0' && mainc <= 'f': es.parameter.WriteRune(mainc) return true + case mainc < 0x40: + es.parameter.WriteRune(mainc) + return true } es.state = ansiText return true @@ -161,128 +170,186 @@ func (es *escapeSequence) convert(st *parseState) bool { } // csToStyle returns tcell.Style from the control sequence. -func csToStyle(style tcell.Style, params string) tcell.Style { +func csToStyle(style tcell.Style, params string) (tcell.Style, error) { if params == "0" || params == "" || params == ";" { - return tcell.StyleDefault.Normal() + return tcell.StyleDefault.Normal(), nil } if s, ok := csiCache.Load(params); ok { style = applyStyle(style, s.(OVStyle)) - return style + return style, nil } - s := parseCSI(params) + s, err := parseCSI(params) + if err != nil { + return style, err + } csiCache.Store(params, s) - style = applyStyle(style, s) - return style + return applyStyle(style, s), nil } // parseCSI actually parses the style and returns ovStyle. -func parseCSI(params string) OVStyle { +func parseCSI(params string) (OVStyle, error) { s := OVStyle{} fields := strings.Split(params, ";") for index := 0; index < len(fields); index++ { field := fields[index] - switch field { - case "1", "01": + num, err := toESCode(field) + if err != nil { + if errors.Is(err, ErrNotSuuport) { + return s, nil + } + return s, err + } + switch num { + case 0: + s = OVStyle{} + case 1: s.Bold = true - case "2", "02": + case 2: s.Dim = true - case "3", "03": + case 3: s.Italic = true - case "4", "04": + case 4: s.Underline = true - case "5", "05": + case 5: s.Blink = true - case "6", "06": + case 6: s.Blink = true - case "7", "07": + case 7: s.Reverse = true - case "8", "08": + case 8: // Invisible On (not implemented) - case "9", "09": + case 9: s.StrikeThrough = true - case "22": + case 22: s.UnBold = true - case "23": + case 23: s.UnItalic = true - case "24": + case 24: s.UnUnderline = true - case "25": + case 25: s.UnBlink = true - case "27": + case 27: s.UnReverse = true - case "28": + case 28: // Invisible Off (not implemented) - case "29": + case 29: s.UnStrikeThrough = true - case "30", "31", "32", "33", "34", "35", "36", "37": - colorNumber, _ := strconv.Atoi(field) - s.Foreground = colorName(colorNumber - 30) - case "38": + case 30, 31, 32, 33, 34, 35, 36, 37: + s.Foreground = colorName(num - 30) + case 38: i, color := csColor(fields[index:]) + if i == 0 { + return s, nil + } index += i s.Foreground = color - case "39": + case 39: s.Foreground = "default" - case "40", "41", "42", "43", "44", "45", "46", "47": - colorNumber, _ := strconv.Atoi(field) - s.Background = colorName(colorNumber - 40) - case "48": + case 40, 41, 42, 43, 44, 45, 46, 47: + s.Background = colorName(num - 40) + case 48: i, color := csColor(fields[index:]) + if i == 0 { + return s, nil + } index += i s.Background = color - case "49": + case 49: s.Background = "default" - case "53": + case 53: s.OverLine = true - case "55": + case 55: s.UnOverLine = true - case "90", "91", "92", "93", "94", "95", "96", "97": - colorNumber, _ := strconv.Atoi(field) - s.Foreground = colorName(colorNumber - 82) - case "100", "101", "102", "103", "104", "105", "106", "107": - colorNumber, _ := strconv.Atoi(field) - s.Background = colorName(colorNumber - 92) + case 90, 91, 92, 93, 94, 95, 96, 97: + s.Foreground = colorName(num - 82) + case 100, 101, 102, 103, 104, 105, 106, 107: + s.Background = colorName(num - 92) } } - return s + return s, nil +} + +// toESCode converts a string to an integer. +// If the code is smaller than 0x40 for compatibility, +// return -1 instead of an error. +func toESCode(str string) (int, error) { + num, err := strconv.Atoi(str) + if err != nil { + for _, char := range str { + if char >= 0x40 { + return 0, ErrInvalidCSI + } + } + return 0, ErrNotSuuport + } + return num, nil } // csColor parses 8-bit color and 24-bit color. func csColor(fields []string) (int, string) { if len(fields) < 2 { + return 0, "" + } + if fields[1] == "" { return 1, "" } + ex, err := strconv.Atoi(fields[1]) + if err != nil { + return 1, "" + } + switch ex { + case 5: // 8-bit colors. + if len(fields) < 3 { + return 1, "" + } + if fields[2] == "" { + return len(fields), "" + } - index := 1 - color := "default" - switch fields[1] { - case "5": // 8-bit colors. - if len(fields) > index+1 { - eColor := fields[2] - if eColor == "" { - return index, color - } - c, err := strconv.Atoi(eColor) - if err != nil { - return index, color + color, err := parse8BitColor(fields[2]) + if err != nil { + return 0, color + } + return 2, color + case 2: // 24-bit colors. + if len(fields) < 5 { + return len(fields), "" + } + for i := 2; i < 5; i++ { + if fields[i] == "" { + return i, "" } - index++ - color = colorName(c) } - case "2": // 24-bit colors. - if len(fields) <= index+3 { - index += len(fields) // Skip the Invalid fields. - return index, color + + color, err := parseRGBColor(fields[2:5]) + if err != nil { + return 0, color } - red, _ := strconv.Atoi(fields[2]) - green, _ := strconv.Atoi(fields[3]) - blue, _ := strconv.Atoi(fields[4]) - index += 3 - color = fmt.Sprintf("#%02x%02x%02x", red, green, blue) + return 4, color + } + return 1, "" +} + +func parse8BitColor(field string) (string, error) { + c, err := strconv.Atoi(field) + if err != nil { + return "", fmt.Errorf("invalid 8-bit color value: %v", field) + } + color := colorName(c) + return color, nil +} + +func parseRGBColor(fields []string) (string, error) { + red, err1 := strconv.Atoi(fields[0]) + green, err2 := strconv.Atoi(fields[1]) + blue, err3 := strconv.Atoi(fields[2]) + if err1 != nil || err2 != nil || err3 != nil { + return "", fmt.Errorf("invalid RGB color values: %v, %v, %v", fields[0], fields[1], fields[2]) } - return index, color + color := fmt.Sprintf("#%02x%02x%02x", red, green, blue) + return color, nil } // colorName returns a string that can be used to specify the color of tcell. diff --git a/oviewer/convert_es_test.go b/oviewer/convert_es_test.go index d7f8b483..0037fc3a 100644 --- a/oviewer/convert_es_test.go +++ b/oviewer/convert_es_test.go @@ -173,9 +173,10 @@ func Test_csToStyle(t *testing.T) { csiParameter string } tests := []struct { - name string - args args - want tcell.Style + name string + args args + want tcell.Style + wantErr bool }{ { name: "color8bit", @@ -183,7 +184,8 @@ func Test_csToStyle(t *testing.T) { style: tcell.StyleDefault, csiParameter: "38;5;1", }, - want: tcell.StyleDefault.Foreground(tcell.ColorMaroon), + want: tcell.StyleDefault.Foreground(tcell.ColorMaroon), + wantErr: false, }, { name: "color8bit2", @@ -191,7 +193,34 @@ func Test_csToStyle(t *testing.T) { style: tcell.StyleDefault, csiParameter: "38;5;21", }, - want: tcell.StyleDefault.Foreground(tcell.GetColor("#0000ff")), + want: tcell.StyleDefault.Foreground(tcell.GetColor("#0000ff")), + wantErr: false, + }, + { + name: "colorTrueColor", + args: args{ + style: tcell.StyleDefault, + csiParameter: "38;2;255;0;0", + }, + want: tcell.StyleDefault.Foreground(tcell.GetColor("#FF0000")), + wantErr: false, + }, + { + name: "colorTrueColorInvalid", + args: args{ + style: tcell.StyleDefault, + csiParameter: "38;2;255;", + }, + want: tcell.StyleDefault, + }, + { + name: "colorTrueColorInvalid2", + args: args{ + style: tcell.StyleDefault, + csiParameter: "38;2;a;b;c", + }, + want: tcell.StyleDefault, + wantErr: false, }, { name: "attributes", @@ -199,14 +228,19 @@ func Test_csToStyle(t *testing.T) { style: tcell.StyleDefault, csiParameter: "2;3;4;5;6;7;8;9", }, - want: tcell.StyleDefault.Dim(true).Italic(true).Underline(true).Blink(true).Reverse(true).StrikeThrough(true), + want: tcell.StyleDefault.Dim(true).Italic(true).Underline(true).Blink(true).Reverse(true).StrikeThrough(true), + wantErr: false, }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := csToStyle(tt.args.style, tt.args.csiParameter); !reflect.DeepEqual(got, tt.want) { + got, err := csToStyle(tt.args.style, tt.args.csiParameter) + if (err != nil) != tt.wantErr { + t.Errorf("csToStyle() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(got, tt.want) { t.Errorf("csToStyle() = %v, want %v", got, tt.want) gfg, gbg, gattr := got.Decompose() wfg, wbg, wattr := tt.want.Decompose() @@ -221,9 +255,10 @@ func Test_parseCSI(t *testing.T) { params string } tests := []struct { - name string - args args - want OVStyle + name string + args args + want OVStyle + wantErr bool }{ { name: "test-attributes", @@ -238,6 +273,7 @@ func Test_parseCSI(t *testing.T) { Reverse: true, StrikeThrough: true, }, + wantErr: false, }, { name: "test-attributesErr", @@ -252,6 +288,7 @@ func Test_parseCSI(t *testing.T) { Reverse: false, StrikeThrough: false, }, + wantErr: false, }, { name: "test-attributesNone", @@ -266,6 +303,7 @@ func Test_parseCSI(t *testing.T) { Reverse: false, StrikeThrough: false, }, + wantErr: false, }, { name: "test-Default", @@ -281,6 +319,7 @@ func Test_parseCSI(t *testing.T) { Reverse: false, StrikeThrough: false, }, + wantErr: false, }, { name: "test-forground2", @@ -299,11 +338,16 @@ func Test_parseCSI(t *testing.T) { want: OVStyle{ Foreground: "#FFAF87", }, + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := parseCSI(tt.args.params); !reflect.DeepEqual(got, tt.want) { + got, err := parseCSI(tt.args.params) + if (err != nil) != tt.wantErr { + t.Errorf("parseCSI() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(got, tt.want) { t.Errorf("parseCSI() = %v, want %v", got, tt.want) } }) diff --git a/oviewer/oviewer.go b/oviewer/oviewer.go index f6f8a8eb..21efa62a 100644 --- a/oviewer/oviewer.go +++ b/oviewer/oviewer.go @@ -426,6 +426,10 @@ var ( ErrNotAlignMode = errors.New("not an align mode") // ErrNoColumnSelected indicates that no column is selected. ErrNoColumnSelected = errors.New("no column selected") + // ErrInvalidCSI indicates that the CSI is invalid. + ErrInvalidCSI = errors.New("invalid CSI") + // ErrNotSuuport indicates that it is not supported. + ErrNotSuuport = errors.New("not support") ) // This is a function of tcell.NewScreen but can be replaced with mock.