fix: the runtime cycle log is keyed by rules and bounded (WR-13)
redirectHopsLogged was a never-evicted map keyed by the anonymous request's URL with its query, so requests to a looping address with fresh query strings grew it without limit and logged each one. The log is now keyed by the cycle's rule ids, repeats at most once an hour and starts over at 1024 keys.
This commit is contained in:
@@ -203,3 +203,34 @@ func TestRedirectGraph(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestCycleLogBounded: the runtime cycle log is keyed by the cycle's rules,
|
||||
// not the request URL, and never holds more than maxCycleLog keys (WR-13).
|
||||
func TestCycleLogBounded(t *testing.T) {
|
||||
set := NewRedirectSet([]models.Redirect{rd(1, "/la", "/lb"), rd(2, "/lb", "/la")})
|
||||
ch, err := set.Follow(rdSite, "/la", "", MaxRedirectHops)
|
||||
if err != nil || !ch.Loop {
|
||||
t.Fatalf("chain = %+v, %v; want a loop", ch, err)
|
||||
}
|
||||
cycleLog.Lock()
|
||||
cycleLog.seen = nil
|
||||
cycleLog.Unlock()
|
||||
for i := 0; i < 5000; i++ {
|
||||
logRedirectCycle(rdSite+"/la?x="+strconv.Itoa(i), ch)
|
||||
}
|
||||
cycleLog.Lock()
|
||||
n := len(cycleLog.seen)
|
||||
cycleLog.Unlock()
|
||||
if n != 1 {
|
||||
t.Fatalf("cycle log keys after one cycle from 5000 URLs = %d, want 1", n)
|
||||
}
|
||||
for i := 0; i < maxCycleLog+10; i++ {
|
||||
logRedirectCycle("/x", Chain{Hops: []RedirectHit{{Rule: rd(uint(100+i), "/a", "/b")}}})
|
||||
}
|
||||
cycleLog.Lock()
|
||||
n = len(cycleLog.seen)
|
||||
cycleLog.Unlock()
|
||||
if n > maxCycleLog {
|
||||
t.Fatalf("cycle log keys = %d, want at most %d", n, maxCycleLog)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"git.golem15.com/golem15/sm-seo-plugin/models"
|
||||
"git.golem15.com/golem15/summercms/modules/diary"
|
||||
@@ -529,15 +530,54 @@ func internalTarget(site, target string) (string, string, bool) {
|
||||
return p, u.RawQuery, true
|
||||
}
|
||||
|
||||
// redirectHopsLogged keeps the URLs whose runtime redirect cycle was
|
||||
// logged, so each is logged once.
|
||||
var redirectHopsLogged sync.Map
|
||||
// cycleLog keeps when each runtime redirect cycle was last logged, keyed
|
||||
// by the rules it runs through (not by the request URL, which an anonymous
|
||||
// client controls), and holds at most maxCycleLog keys (WR-13).
|
||||
var cycleLog struct {
|
||||
sync.Mutex
|
||||
seen map[string]time.Time
|
||||
}
|
||||
|
||||
// logRedirectCycle logs a redirect cycle met at runtime, once per URL.
|
||||
// maxCycleLog bounds cycleLog; when it is full it starts over.
|
||||
const maxCycleLog = 1024
|
||||
|
||||
// cycleLogEvery is how often one cycle is logged again.
|
||||
const cycleLogEvery = time.Hour
|
||||
|
||||
// cycleKey identifies a cycle by the sorted ids of the rules it fired.
|
||||
func cycleKey(ch Chain) string {
|
||||
ids := make([]int, 0, len(ch.Hops))
|
||||
for _, h := range ch.Hops {
|
||||
ids = append(ids, int(h.Rule.ID))
|
||||
}
|
||||
sort.Ints(ids)
|
||||
var b strings.Builder
|
||||
for i, id := range ids {
|
||||
if i > 0 && ids[i-1] == id {
|
||||
continue
|
||||
}
|
||||
b.WriteString(strconv.Itoa(id))
|
||||
b.WriteByte(',')
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// logRedirectCycle logs a redirect cycle met at runtime, at most once an
|
||||
// hour per set of rules.
|
||||
func logRedirectCycle(url string, ch Chain) {
|
||||
if _, seen := redirectHopsLogged.LoadOrStore(url, true); seen {
|
||||
key := cycleKey(ch)
|
||||
now := time.Now()
|
||||
cycleLog.Lock()
|
||||
if cycleLog.seen == nil || len(cycleLog.seen) >= maxCycleLog {
|
||||
cycleLog.seen = map[string]time.Time{}
|
||||
}
|
||||
at, seen := cycleLog.seen[key]
|
||||
if seen && now.Sub(at) < cycleLogEvery {
|
||||
cycleLog.Unlock()
|
||||
return
|
||||
}
|
||||
cycleLog.seen[key] = now
|
||||
cycleLog.Unlock()
|
||||
log.Warn("golem15.seo: redirect cycle, answering not_found", "url", url, "path", strings.Join(ch.Visited, " → "))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user