Merge remote-tracking branch 'origin/master' into remove-net-http-wasm

Author: paralin

.github/FUNDING.yml

@@ -0,0 +1,24 @@
+version: 2
+updates:
+  # Track in case we ever add dependencies.
+  - package-ecosystem: 'gomod'
+    directory: '/'
+    schedule:
+      interval: 'weekly'
+    commit-message:
+      prefix: 'chore'
+
+  # Keep example and test/benchmark deps up-to-date.
+  - package-ecosystem: 'gomod'
+    directories:
+      - '/internal/examples'
+      - '/internal/thirdparty'
+    schedule:
+      interval: 'monthly'
+    commit-message:
+      prefix: 'chore'
+    labels: []
+    groups:
+      internal-deps:
+        patterns:
+          - '*'

.github/dependabot.yml

@@ -1,5 +1,11 @@
 name: ci
-on: [push, pull_request]
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
 concurrency:
   group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}
   cancel-in-progress: true
@@ -12,25 +18,33 @@ jobs:
       - uses: actions/setup-go@v5
         with:
           go-version-file: ./go.mod
-      - run: ./ci/fmt.sh
+      - run: make fmt
 
   lint:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - run: go version
       - uses: actions/setup-go@v5
-      - run: ./ci/lint.sh
+        with:
+          go-version-file: ./go.mod
+      - run: make lint
 
   test:
     runs-on: ubuntu-latest
     steps:
+      - name: Disable AppArmor
+        if: runner.os == 'Linux'
+        run: |
+          # Disable AppArmor for Ubuntu 23.10+.
+          # https://chromium.googlesource.com/chromium/src/+/main/docs/security/apparmor-userns-restrictions.md
+          echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
           go-version-file: ./go.mod
-      - run: ./ci/test.sh
-      - uses: actions/upload-artifact@v3
+      - run: make test
+      - uses: actions/upload-artifact@v4
         with:
           name: coverage.html
           path: ./ci/out/coverage.html
@@ -42,4 +56,4 @@ jobs:
       - uses: actions/setup-go@v5
         with:
           go-version-file: ./go.mod
-      - run: ./ci/bench.sh
+      - run: make bench

.github/workflows/ci.yml

@@ -15,16 +15,22 @@ jobs:
       - uses: actions/setup-go@v5
         with:
           go-version-file: ./go.mod
-      - run: AUTOBAHN=1 ./ci/bench.sh
+      - run: AUTOBAHN=1 make bench
   test:
     runs-on: ubuntu-latest
     steps:
+      - name: Disable AppArmor
+        if: runner.os == 'Linux'
+        run: |
+          # Disable AppArmor for Ubuntu 23.10+.
+          # https://chromium.googlesource.com/chromium/src/+/main/docs/security/apparmor-userns-restrictions.md
+          echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
           go-version-file: ./go.mod
-      - run: AUTOBAHN=1 ./ci/test.sh
-      - uses: actions/upload-artifact@v3
+      - run: AUTOBAHN=1 make test
+      - uses: actions/upload-artifact@v4
         with:
           name: coverage.html
           path: ./ci/out/coverage.html
@@ -37,18 +43,24 @@ jobs:
       - uses: actions/setup-go@v5
         with:
           go-version-file: ./go.mod
-      - run: AUTOBAHN=1 ./ci/bench.sh
+      - run: AUTOBAHN=1 make bench
   test-dev:
     runs-on: ubuntu-latest
     steps:
+      - name: Disable AppArmor
+        if: runner.os == 'Linux'
+        run: |
+          # Disable AppArmor for Ubuntu 23.10+.
+          # https://chromium.googlesource.com/chromium/src/+/main/docs/security/apparmor-userns-restrictions.md
+          echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns
       - uses: actions/checkout@v4
         with:
           ref: dev
       - uses: actions/setup-go@v5
         with:
           go-version-file: ./go.mod
-      - run: AUTOBAHN=1 ./ci/test.sh
-      - uses: actions/upload-artifact@v3
+      - run: AUTOBAHN=1 make test
+      - uses: actions/upload-artifact@v4
         with:
           name: coverage-dev.html
           path: ./ci/out/coverage.html

.github/workflows/daily.yml

@@ -22,6 +22,12 @@ jobs:
       url: ${{ steps.deployment.outputs.page_url }}
     runs-on: ubuntu-latest
     steps:
+      - name: Disable AppArmor
+        if: runner.os == 'Linux'
+        run: |
+          # Disable AppArmor for Ubuntu 23.10+.
+          # https://chromium.googlesource.com/chromium/src/+/main/docs/security/apparmor-userns-restrictions.md
+          echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns
       - name: Checkout
         uses: actions/checkout@v4
       - name: Setup Pages
@@ -32,7 +38,7 @@ jobs:
           go-version-file: ./go.mod
       - name: Generate coverage and badge
         run: |
-          ./ci/test.sh
+          make test
           mkdir -p ./ci/out/static
           cp ./ci/out/coverage.html ./ci/out/static/coverage.html
           percent=$(go tool cover -func ./ci/out/coverage.prof | tail -n1 | awk '{print $3}' | tr -d '%')

.github/workflows/static.yml

@@ -0,0 +1,18 @@
+.PHONY: all
+all: fmt lint test
+
+.PHONY: fmt
+fmt:
+	./ci/fmt.sh
+
+.PHONY: lint
+lint:
+	./ci/lint.sh
+
+.PHONY: test
+test:
+	./ci/test.sh
+
+.PHONY: bench
+bench:
+	./ci/bench.sh

Makefile

@@ -63,7 +63,9 @@ http.HandlerFunc(func (w http.ResponseWriter, r *http.Request) {
 	}
 	defer c.CloseNow()
 
-	ctx, cancel := context.WithTimeout(r.Context(), time.Second*10)
+	// Set the context as needed. Use of r.Context() is not recommended
+	// to avoid surprising behavior (see http.Hijacker).
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
 	defer cancel()
 
 	var v interface{}

README.md

@@ -5,6 +5,7 @@ package websocket
 
 import (
 	"bytes"
+	"context"
 	"crypto/sha1"
 	"encoding/base64"
 	"errors"
@@ -14,7 +15,7 @@ import (
 	"net/http"
 	"net/textproto"
 	"net/url"
-	"path/filepath"
+	"path"
 	"strings"
 
 	"github.com/coder/websocket/internal/errd"
@@ -41,8 +42,8 @@ type AcceptOptions struct {
 	// One would set this field to []string{"example.com"} to authorize example.com to connect.
 	//
 	// Each pattern is matched case insensitively against the request origin host
-	// with filepath.Match.
-	// See https://golang.org/pkg/path/filepath/#Match
+	// with path.Match.
+	// See https://golang.org/pkg/path/#Match
 	//
 	// Please ensure you understand the ramifications of enabling this.
 	// If used incorrectly your WebSocket server will be open to CSRF attacks.
@@ -62,6 +63,22 @@ type AcceptOptions struct {
 	// Defaults to 512 bytes for CompressionNoContextTakeover and 128 bytes
 	// for CompressionContextTakeover.
 	CompressionThreshold int
+
+	// OnPingReceived is an optional callback invoked synchronously when a ping frame is received.
+	//
+	// The payload contains the application data of the ping frame.
+	// If the callback returns false, the subsequent pong frame will not be sent.
+	// To avoid blocking, any expensive processing should be performed asynchronously using a goroutine.
+	OnPingReceived func(ctx context.Context, payload []byte) bool
+
+	// OnPongReceived is an optional callback invoked synchronously when a pong frame is received.
+	//
+	// The payload contains the application data of the pong frame.
+	// To avoid blocking, any expensive processing should be performed asynchronously using a goroutine.
+	//
+	// Unlike OnPingReceived, this callback does not return a value because a pong frame
+	// is a response to a ping and does not trigger any further frame transmission.
+	OnPongReceived func(ctx context.Context, payload []byte)
 }
 
 func (opts *AcceptOptions) cloneWithDefaults() *AcceptOptions {
@@ -79,6 +96,9 @@ func (opts *AcceptOptions) cloneWithDefaults() *AcceptOptions {
 // See the InsecureSkipVerify and OriginPatterns options to allow cross origin requests.
 //
 // Accept will write a response to w on all errors.
+//
+// Note that using the http.Request Context after Accept returns may lead to
+// unexpected behavior (see http.Hijacker).
 func Accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (*Conn, error) {
 	return accept(w, r, opts)
 }
@@ -96,7 +116,7 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con
 	if !opts.InsecureSkipVerify {
 		err = authenticateOrigin(r, opts.OriginPatterns)
 		if err != nil {
-			if errors.Is(err, filepath.ErrBadPattern) {
+			if errors.Is(err, path.ErrBadPattern) {
 				log.Printf("websocket: %v", err)
 				err = errors.New(http.StatusText(http.StatusForbidden))
 			}
@@ -105,7 +125,7 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con
 		}
 	}
 
-	hj, ok := w.(http.Hijacker)
+	hj, ok := hijacker(w)
 	if !ok {
 		err = errors.New("http.ResponseWriter does not implement http.Hijacker")
 		http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented)
@@ -153,6 +173,8 @@ func accept(w http.ResponseWriter, r *http.Request, opts *AcceptOptions) (_ *Con
 		client:         false,
 		copts:          copts,
 		flateThreshold: opts.CompressionThreshold,
+		onPingReceived: opts.OnPingReceived,
+		onPongReceived: opts.OnPongReceived,
 
 		br: brw.Reader,
 		bw: brw.Writer,
@@ -221,7 +243,7 @@ func authenticateOrigin(r *http.Request, originHosts []string) error {
 	for _, hostPattern := range originHosts {
 		matched, err := match(hostPattern, u.Host)
 		if err != nil {
-			return fmt.Errorf("failed to parse filepath pattern %q: %w", hostPattern, err)
+			return fmt.Errorf("failed to parse path pattern %q: %w", hostPattern, err)
 		}
 		if matched {
 			return nil
@@ -234,7 +256,7 @@ func authenticateOrigin(r *http.Request, originHosts []string) error {
 }
 
 func match(pattern, s string) (bool, error) {
-	return filepath.Match(strings.ToLower(pattern), strings.ToLower(s))
+	return path.Match(strings.ToLower(pattern), strings.ToLower(s))
 }
 
 func selectSubprotocol(r *http.Request, subprotocols []string) string {

accept.go

@@ -143,6 +143,33 @@ func TestAccept(t *testing.T) {
 		_, err := Accept(w, r, nil)
 		assert.Contains(t, err, `failed to hijack connection`)
 	})
+
+	t.Run("wrapperHijackerIsUnwrapped", func(t *testing.T) {
+		t.Parallel()
+
+		rr := httptest.NewRecorder()
+		w := mockUnwrapper{
+			ResponseWriter: rr,
+			unwrap: func() http.ResponseWriter {
+				return mockHijacker{
+					ResponseWriter: rr,
+					hijack: func() (conn net.Conn, writer *bufio.ReadWriter, err error) {
+						return nil, nil, errors.New("haha")
+					},
+				}
+			},
+		}
+
+		r := httptest.NewRequest("GET", "/", nil)
+		r.Header.Set("Connection", "Upgrade")
+		r.Header.Set("Upgrade", "websocket")
+		r.Header.Set("Sec-WebSocket-Version", "13")
+		r.Header.Set("Sec-WebSocket-Key", xrand.Base64(16))
+
+		_, err := Accept(w, r, nil)
+		assert.Contains(t, err, "failed to hijack connection")
+	})
+
 	t.Run("closeRace", func(t *testing.T) {
 		t.Parallel()
 
@@ -534,3 +561,14 @@ var _ http.Hijacker = mockHijacker{}
 func (mj mockHijacker) Hijack() (net.Conn, *bufio.ReadWriter, error) {
 	return mj.hijack()
 }
+
+type mockUnwrapper struct {
+	http.ResponseWriter
+	unwrap func() http.ResponseWriter
+}
+
+var _ rwUnwrapper = mockUnwrapper{}
+
+func (mu mockUnwrapper) Unwrap() http.ResponseWriter {
+	return mu.unwrap()
+}

accept_test.go

@@ -92,7 +92,7 @@ func TestAutobahn(t *testing.T) {
 		}
 	})
 
-	c, _, err := websocket.Dial(ctx, fmt.Sprintf(wstestURL+"/updateReports?agent=main"), nil)
+	c, _, err := websocket.Dial(ctx, wstestURL+"/updateReports?agent=main", nil)
 	assert.Success(t, err)
 	c.Close(websocket.StatusNormalClosure, "")