mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(agent): set utf8 locale for tmux terminals (#25530)
> Mux is updating this PR on behalf of Mike. ## Summary - Set a UTF-8 `LC_CTYPE` fallback for reconnecting PTYs when no effective UTF-8 locale is present. - Preserve non-empty `LC_ALL` so explicit user locale choices still win. - Add tmux glyph regression coverage for reconnecting PTYs, plus unit coverage for the env helper. - Stabilize the tmux regression by keeping the pane alive until the glyph output is observed. - Keep the env helper unit test expectations OS-aware for Windows and cover unhyphenated UTF8 locales. ## Validation - `go test ./agent/reconnectingpty -run TestWithTerminalEnv -count=1` - `go test ./agent -run '^TestAgent_ReconnectingPTY$/Buffered$' -count=1` - `go test ./agent -run '^TestAgent_ReconnectingPTY$' -count=1` - `make lint` - `git commit` pre-commit hook - `git push` pre-push hook
This commit is contained in:
+46
-1
@@ -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)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user