From 78435f2d71c2cdab7297052d5de2fa110db4baad Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Tue, 19 Aug 2025 09:32:27 -0700 Subject: [PATCH 1/3] Strip out BELL/ALERT sequence --- helpers_windows.go | 9 +++++---- helpers_windows_test.go | 7 +++++++ termtest_windows.go | 3 +++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/helpers_windows.go b/helpers_windows.go index b466ff5..cfb651e 100644 --- a/helpers_windows.go +++ b/helpers_windows.go @@ -2,12 +2,8 @@ package termtest import ( "bytes" - - "golang.org/x/sys/windows" ) -var ERR_ACCESS_DENIED = windows.ERROR_ACCESS_DENIED - const UnicodeEscapeRune = '\u001B' const UnicodeBellRune = '\u0007' const UnicodeBackspaceRune = '\u0008' // Note in the docs this is \u007f, but in actual use we're seeing \u0008. Possibly badly documented. @@ -51,6 +47,11 @@ func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) ([]byte, int switch { // SEQUENCE START + // Delete alert / bell sequence + case !inEscapeSequence && r == '\a': + dropPos(pos) + continue + // Detect start of escape sequence case !inEscapeSequence && r == UnicodeEscapeRune: inEscapeSequence = true diff --git a/helpers_windows_test.go b/helpers_windows_test.go index 851e9f2..f395a00 100644 --- a/helpers_windows_test.go +++ b/helpers_windows_test.go @@ -116,6 +116,13 @@ func Test_cleanPtySequences(t *testing.T) { []byte("foo"), -1, }, + { + "Alert / bell character", + []byte("\aP\x1b[?25lython 3.9.5"), + 0, + []byte("Python 3.9.5"), + -1, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/termtest_windows.go b/termtest_windows.go index 68ca81d..5831f7c 100644 --- a/termtest_windows.go +++ b/termtest_windows.go @@ -7,8 +7,11 @@ import ( "time" gopsutil "github.com/shirou/gopsutil/v3/process" + "golang.org/x/sys/windows" ) +var ERR_ACCESS_DENIED = windows.ERROR_ACCESS_DENIED + func syscallErrorCode(err error) int { if errv, ok := err.(syscall.Errno); ok { return int(errv) From ed1ac05ea3842fb671628c1ffb987959af634c8f Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Tue, 19 Aug 2025 12:01:38 -0700 Subject: [PATCH 2/3] Handle cleanup of output regardless of final line ending --- helpers_unix.go | 4 +- helpers_windows.go | 22 +++++-- helpers_windows_test.go | 7 +++ outputproducer.go | 54 ++++++----------- outputproducer_test.go | 131 +++++++--------------------------------- 5 files changed, 69 insertions(+), 149 deletions(-) diff --git a/helpers_unix.go b/helpers_unix.go index 5034992..d5e251a 100644 --- a/helpers_unix.go +++ b/helpers_unix.go @@ -9,6 +9,6 @@ import ( var ERR_ACCESS_DENIED = errors.New("only used on windows, this should never match") -func cleanPtySnapshot(b []byte, cursorPos int, _ bool) ([]byte, int) { - return b, cursorPos +func cleanPtySnapshot(b []byte, cursorPos int, _ bool) ([]byte, int, int) { + return b, cursorPos, len(b) } diff --git a/helpers_windows.go b/helpers_windows.go index cfb651e..17a9f7d 100644 --- a/helpers_windows.go +++ b/helpers_windows.go @@ -12,9 +12,9 @@ const UnicodeBackspaceRune = '\u0008' // Note in the docs this is \u007f, but in // Ultimately we want to emulate the windows console here, just like we're doing for v10x on posix. // The current implementation is geared towards our needs, and won't be able to handle all escape sequences as a result. // For details on escape sequences see https://learn.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences -func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) ([]byte, int) { +func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) (_output []byte, _cursorPos int, _cleanUptoPos int) { if isPosix { - return snapshot, cursorPos + return snapshot, cursorPos, len(snapshot) } // Most escape sequences appear to end on `A-Za-z@` @@ -37,7 +37,10 @@ func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) ([]byte, int } var result []rune + var unterminatedEscape []rune runes := bytes.Runes(snapshot) + escapeStartPos := -1 + for pos, r := range runes { // Reset code recording outside of escape sequence, so we don't have to manually handle this throughout if !inEscapeSequence { @@ -48,7 +51,7 @@ func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) ([]byte, int // SEQUENCE START // Delete alert / bell sequence - case !inEscapeSequence && r == '\a': + case !inEscapeSequence && r == UnicodeBellRune: dropPos(pos) continue @@ -56,6 +59,7 @@ func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) ([]byte, int case !inEscapeSequence && r == UnicodeEscapeRune: inEscapeSequence = true recordingCode = true + escapeStartPos = pos dropPos(pos) continue @@ -71,6 +75,7 @@ func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) ([]byte, int // Detect end of escape sequence case inEscapeSequence && !inTitleEscapeSequence && bytes.ContainsRune(plainVirtualEscapeSeqEndValues, r): inEscapeSequence = false + escapeStartPos = -1 dropPos(pos) continue @@ -78,6 +83,7 @@ func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) ([]byte, int case inTitleEscapeSequence && r == UnicodeBellRune: inEscapeSequence = false inTitleEscapeSequence = false + escapeStartPos = -1 dropPos(pos) continue @@ -108,5 +114,13 @@ func cleanPtySnapshot(snapshot []byte, cursorPos int, isPosix bool) ([]byte, int result = append(result, r) } } - return []byte(string(result)), newCursorPos + + // If we're still in an escape sequence at the end, retain the unterminated sequence + cleanUptoPos := len(result) + if inEscapeSequence && escapeStartPos >= 0 { + unterminatedEscape = runes[escapeStartPos:] + result = append(result, unterminatedEscape...) + } + + return []byte(string(result)), newCursorPos, cleanUptoPos } diff --git a/helpers_windows_test.go b/helpers_windows_test.go index f395a00..db9ccd3 100644 --- a/helpers_windows_test.go +++ b/helpers_windows_test.go @@ -123,6 +123,13 @@ func Test_cleanPtySequences(t *testing.T) { []byte("Python 3.9.5"), -1, }, + { + "Unterminated escape sequence", + []byte("\x1b[?25"), + 0, + []byte("\x1b[?25"), + -1, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/outputproducer.go b/outputproducer.go index 178a06b..0d46399 100644 --- a/outputproducer.go +++ b/outputproducer.go @@ -2,7 +2,6 @@ package termtest import ( "bufio" - "bytes" "errors" "fmt" "io" @@ -66,13 +65,15 @@ func (o *outputProducer) listen(r io.Reader, w io.Writer, appendBuffer func([]by var PtyEOF = errors.New("pty closed") func (o *outputProducer) processNextRead(r io.Reader, w io.Writer, appendBuffer func([]byte, bool) error, size int) error { + isEOF := false o.opts.Logger.Printf("processNextRead started with size: %d\n", size) - defer o.opts.Logger.Println("processNextRead stopped") + defer func() { + o.opts.Logger.Printf("processNextRead stopped, isEOF: %v\n", isEOF) + }() snapshot := make([]byte, size) n, errRead := r.Read(snapshot) - isEOF := false if errRead != nil { pathError := &fs.PathError{} if errors.Is(errRead, fs.ErrClosed) || errors.Is(errRead, io.EOF) || (runtime.GOOS == "linux" && errors.As(errRead, &pathError)) { @@ -116,13 +117,13 @@ func (o *outputProducer) appendBuffer(value []byte, isFinal bool) error { // Clean output var err error - o.output, o.cursorPos, o.cleanUptoPos, err = o.processDirtyOutput(output, o.cursorPos, o.cleanUptoPos, isFinal, func(output []byte, cursorPos int) ([]byte, int, error) { + o.output, o.cursorPos, o.cleanUptoPos, err = o.processDirtyOutput(output, o.cursorPos, o.cleanUptoPos, isFinal, func(output []byte, cursorPos int) ([]byte, int, int, error) { var err error - output, cursorPos = cleanPtySnapshot(output, cursorPos, o.opts.Posix) + output, cursorPos, cleanCursorPos := cleanPtySnapshot(output, cursorPos, o.opts.Posix) if o.opts.OutputSanitizer != nil { - output, cursorPos, err = o.opts.OutputSanitizer(output, cursorPos) + output, cursorPos, cleanCursorPos, err = o.opts.OutputSanitizer(output, cursorPos) } - return output, cursorPos, err + return output, cursorPos, cleanCursorPos, err }) if err != nil { return fmt.Errorf("cleaning output failed: %w", err) @@ -138,7 +139,7 @@ func (o *outputProducer) appendBuffer(value []byte, isFinal bool) error { return nil } -type cleanerFunc func([]byte, int) ([]byte, int, error) +type cleanerFunc func(snapshot []byte, cursorPos int) (newSnapshot []byte, newCursorPos int, cleanUptoPos int, err error) // processDirtyOutput will sanitize the output received, but we have to be careful not to clean output that hasn't fully arrived // For example we may be inside an escape sequence and the escape sequence hasn't finished @@ -146,40 +147,28 @@ type cleanerFunc func([]byte, int) ([]byte, int, error) // In order for this to work properly the invoker must ensure the output and cleanUptoPos are consistent with each other. func (o *outputProducer) processDirtyOutput(output []byte, cursorPos int, cleanUptoPos int, isFinal bool, cleaner cleanerFunc) (_output []byte, _cursorPos int, _cleanUptoPos int, _err error) { defer func() { - o.opts.Logger.Printf("Cleaned output from %d to %d\n", cleanUptoPos, _cleanUptoPos) + o.opts.Logger.Printf("Cleaned output from %d to %d (isFinal: %v)\n", cleanUptoPos, _cleanUptoPos, isFinal) }() alreadyCleanedOutput := copyBytes(output[:cleanUptoPos]) processedOutput := []byte{} unprocessedOutput := copyBytes(output[cleanUptoPos:]) - processedCursorPos := cursorPos - len(alreadyCleanedOutput) - - if isFinal { - // If we've reached the end there's no point looking for the most recent line break as there's no guarantee the - // output will be terminated by a newline. - processedOutput = copyBytes(unprocessedOutput) - unprocessedOutput = []byte{} - } else { - // Find the most recent line break, and only clean until that point. - // Any output after the most recent line break is considered not ready for cleaning as cleaning depends on - // multiple consecutive characters. - lineSepN := bytes.LastIndex(unprocessedOutput, []byte("\n")) - if lineSepN != -1 { - processedOutput = copyBytes(unprocessedOutput[0 : lineSepN+1]) - unprocessedOutput = unprocessedOutput[lineSepN+1:] - } - } + relativeCursorPos := cursorPos - len(alreadyCleanedOutput) // Invoke the cleaner now that we have output that can be cleaned - if len(processedOutput) > 0 { + newCleanUptoPos := cleanUptoPos + if len(unprocessedOutput) > 0 { var err error - processedOutput, processedCursorPos, err = cleaner(processedOutput, processedCursorPos) + var processedCleanUptoPos int + processedOutput, relativeCursorPos, processedCleanUptoPos, err = cleaner(unprocessedOutput, relativeCursorPos) if err != nil { - return processedOutput, processedCursorPos, cleanUptoPos, fmt.Errorf("cleaner failed: %w", err) + return processedOutput, relativeCursorPos, processedCleanUptoPos, fmt.Errorf("cleaner failed: %w", err) } + // Keep a record of what point we're up to + newCleanUptoPos += processedCleanUptoPos } // Convert cursor position back to absolute - processedCursorPos += len(alreadyCleanedOutput) + processedCursorPos := relativeCursorPos + len(alreadyCleanedOutput) if processedCursorPos < 0 { // Because the cleaner function needs to support a negative cursor position it is impossible for the cleaner @@ -188,11 +177,8 @@ func (o *outputProducer) processDirtyOutput(output []byte, cursorPos int, cleanU processedCursorPos = 0 } - // Keep a record of what point we're up to - newCleanUptoPos := cleanUptoPos + len(processedOutput) - // Stitch everything back together - return append(append(alreadyCleanedOutput, processedOutput...), unprocessedOutput...), processedCursorPos, newCleanUptoPos, nil + return append(alreadyCleanedOutput, processedOutput...), processedCursorPos, newCleanUptoPos, nil } func (o *outputProducer) closeConsumers(reason error) { diff --git a/outputproducer_test.go b/outputproducer_test.go index 31b4f78..7b14527 100644 --- a/outputproducer_test.go +++ b/outputproducer_test.go @@ -293,13 +293,11 @@ func Test_outputProducer_appendBuffer(t *testing.T) { } func Test_outputProducer_cleanOutput(t *testing.T) { - phraseSanitizer := func(b []byte, cursorPos int) ([]byte, int, error) { - return regexp.MustCompile(`([\w ]+)`).ReplaceAll(b, []byte("sanitized")), cursorPos, nil - } inc := 0 - incrementalPhraseSanitizer := func(b []byte, cursorPos int) ([]byte, int, error) { + incrementalPhraseSanitizer := func(b []byte, cursorPos int) ([]byte, int, int, error) { defer func() { inc++ }() - return regexp.MustCompile(`([\w ]+)`).ReplaceAll(b, []byte(fmt.Sprintf("sanitized%d", inc))), cursorPos, nil + sanitized := regexp.MustCompile(`([\w ]+)`).ReplaceAll(b, []byte(fmt.Sprintf("sanitized%d", inc))) + return sanitized, cursorPos, len(sanitized), nil } type invocation struct { @@ -320,34 +318,13 @@ func Test_outputProducer_cleanOutput(t *testing.T) { cleaner cleanerFunc invocations []invocation }{ - { - "Do not sanitize unfinished", - newOutputProducer(newTestOpts(nil, t)), - 0, - 0, - func([]byte, int) ([]byte, int, error) { - return []byte(""), -1, fmt.Errorf("I should not have been invoked") - }, - []invocation{ - { - []byte("not final cause I dont have a line end"), - false, - []byte("not final cause I dont have a line end"), - 0, - 0, - 0, - 0, - require.NoError, - }, - }, - }, { "Sanitize finished", newOutputProducer(newTestOpts(nil, t)), 0, 0, - func([]byte, int) ([]byte, int, error) { - return []byte("sanitized"), 0, nil + func([]byte, int) ([]byte, int, int, error) { + return []byte("sanitized"), 0, 9, nil }, []invocation{ { @@ -362,44 +339,6 @@ func Test_outputProducer_cleanOutput(t *testing.T) { }, }, }, - { - "Sanitize up to final line end", - newOutputProducer(newTestOpts(nil, t)), - 0, - 0, - phraseSanitizer, - []invocation{ - { - []byte("sanitize\nsanitize\ndont sanitize"), - false, - []byte("sanitized\nsanitized\ndont sanitize"), - 0, - 0, - 0, - 20, - require.NoError, - }, - }, - }, - { - "Sanitize from pos up to final line end", - newOutputProducer(newTestOpts(nil, t)), - 0, - 21, - phraseSanitizer, - []invocation{ - { - []byte("previously sanitized\nsanitize\ndont sanitize"), - false, - []byte("previously sanitized\nsanitized\ndont sanitize"), - -21, - -21, - 0, - 31, - require.NoError, - }, - }, - }, { "Consecutive Invocations", newOutputProducer(newTestOpts(nil, t)), @@ -408,51 +347,25 @@ func Test_outputProducer_cleanOutput(t *testing.T) { incrementalPhraseSanitizer, []invocation{ { - // This won't result in anything being sanitized, because isFinal=false and there is no line end + // Should sanitize "sanitize me" []byte("sanitize me"), false, - []byte("sanitize me"), + []byte("sanitized0"), 0, 0, 0, - 0, - require.NoError, - }, - { - // The new text won't get sanitized, but because we're adding a line break at the start here which - // will get appended to the previous invocation we should now get the bytes produced by the previous - // invocation sanitized. - []byte("\nsanitize me"), - false, - []byte("sanitized0\nsanitize me"), - 0, - 0, - 0, - 11, - require.NoError, - }, - { - // We're just appending a new line end here, so all output produced up to this point should now get - // sanitized. The integer at the end of the sanitized output lets us know which invocation it got - // sanitized on. - []byte("\n"), - false, - []byte("sanitized0\nsanitized1\n"), - -11, - -11, - 0, - 22, + 10, require.NoError, }, { // No line end on the new output, but we're sending isFinal=true, so the output should be sanitized []byte("sanitize me"), true, - []byte("sanitized0\nsanitized1\nsanitized2"), - -22, - -22, + []byte("sanitized0sanitized1"), + -10, + -10, 0, - 32, + 20, require.NoError, }, }, @@ -462,8 +375,8 @@ func Test_outputProducer_cleanOutput(t *testing.T) { newOutputProducer(newTestOpts(nil, t)), 27, // Space before "me" in "sanitize me" 18, // End of first linebreak - func([]byte, int) ([]byte, int, error) { - return []byte("sanitized\n"), 0, nil + func([]byte, int) ([]byte, int, int, error) { + return []byte("sanitized\n"), 0, 10, nil }, []invocation{ { @@ -490,18 +403,18 @@ func Test_outputProducer_cleanOutput(t *testing.T) { output = append(output, inv.appendBytes...) got, newCursorPos, newCleanerPos, err := o.processDirtyOutput( output, cursorPos, cleanerPos, inv.isFinal, - func(output []byte, cursorPos int) ([]byte, int, error) { - require.Equal(t, inv.wantRelCursorPosInput, cursorPos) - out, newCursorPos, err := tt.cleaner(output, cursorPos) - require.Equal(t, inv.wantRelCursorPosOutput, newCursorPos) - return out, newCursorPos, err + func(output []byte, cursorPos int) ([]byte, int, int, error) { + require.Equal(t, inv.wantRelCursorPosInput, cursorPos, "wamtRelCursorPosInput") + out, newCursorPos, newCleanerPos, err := tt.cleaner(output, cursorPos) + require.Equal(t, inv.wantRelCursorPosOutput, newCursorPos, "wamtRelCursorPosOutput") + return out, newCursorPos, newCleanerPos, err }) inv.wantErr(t, err) - require.Equal(t, string(inv.wantOutput), string(got)) - require.Equal(t, inv.wantAbsCursorPos, newCursorPos) - require.Equal(t, inv.wantAbsCleanerPos, newCleanerPos) + require.Equal(t, string(inv.wantOutput), string(got), "wantOutput") + require.Equal(t, inv.wantAbsCursorPos, newCursorPos, newCleanerPos, "wantAbsCursorPos") + require.Equal(t, inv.wantAbsCleanerPos, newCleanerPos, "wantAbsCleanerPos") cleanerPos = newCleanerPos cursorPos = newCursorPos From 90926e4be6bc23c30d615a773deccfaa37b0b646 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 20 Aug 2025 09:35:42 -0700 Subject: [PATCH 3/3] Fix windows test --- helpers_windows_test.go | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/helpers_windows_test.go b/helpers_windows_test.go index db9ccd3..43eed2d 100644 --- a/helpers_windows_test.go +++ b/helpers_windows_test.go @@ -13,6 +13,7 @@ func Test_cleanPtySequences(t *testing.T) { cursorPos int want []byte wantCursorPos int + wantCleanUpto int }{ { "Window title, cursor after", @@ -20,6 +21,7 @@ func Test_cleanPtySequences(t *testing.T) { 86, // First two characters of Hello []byte("Hello"), 2, + 5, }, { "Window title, cursor preceding", @@ -27,6 +29,7 @@ func Test_cleanPtySequences(t *testing.T) { 1, // First two characters of Hello []byte("HelloWorld"), 1, + 10, }, { "Window title, cursor on top", @@ -34,6 +37,7 @@ func Test_cleanPtySequences(t *testing.T) { 10, // Inside title escape sequence []byte("HelloWorld"), 4, + 10, }, { "Backspace character", @@ -41,6 +45,7 @@ func Test_cleanPtySequences(t *testing.T) { 7, // End of string []byte("FooBar"), 5, + 6, }, { "Backspace character, cursor on top of backspace", @@ -48,6 +53,7 @@ func Test_cleanPtySequences(t *testing.T) { 5, // End of string []byte("FooBar"), 3, + 6, }, { "Cursor position preceding cleaned sequence", @@ -55,6 +61,7 @@ func Test_cleanPtySequences(t *testing.T) { 2, // End of "Foo" []byte("FooBar"), 2, + 6, }, { "Cursor position succeeding cleaned sequence", @@ -62,6 +69,7 @@ func Test_cleanPtySequences(t *testing.T) { 9, // End of "Bar" []byte("FooBar"), 5, + 6, }, { "Cursor position on top of cleaned sequence", @@ -69,6 +77,7 @@ func Test_cleanPtySequences(t *testing.T) { 4, // Unicode code point []byte("FooBar"), 2, + 6, }, { "Negative cursor position", @@ -76,6 +85,7 @@ func Test_cleanPtySequences(t *testing.T) { -10, // End of "Foo" []byte("FooBar"), -10, + 6, }, { // Running on ANSI escape codes obviously is not the intent, but without being able to easily identify @@ -85,6 +95,7 @@ func Test_cleanPtySequences(t *testing.T) { 165, []byte("25h 25l █ Installing Runtime (Unconfigured)25h 25l █25h 25l █ Installing Runtime Environment25h 25l Setting Up Runtime \n Resolving Dependencies |25h"), 159, + 158, }, { "Escape at first character", @@ -94,6 +105,7 @@ func Test_cleanPtySequences(t *testing.T) { // Since the cleaner handles an absolute cursor position against relative output, we can't determine start // of output and so we return a negative -1, + 3, }, { "Cursor character (NOT position)", @@ -101,6 +113,7 @@ func Test_cleanPtySequences(t *testing.T) { 0, []byte("foobar"), 0, + 6, }, { "Home key", @@ -108,6 +121,7 @@ func Test_cleanPtySequences(t *testing.T) { 0, []byte("foo"), -1, + 3, }, { "Home key with Window title following", @@ -115,6 +129,7 @@ func Test_cleanPtySequences(t *testing.T) { 0, []byte("foo"), -1, + 3, }, { "Alert / bell character", @@ -122,13 +137,15 @@ func Test_cleanPtySequences(t *testing.T) { 0, []byte("Python 3.9.5"), -1, + 12, }, { "Unterminated escape sequence", - []byte("\x1b[?25"), + []byte("foo\x1b[?25"), 0, - []byte("\x1b[?25"), - -1, + []byte("foo\x1b[?25"), + 0, + 3, }, } for _, tt := range tests { @@ -139,9 +156,10 @@ func Test_cleanPtySequences(t *testing.T) { if tt.wantCursorPos > len(tt.want) { t.Fatal("Wanted cursor position cannot be larger than wanted output") } - cleaned, cursorPos := cleanPtySnapshot(tt.b, tt.cursorPos, false) - assert.Equal(t, string(tt.want), string(cleaned)) - assert.Equal(t, tt.wantCursorPos, cursorPos) + cleaned, cursorPos, cleanUptoPos := cleanPtySnapshot(tt.b, tt.cursorPos, false) + assert.Equal(t, string(tt.want), string(cleaned), "wanted output") + assert.Equal(t, tt.wantCursorPos, cursorPos, "wanted cursor position") + assert.Equal(t, tt.wantCleanUpto, cleanUptoPos, "wanted clean upto position") }) } }