From e4b74cffd77e89bc1dd44ed475c693d918e12cf3 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 13 Aug 2026 08:43:08 +0100 Subject: [PATCH] Apply configured conn limits in pooled transport (round 41) http_client_pool.go SharedTransportPool.GetOrCreateTransport built the shared http.Transport with hardcoded MaxConnsPerHost=5, MaxIdleConnsPerHost=2, MaxIdleConns=10, IdleConnTimeout=30s while HTTPClientConfig's versions of those fields drove configKey (cache identity) and were validated and tuned per preset (Default: 20/10/50/30s; OIDCProvider: 50/25/100/90s). The config was thus valid, keyed, and validated but silently ignored in the transport used by main.go (CreatePooledHTTPClient) - dead config that left OIDC-provider connection tuning without effect. The sibling factory path (http_client_factory.go) already applied these values. Now the pooled transport derives MaxConnsPerHost, MaxIdleConnsPerHost, MaxIdleConns, and IdleConnTimeout from the config, consistent with the factory path and with configKey. Test: added TestSharedTransportPoolAppliesConfigLimits, which fails on the old hardcoded code (fixed values) and passes now. Verified: full source-order suite, full -race -count=1 -shuffle=on ./... (all packages), golangci-lint run ./... (0 issues). --- http_client_pool.go | 8 ++++---- http_client_pool_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/http_client_pool.go b/http_client_pool.go index 6eaf622..8dc2932 100644 --- a/http_client_pool.go +++ b/http_client_pool.go @@ -121,11 +121,11 @@ func (p *SharedTransportPool) GetOrCreateTransport(config HTTPClientConfig) *htt ForceAttemptHTTP2: config.ForceHTTP2, TLSHandshakeTimeout: config.TLSHandshakeTimeout, ExpectContinueTimeout: config.ExpectContinueTimeout, - MaxIdleConns: 10, // SECURITY FIX: Further reduced - MaxIdleConnsPerHost: 2, // SECURITY FIX: Limited connections - IdleConnTimeout: 30 * time.Second, // Reduced from 5 minutes + MaxIdleConns: config.MaxIdleConns, + MaxIdleConnsPerHost: config.MaxIdleConnsPerHost, + IdleConnTimeout: config.IdleConnTimeout, DisableKeepAlives: config.DisableKeepAlives, - MaxConnsPerHost: 5, // SECURITY FIX: Strict limit + MaxConnsPerHost: config.MaxConnsPerHost, ResponseHeaderTimeout: config.ResponseHeaderTimeout, DisableCompression: config.DisableCompression, WriteBufferSize: config.WriteBufferSize, diff --git a/http_client_pool_test.go b/http_client_pool_test.go index 9bf96cd..f5ec70e 100644 --- a/http_client_pool_test.go +++ b/http_client_pool_test.go @@ -689,3 +689,38 @@ func TestSharedTransportPoolEdgeCases(t *testing.T) { assert.Equal(t, int32(0), finalCount, "client count should decrement on cleanup") }) } + +// TestSharedTransportPoolAppliesConfigLimits is a regression test: the pooled +// transport must apply the configured connection limits (MaxConnsPerHost, +// MaxIdleConnsPerHost, MaxIdleConns, IdleConnTimeout) rather than hardcoded +// constants. Previously GetOrCreateTransport built the transport with fixed +// values (5/2/10/30s) while configKey still incorporated these config fields, +// so the config was valid, keyed, and validated but silently ignored +// (dead config) — e.g. OIDCProviderHTTPClientConfig's tuning had no effect. +func TestSharedTransportPoolAppliesConfigLimits(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + pool := &SharedTransportPool{ + transports: make(map[string]*sharedTransport), + maxConns: 20, + clientCount: 0, + maxClients: 5, + ctx: ctx, + cancel: cancel, + } + + config := DefaultHTTPClientConfig() + config.MaxConnsPerHost = 37 + config.MaxIdleConnsPerHost = 13 + config.MaxIdleConns = 41 + config.IdleConnTimeout = 7 * time.Second + + transport := pool.GetOrCreateTransport(config) + require.NotNil(t, transport) + + assert.Equal(t, 37, transport.MaxConnsPerHost, "MaxConnsPerHost must come from config") + assert.Equal(t, 13, transport.MaxIdleConnsPerHost, "MaxIdleConnsPerHost must come from config") + assert.Equal(t, 41, transport.MaxIdleConns, "MaxIdleConns must come from config") + assert.Equal(t, 7*time.Second, transport.IdleConnTimeout, "IdleConnTimeout must come from config") +}