More test coverage and updated CONTRIBUTING.md

Author: nhooyr

ci/image/Dockerfile

@@ -6,8 +6,7 @@ ENV GOFLAGS="-mod=readonly"
 ENV PAGER=cat
 
 RUN apt-get update && \
-  apt-get install -y shellcheck python-pip npm && \
-  pip2 install autobahntestsuite && \
+  apt-get install -y shellcheck npm && \
   npm install -g prettier
 
-RUN git config --global color.ui always
+RUN git config --global color.ui always

docs/CONTRIBUTING.md

@@ -34,16 +34,21 @@ browse coverage.
 
 You can run CI locally. The various steps are located in `ci/*.sh`.
 
-1. `ci/fmt.sh` requires node (specifically prettier).
-1. `ci/lint.sh` requires [shellcheck](https://github.com/koalaman/shellcheck#installing).
-1. `ci/test.sh` requires the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite).
-1. `ci/run.sh` runs the above scripts in order.
+1. `ci/fmt.sh` which requires node (specifically prettier).
+1. `ci/lint.sh` which requires [shellcheck](https://github.com/koalaman/shellcheck#installing).
+1. `ci/test.sh`
+1. `ci/run.sh` which runs the above scripts in order.
 
 For coverage details locally, please see `ci/out/coverage.html` after running `ci/test.sh`.
 
 See [ci/image/Dockerfile](ci/image/Dockerfile) for the installation of the CI dependencies on Ubuntu.
 
-You can also run tests normally with `go test` once you have the
-[Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite)
-installed. `ci/test.sh` just passes a default set of flags to `go test` to collect coverage,
+You can also run tests normally with `go test`.
+`ci/test.sh` just passes a default set of flags to `go test` to collect coverage,
 enable the race detector, run benchmarks and also prettifies the output.
+
+If you pass flags to `ci/test.sh`, it will pass those flags directly to `go test` but will also
+collect coverage for you. This is nice for when you don't want to wait for benchmarks
+or the race detector but want to have coverage.
+
+Coverage percentage from codecov and the CI scripts will be different because they are calculated differently.

export_test.go

@@ -8,6 +8,13 @@ type Addr = websocketAddr
 
 type Header = header
 
+const OPClose = opClose
+const OPPing = opPing
+
 func (c *Conn) WriteFrame(ctx context.Context, fin bool, opcode opcode, p []byte) (int, error) {
 	return c.writeFrame(ctx, fin, opcode, p)
 }
+
+func (c *Conn) Flush() error {
+	return c.bw.Flush()
+}

websocket_test.go

@@ -4,8 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"github.com/golang/protobuf/proto"
-	"github.com/golang/protobuf/ptypes/timestamp"
 	"io"
 	"io/ioutil"
 	"math/rand"
@@ -23,8 +21,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/ptypes"
 	"github.com/golang/protobuf/ptypes/duration"
+	"github.com/golang/protobuf/ptypes/timestamp"
 	"github.com/google/go-cmp/cmp"
 	"golang.org/x/xerrors"
 
@@ -592,7 +592,7 @@ func TestConn(t *testing.T) {
 				return nil
 			},
 			client: func(ctx context.Context, c *websocket.Conn) error {
-				go c.CloseRead(ctx)
+				c.CloseRead(ctx)
 
 				err := c.Write(ctx, websocket.MessageBinary, []byte(strings.Repeat("x", 32769)))
 				if err != nil {
@@ -775,7 +775,7 @@ func TestConn(t *testing.T) {
 				}
 				<-ctx.Done()
 				_, err = r.Read(make([]byte, 1))
-				if !xerrors.Is(err, context.DeadlineExceeded){
+				if !xerrors.Is(err, context.DeadlineExceeded) {
 					return xerrors.Errorf("expected deadline exceeded error: %+v", err)
 				}
 				return nil
@@ -829,6 +829,152 @@ func TestConn(t *testing.T) {
 				return nil
 			},
 		},
+		{
+			name: "largeControlFrame",
+			server: func(ctx context.Context, c *websocket.Conn) error {
+				_, err := c.WriteFrame(ctx, true, websocket.OPClose, []byte(strings.Repeat("x", 4096)))
+				if err != nil {
+					return err
+				}
+				_, _, err = c.Read(ctx)
+				cerr := &websocket.CloseError{}
+				if !xerrors.As(err, cerr) || cerr.Code != websocket.StatusProtocolError {
+					return xerrors.Errorf("expected close error with StatusProtocolError: %+v", err)
+				}
+				return nil
+			},
+			client: func(ctx context.Context, c *websocket.Conn) error {
+				_, _, err := c.Read(ctx)
+				if err == nil || !strings.Contains(err.Error(), "too large") {
+					return xerrors.Errorf("expected error that contains too large: %+v", err)
+				}
+				return nil
+			},
+		},
+		{
+			name: "fragmentedControlFrame",
+			server: func(ctx context.Context, c *websocket.Conn) error {
+				_, err := c.WriteFrame(ctx, false, websocket.OPPing, []byte(strings.Repeat("x", 32)))
+				if err != nil {
+					return err
+				}
+				err = c.Flush()
+				if err != nil {
+					return err
+				}
+				_, _, err = c.Read(ctx)
+				cerr := &websocket.CloseError{}
+				if !xerrors.As(err, cerr) || cerr.Code != websocket.StatusProtocolError {
+					return xerrors.Errorf("expected close error with StatusProtocolError: %+v", err)
+				}
+				return nil
+			},
+			client: func(ctx context.Context, c *websocket.Conn) error {
+				_, _, err := c.Read(ctx)
+				if err == nil || !strings.Contains(err.Error(), "fragmented") {
+					return xerrors.Errorf("expected error that contains fragmented: %+v", err)
+				}
+				return nil
+			},
+		},
+		{
+			name: "invalidClosePayload",
+			server: func(ctx context.Context, c *websocket.Conn) error {
+				_, err := c.WriteFrame(ctx, true, websocket.OPClose, []byte{0x17, 0x70})
+				if err != nil {
+					return err
+				}
+				_, _, err = c.Read(ctx)
+				cerr := &websocket.CloseError{}
+				if !xerrors.As(err, cerr) || cerr.Code != websocket.StatusProtocolError {
+					return xerrors.Errorf("expected close error with StatusProtocolError: %+v", err)
+				}
+				return nil
+			},
+			client: func(ctx context.Context, c *websocket.Conn) error {
+				_, _, err := c.Read(ctx)
+				if err == nil || !strings.Contains(err.Error(), "invalid status code") {
+					return xerrors.Errorf("expected error that contains invalid status code: %+v", err)
+				}
+				return nil
+			},
+		},
+		{
+			name: "doubleReader",
+			server: func(ctx context.Context, c *websocket.Conn) error {
+				_, r, err := c.Reader(ctx)
+				if err != nil {
+					return err
+				}
+				p := make([]byte, 10)
+				_, err = io.ReadFull(r, p)
+				if err != nil {
+					return err
+				}
+				_, _, err = c.Reader(ctx)
+				if err == nil {
+					return xerrors.Errorf("expected non nil error: %v", err)
+				}
+				return nil
+			},
+			client: func(ctx context.Context, c *websocket.Conn) error {
+				err := c.Write(ctx, websocket.MessageBinary, []byte(strings.Repeat("x", 11)))
+				if err != nil {
+					return err
+				}
+				_, _, err = c.Read(ctx)
+				if err == nil {
+					return xerrors.Errorf("expected non nil error: %v", err)
+				}
+				return nil
+			},
+		},
+		{
+			name: "doubleFragmentedReader",
+			server: func(ctx context.Context, c *websocket.Conn) error {
+				_, r, err := c.Reader(ctx)
+				if err != nil {
+					return err
+				}
+				p := make([]byte, 10)
+				_, err = io.ReadFull(r, p)
+				if err != nil {
+					return err
+				}
+				_, _, err = c.Reader(ctx)
+				if err == nil {
+					return xerrors.Errorf("expected non nil error: %v", err)
+				}
+				return nil
+			},
+			client: func(ctx context.Context, c *websocket.Conn) error {
+				w, err := c.Writer(ctx, websocket.MessageBinary)
+				if err != nil {
+					return err
+				}
+				_, err = w.Write([]byte(strings.Repeat("x", 10)))
+				if err != nil {
+					return xerrors.Errorf("expected non nil error")
+				}
+				err = c.Flush()
+				if err != nil {
+					return xerrors.Errorf("failed to flush: %w", err)
+				}
+				_, err = w.Write([]byte(strings.Repeat("x", 10)))
+				if err != nil {
+					return xerrors.Errorf("expected non nil error")
+				}
+				err = c.Flush()
+				if err != nil {
+					return xerrors.Errorf("failed to flush: %w", err)
+				}
+				_, _, err = c.Read(ctx)
+				if err == nil {
+					return xerrors.Errorf("expected non nil error: %v", err)
+				}
+				return nil
+			},
+		},
 	}
 	for _, tc := range testCases {
 		tc := tc