amadan
console feed: editions must be real HTTP dates; pointer size capped at write; unchanged editions record success (Astra code review R7)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+55
−4
internal/console/geoipfeed.go+8 −4internal/console/geoipfeed_test.go+42 −0internal/console/geoippublish.go+5 −0
CI passed — run details
diff --git a/internal/console/geoipfeed.go b/internal/console/geoipfeed.go| index 9adfb682e..46e7eb493 100644 |
| --- a/internal/console/geoipfeed.go |
| +++ b/internal/console/geoipfeed.go |
| @@ -198,7 +198,7 @@ func (f *geoipFeeder) checkDBIP(ctx context.Context, cur geoip.Pointer, ours boo |
| now := f.now().UTC() |
| this := time.Date(now.Year(), now.Month(), 1, 0, 0, 0, 0, time.UTC) |
| if ours && cur.Edition == this.Format("2006-01") { |
| - return "", nil |
| + return cur.Edition, nil // confirmed current: a success (code review R7-2) |
| } |
| months := []time.Time{this} |
| if !ours { |
| @@ -241,12 +241,16 @@ func (f *geoipFeeder) checkMaxMind(ctx context.Context, cur geoip.Pointer, ours |
| if head.StatusCode != http.StatusOK { |
| return "", feedFail("download") |
| } |
| - edition := head.Header.Get("Last-Modified") |
| - if edition == "" { |
| + // The edition is the provider's Last-Modified, but only as a real HTTP |
| + // date, re-rendered by us: an arbitrary header value must never reach |
| + // the pointer or feed.json (code review R7-1). |
| + lm, err := http.ParseTime(head.Header.Get("Last-Modified")) |
| + if err != nil { |
| return "", feedFail("download") |
| } |
| + edition := lm.UTC().Format(http.TimeFormat) |
| if ours && cur.Edition == edition { |
| - return "", nil |
| + return edition, nil // confirmed current: a success (code review R7-2) |
| } |
| sumRes, err := f.get(ctx, geoip.MaxMindURL(f.maxmindBase, f.spec, "tar.gz.sha256"), true) |
| if err != nil { |
diff --git a/internal/console/geoipfeed_test.go b/internal/console/geoipfeed_test.go| index bca90b89b..c7a133562 100644 |
| --- a/internal/console/geoipfeed_test.go |
| +++ b/internal/console/geoipfeed_test.go |
| @@ -363,3 +363,45 @@ func TestRetryClearFeedStatusRecoversWithoutARestart(t *testing.T) { |
| t.Fatalf("feed.json survived the retries: %v", err) |
| } |
| } |
| + |
| +// Code review R7-1: a Last-Modified that is not an HTTP date never becomes |
| +// an edition, and the published generation survives. |
| +func TestFeedRefusesAnInvalidEdition(t *testing.T) { |
| + withFixtureProbes(t) |
| + archive := tarGzBytes("x/GeoLite2-City.mmdb", fixture(t, "GeoIP2-City-Test.mmdb")) |
| + sum := sha256.Sum256(archive) |
| + var gets atomic.Int32 |
| + f, s := newTestFeeder(t, "maxmind", maxmindHandler(t, archive, hex.EncodeToString(sum[:]), strings.Repeat("x", 70<<10), &gets)) |
| + good, err := f.pub.Publish(context.Background(), bytes.NewReader(fixture(t, "GeoIP2-Country-Test.mmdb")), geoipPublishOpts{Origin: "manual"}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := f.Check(context.Background()); err == nil { |
| + t.Fatal("a junk Last-Modified was accepted") |
| + } |
| + if cur, _, _, _ := f.pub.Current(context.Background()); cur.SHA256 != good.SHA256 { |
| + t.Fatal("the usable generation was replaced") |
| + } |
| + if gets.Load() != 0 { |
| + t.Fatal("downloaded on a junk Last-Modified") |
| + } |
| + _ = s |
| +} |
| + |
| +// Code review R7-2: re-enabling a feed whose edition is already published |
| +// records a success, not "not run yet". |
| +func TestFeedUnchangedEditionIsASuccess(t *testing.T) { |
| + withFixtureProbes(t) |
| + body := gzipBytes(fixture(t, "GeoIP2-City-Test.mmdb")) |
| + f, s := newTestFeeder(t, "dbip", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write(body) })) |
| + if err := f.Check(context.Background()); err != nil { |
| + t.Fatal(err) |
| + } |
| + s.Delete(context.Background(), geoip.FeedKey) // disabled and cleared, then re-enabled |
| + if err := f.Check(context.Background()); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st := feedStatus(t, s); st.LastSuccess.IsZero() || st.LastEdition != "2026-10" { |
| + t.Fatalf("feed.json after an unchanged check = %+v", st) |
| + } |
| +} |
diff --git a/internal/console/geoippublish.go b/internal/console/geoippublish.go| index 165a9b4dc..2f78e2cf6 100644 |
| --- a/internal/console/geoippublish.go |
| +++ b/internal/console/geoippublish.go |
| @@ -264,6 +264,11 @@ func (p *geoipPublisher) Commit(ctx context.Context, st *geoipStaged, o geoipPub |
| if err != nil { |
| return geoip.Pointer{}, err |
| } |
| + if len(body) > geoip.MaxPointerBytes { |
| + // Every reader caps the pointer at this size: one larger would |
| + // publish a database no edge can ever read (code review R7-1). |
| + return geoip.Pointer{}, refused("the pointer would be larger than readers accept") |
| + } |
| if !present { |
| etag = "" |
| } |