fix(websocket): replace invalid UTF-8 instead of dropping the session - #646
Open
rawsun007 wants to merge 1 commit into
Open
fix(websocket): replace invalid UTF-8 instead of dropping the session#646rawsun007 wants to merge 1 commit into
rawsun007 wants to merge 1 commit into
Conversation
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 GoMudEngine#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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #631, and the belt-and-braces half I offered there rather than assumed.
A websocket text frame has to be valid UTF-8.
ConnectionDetails.Writealready guards one source of an invalid frame - thep[0] == term.TELNET_IACcheck - and the comment above it names the consequence: "Websocket client complains, disconnects, error is raised: close 1002 (protocol error): Invalid UTF-8 in text frame."Anything else that cuts a multi-byte rune hits the same wall, and the player loses the session rather than seeing a glitch. #631 is exactly that:
ansitags.SplitStringOnSpacesmeasures in bytes, so the░blocks in thequestsprogress bar get split and the browser closes with 1007.This replaces invalid bytes with U+FFFD and logs the payload, so the frame is deliverable and the cause shows up in the log.
Writestill reportslen(p)to the caller, since the write consumed all of it.Deliberately not a fix for the wrap - that belongs upstream in GoMudEngine/ansitags#14, and I have not touched it here. This only stops that class of bug from being fatal, and does nothing at all when the payload is already valid (the common path returns the original slice, no allocation).
validUTF8Payloadis split out so it is testable without a live socket. The test covers ASCII, a 25-rune░bar, a bar cut after the first byte of a rune (the #631 shape), orphaned continuation bytes, and a lone0xff; it also asserts a valid payload comes back byte-identical.Verified:
make validateclean,go test ./...green, and the new test fails to compile againstmainwithout the helper.Assisted-by: Claude Code (Claude Opus 5) - disclosed here and in the commit trailer.