[net] websocket: handle solicited and unsolicited Ping/Pong frames correctly

[Mikio Hara (Gerrit)]

Reviewers: Ian Lance Taylor, Andrew Gerrand

Mikio Hara uploaded a change:
https://go-review.googlesource.com/13054

websocket: handle solicited and unsolicited Ping/Pong frames correctly

Fixes #6377.
Fixes #7825.
Fixes #10156.

Change-Id: I600cf493de3671d7e3d11e2e12d32f43928b7bfc
---
M websocket/hybi.go
M websocket/hybi_test.go
M websocket/websocket_test.go
3 files changed, 112 insertions(+), 13 deletions(-)



diff --git a/websocket/hybi.go b/websocket/hybi.go
index 41c854e..713ee34 100644
--- a/websocket/hybi.go
+++ b/websocket/hybi.go
@@ -264,7 +264,7 @@
payloadType byte
}

-func (handler *hybiFrameHandler) HandleFrame(frame frameReader) (r
frameReader, err error) {
+func (handler *hybiFrameHandler) HandleFrame(frame frameReader)
(frameReader, error) {
if handler.conn.IsServerConn() {
// The client MUST mask all frames sent to the server.
if frame.(*hybiFrameReader).header.MaskingKey == nil {
@@ -288,20 +288,19 @@
handler.payloadType = frame.PayloadType()
case CloseFrame:
return nil, io.EOF
- case PingFrame:
- pingMsg := make([]byte, maxControlFramePayloadLength)
- n, err := io.ReadFull(frame, pingMsg)
- if err != nil && err != io.ErrUnexpectedEOF {
+ case PingFrame, PongFrame:
+ b := make([]byte, maxControlFramePayloadLength)
+ n, err := io.ReadFull(frame, b)
+ if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF {
return nil, err
}
io.Copy(ioutil.Discard, frame)
- n, err = handler.WritePong(pingMsg[:n])
- if err != nil {
- return nil, err
+ if frame.PayloadType() == PingFrame {
+ if _, err := handler.WritePong(b[:n]); err != nil {
+ return nil, err
+ }
}
return nil, nil
- case PongFrame:
- return nil, ErrNotImplemented
}
return frame, nil
}
diff --git a/websocket/hybi_test.go b/websocket/hybi_test.go
index d6a1910..722ab31 100644
--- a/websocket/hybi_test.go
+++ b/websocket/hybi_test.go
@@ -326,7 +326,7 @@
}
payload := make([]byte, len(testPayload))
_, err = r.Read(payload)
- if err != nil {
+ if err != nil && err != io.EOF {
t.Errorf("read %v", err)
}
if !bytes.Equal(testPayload, payload) {
@@ -363,13 +363,20 @@
}

func TestHybiControlFrame(t *testing.T) {
- frameHeader := &hybiFrameHeader{Fin: true, OpCode: PingFrame}
payload := []byte("hello")
+
+ frameHeader := &hybiFrameHeader{Fin: true, OpCode: PingFrame}
testHybiFrame(t, []byte{0x89, 0x05}, payload, payload, frameHeader)
+
+ frameHeader = &hybiFrameHeader{Fin: true, OpCode: PingFrame}
+ testHybiFrame(t, []byte{0x89, 0x00}, nil, nil, frameHeader)

frameHeader = &hybiFrameHeader{Fin: true, OpCode: PongFrame}
testHybiFrame(t, []byte{0x8A, 0x05}, payload, payload, frameHeader)

+ frameHeader = &hybiFrameHeader{Fin: true, OpCode: PongFrame}
+ testHybiFrame(t, []byte{0x8A, 0x00}, nil, nil, frameHeader)
+
frameHeader = &hybiFrameHeader{Fin: true, OpCode: CloseFrame}
payload = []byte{0x03, 0xe8} // 1000
testHybiFrame(t, []byte{0x88, 0x02}, payload, payload, frameHeader)
diff --git a/websocket/websocket_test.go b/websocket/websocket_test.go
index 7841af2..c8a945f 100644
--- a/websocket/websocket_test.go
+++ b/websocket/websocket_test.go
@@ -24,7 +24,10 @@
var serverAddr string
var once sync.Once

-func echoServer(ws *Conn) { io.Copy(ws, ws) }
+func echoServer(ws *Conn) {
+ defer ws.Close()
+ io.Copy(ws, ws)
+}

type Count struct {
S string
@@ -32,6 +35,7 @@
}

func countServer(ws *Conn) {
+ defer ws.Close()
for {
var count Count
err := JSON.Receive(ws, &count)
@@ -43,6 +47,55 @@
err = JSON.Send(ws, count)
if err != nil {
return
+ }
+ }
+}
+
+type testCtrlAndDataHandler struct {
+ hybiFrameHandler
+}
+
+func (h *testCtrlAndDataHandler) WritePing(b []byte) (int, error) {
+ h.hybiFrameHandler.conn.wio.Lock()
+ defer h.hybiFrameHandler.conn.wio.Unlock()
+ w, err :=
h.hybiFrameHandler.conn.frameWriterFactory.NewFrameWriter(PingFrame)
+ if err != nil {
+ return 0, err
+ }
+ n, err := w.Write(b)
+ w.Close()
+ return n, err
+}
+
+func ctrlAndDataServer(ws *Conn) {
+ defer ws.Close()
+ h := &testCtrlAndDataHandler{hybiFrameHandler: hybiFrameHandler{conn: ws}}
+ ws.frameHandler = h
+
+ go func() {
+ for i := 0; ; i++ {
+ var b []byte
+ if i%2 != 0 { // with or without payload
+ b = []byte(fmt.Sprintf("#%d-CONTROL-FRAME-FROM-SERVER", i))
+ }
+ if _, err := h.WritePing(b); err != nil {
+ break
+ }
+ if _, err := h.WritePong(b); err != nil { // unsolicited pong
+ break
+ }
+ time.Sleep(10 * time.Millisecond)
+ }
+ }()
+
+ b := make([]byte, 128)
+ for {
+ n, err := ws.Read(b)
+ if err != nil {
+ break
+ }
+ if _, err := ws.Write(b[:n]); err != nil {
+ break
}
}
}
@@ -66,6 +119,7 @@
func startServer() {
http.Handle("/echo", Handler(echoServer))
http.Handle("/count", Handler(countServer))
+ http.Handle("/ctrldata", Handler(ctrlAndDataServer))
subproto := Server{
Handshake: subProtocolHandshake,
Handler: Handler(subProtoServer),
@@ -492,3 +546,42 @@
}
}
}
+
+func TestCtrlAndData(t *testing.T) {
+ once.Do(startServer)
+
+ c, err := net.Dial("tcp", serverAddr)
+ if err != nil {
+ t.Fatal(err)
+ }
+ ws, err := NewClient(newConfig(t, "/ctrldata"), c)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer ws.Close()
+
+ h := &testCtrlAndDataHandler{hybiFrameHandler: hybiFrameHandler{conn: ws}}
+ ws.frameHandler = h
+
+ b := make([]byte, 128)
+ for i := 0; i < 2; i++ {
+ var ctrl []byte
+ if i%2 != 0 { // with or without payload
+ ctrl = []byte(fmt.Sprintf("#%d-CONTROL-FRAME-FROM-CLIENT", i))
+ }
+ data := []byte(fmt.Sprintf("#%d-DATA-FRAME-FROM-CLIENT", i))
+ if _, err := ws.Write(data); err != nil {
+ t.Fatalf("#%d: %v", i, err)
+ }
+ if _, err := h.WritePing(ctrl); err != nil {
+ t.Fatalf("#%d: %v", i, err)
+ }
+ n, err := ws.Read(b)
+ if err != nil {
+ t.Fatalf("#%d: %v", i, err)
+ }
+ if !bytes.Equal(b[:n], data) {
+ t.Fatalf("#%d: got %v; want %v", i, b[:n], data)
+ }
+ }
+}

--
https://go-review.googlesource.com/13054
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

[Mikio Hara (Gerrit)]

Mikio Hara uploaded a new patch set:

https://go-review.googlesource.com/13054

websocket: handle solicited and unsolicited Ping/Pong frames correctly

This change makes Read not return io.EOF, io.ErrUnexpectedEOF on
exchanging both control frames such as ping and pong.

Fixes #6377.
Fixes #7825.
Fixes #10156.

Change-Id: I600cf493de3671d7e3d11e2e12d32f43928b7bfc
---
M websocket/hybi.go
M websocket/hybi_test.go
M websocket/websocket_test.go
3 files changed, 112 insertions(+), 13 deletions(-)

[Mikio Hara (Gerrit)]

Mikio Hara uploaded a new patch set:

https://go-review.googlesource.com/13054

websocket: handle solicited and unsolicited Ping/Pong frames correctly

This change makes Read not return io.EOF, io.ErrUnexpectedEOF on
exchanging both control frames such as ping and pong.

Fixes golang/go#6377.
Fixes golang/go#7825.
Fixes golang/go#10156.

Change-Id: I600cf493de3671d7e3d11e2e12d32f43928b7bfc
---
M websocket/hybi.go
M websocket/hybi_test.go
M websocket/websocket_test.go
3 files changed, 112 insertions(+), 13 deletions(-)

[Mikio Hara (Gerrit)]

Mikio Hara uploaded a new patch set:

https://go-review.googlesource.com/13054

websocket: handle solicited and unsolicited Ping/Pong frames correctly

This change prevents Read from failing with io.EOF, ErrNotImplemented on
exchanging control frames such as ping and pong.

Fixes golang/go#6377.
Fixes golang/go#7825.
Fixes golang/go#10156.

Change-Id: I600cf493de3671d7e3d11e2e12d32f43928b7bfc
---
M websocket/hybi.go
M websocket/hybi_test.go
M websocket/websocket_test.go
3 files changed, 112 insertions(+), 13 deletions(-)

[Andrew Gerrand (Gerrit)]

Andrew Gerrand has posted comments on this change.

websocket: handle solicited and unsolicited Ping/Pong frames correctly

Patch Set 4: Code-Review+2

--
https://go-review.googlesource.com/13054
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-HasComments: No

[Mikio Hara (Gerrit)]

Mikio Hara has submitted this change and it was merged.

websocket: handle solicited and unsolicited Ping/Pong frames correctly

This change prevents Read from failing with io.EOF, ErrNotImplemented on
exchanging control frames such as ping and pong.

Fixes golang/go#6377.
Fixes golang/go#7825.
Fixes golang/go#10156.

Change-Id: I600cf493de3671d7e3d11e2e12d32f43928b7bfc

Reviewed-on: https://go-review.googlesource.com/13054
Reviewed-by: Andrew Gerrand <a...@golang.org>

---
M websocket/hybi.go
M websocket/hybi_test.go
M websocket/websocket_test.go
3 files changed, 112 insertions(+), 13 deletions(-)

Approvals:
Andrew Gerrand: Looks good to me, approved

--
https://go-review.googlesource.com/13054
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Reviewer: Mikio Hara <mikioh...@gmail.com>