diff --git a/internal/inputhandlers/term_iac_test.go b/internal/inputhandlers/term_iac_test.go index b29dd84560..1bfad1b4ee 100644 --- a/internal/inputhandlers/term_iac_test.go +++ b/internal/inputhandlers/term_iac_test.go @@ -3,7 +3,9 @@ package inputhandlers import ( "testing" + "github.com/GoMudEngine/GoMud/internal/connections" "github.com/GoMudEngine/GoMud/internal/term" + "github.com/stretchr/testify/assert" ) // newEnvironPayload builds the bytes that appear between `IAC SB NEW-ENVIRON IS` @@ -90,3 +92,17 @@ func TestNewEnvironResponseMatcher(t *testing.T) { t.Errorf("expected extracted payload to be detected as Mudlet, payload=%v", payload) } } + +// TestTelnetIACHandlerShortScreenSizeReport sends a NAWS report cut short after +// three bytes. It used to index past the payload and panic on the connection +// goroutine, which has no recover, so the whole server exited. +func TestTelnetIACHandlerShortScreenSizeReport(t *testing.T) { + clientInput := &connections.ClientInput{ + ConnectionId: 1, + DataIn: []byte{term.TELNET_IAC, term.TELNET_SB, term.TELNET_OPT_NAWS, 0, 80, 0}, + } + + assert.NotPanics(t, func() { + TelnetIACHandler(clientInput, map[string]any{}) + }) +} diff --git a/internal/term/telnet.go b/internal/term/telnet.go index 48b5e80645..3470b3e320 100644 --- a/internal/term/telnet.go +++ b/internal/term/telnet.go @@ -167,7 +167,9 @@ func TelnetDONT(what IACByte) []IACByte { func TelnetParseScreenSizePayload(info []byte) (width int, height int, err error) { - if len(info) >= 3 { + // Width and height are two bytes each, so a report shorter than four + // bytes cannot be read. Any client can send one. + if len(info) >= 4 { width = (int(info[0]) << 8) | int(info[1]) height = (int(info[2]) << 8) | int(info[3]) diff --git a/internal/term/telnet_test.go b/internal/term/telnet_test.go new file mode 100644 index 0000000000..6e60562d5d --- /dev/null +++ b/internal/term/telnet_test.go @@ -0,0 +1,43 @@ +package term + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A NAWS report is four bytes: width then height, each high byte first. A +// client can send fewer, and that must come back as an error rather than a +// read past the end of the payload. +func TestTelnetParseScreenSizePayload(t *testing.T) { + tests := []struct { + name string + payload []byte + wantWidth int + wantHeight int + wantErr bool + }{ + {name: "80x24", payload: []byte{0, 80, 0, 24}, wantWidth: 80, wantHeight: 24}, + {name: "high bytes", payload: []byte{1, 0, 0, 50}, wantWidth: 256, wantHeight: 50}, + {name: "trailing IAC SE", payload: []byte{0, 120, 0, 40, TELNET_IAC, TELNET_SE}, wantWidth: 120, wantHeight: 40}, + {name: "three bytes", payload: []byte{0, 80, 0}, wantErr: true}, + {name: "two bytes", payload: []byte{0, 80}, wantErr: true}, + {name: "empty", payload: []byte{}, wantErr: true}, + {name: "nil", payload: nil, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + width, height, err := TelnetParseScreenSizePayload(tt.payload) + if tt.wantErr { + assert.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.wantWidth, width) + assert.Equal(t, tt.wantHeight, height) + }) + } +}