Merge pull request #187 from nhooyr/release-v1.8.0

Release v1.8.0

Makefile

@@ -2,12 +2,6 @@ all: fmt lint test
 
 .SILENT:
 
-.PHONY: *
-
-.ONESHELL:
-SHELL = bash
-.SHELLFLAGS = -ceuo pipefail
-
 include ci/fmt.mk
 include ci/lint.mk
 include ci/test.mk

README.md

@@ -1,9 +1,6 @@
 # websocket
 
-[![release](https://img.shields.io/github/v/release/nhooyr/websocket?color=6b9ded&sort=semver)](https://github.com/nhooyr/websocket/releases)
 [![godoc](https://godoc.org/nhooyr.io/websocket?status.svg)](https://godoc.org/nhooyr.io/websocket)
-[![coverage](https://img.shields.io/coveralls/github/nhooyr/websocket?color=65d6a4)](https://coveralls.io/github/nhooyr/websocket)
-[![ci](https://github.com/nhooyr/websocket/workflows/ci/badge.svg)](https://github.com/nhooyr/websocket/actions)
 
 websocket is a minimal and idiomatic WebSocket library for Go.
 
@@ -17,7 +14,8 @@ go get nhooyr.io/websocket
 
 - Minimal and idiomatic API
 - First class [context.Context](https://blog.golang.org/context) support
-- Thorough tests, fully passes the WebSocket [autobahn-testsuite](https://github.com/crossbario/autobahn-testsuite)
+- Fully passes the WebSocket [autobahn-testsuite](https://github.com/crossbario/autobahn-testsuite)
+- Thorough unit tests with [90% coverage](https://coveralls.io/github/nhooyr/websocket)
 - [Minimal dependencies](https://godoc.org/nhooyr.io/websocket?imports)
 - JSON and protobuf helpers in the [wsjson](https://godoc.org/nhooyr.io/websocket/wsjson) and [wspb](https://godoc.org/nhooyr.io/websocket/wspb) subpackages
 - Zero alloc reads and writes
@@ -111,8 +109,7 @@ Advantages of nhooyr.io/websocket:
   - Gorilla's implementation is slower and uses [unsafe](https://golang.org/pkg/unsafe/).
 - Full [permessage-deflate](https://tools.ietf.org/html/rfc7692) compression extension support
   - Gorilla only supports no context takeover mode
-  - Uses [klauspost/compress](https://github.com/klauspost/compress) for optimized compression
-  - See [gorilla/websocket#203](https://github.com/gorilla/websocket/issues/203)
+  - We use [klauspost/compress](https://github.com/klauspost/compress) for much lower memory usage ([gorilla/websocket#203](https://github.com/gorilla/websocket/issues/203))
 - [CloseRead](https://godoc.org/nhooyr.io/websocket#Conn.CloseRead) helper ([gorilla/websocket#492](https://github.com/gorilla/websocket/issues/492))
 - Actively maintained ([gorilla/websocket#370](https://github.com/gorilla/websocket/issues/370))
 

accept.go

@@ -6,14 +6,15 @@ import (
 	"bytes"
 	"crypto/sha1"
 	"encoding/base64"
+	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/textproto"
 	"net/url"
+	"strconv"
 	"strings"
 
-	"golang.org/x/xerrors"
-
 	"nhooyr.io/websocket/internal/errd"
 )
 
@@ -85,7 +86,7 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con
 
 	hj, ok := w.(http.Hijacker)
 	if !ok {
-		err = xerrors.New("http.ResponseWriter does not implement http.Hijacker")
+		err = errors.New("http.ResponseWriter does not implement http.Hijacker")
 		http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented)
 		return nil, err
 	}
@@ -110,7 +111,7 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con
 
 	netConn, brw, err := hj.Hijack()
 	if err != nil {
-		err = xerrors.Errorf("failed to hijack connection: %w", err)
+		err = fmt.Errorf("failed to hijack connection: %w", err)
 		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return nil, err
 	}
@@ -133,32 +134,32 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con
 
 func verifyClientRequest(w http.ResponseWriter, r *http.Request) (errCode int, _ error) {
 	if !r.ProtoAtLeast(1, 1) {
-		return http.StatusUpgradeRequired, xerrors.Errorf("WebSocket protocol violation: handshake request must be at least HTTP/1.1: %q", r.Proto)
+		return http.StatusUpgradeRequired, fmt.Errorf("WebSocket protocol violation: handshake request must be at least HTTP/1.1: %q", r.Proto)
 	}
 
 	if !headerContainsToken(r.Header, "Connection", "Upgrade") {
 		w.Header().Set("Connection", "Upgrade")
 		w.Header().Set("Upgrade", "websocket")
-		return http.StatusUpgradeRequired, xerrors.Errorf("WebSocket protocol violation: Connection header %q does not contain Upgrade", r.Header.Get("Connection"))
+		return http.StatusUpgradeRequired, fmt.Errorf("WebSocket protocol violation: Connection header %q does not contain Upgrade", r.Header.Get("Connection"))
 	}
 
 	if !headerContainsToken(r.Header, "Upgrade", "websocket") {
 		w.Header().Set("Connection", "Upgrade")
 		w.Header().Set("Upgrade", "websocket")
-		return http.StatusUpgradeRequired, xerrors.Errorf("WebSocket protocol violation: Upgrade header %q does not contain websocket", r.Header.Get("Upgrade"))
+		return http.StatusUpgradeRequired, fmt.Errorf("WebSocket protocol violation: Upgrade header %q does not contain websocket", r.Header.Get("Upgrade"))
 	}
 
 	if r.Method != "GET" {
-		return http.StatusMethodNotAllowed, xerrors.Errorf("WebSocket protocol violation: handshake request method is not GET but %q", r.Method)
+		return http.StatusMethodNotAllowed, fmt.Errorf("WebSocket protocol violation: handshake request method is not GET but %q", r.Method)
 	}
 
 	if r.Header.Get("Sec-WebSocket-Version") != "13" {
 		w.Header().Set("Sec-WebSocket-Version", "13")
-		return http.StatusBadRequest, xerrors.Errorf("unsupported WebSocket protocol version (only 13 is supported): %q", r.Header.Get("Sec-WebSocket-Version"))
+		return http.StatusBadRequest, fmt.Errorf("unsupported WebSocket protocol version (only 13 is supported): %q", r.Header.Get("Sec-WebSocket-Version"))
 	}
 
 	if r.Header.Get("Sec-WebSocket-Key") == "" {
-		return http.StatusBadRequest, xerrors.New("WebSocket protocol violation: missing Sec-WebSocket-Key")
+		return http.StatusBadRequest, errors.New("WebSocket protocol violation: missing Sec-WebSocket-Key")
 	}
 
 	return 0, nil
@@ -169,10 +170,10 @@ func authenticateOrigin(r *http.Request) error {
 	if origin != "" {
 		u, err := url.Parse(origin)
 		if err != nil {
-			return xerrors.Errorf("failed to parse Origin header %q: %w", origin, err)
+			return fmt.Errorf("failed to parse Origin header %q: %w", origin, err)
 		}
 		if !strings.EqualFold(u.Host, r.Host) {
-			return xerrors.Errorf("request Origin %q is not authorized for Host %q", origin, r.Host)
+			return fmt.Errorf("request Origin %q is not authorized for Host %q", origin, r.Host)
 		}
 	}
 	return nil
@@ -208,6 +209,7 @@ func acceptCompression(r *http.Request, w http.ResponseWriter, mode CompressionM
 
 func acceptDeflate(w http.ResponseWriter, ext websocketExtension, mode CompressionMode) (*compressionOptions, error) {
 	copts := mode.opts()
+	copts.serverMaxWindowBits = 8
 
 	for _, p := range ext.params {
 		switch p {
@@ -219,11 +221,31 @@ func acceptDeflate(w http.ResponseWriter, ext websocketExtension, mode Compressi
 			continue
 		}
 
-		if strings.HasPrefix(p, "client_max_window_bits") || strings.HasPrefix(p, "server_max_window_bits") {
+		if strings.HasPrefix(p, "client_max_window_bits") {
+			continue
+
+			// bits, ok := parseExtensionParameter(p, 15)
+			// if !ok || bits < 8 || bits > 16 {
+			// 	err := fmt.Errorf("invalid client_max_window_bits: %q", p)
+			// 	http.Error(w, err.Error(), http.StatusBadRequest)
+			// 	return nil, err
+			// }
+			// copts.clientMaxWindowBits = bits
+			// continue
+		}
+
+		if false && strings.HasPrefix(p, "server_max_window_bits") {
+			// We always send back 8 but make sure to validate.
+			bits, ok := parseExtensionParameter(p, 0)
+			if !ok || bits < 8 || bits > 16 {
+				err := fmt.Errorf("invalid server_max_window_bits: %q", p)
+				http.Error(w, err.Error(), http.StatusBadRequest)
+				return nil, err
+			}
 			continue
 		}
 
-		err := xerrors.Errorf("unsupported permessage-deflate parameter: %q", p)
+		err := fmt.Errorf("unsupported permessage-deflate parameter: %q", p)
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return nil, err
 	}
@@ -233,6 +255,21 @@ func acceptDeflate(w http.ResponseWriter, ext websocketExtension, mode Compressi
 	return copts, nil
 }
 
+// parseExtensionParameter parses the value in the extension parameter p.
+// It falls back to defaultVal if there is no value.
+// If defaultVal == 0, then ok == false if there is no value.
+func parseExtensionParameter(p string, defaultVal int) (int, bool) {
+	ps := strings.Split(p, "=")
+	if len(ps) == 1 {
+		if defaultVal > 0 {
+			return defaultVal, true
+		}
+		return 0, false
+	}
+	i, e := strconv.Atoi(strings.Trim(ps[1], `"`))
+	return i, e == nil
+}
+
 func acceptWebkitDeflate(w http.ResponseWriter, ext websocketExtension, mode CompressionMode) (*compressionOptions, error) {
 	copts := mode.opts()
 	// The peer must explicitly request it.
@@ -253,7 +290,7 @@ func acceptWebkitDeflate(w http.ResponseWriter, ext websocketExtension, mode Com
 		//
 		// Either way, we're only implementing this for webkit which never sends the max_window_bits
 		// parameter so we don't need to worry about it.
-		err := xerrors.Errorf("unsupported x-webkit-deflate-frame parameter: %q", p)
+		err := fmt.Errorf("unsupported x-webkit-deflate-frame parameter: %q", p)
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return nil, err
 	}

accept_js.go

@@ -1,9 +1,8 @@
 package websocket
 
 import (
+	"errors"
 	"net/http"
-
-	"golang.org/x/xerrors"
 )
 
 // AcceptOptions represents Accept's options.
@@ -16,5 +15,5 @@ type AcceptOptions struct {
 
 // Accept is stubbed out for Wasm.
 func Accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (*Conn, error) {
-	return nil, xerrors.New("unimplemented")
+	return nil, errors.New("unimplemented")
 }

accept_test.go

@@ -4,14 +4,13 @@ package websocket
 
 import (
 	"bufio"
+	"errors"
 	"net"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
-	"golang.org/x/xerrors"
-
 	"nhooyr.io/websocket/internal/test/assert"
 )
 
@@ -80,7 +79,7 @@ func TestAccept(t *testing.T) {
 		w := mockHijacker{
 			ResponseWriter: httptest.NewRecorder(),
 			hijack: func() (conn net.Conn, writer *bufio.ReadWriter, err error) {
-				return nil, nil, xerrors.New("haha")
+				return nil, nil, errors.New("haha")
 			},
 		}
 
@@ -328,6 +327,7 @@ func Test_acceptCompression(t *testing.T) {
 			expCopts: &compressionOptions{
 				clientNoContextTakeover: true,
 				serverNoContextTakeover: true,
+				serverMaxWindowBits:     8,
 			},
 		},
 		{

autobahn_test.go

@@ -15,8 +15,6 @@ import (
 	"testing"
 	"time"
 
-	"golang.org/x/xerrors"
-
 	"nhooyr.io/websocket"
 	"nhooyr.io/websocket/internal/errd"
 	"nhooyr.io/websocket/internal/test/assert"
@@ -108,7 +106,7 @@ func wstestClientServer(ctx context.Context) (url string, closeFn func(), err er
 		"exclude-cases": excludedAutobahnCases,
 	})
 	if err != nil {
-		return "", nil, xerrors.Errorf("failed to write spec: %w", err)
+		return "", nil, fmt.Errorf("failed to write spec: %w", err)
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute*15)
@@ -126,7 +124,7 @@ func wstestClientServer(ctx context.Context) (url string, closeFn func(), err er
 	wstest := exec.CommandContext(ctx, "wstest", args...)
 	err = wstest.Start()
 	if err != nil {
-		return "", nil, xerrors.Errorf("failed to start wstest: %w", err)
+		return "", nil, fmt.Errorf("failed to start wstest: %w", err)
 	}
 
 	return url, func() {
@@ -209,20 +207,20 @@ func unusedListenAddr() (_ string, err error) {
 func tempJSONFile(v interface{}) (string, error) {
 	f, err := ioutil.TempFile("", "temp.json")
 	if err != nil {
-		return "", xerrors.Errorf("temp file: %w", err)
+		return "", fmt.Errorf("temp file: %w", err)
 	}
 	defer f.Close()
 
 	e := json.NewEncoder(f)
 	e.SetIndent("", "\t")
 	err = e.Encode(v)
 	if err != nil {
-		return "", xerrors.Errorf("json encode: %w", err)
+		return "", fmt.Errorf("json encode: %w", err)
 	}
 
 	err = f.Close()
 	if err != nil {
-		return "", xerrors.Errorf("close temp file: %w", err)
+		return "", fmt.Errorf("close temp file: %w", err)
 	}
 
 	return f.Name(), nil

ci/ensure_fmt.sh

@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+main() {
+  local files
+  mapfile -t files < <(git ls-files --other --modified --exclude-standard)
+  if [[ ${files[*]} == "" ]]; then
+    return
+  fi
+
+  echo "Files need generation or are formatted incorrectly:"
+  for f in "${files[@]}"; do
+    echo "  $f"
+  done
+
+  echo
+  echo "Please run the following locally:"
+  echo "  make fmt"
+  exit 1
+}
+
+main "$@"

ci/fmt.mk

@@ -1,12 +1,6 @@
-fmt: modtidy gofmt goimports prettier
+fmt: modtidy gofmt goimports prettier shfmt
 ifdef CI
-	if [[ $$(git ls-files --other --modified --exclude-standard) != "" ]]; then
-	  echo "Files need generation or are formatted incorrectly:"
-	  git -c color.ui=always status | grep --color=no '\e\[31m'
-	  echo "Please run the following locally:"
-	  echo "  make fmt"
-	  exit 1
-	fi
+	./ci/ensure_fmt.sh
 endif
 
 modtidy: gen
@@ -23,3 +17,6 @@ prettier:
 
 gen:
 	stringer -type=opcode,MessageType,StatusCode -output=stringer.go
+
+shfmt:
+	shfmt -i 2 -w -s -sr $$(git ls-files "*.sh")

ci/image/Dockerfile

@@ -1,10 +1,12 @@
 FROM golang:1
 
 RUN apt-get update
-RUN apt-get install -y chromium npm
+RUN apt-get install -y chromium npm shellcheck
+
+ARG SHFMT_URL=https://github.com/mvdan/sh/releases/download/v3.0.1/shfmt_v3.0.1_linux_amd64
+RUN curl -L $SHFMT_URL > /usr/local/bin/shfmt && chmod +x /usr/local/bin/shfmt
 
 ENV GOFLAGS="-mod=readonly"
-ENV PAGER=cat
 ENV CI=true
 ENV MAKEFLAGS="--jobs=16 --output-sync=target"
 

ci/lint.mk

@@ -1,4 +1,4 @@
-lint: govet golint
+lint: govet golint govet-wasm golint-wasm shellcheck
 
 govet:
 	go vet ./...
@@ -11,3 +11,6 @@ golint:
 
 golint-wasm:
 	GOOS=js GOARCH=wasm golint -set_exit_status ./...
+
+shellcheck:
+	shellcheck $$(git ls-files "*.sh")

ci/test.mk

@@ -7,7 +7,6 @@ ci/out/coverage.html: gotest
 	go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html
 
 coveralls: gotest
-	# https://github.com/coverallsapp/github-action/blob/master/src/run.ts
 	echo "--- coveralls"
 	goveralls -coverprofile=ci/out/coverage.prof