diff --git a/internal/api/rate_limit_config_test.go b/internal/api/rate_limit_config_test.go index 6f81451df..d968f4a26 100644 --- a/internal/api/rate_limit_config_test.go +++ b/internal/api/rate_limit_config_test.go @@ -34,6 +34,7 @@ func TestGeneralAPIRateLimitDevelopmentOverride(t *testing.T) { } func TestGetRateLimiterForEndpoint(t *testing.T) { + isolateRateLimitFixture(t) // Ensure rate limiters are initialized InitializeRateLimiters() @@ -330,6 +331,7 @@ func identifyLimiter(rl *RateLimiter) string { } func TestGetRateLimiterForEndpoint_PriorityOrder(t *testing.T) { + isolateRateLimitFixture(t) // Test that more specific patterns match before general ones InitializeRateLimiters() @@ -376,12 +378,7 @@ func TestGetRateLimiterForEndpoint_PriorityOrder(t *testing.T) { } func TestGetRateLimiterForEndpoint_InitializesIfNeeded(t *testing.T) { - // Save and restore global state - saved := globalRateLimitConfig - globalRateLimitConfig = nil - t.Cleanup(func() { - globalRateLimitConfig = saved - }) + isolateRateLimitFixture(t) // Call GetRateLimiterForEndpoint with nil config - should initialize got := GetRateLimiterForEndpoint("/api/login", http.MethodPost) @@ -394,6 +391,7 @@ func TestGetRateLimiterForEndpoint_InitializesIfNeeded(t *testing.T) { } func TestUniversalRateLimitMiddleware_HeaderFormat(t *testing.T) { + isolateRateLimitFixture(t) handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }) @@ -441,12 +439,7 @@ func TestUniversalRateLimitMiddleware_HeaderFormat(t *testing.T) { } func TestUniversalRateLimitMiddleware_InitializesIfNeeded(t *testing.T) { - // Save and restore global state - saved := globalRateLimitConfig - globalRateLimitConfig = nil - t.Cleanup(func() { - globalRateLimitConfig = saved - }) + isolateRateLimitFixture(t) handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) @@ -475,7 +468,9 @@ func TestUniversalRateLimitMiddlewareWithConfig_UsesIndependentState(t *testing. }) firstConfig := newEndpointRateLimitConfig() + t.Cleanup(func() { stopRateLimitFixture(firstConfig) }) secondConfig := newEndpointRateLimitConfig() + t.Cleanup(func() { stopRateLimitFixture(secondConfig) }) first := UniversalRateLimitMiddlewareWithConfig(firstConfig, handler) second := UniversalRateLimitMiddlewareWithConfig(secondConfig, handler) @@ -511,6 +506,7 @@ func TestUniversalRateLimitMiddlewareWithConfig_UsesIndependentState(t *testing. } func TestUniversalRateLimitMiddleware_StaticAssetBypass(t *testing.T) { + isolateRateLimitFixture(t) InitializeRateLimiters() handlerCalled := false @@ -549,6 +545,7 @@ func TestUniversalRateLimitMiddleware_StaticAssetBypass(t *testing.T) { } func TestResetRateLimitForIP(t *testing.T) { + isolateRateLimitFixture(t) t.Run("nil globalRateLimitConfig does not panic", func(t *testing.T) { // Save current config and restore after test savedConfig := globalRateLimitConfig diff --git a/internal/api/rate_limit_fixture_cleanup_test.go b/internal/api/rate_limit_fixture_cleanup_test.go new file mode 100644 index 000000000..d071bfceb --- /dev/null +++ b/internal/api/rate_limit_fixture_cleanup_test.go @@ -0,0 +1,64 @@ +package api + +import "testing" + +func rateLimitFixtureWorkers(cfg *EndpointRateLimitConfig) []*RateLimiter { + if cfg == nil { + return nil + } + return []*RateLimiter{cfg.AuthEndpoints, cfg.ConfigEndpoints, cfg.ExportEndpoints, + cfg.RecoveryEndpoints, cfg.UpdateEndpoints, cfg.WebSocketEndpoints, cfg.GeneralAPI, cfg.PublicEndpoints} +} + +func stopRateLimitFixture(cfg *EndpointRateLimitConfig) { + for _, limiter := range rateLimitFixtureWorkers(cfg) { + limiter.Stop() + } +} + +// These fixtures mutate a package global and must remain sequential. Each +// replacement belongs to the fixture, not to the previous global owner. +func isolateRateLimitFixture(t *testing.T) { + t.Helper() + saved := globalRateLimitConfig + globalRateLimitConfig = nil + t.Cleanup(func() { + stopRateLimitFixture(globalRateLimitConfig) + globalRateLimitConfig = saved + }) +} + +// Exercise the actual fixtures rather than infer worker ownership from total +// process goroutine counts. No router, database or endpoint timing is involved. +func TestRateLimitInitializationFixturesStopWorkers(t *testing.T) { + for _, tc := range []struct { + name string + run func(*testing.T) + }{ + {"endpoint", TestGetRateLimiterForEndpoint_InitializesIfNeeded}, + {"middleware", TestUniversalRateLimitMiddleware_InitializesIfNeeded}, + } { + t.Run(tc.name, func(t *testing.T) { + saved := globalRateLimitConfig + var created *EndpointRateLimitConfig + t.Run("fixture", func(t *testing.T) { + tc.run(t) + created = globalRateLimitConfig + }) + if created == nil { + t.Fatal("fixture did not initialize rate limiters") + } + defer stopRateLimitFixture(created) // The failing proof must not leak either. + if globalRateLimitConfig != saved { + t.Error("fixture did not restore the previous configuration") + } + for i, limiter := range rateLimitFixtureWorkers(created) { + select { + case <-limiter.stopCleanup: + default: + t.Errorf("fixture left cleanup worker %d active", i) + } + } + }) + } +}