Harden redirect-count accessors against pooled-session data race (round 44)

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()
+}

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
 }