From a0245fc0c8f5aaa9b1199325e2d07df6f361477b Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Fri, 24 Mar 2017 09:45:22 -0600 Subject: [PATCH] [FIXED] Server not sending PINGs to TSL connections (clients or routes) - Removed unnecessary cast check to (*net.TCPConn). When the timer fires, the connection is already established. Replaced with check that connection has not been closed. - Add PING test that checks that pings are sent to TLS connections. - Changed Go version to 1.7.5 in travis. - Removed test package from code coverage. --- .travis.yml | 2 +- scripts/cov.sh | 3 +-- server/client.go | 4 +-- test/ping_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index a471d0d8..92b22462 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go go: - 1.6.4 -- 1.7.4 +- 1.7.5 - 1.8 install: - go get github.com/nats-io/go-nats diff --git a/scripts/cov.sh b/scripts/cov.sh index c4e05f22..e51bc93f 100755 --- a/scripts/cov.sh +++ b/scripts/cov.sh @@ -7,8 +7,7 @@ go test -v -covermode=atomic -coverprofile=./cov/auth.out ./auth go test -v -covermode=atomic -coverprofile=./cov/conf.out ./conf go test -v -covermode=atomic -coverprofile=./cov/log.out ./logger go test -v -covermode=atomic -coverprofile=./cov/server.out ./server -go test -v -covermode=atomic -coverprofile=./cov/test.out ./test -go test -v -covermode=atomic -coverprofile=./cov/test2.out -coverpkg=./server ./test +go test -v -covermode=atomic -coverprofile=./cov/test.out -coverpkg=./server ./test gocovmerge ./cov/*.out > acc.out rm -rf ./cov diff --git a/server/client.go b/server/client.go index 5a64a79c..7199e185 100644 --- a/server/client.go +++ b/server/client.go @@ -1193,8 +1193,8 @@ func (c *client) processPingTimer() { c.mu.Lock() defer c.mu.Unlock() c.ptmr = nil - // Check if we are ready yet.. - if _, ok := c.nc.(*net.TCPConn); !ok { + // Check if connection is still opened + if c.nc == nil { return } diff --git a/test/ping_test.go b/test/ping_test.go index f259b8af..56f68d09 100644 --- a/test/ping_test.go +++ b/test/ping_test.go @@ -3,6 +3,8 @@ package test import ( + "crypto/tls" + "fmt" "net" "testing" "time" @@ -24,6 +26,68 @@ func runPingServer() *server.Server { return RunServer(&opts) } +func TestPingSentToTLSConnection(t *testing.T) { + opts := DefaultTestOptions + opts.Port = PING_TEST_PORT + opts.PingInterval = PING_INTERVAL + opts.MaxPingsOut = PING_MAX + opts.TLSCert = "configs/certs/server-cert.pem" + opts.TLSKey = "configs/certs/server-key.pem" + opts.TLSCaCert = "configs/certs/ca.pem" + + tc := server.TLSConfigOpts{} + tc.CertFile = opts.TLSCert + tc.KeyFile = opts.TLSKey + tc.CaFile = opts.TLSCaCert + + opts.TLSConfig, _ = server.GenTLSConfig(&tc) + s := RunServer(&opts) + defer s.Shutdown() + + c := createClientConn(t, "localhost", PING_TEST_PORT) + defer c.Close() + + checkInfoMsg(t, c) + c = tls.Client(c, &tls.Config{InsecureSkipVerify: true}) + tlsConn := c.(*tls.Conn) + tlsConn.Handshake() + + cs := fmt.Sprintf("CONNECT {\"verbose\":%v,\"pedantic\":%v,\"ssl_required\":%v}\r\n", false, false, true) + sendProto(t, c, cs) + + expect := expectCommand(t, c) + + // Expect the max to be delivered correctly.. + 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.. + time.Sleep(PING_INTERVAL) + 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") + } +} + func TestPingInterval(t *testing.T) { s := runPingServer() defer s.Shutdown()