diff --git a/pkg/surrogate/providers/common.go b/pkg/surrogate/providers/common.go index 16d5f3f8f..5836b4d41 100644 --- a/pkg/surrogate/providers/common.go +++ b/pkg/surrogate/providers/common.go @@ -189,6 +189,19 @@ func containsCacheKey(currentValue, cacheKey string) bool { } func (s *baseStorage) storeTag(tag string, cacheKey string) { + if tag == "" { + // A response carrying none of the surrogate-key headers still reaches + // here with an empty tag: getSurrogateKey returns "" and ParseHeaders is + // strings.Split, which returns [""] for an empty input rather than an + // empty slice, so Store's loop runs once with an empty key. + // + // Indexing under "" collects every cache key of every response into the + // single SURROGATE_ entry, which this function then read-modify-writes + // while holding s.mu on every store. That makes each store O(number of + // cached objects) and degrades as the cache grows. + return + } + defer s.mu.Unlock() s.mu.Lock() currentValue := string(s.Storage.Get(surrogatePrefix + tag)) diff --git a/pkg/surrogate/providers/common_test.go b/pkg/surrogate/providers/common_test.go index 3a4b3d6a7..63e5b1006 100644 --- a/pkg/surrogate/providers/common_test.go +++ b/pkg/surrogate/providers/common_test.go @@ -6,6 +6,7 @@ import ( "strings" "sync" "testing" + "time" "github.com/darkweak/souin/configurationtypes" "github.com/darkweak/souin/pkg/storage" @@ -208,3 +209,32 @@ func TestContainsCacheKey(t *testing.T) { }) } } + +func TestBaseStorage_Store_NoSurrogateKeyHeaderDoesNotIndexUnderEmptyTag(t *testing.T) { + // A response carrying none of the surrogate-key headers must not be indexed + // at all under the empty tag. getSurrogateKey returns "" for such a + // response and ParseHeaders (strings.Split) turns that into [""], so Store + // still iterates once with an empty key. Writing that to SURROGATE_ funnels + // every cache key of every response into one entry, which storeTag + // read-modify-writes under a global mutex on each store. + res := http.Response{Header: http.Header{}} + bs := mockCommonProvider() + // The mock storer maps to a zero TTL, under which nothing is retained; give + // it a real one so the assertions below observe what was actually written. + bs.duration = time.Minute + + for i := 0; i < 50; i++ { + if e := bs.Store(&res, fmt.Sprintf("cache_key_%d", i), fmt.Sprintf("/uri/%d", i)); e != nil { + t.Errorf("It shouldn't throw an error: %v.", e) + } + } + + if v := bs.Storage.Get(surrogatePrefix); len(v) != 0 { + t.Errorf("The empty surrogate tag must stay empty, %q given.", string(v)) + } + + // The per-URI tags must still be written, so purge-by-URI keeps working. + if v := bs.Storage.Get(surrogatePrefix + "/uri/7"); !strings.Contains(string(v), "cache_key_7") { + t.Errorf("The URI tag must still index its cache key, %q given.", string(v)) + } +}