diff --git a/agent/agent_test.go b/agent/agent_test.go index 410b0941f9..1fe8ad2725 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -2164,8 +2164,13 @@ func TestAgent_ReconnectingPTY(t *testing.T) { _, err := exec.LookPath("screen") hasScreen := err == nil - // Make sure UTF-8 works even with LANG set to something like C. + tmuxPath, err := exec.LookPath("tmux") + hasTmux := err == nil + + // Make sure UTF-8 works even with locale variables set to C. t.Setenv("LANG", "C") + t.Setenv("LC_CTYPE", "C") + t.Setenv("LC_ALL", "") for _, backendType := range backends { t.Run(backendType, func(t *testing.T) { @@ -2308,6 +2313,46 @@ func TestAgent_ReconnectingPTY(t *testing.T) { bytes, err := io.ReadAll(netConn5) require.NoError(t, err) require.Contains(t, string(bytes), "❯") + + if !hasTmux { + t.Log("`tmux` not found, skipping tmux glyph regression") + } else { + glyphs := "⚠╭╮╰╯•›│─█▓░▄❯✔╌" + tmuxSocket := "coder-test-" + strings.ReplaceAll(uuid.NewString(), "-", "") + t.Cleanup(func() { + _ = exec.Command(tmuxPath, "-L", tmuxSocket, "kill-server").Run() + }) + // Keep the pane alive with a shell builtin until the read loop sees + // the glyphs, otherwise tmux can restore the alternate screen first. + command := fmt.Sprintf( + "%s -L %s new-session %q", + strconv.Quote(tmuxPath), + tmuxSocket, + fmt.Sprintf("printf '%%s\\n' '%s'; read _", glyphs), + ) + netConn6, err := conn.ReconnectingPTY(ctx, uuid.New(), 80, 80, command) + require.NoError(t, err) + defer netConn6.Close() + + var output strings.Builder + buffer := make([]byte, 1024) + deadline := time.Now().Add(testutil.WaitMedium) + for !strings.Contains(output.String(), glyphs) { + if time.Now().After(deadline) { + require.Contains(t, output.String(), glyphs) + } + require.NoError(t, netConn6.SetReadDeadline(time.Now().Add(testutil.IntervalMedium))) + read, err := netConn6.Read(buffer) + if read > 0 { + _, _ = output.Write(buffer[:read]) + } + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + continue + } + require.NoError(t, err) + } + } }) } } diff --git a/agent/reconnectingpty/buffered.go b/agent/reconnectingpty/buffered.go index 385d6ef7f7..2d3b5ef27f 100644 --- a/agent/reconnectingpty/buffered.go +++ b/agent/reconnectingpty/buffered.go @@ -56,11 +56,10 @@ func newBuffered(ctx context.Context, logger slog.Logger, execer agentexec.Exece } rpty.circularBuffer = circularBuffer - // Add TERM then start the command with a pty. pty.Cmd duplicates Path as the - // first argument so remove it. + // Add terminal environment then start the command with a pty. pty.Cmd + // duplicates Path as the first argument so remove it. cmdWithEnv := execer.PTYCommandContext(ctx, cmd.Path, cmd.Args[1:]...) - //nolint:gocritic - cmdWithEnv.Env = append(rpty.command.Env, "TERM="+xterm256Color) + cmdWithEnv.Env = withTerminalEnv(rpty.command.Env) cmdWithEnv.Dir = rpty.command.Dir ptty, process, err := pty.Start(cmdWithEnv) if err != nil { diff --git a/agent/reconnectingpty/reconnectingpty.go b/agent/reconnectingpty/reconnectingpty.go index c3fa833b41..f95bf3e34b 100644 --- a/agent/reconnectingpty/reconnectingpty.go +++ b/agent/reconnectingpty/reconnectingpty.go @@ -7,6 +7,7 @@ import ( "net" "os/exec" "runtime" + "strings" "sync" "time" @@ -31,6 +32,62 @@ const ( xterm256Color = "xterm-256color" ) +// withTerminalEnv returns env with the terminal type and UTF-8 character locale expected by the web terminal. +func withTerminalEnv(env []string) []string { + next := make([]string, 0, len(env)+2) + next = append(next, env...) + next = append(next, "TERM="+xterm256Color) + // Some terminal applications use the process locale for glyph width and + // replacement behavior. Set only LC_CTYPE so other locale categories keep + // the user's settings. Preserve non-empty LC_ALL because it has higher + // precedence than LC_CTYPE. + if runtime.GOOS != "windows" && !effectiveLocaleIsUTF8(next) && !hasNonEmptyEnv(next, "LC_ALL") { + next = append(next, "LC_CTYPE="+terminalUTF8Locale()) + } + return next +} + +// terminalUTF8Locale returns a widely available UTF-8 character locale for the host OS. +func terminalUTF8Locale() string { + if runtime.GOOS == "darwin" { + return "UTF-8" + } + return "C.UTF-8" +} + +// effectiveLocaleIsUTF8 reports whether the locale precedence chain resolves to UTF-8. +func effectiveLocaleIsUTF8(env []string) bool { + for _, name := range []string{"LC_ALL", "LC_CTYPE", "LANG"} { + value, ok := envValue(env, name) + if !ok || value == "" { + continue + } + return localeIsUTF8(value) + } + return false +} + +func localeIsUTF8(locale string) bool { + lower := strings.ToLower(locale) + return strings.Contains(lower, "utf-8") || strings.Contains(lower, "utf8") +} + +func hasNonEmptyEnv(env []string, name string) bool { + value, ok := envValue(env, name) + return ok && value != "" +} + +// envValue returns the effective value for name using the last assignment. +func envValue(env []string, name string) (string, bool) { + prefix := name + "=" + for i := len(env) - 1; i >= 0; i-- { + if value, ok := strings.CutPrefix(env[i], prefix); ok { + return value, true + } + } + return "", false +} + // Options allows configuring the reconnecting pty. type Options struct { // Timeout describes how long to keep the pty alive without any connections. diff --git a/agent/reconnectingpty/reconnectingpty_test.go b/agent/reconnectingpty/reconnectingpty_test.go new file mode 100644 index 0000000000..23f8cd26b1 --- /dev/null +++ b/agent/reconnectingpty/reconnectingpty_test.go @@ -0,0 +1,99 @@ +//nolint:testpackage // Tests private env helpers directly. +package reconnectingpty + +import ( + "runtime" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestWithTerminalEnv(t *testing.T) { + t.Parallel() + + defaultLocale := "C.UTF-8" + if runtime.GOOS == "darwin" { + defaultLocale = "UTF-8" + } + + tests := []struct { + name string + env []string + wantLCCTYPE string + wantLCCTYPESet bool + }{ + { + name: "adds locale when missing", + env: []string{"PATH=/bin"}, + wantLCCTYPE: defaultLocale, + wantLCCTYPESet: true, + }, + { + name: "adds locale when lang is not utf8", + env: []string{"LANG=C"}, + wantLCCTYPE: defaultLocale, + wantLCCTYPESet: true, + }, + { + name: "keeps utf8 lang", + env: []string{"LANG=C.UTF-8"}, + }, + { + name: "keeps unhyphenated utf8 lang", + env: []string{"LANG=C.UTF8"}, + }, + { + name: "keeps utf8 ctype", + env: []string{"LC_CTYPE=C.UTF-8"}, + wantLCCTYPE: "C.UTF-8", + wantLCCTYPESet: true, + }, + { + name: "overrides non utf8 ctype", + env: []string{"LANG=C.UTF-8", "LC_CTYPE=C"}, + wantLCCTYPE: defaultLocale, + wantLCCTYPESet: true, + }, + { + name: "keeps utf8 lc all", + env: []string{"LC_ALL=C.UTF-8"}, + }, + { + name: "preserves non empty lc all", + env: []string{"LC_ALL=C"}, + }, + { + name: "ignores empty lc all", + env: []string{"LC_ALL="}, + wantLCCTYPE: defaultLocale, + wantLCCTYPESet: true, + }, + { + name: "continues after empty lc all", + env: []string{"LC_ALL=", "LANG=C.UTF-8"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := withTerminalEnv(tt.env) + term, ok := envValue(got, "TERM") + require.True(t, ok) + require.Equal(t, xterm256Color, term) + + wantLCCTYPE := tt.wantLCCTYPE + wantLCCTYPESet := tt.wantLCCTYPESet + if runtime.GOOS == "windows" { + wantLCCTYPE, wantLCCTYPESet = envValue(tt.env, "LC_CTYPE") + } + + locale, ok := envValue(got, "LC_CTYPE") + require.Equal(t, wantLCCTYPESet, ok) + if wantLCCTYPESet { + require.Equal(t, wantLCCTYPE, locale) + } + }) + } +} diff --git a/agent/reconnectingpty/screen.go b/agent/reconnectingpty/screen.go index a2fcb4fba2..1540bd067a 100644 --- a/agent/reconnectingpty/screen.go +++ b/agent/reconnectingpty/screen.go @@ -236,8 +236,7 @@ func (rpty *screenReconnectingPTY) doAttach(ctx context.Context, conn net.Conn, rpty.command.Path, // pty.Cmd duplicates Path as the first argument so remove it. }, rpty.command.Args[1:]...)...) - //nolint:gocritic - cmd.Env = append(rpty.command.Env, "TERM="+xterm256Color) + cmd.Env = withTerminalEnv(rpty.command.Env) cmd.Dir = rpty.command.Dir ptty, process, err := pty.Start(cmd, pty.WithPTYOption( pty.WithSSHRequest(ssh.Pty{ @@ -352,8 +351,7 @@ func (rpty *screenReconnectingPTY) sendCommand(ctx context.Context, command stri // -X runs a command in the matching session. "-X", command, ) - //nolint:gocritic - cmd.Env = append(rpty.command.Env, "TERM="+xterm256Color) + cmd.Env = withTerminalEnv(rpty.command.Env) cmd.Dir = rpty.command.Dir cmd.Stdout = &stdout err := cmd.Run()