jub0bs / cors

perhaps the best CORS middleware library for Go
https://pkg.go.dev/github.com/jub0bs/cors
MIT License
44 stars 1 forks source link

Synchronization bug related to debug mode #5

Closed jub0bs closed 2 months ago

jub0bs commented 2 months ago

Method (*internalConfig)handleCORSPreflight performs an unsynchronized read access of the debug field of its receiver (here), a field which may be updated concurrently (due to concurrent calls to method (*Middleware).SetDebug). This synchronization bug can give rise to data races, as demonstrated by the following test:

package cors_test

import (
    "net/http"
    "net/http/httptest"
    "sync"
    "testing"

    "github.com/jub0bs/cors"
)

func TestSynchronizationBugRelatedToDebugMode(t *testing.T) {
    corsMw, err := cors.NewMiddleware(cors.Config{
        Origins: []string{"*"},
    })
    if err != nil {
        t.Fatal(err)
    }

    handler := corsMw.Wrap(handler)

    req := httptest.NewRequest(http.MethodOptions, "https://whatever", nil)
    req.Header.Add("Origin", "https://example.com")
    req.Header.Add("Access-Control-Request-Method", http.MethodGet)

    const n = 128
    var wg sync.WaitGroup
    for range n {
        wg.Add(1)
        go func() {
            defer wg.Done()
            rec := httptest.NewRecorder()
            handler.ServeHTTP(rec, req)
        }()
    }
    for range n {
        wg.Add(1)
        go func() {
            defer wg.Done()
            corsMw.SetDebug(false)
        }()
    }
    wg.Wait()
}

var handler http.Handler = http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})

Run that test with the race detector on:

go test -race -run ^TestSynchronizationBugRelatedToDebugMode$ github.com/jub0bs/cors

The output reports a data race:

==================
WARNING: DATA RACE
Write at 0x00c000130950 by goroutine 137:
  github.com/jub0bs/cors.(*Middleware).SetDebug()
      REDACTED/cors/middleware.go:486 +0x74
  github.com/jub0bs/cors_test.TestSynchronizationBugRelatedToDebugMode.func2()
      REDACTED/cors/race_test.go:40 +0x84

Previous read at 0x00c000130950 by goroutine 132:
  github.com/jub0bs/cors.(*internalConfig).handleCORSPreflight()
      REDACTED/cors/middleware.go:211 +0x34e
  github.com/jub0bs/cors_test.TestSynchronizationBugRelatedToDebugMode.(*Middleware).Wrap.func3()
      REDACTED/cors/middleware.go:133 +0x3ae
  net/http.HandlerFunc.ServeHTTP()
      REDACTED/src/net/http/server.go:2220 +0x1c6
  github.com/jub0bs/cors_test.TestSynchronizationBugRelatedToDebugMode.func1()
      REDACTED/cors/race_test.go:33 +0x1a0

Goroutine 137 (running) created at:
  github.com/jub0bs/cors_test.TestSynchronizationBugRelatedToDebugMode()
      REDACTED/cors/race_test.go:38 +0x716
  testing.tRunner()
      REDACTED/src/testing/testing.go:1690 +0x226
  testing.(*T).Run.gowrap1()
      REDACTED/src/testing/testing.go:1743 +0x44

Goroutine 132 (finished) created at:
  github.com/jub0bs/cors_test.TestSynchronizationBugRelatedToDebugMode()
      REDACTED/cors/race_test.go:30 +0x5ba
  testing.tRunner()
      REDACTED/src/testing/testing.go:1690 +0x226
  testing.(*T).Run.gowrap1()
      REDACTED/src/testing/testing.go:1743 +0x44
==================
--- FAIL: TestSynchronizationBugRelatedToDebugMode (0.00s)
    testing.go:1399: race detected during execution of test
FAIL
FAIL    github.com/jub0bs/cors  0.625s
FAIL
jub0bs commented 2 months ago

A fix for this will land in v0.3.0.