diff --git a/README.md b/README.md index f5de104..dee9c85 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,7 @@ The pipeline runs normalisation, then redirects on the normalised path (followed {"action": "redirect", "status": 301, "location": "https://example.com/pages/b", "url": "https://example.com/pages/a", "locale": "", "staging": false, "seo": null} ``` -`status` is 302 when a temporary redirect takes part. A 410 rule answers `"action": "gone"`, `"status": 410`. A redirect cycle met at runtime (stored past the save check) stops after 10 hops with `not_found` and is logged once per URL. A path that no published record owns and no URL rule covers answers `"action": "not_found"`, `"status": 404` and `"seo": null` (the home page `/` is always known). A `url` that is neither a path nor an http(s) URL is a `422`. Absolute URLs are built from `golem15.seo.site_url`, or from the request URL when it names the site's host or its `www` twin (for the scheme and host rules), never from the request's Host header. +`status` is 302 when a temporary redirect takes part. A 410 rule answers `"action": "gone"`, `"status": 410`. A redirect cycle met at runtime (stored past the save check) stops after 10 hops with `not_found` and is logged at most once an hour per cycle (keyed by its rules, not by the requested URL). A path that no published record owns and no URL rule covers answers `"action": "not_found"`, `"status": 404` and `"seo": null` (the home page `/` is always known). A `url` that is neither a path nor an http(s) URL is a `422`. Absolute URLs are built from `golem15.seo.site_url`, or from the request URL when it names the site's host or its `www` twin (for the scheme and host rules), never from the request's Host header. ### The 404 report endpoint diff --git a/classes/redirect_graph_test.go b/classes/redirect_graph_test.go index 8117417..d87f643 100644 --- a/classes/redirect_graph_test.go +++ b/classes/redirect_graph_test.go @@ -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) + } +} diff --git a/classes/redirects.go b/classes/redirects.go index 5056759..f2cb3ff 100644 --- a/classes/redirects.go +++ b/classes/redirects.go @@ -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, " → ")) }