From 0106f107d1888a3d83c37ed70103771dc9d215ea Mon Sep 17 00:00:00 2001 From: Roshan Ramani Date: Fri, 4 Sep 2026 19:45:51 +0530 Subject: [PATCH] fix(websocket): replace invalid UTF-8 instead of dropping the session A websocket text frame must be valid UTF-8, so a payload that splits a rune makes the client close with 1007 and the player loses the session. The existing first-byte TELNET_IAC check guards one source of that; the comment beside it already names the failure ('Invalid UTF-8 in text frame'). Anything that cuts a multi-byte rune hits the same wall - see #631, where a byte-counting wrap splits the U+2591 blocks in the quests progress bar. Sanitize the payload and log it, so the frame is deliverable and the cause is visible rather than the connection disappearing. This does not fix the wrap itself, which is upstream in ansitags (GoMudEngine/ansitags#14). It stops that class of bug from being fatal. Assisted-by: Claude Code (Claude Opus 5) --- internal/connections/connectiondetails.go | 25 ++++++++++- .../connectiondetails_utf8_test.go | 44 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 internal/connections/connectiondetails_utf8_test.go diff --git a/internal/connections/connectiondetails.go b/internal/connections/connectiondetails.go index 1753b8682..620605293 100644 --- a/internal/connections/connectiondetails.go +++ b/internal/connections/connectiondetails.go @@ -8,6 +8,7 @@ import ( "sync" "sync/atomic" "time" + "unicode/utf8" "github.com/GoMudEngine/GoMud/internal/mudlog" "github.com/GoMudEngine/GoMud/internal/term" @@ -296,16 +297,38 @@ func (cd *ConnectionDetails) Write(p []byte) (n int, err error) { return 0, nil } - err := cd.wsConn.WriteMessage(websocket.TextMessage, p) + // A text frame must be valid UTF-8 or the client closes the connection + // (1007), which is what the first-byte check above is guarding one case of. + // Anything else that splits a rune - a width-aware wrap that counts bytes, + // for instance - would drop the session mid-sentence, so replace the bad + // bytes and log rather than hand the client something it must reject. + payload, replaced := validUTF8Payload(p) + if replaced { + mudlog.Error("conn.Write", "error", "Invalid UTF-8 in websocket payload; replaced", "bytes", p) + } + + err := cd.wsConn.WriteMessage(websocket.TextMessage, payload) if err != nil { return 0, err } + // Report the caller's length: the write consumed all of p, whatever the + // replacement did to the byte count. return len(p), nil } return cd.conn.Write(p) } +// validUTF8Payload returns a websocket-safe copy of p, reporting whether any +// invalid byte had to be replaced. A valid payload is returned untouched so the +// common path allocates nothing. +func validUTF8Payload(p []byte) (payload []byte, replaced bool) { + if utf8.Valid(p) { + return p, false + } + return []byte(strings.ToValidUTF8(string(p), string(utf8.RuneError))), true +} + func (cd *ConnectionDetails) Read(p []byte) (n int, err error) { if cd.sshChannel != nil { diff --git a/internal/connections/connectiondetails_utf8_test.go b/internal/connections/connectiondetails_utf8_test.go new file mode 100644 index 000000000..d269cc6c2 --- /dev/null +++ b/internal/connections/connectiondetails_utf8_test.go @@ -0,0 +1,44 @@ +package connections + +import ( + "strings" + "testing" + "unicode/utf8" +) + +// A websocket text frame must be valid UTF-8, so a payload that splits a rune +// closes the connection with 1007 rather than showing the player anything. +func TestValidUTF8Payload(t *testing.T) { + bar := strings.Repeat("░", 25) + + tests := []struct { + name string + in []byte + replaced bool + }{ + {name: "ascii", in: []byte("You see a rusty sword."), replaced: false}, + {name: "multi-byte runes", in: []byte(bar), replaced: false}, + {name: "empty", in: []byte{}, replaced: false}, + // The tail of a progress bar cut after the first byte of a 3-byte rune. + {name: "rune split at the end", in: []byte(bar)[:len(bar)-2], replaced: true}, + // Orphaned continuation bytes, i.e. the other half of that cut. + {name: "orphan continuation bytes", in: []byte{0x96, 0x91, 'h', 'i'}, replaced: true}, + {name: "lone 0xff", in: []byte{'a', 0xff, 'b'}, replaced: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + payload, replaced := validUTF8Payload(test.in) + + if replaced != test.replaced { + t.Fatalf("replaced = %v, want %v", replaced, test.replaced) + } + if !utf8.Valid(payload) { + t.Fatalf("payload is still not valid UTF-8: %q", payload) + } + if !test.replaced && string(payload) != string(test.in) { + t.Fatalf("valid payload was altered: got %q, want %q", payload, test.in) + } + }) + } +}