Read returns a truncated fragmented message as complete when the connection closes mid-message

Start in msgReader.read and msgReader.Read in read.go, and check how readLoop and readFramePayload surface transport EOF. Run the reproduction test, which should fail because a truncated fragmented message returns nil error. Done when a connection closed mid-message yields io.ErrUnexpectedEOF and go test ./... passes. The compressed final-frame case mentioned in the issue still needs handling.

Summary

If the peer closes the connection in the middle of a fragmented message, Conn.Read (and the io.Reader from Conn.Reader) returns the frames received so far as a complete message with a nil error, instead of failing. The caller can't tell that the message is truncated. The next read then fails with previous message not read to completion.

Version: v1.8.15 (also on master).

Cause

msgReader.read returns the transport error as is when the connection closes while it waits for the next continuation frame's header (readLoop) or reads a frame payload (readFramePayload). Those errors wrap io.EOF. msgReader.Read then treats any io.EOF as a normal end of message:

// read.go
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) && mr.fin && mr.flate {
	mr.putFlateReader()
	return n, io.EOF
}

So io.ReadAll / Conn.Read see a clean end of message.

Reproduction

A server that accepts a connection and calls Read; a raw client sends a text frame with FIN=0 and then closes its side.

package wsrepro

import (
	"bufio"
	"context"
	"net"
	"net/http"
	"net/http/httptest"
	"strings"
	"testing"

	"github.com/coder/websocket"
)

func TestTruncatedFragmentedMessageIsReturnedAsComplete(t *testing.T) {
	type result struct {
		typ  websocket.MessageType
		data []byte
		err  error
	}
	results := make(chan result, 1)
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		c, err := websocket.Accept(w, r, nil)
		if err != nil {
			results <- result{err: err}
			return
		}
		typ, data, err := c.Read(context.Background())
		results <- result{typ, data, err}
	}))
	defer srv.Close()

	conn, err := net.Dial("tcp", strings.TrimPrefix(srv.URL, "http://"))
	if err != nil {
		t.Fatal(err)
	}
	_, _ = conn.Write([]byte("GET / HTTP/1.1\r\nHost: x\r\nUpgrade: websocket\r\nConnection: Upgrade\r\n" +
		"Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\nSec-WebSocket-Version: 13\r\n\r\n"))
	resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
	if err != nil || resp.StatusCode != http.StatusSwitchingProtocols {
		t.Fatalf("handshake: %v %v", resp, err)
	}

	payload := []byte("hello")
	frame := []byte{0x01, 0x80 | byte(len(payload)), 0, 0, 0, 0} // text, FIN=0, masked with a zero key
	frame = append(frame, payload...)
	_, _ = conn.Write(frame)
	_ = conn.(*net.TCPConn).CloseWrite()

	got := <-results
	t.Logf("type=%v data=%q err=%v", got.typ, got.data, got.err)
	if got.err == nil {
		t.Fatalf("got a complete message %q from a fragmented message whose final frame never arrived", got.data)
	}
}

Output on v1.8.15:

main_test.go:51: type=MessageText data="hello" err=<nil>
main_test.go:53: got a complete message "hello" from a fragmented message whose final frame never arrived

The same happens if the connection closes after a frame header but before its payload, since io.ReadFull returns io.EOF when it reads no bytes.

Possible fix

Parenthesizing the condition in msgReader.Read isn't enough: without compression, msgReader.read's own end-of-message signal is also a bare io.EOF, and several existing tests rely on it. Converting a transport EOF inside a message to io.ErrUnexpectedEOF in msgReader.read fixes the reproduction above and keeps go test ./... passing:

@@ func (mr *msgReader) read(p []byte) (int, error) {
 			h, err := mr.c.readLoop(mr.ctx)
 			if err != nil {
+				if errors.Is(err, io.EOF) {
+					return 0, fmt.Errorf("connection closed before the message was complete: %w", io.ErrUnexpectedEOF)
+				}
 				return 0, err
 			}
@@
 		n, err := mr.c.readFramePayload(mr.ctx, p)
 		if err != nil {
+			if errors.Is(err, io.EOF) {
+				return n, fmt.Errorf("connection closed before the message was complete: %w", io.ErrUnexpectedEOF)
+			}
 			return n, err
 		}

One thing I didn't check: with compression, a cut inside the final frame's payload still matches io.ErrUnexpectedEOF && mr.fin && mr.flate and would still be reported as io.EOF. That case probably needs to tell the flate tail apart from a transport error too.