Merge pull request #84 from nhooyr/stable

Polish for stable release

Author: nhooyr

Diff for: .gitignore

@@ -5,24 +5,22 @@
 
 websocket is a minimal and idiomatic WebSocket library for Go.
 
-This library is not final and the API is subject to change.
-
 ## Install
 
 ```bash
-go get nhooyr.io/websocket@v0.2.0
+go get nhooyr.io/websocket@v1.0.0
 ```
 
 ## Features
 
 - Minimal and idiomatic API
-- Tiny codebase at 1400 lines
+- Tiny codebase at 1700 lines
 - First class context.Context support
 - Thorough tests, fully passes the [autobahn-testsuite](https://github.com/crossbario/autobahn-testsuite)
 - Zero dependencies outside of the stdlib for the core library
 - JSON and ProtoBuf helpers in the wsjson and wspb subpackages
-- High performance
-- Concurrent reads and writes out of the box
+- Highly optimized by default
+- Concurrent writes out of the box
 
 ## Roadmap
 
@@ -88,8 +86,9 @@ c.Close(websocket.StatusNormalClosure, "")
 - net.Conn is never exposed as WebSocket over HTTP/2 will not have a net.Conn.
 - Using net/http's Client for dialing means we do not have to reinvent dialing hooks
   and configurations like other WebSocket libraries
-- We do not support the compression extension because Go's compress/flate library is very memory intensive
-  and browsers do not handle WebSocket compression intelligently. See [#5](https://github.com/nhooyr/websocket/issues/5)
+- We do not support the deflate compression extension because Go's compress/flate library
+  is very memory intensive and browsers do not handle WebSocket compression intelligently.
+  See [#5](https://github.com/nhooyr/websocket/issues/5)
 
 ## Comparison
 
@@ -111,7 +110,7 @@ Just compare the godoc of
 
 The API for nhooyr/websocket has been designed such that there is only one way to do things
 which makes it easy to use correctly. Not only is the API simpler, the implementation is
-only 1400 lines whereas gorilla/websocket is at 3500 lines. That's more code to maintain,
+only 1700 lines whereas gorilla/websocket is at 3500 lines. That's more code to maintain,
  more code to test, more code to document and more surface area for bugs.
 
 The future of gorilla/websocket is also uncertain. See [gorilla/websocket#370](https://github.com/gorilla/websocket/issues/370).
@@ -121,11 +120,23 @@ also uses net/http's Client and ResponseWriter directly for WebSocket handshakes
 gorilla/websocket writes its handshakes to the underlying net.Conn which means
 it has to reinvent hooks for TLS and proxies and prevents support of HTTP/2.
 
-Some more advantages of nhooyr/websocket are that it supports concurrent reads,
-writes and makes it very easy to close the connection with a status code and reason.
+Some more advantages of nhooyr/websocket are that it supports concurrent writes and
+makes it very easy to close the connection with a status code and reason.
+
+nhooyr/websocket also responds to pings, pongs and close frames in a separate goroutine so that
+your application doesn't always need to read from the connection unless it expects a data message.
+gorilla/websocket requires you to constantly read from the connection to respond to control frames
+even if you don't expect the peer to send any messages.
+
+In terms of performance, the differences depend on your application code. nhooyr/websocket
+reuses buffers efficiently out of the box if you use the wsjson and wspb subpackages whereas
+gorilla/websocket does not. As mentioned above, nhooyr/websocket also supports concurrent
+writers out of the box.
 
-In terms of performance, the only difference is nhooyr/websocket is forced to use one extra
-goroutine for context.Context support. Otherwise, they perform identically.
+The only performance con to nhooyr/websocket is that uses two extra goroutines. One for
+reading pings, pongs and close frames async to application code and another to support
+context.Context cancellation. This costs 4 KB of memory which is cheap compared
+to the benefits.
 
 ### x/net/websocket
 

Diff for: README.md

@@ -76,12 +76,12 @@ func verifyClientRequest(w http.ResponseWriter, r *http.Request) error {
 }
 
 // Accept accepts a WebSocket handshake from a client and upgrades the
-// the connection to WebSocket.
+// the connection to a WebSocket.
 //
 // Accept will reject the handshake if the Origin domain is not the same as the Host unless
 // the InsecureSkipVerify option is set.
 //
-// The returned connection will be bound by r.Context(). Use c.Context() to change
+// The returned connection will be bound by r.Context(). Use conn.Context() to change
 // the bounding context.
 func Accept(w http.ResponseWriter, r *http.Request, opts AcceptOptions) (*Conn, error) {
 	c, err := accept(w, r, opts)
@@ -107,15 +107,15 @@ func accept(w http.ResponseWriter, r *http.Request, opts AcceptOptions) (*Conn,
 
 	hj, ok := w.(http.Hijacker)
 	if !ok {
-		err = xerrors.New("response writer must implement http.Hijacker")
+		err = xerrors.New("passed ResponseWriter does not implement http.Hijacker")
 		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return nil, err
 	}
 
 	w.Header().Set("Upgrade", "websocket")
 	w.Header().Set("Connection", "Upgrade")
 
-	handleKey(w, r)
+	handleSecWebSocketKey(w, r)
 
 	subproto := selectSubprotocol(r, opts.Subprotocols)
 	if subproto != "" {
@@ -163,7 +163,7 @@ func selectSubprotocol(r *http.Request, subprotocols []string) string {
 
 var keyGUID = []byte("258EAFA5-E914-47DA-95CA-C5AB0DC85B11")
 
-func handleKey(w http.ResponseWriter, r *http.Request) {
+func handleSecWebSocketKey(w http.ResponseWriter, r *http.Request) {
 	key := r.Header.Get("Sec-WebSocket-Key")
 	h := sha1.New()
 	h.Write([]byte(key))
@@ -185,5 +185,5 @@ func authenticateOrigin(r *http.Request) error {
 	if strings.EqualFold(u.Host, r.Host) {
 		return nil
 	}
-	return xerrors.Errorf("request origin %q is not authorized for host %q", origin, r.Host)
+	return xerrors.Errorf("request Origin %q is not authorized for Host %q", origin, r.Host)
 }

Diff for: accept.go

@@ -2,16 +2,14 @@
 
 source ci/lib.sh || exit 1
 
-mkdir -p profs
-
-go test --vet=off --run=^$ -bench=. \
-	-cpuprofile=profs/cpu \
-	-memprofile=profs/mem \
-	-blockprofile=profs/block \
-	-mutexprofile=profs/mutex \
+go test --vet=off --run=^$ -bench=. -o=ci/out/websocket.test \
+	-cpuprofile=ci/out/cpu.prof \
+	-memprofile=ci/out/mem.prof \
+	-blockprofile=ci/out/block.prof \
+	-mutexprofile=ci/out/mutex.prof \
 	.
 
 set +x
 echo
-echo "profiles are in ./profs
+echo "profiles are in ./ci/out/*.prof
 keep in mind that every profiler Go provides is enabled so that may skew the benchmarks"

Diff for: ci/bench/entrypoint.sh

@@ -7,5 +7,5 @@ source ci/lib.sh || exit 1
 	shellcheck ./**/*.sh
 )
 
-go vet -composites=false -lostcancel=false ./...
+go vet ./...
 go run golang.org/x/lint/golint -set_exit_status ./...

Diff for: ci/lint/entrypoint.sh

@@ -0,0 +1 @@
+*

Diff for: ci/out/.gitignore

@@ -2,8 +2,6 @@
 
 source ci/lib.sh || exit 1
 
-mkdir -p profs
-
 set +x
 echo
 echo "this step includes benchmarks for race detection and coverage purposes
@@ -12,15 +10,15 @@ accurate numbers"
 echo
 set -x
 
-go test -race -coverprofile=profs/coverage --vet=off -bench=. ./...
-go tool cover -func=profs/coverage
+go test -race -coverprofile=ci/out/coverage.prof --vet=off -bench=. ./...
+go tool cover -func=ci/out/coverage.prof
 
 if [[ $CI ]]; then
-	bash <(curl -s https://codecov.io/bash) -f profs/coverage
+	bash <(curl -s https://codecov.io/bash) -f ci/out/coverage.prof
 else
-	go tool cover -html=profs/coverage -o=profs/coverage.html
+	go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html
 
 	set +x
 	echo
-	echo "please open profs/coverage.html to see detailed test coverage stats"
+	echo "please open ci/out/coverage.html to see detailed test coverage stats"
 fi

Diff for: ci/test/entrypoint.sh

@@ -18,9 +18,9 @@ import (
 // DialOptions represents the options available to pass to Dial.
 type DialOptions struct {
 	// HTTPClient is the http client used for the handshake.
-	// Its Transport must use HTTP/1.1 and return writable bodies
-	// for WebSocket handshakes. This was introduced in Go 1.12.
-	// http.Transport does this all correctly.
+	// Its Transport must return writable bodies
+	// for WebSocket handshakes.
+	// http.Transport does this correctly beginning with Go 1.12.
 	HTTPClient *http.Client
 
 	// HTTPHeader specifies the HTTP headers included in the handshake request.
@@ -30,7 +30,7 @@ type DialOptions struct {
 	Subprotocols []string
 }
 
-// We use this key for all client requests as the Sec-WebSocket-Key header is useless.
+// We use this key for all client requests as the Sec-WebSocket-Key header doesn't do anything.
 // See https://stackoverflow.com/a/37074398/4283659.
 // We also use the same mask key for every message as it too does not make a difference.
 var secWebSocketKey = base64.StdEncoding.EncodeToString(make([]byte, 16))
@@ -108,7 +108,7 @@ func dial(ctx context.Context, u string, opts DialOptions) (_ *Conn, _ *http.Res
 
 	rwc, ok := resp.Body.(io.ReadWriteCloser)
 	if !ok {
-		return nil, resp, xerrors.Errorf("response body is not a read write closer: %T", rwc)
+		return nil, resp, xerrors.Errorf("response body is not a io.ReadWriteCloser: %T", rwc)
 	}
 
 	c := &Conn{

Diff for: dial.go

@@ -20,7 +20,7 @@ import (
 // dials the server and then sends 5 different messages
 // and prints out the server's responses.
 func Example_echo() {
-	// First we listen on port 0, that means the OS will
+	// First we listen on port 0 which means the OS will
 	// assign us a random free port. This is the listener
 	// the server will serve on and the client will connect to.
 	l, err := net.Listen("tcp", "localhost:0")
@@ -51,7 +51,6 @@ func Example_echo() {
 
 	// Now we dial the server, send the messages and echo the responses.
 	err = client("ws://" + l.Addr().String())
-	time.Sleep(time.Second)
 	if err != nil {
 		log.Fatalf("client failed: %v", err)
 	}

Diff for: example_echo_test.go

@@ -0,0 +1,34 @@
+package websocket
+
+import (
+	"fmt"
+	"io"
+
+	"golang.org/x/xerrors"
+)
+
+type limitedReader struct {
+	c     *Conn
+	r     io.Reader
+	left  int64
+	limit int64
+}
+
+func (lr *limitedReader) Read(p []byte) (int, error) {
+	if lr.limit == 0 {
+		lr.limit = lr.left
+	}
+
+	if lr.left <= 0 {
+		msg := fmt.Sprintf("read limited at %v bytes", lr.limit)
+		lr.c.Close(StatusPolicyViolation, msg)
+		return 0, xerrors.Errorf(msg)
+	}
+
+	if int64(len(p)) > lr.left {
+		p = p[:lr.left]
+	}
+	n, err := lr.r.Read(p)
+	lr.left -= int64(n)
+	return n, err
+}

Diff for: limitedreader.go

@@ -13,3 +13,5 @@ const (
 	// MessageBinary is for binary messages like Protobufs.
 	MessageBinary MessageType = MessageType(opBinary)
 )
+
+// Above I've explicitly included the types of the constants for stringer.

Diff for: messagetype.go

@@ -60,7 +60,7 @@ func parseClosePayload(p []byte) (CloseError, error) {
 	}
 
 	if len(p) < 2 {
-		return CloseError{}, xerrors.Errorf("close payload too small, cannot even contain the 2 byte status code")
+		return CloseError{}, xerrors.Errorf("close payload %q too small, cannot even contain the 2 byte status code", p)
 	}
 
 	ce := CloseError{
@@ -78,13 +78,13 @@ func parseClosePayload(p []byte) (CloseError, error) {
 // See http://www.iana.org/assignments/websocket/websocket.xhtml#close-code-number
 // and https://tools.ietf.org/html/rfc6455#section-7.4.1
 func validWireCloseCode(code StatusCode) bool {
-	if code >= StatusNormalClosure && code <= statusTLSHandshake {
-		switch code {
-		case 1004, StatusNoStatusRcvd, statusAbnormalClosure, statusTLSHandshake:
-			return false
-		default:
-			return true
-		}
+	switch code {
+	case 1004, StatusNoStatusRcvd, statusAbnormalClosure, statusTLSHandshake:
+		return false
+	}
+
+	if code >= StatusNormalClosure && code <= StatusBadGateway {
+		return true
 	}
 	if code >= 3000 && code <= 4999 {
 		return true