Read returns a truncated fragmented message as complete when the connection closes mid-message
評価
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 初心者へのやさしさ
- 62/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- go
- 領域
- networking
調査の方向性
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.
索引モデルが issue の本文から書いたものです。
説明
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.
- 主要言語
- Go
- スター
- 5.5k
- フォーク
- 379
- PR マージ指標
- 30日以内にマージされた PR はありません
環境構築
このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
coder/websocket のほかの issue
-
Must not wrap io.EOF対応中かも @Yanhu007 が 188 日前に担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
-
export wstestオープンenhancement
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
-
難易度 5/5 1週間以上 初心者へのやさしさ 22/100
-
wasm: Close with codes other than 1000 throws, so CloseNow never closes対応中かも @0pcom が 15 日前に担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 20/100
coder/websocket の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
blinklabs-io/dingo#5368 ·
メンテナーはふだん 1 日以内に返信
-
difficulty: easy enhancement frontend good first issue help wanted priority: P2
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
rafaelsilvabr/pdfHandler#18 ·
-
難易度 2/5 1〜2日 初心者へのやさしさ 78/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 83/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
メンテナーはふだん 1 日以内に返信