darkweak/souin

Surrogate keys not being set properly for Redis

p0358 opened this issue · 0 comments

It executes a query like this:

"SET" "SURROGATE_article1234" ",GET-https-www.example.com-%2Findex.php%3Fid%3D1" "EX" "0"

Resulting in error (and thus the key is not actually being set):

"ERR invalid expire time in 'set' command"

According to Redis docs, EX must be a positive integer, otherwise if we want infinite, EX must not be provided at all.

So I guess simplest workaround is that in this function, it shouldn't call the Ex() param if expiration time is 0 (and then it should discard the stale parameter too, or we run the risk of making something very short time instead of infinite!):

func (provider *Redis) Set(key string, value []byte, url t.URL, duration time.Duration) error {
err := provider.inClient.Do(provider.ctx, provider.inClient.B().Set().Key(key).Value(string(value)).Ex(duration+provider.stale).Build()).Error()
if err != nil {
provider.logger.Sugar().Errorf("Impossible to set value into Redis, %v", err)
}
return err
}

The call where it's set is (and 0 comes from infinite value for redis being defined this way at the top):

func (s *baseStorage) storeTag(tag string, cacheKey string, re *regexp.Regexp) {
defer s.mu.Unlock()
s.mu.Lock()
currentValue := string(s.Storage.Get(surrogatePrefix + tag))
if !re.MatchString(currentValue) {
s.logger.Sugar().Debugf("Store the tag %s", tag)
_ = s.Storage.Set(surrogatePrefix+tag, []byte(currentValue+souinStorageSeparator+cacheKey), configurationtypes.URL{}, s.duration)
}
}

Also, setting value as []byte(currentValue+souinStorageSeparator+cacheKey) even if currentValue is empty leads to an empty comma at the beginning of the value. It'd be better to only prepend currentValue+souinStorageSeparator if currentValue is non-empty.