diff --git a/review_r44_regression_test.go b/review_r44_regression_test.go new file mode 100644 index 0000000..7175632 --- /dev/null +++ b/review_r44_regression_test.go @@ -0,0 +1,38 @@ +package traefikoidc + +import ( + "sync" + "testing" + + "github.com/gorilla/sessions" +) + +// TestSessionDataRedirectCountConcurrency is a regression test for the redirect +// counter data race. GetRedirectCount / IncrementRedirectCount / +// ResetRedirectCount previously read and wrote sd.mainSession.Values without +// acquiring sd.sessionMutex, while other session methods (SetAuthenticated, +// SetIDToken, Save) write the same map under that lock. A SessionData can be +// returned to the object pool (Clear -> returnToPoolSafely) and reused by +// another goroutine while the original caller still mutates it, so concurrent +// lock-free map writes could occur. Run under -race: this failed on the old +// code (concurrent map writes), passes now. +func TestSessionDataRedirectCountConcurrency(t *testing.T) { + sd := &SessionData{ + sessionMutex: sync.RWMutex{}, + mainSession: &sessions.Session{Values: make(map[interface{}]interface{})}, + } + + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < 2000; j++ { + sd.IncrementRedirectCount() + sd.ResetRedirectCount() + _ = sd.GetRedirectCount() + } + }() + } + wg.Wait() +} diff --git a/session.go b/session.go index adf0cdd..e8541dc 100644 --- a/session.go +++ b/session.go @@ -2848,6 +2848,8 @@ func (sd *SessionData) SetIDToken(token string) { // Returns: // - The current redirect count, 0 if not set. func (sd *SessionData) GetRedirectCount() int { + sd.sessionMutex.RLock() + defer sd.sessionMutex.RUnlock() if count, ok := sd.mainSession.Values["redirect_count"].(int); ok { return count } @@ -2858,7 +2860,9 @@ func (sd *SessionData) GetRedirectCount() int { // STABILITY FIX: Prevents infinite redirect loops by tracking successive redirects. // Used to detect potential redirect loops and abort authentication if too many occur. func (sd *SessionData) IncrementRedirectCount() { - currentCount := sd.GetRedirectCount() + sd.sessionMutex.Lock() + defer sd.sessionMutex.Unlock() + currentCount := sd.getRedirectCountUnsafe() sd.mainSession.Values["redirect_count"] = currentCount + 1 sd.dirty = true } @@ -2867,6 +2871,8 @@ func (sd *SessionData) IncrementRedirectCount() { // STABILITY FIX: Prevents infinite redirect loops by clearing the counter // when authentication completes successfully or when starting a new flow. func (sd *SessionData) ResetRedirectCount() { + sd.sessionMutex.Lock() + defer sd.sessionMutex.Unlock() sd.mainSession.Values["redirect_count"] = 0 sd.dirty = true }