From 76324844a9f23bdcf88e0537fcbf895b1add7e9c Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Wed, 23 Mar 2016 14:26:06 -0700 Subject: [PATCH] Fixed handling of unprompted PONG protocols - The number of outstanding PINGs is now reset whenever the server receives a PONG from the client. - Updated parser test to check c.pout. - Added a test to check for unprompted PONGs. Resolves issue: https://github.com/nats-io/gnatsd/issues/168 --- server/client.go | 2 +- server/parser_test.go | 15 +++++++++++--- test/ping_test.go | 46 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/server/client.go b/server/client.go index f3d9eefb..8a90194c 100644 --- a/server/client.go +++ b/server/client.go @@ -356,7 +356,7 @@ func (c *client) processPing() { func (c *client) processPong() { c.traceInOp("PONG", nil) c.mu.Lock() - c.pout-- + c.pout = 0 c.mu.Unlock() } diff --git a/server/parser_test.go b/server/parser_test.go index 06696b8e..63d0403a 100644 --- a/server/parser_test.go +++ b/server/parser_test.go @@ -93,10 +93,16 @@ func TestParsePong(t *testing.T) { if err != nil || c.state != OP_START { t.Fatalf("Unexpected: %d : %v\n", c.state, err) } + if c.pout != 0 { + t.Fatalf("Unexpected pout value: %d vs 0\n", c.pout) + } err = c.parse(pong) if err != nil || c.state != OP_START { t.Fatalf("Unexpected: %d : %v\n", c.state, err) } + if c.pout != 0 { + t.Fatalf("Unexpected pout value: %d vs 0\n", c.pout) + } // Should tolerate spaces pong = []byte("PONG \r") err = c.parse(pong) @@ -109,8 +115,11 @@ func TestParsePong(t *testing.T) { if err != nil || c.state != OP_START { t.Fatalf("Unexpected: %d : %v\n", c.state, err) } + if c.pout != 0 { + t.Fatalf("Unexpected pout value: %d vs 0\n", c.pout) + } - // Should be adjusting c.pout, Pings Outstanding + // Should be adjusting c.pout (Pings Outstanding): reset to 0 c.state = OP_START c.pout = 10 pong = []byte("PONG\r\n") @@ -118,8 +127,8 @@ func TestParsePong(t *testing.T) { if err != nil || c.state != OP_START { t.Fatalf("Unexpected: %d : %v\n", c.state, err) } - if c.pout != 9 { - t.Fatalf("Unexpected pout: %d vs %d\n", c.pout, 9) + if c.pout != 0 { + t.Fatalf("Unexpected pout: %d vs 0\n", c.pout) } } diff --git a/test/ping_test.go b/test/ping_test.go index a92bcff7..f259b8af 100644 --- a/test/ping_test.go +++ b/test/ping_test.go @@ -65,3 +65,49 @@ func TestPingInterval(t *testing.T) { t.Fatal("timeout: Expected to have connection closed") } } + +func TestUnpromptedPong(t *testing.T) { + s := runPingServer() + defer s.Shutdown() + + c := createClientConn(t, "localhost", PING_TEST_PORT) + defer c.Close() + + doConnect(t, c, false, false, false) + + expect := expectCommand(t, c) + + // Send lots of PONGs in a row... + for i := 0; i < 100; i++ { + c.Write([]byte("PONG\r\n")) + } + + // The server should still send the max number of PINGs and then + // close the connection. + for i := 0; i < PING_MAX; i++ { + time.Sleep(PING_INTERVAL / 2) + expect(pingRe) + } + + // We should get an error from the server + time.Sleep(PING_INTERVAL) + expect(errRe) + + // Server should close the connection at this point.. + c.SetWriteDeadline(time.Now().Add(PING_INTERVAL)) + var err error + for { + _, err = c.Write([]byte("PING\r\n")) + if err != nil { + break + } + } + c.SetWriteDeadline(time.Time{}) + + if err == nil { + t.Fatal("No error: Expected to have connection closed") + } + if ne, ok := err.(net.Error); ok && ne.Timeout() { + t.Fatal("timeout: Expected to have connection closed") + } +}