geoip: strip the whole X-Carlos-Geo-* namespace; source never empty; publish refuses expired candidates, verifies an existing blob, compares against the pointer read at stage (Astra plan R1-1/6/20, spec R7-1/2); spec rev 8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
7 files changed,
+256
−22
docs/superpowers/specs/2026-10-06-platform-geoip-design.md+44 −10internal/console/geoippublish.go+55 −7internal/console/geoippublish_test.go+110 −1internal/geo/mmdb.go+5 −0internal/geo/mmdb_test.go+9 −0internal/geoip/answer.go+12 −4internal/geoip/answer_test.go+21 −0
diff --git a/docs/superpowers/specs/2026-10-06-platform-geoip-design.md b/docs/superpowers/specs/2026-10-06-platform-geoip-design.md| index 264d59e6b..04d158f99 100644 |
| --- a/docs/superpowers/specs/2026-10-06-platform-geoip-design.md |
| +++ b/docs/superpowers/specs/2026-10-06-platform-geoip-design.md |
| @@ -1,7 +1,7 @@ |
| # Platform GeoIP for apps: headers, a mirrored database, a lookup endpoint |
| **Date:** 2026-10-06 |
| -**Revision:** 7 |
| +**Revision:** 8 |
| **Discussion:** amadan carlos/platform #241 (quoted in Appendix A) |
| **Branch:** `geoip-for-apps` |
| @@ -54,6 +54,12 @@ |
| - It takes **D2**, which reverses round 2's TLS-only tunnel trust. |
| - Uploads stage in a directory the console clears at startup. |
| - A box is retired explicitly, by deleting its edges record. |
| +- Rev 8 answers Astra's round 7 (`CODEX-FINDINGS-GEOIP-R7.md`), marked |
| + "(codex R7-N)": |
| + - an expired database is refused at publish; |
| + - a source can never sanitise to empty; |
| + - attributed sources are never folded into `_other`; |
| + - the spec's signatures now match the committed code. |
| ## 0. Rulings and decisions |
| @@ -278,13 +284,18 @@ by both the CLI verb and the feed: |
| `208.67.222.222` and `2001:4860:4860::8888`. Refuse the database unless |
| at least three of them return a country. That catches an ASN database or |
| a schema the decoder can't read. |
| -5. **Shrink check:** when the new database has the same `database_type` |
| +5. **Expiry check** (codex R7-1): refuse a database that D1 already |
| + counts as expired. `force` never overrides this. The check runs again |
| + immediately before the pointer is written, because a long upload can |
| + cross the expiry. A refusal leaves the previous pointer and blob |
| + untouched. |
| +6. **Shrink check:** when the new database has the same `database_type` |
| as the current pointer's, refuse it if it has fewer than half that |
| pointer's `node_count`. This guards against a truncated file. The check |
| is skipped when the types differ, because node counts across products |
| aren't comparable. A manual publish can override the check with |
| `force`. The feed can't (codex R2 on R1-14). |
| -6. Publish (§3). |
| +7. Publish (§3). |
| Plan 1, task 1 measures `Verify`'s peak memory on a real DB-IP City Lite |
| file and a real GeoLite2 City file, and records the result in the plan. |
| @@ -293,12 +304,15 @@ file's size. If the peak would push the console box past its memory |
| headroom, the plan says so and proposes a limit (codex R1-7). |
| **Streaming upload** (codex R1-8). `Store.Put` takes the whole body as |
| -`[]byte`. Plan 1 adds one optional interface: |
| +`[]byte`. Plan 1 adds one optional interface. Its body is an |
| +`io.ReadSeeker`, because the publish path always streams from its staged |
| +file, and the AWS SDK signs a seekable body without buffering it (codex |
| +R7-4): |
| ```go |
| -// release.PutStreamer is implemented by stores that can upload from a reader. |
| +// release.PutStreamer is implemented by stores that can upload from a seekable reader. |
| type PutStreamer interface { |
| - PutFrom(ctx context.Context, key string, body io.Reader, size int64, ifMatch string) (etag string, err error) |
| + PutFrom(ctx context.Context, key string, body io.ReadSeeker, size int64, ifMatch string) (etag string, err error) |
| } |
| ``` |
| @@ -607,12 +621,17 @@ type Result struct { |
| HasPoint bool // true only for a real location record, never a centroid |
| } |
| func (m *MMDB) Lookup(ip net.IP) (Result, bool) |
| -func (m *MMDB) Source() string // metadata.DatabaseType, reduced to [A-Za-z0-9._-], at most 64 bytes |
| -func (m *MMDB) BuildEpoch() uint |
| -func (m *MMDB) NodeCount() uint |
| +func (m *MMDB) Source() string // metadata.DatabaseType, reduced to [A-Za-z0-9._-], at most 64 bytes; "mmdb" if that leaves nothing |
| +func (m *MMDB) BuildEpoch() time.Time // UTC; the pointer stores BuildEpoch().Unix() |
| +func (m *MMDB) NodeCount() uint64 |
| func OpenMMDBTrusted(path string) (*MMDB, error) // opens without Verify |
| ``` |
| +- **The source is never empty** (codex R7-2). A database type that |
| + sanitises to nothing becomes `mmdb`, which satisfies the header grammar |
| + (§8.4). A loaded reader therefore always has a source that crosses a |
| + hop or the tunnel intact, and a local hit is never turned into `off` |
| + downstream. |
| - **Shared:** `decode(ip) (mmdbRecord, bool)` applies the `Locatable` |
| filter, `To4` normalisation and the fixed-category error log, and treats |
| a malformed location as a miss of the whole record. |
| @@ -840,6 +859,11 @@ show that, and only then publishes a database and the app docs. |
| and `geo_sources`. |
| - Both maps are bounded: countries by the ISO list plus `_unknown` and |
| `_other`, sources at 8 entries plus `_other`. |
| + - **A source with a known attribution** (§13: the `DBIP-` and |
| + `GeoLite2-` prefixes) is never folded into `_other`, whatever the cap. |
| + Only unattributed types fold (codex R7-3). Those prefixes cover a |
| + handful of product names, so the map stays small, and every credit a |
| + retained range owes stays readable. |
| - **Old and new days:** days written before plan 1 have no `geo_sources` |
| and an empty `countries`. They contribute nothing, and the read path |
| treats the missing field as empty. A mixed range tests this. |
| @@ -1172,7 +1196,7 @@ Plans 2 and 3 roll the console, and plan 2 also rolls the boxes for the |
| | Plan | Contents | |
| |---|---| |
| -| 1 | The `control/geoip/` lifecycle rule in `infra/modules/deployment-buckets` (applied later, §15); `geo.Lookup`/`OpenMMDBTrusted`/`Swappable`/`Snapshot`; `release.PutStreamer` and its wrappers; the console publish path and `carlos system geoip publish/status/off`; the edge mirror, its local layout and box status; capture/decide/expose ordering, ingress computing, adopting, `X-Carlos-Hop-Geo`, `setGeo`, the rejected-hop rule and the TLS-only tunnel mark; GeoLite2 expiry; analytics `Countries`/`GeoSources` through to the console table; operator and app docs | |
| +| 1 | The `control/geoip/` lifecycle rule in `infra/modules/deployment-buckets` (applied later, §15); `geo.Lookup`/`OpenMMDBTrusted`/`Swappable`/`Snapshot`; `release.PutStreamer` and its wrappers; the console publish path and `carlos system geoip publish/status/off`; the edge mirror, its local layout and box status; capture/decide/expose ordering, ingress computing, adopting, `X-Carlos-Hop-Geo`, `setGeo`, the rejected-hop rule and the transport-independent tunnel mark (D2); GeoLite2 expiry; analytics `Countries`/`GeoSources` through to the console table; operator and app docs | |
| | 2 | The feed (DB-IP and MaxMind sources, bounded input, integrity, type and monotonic checks, `feed.json`) and the `geoip` box rule | |
| | 3 | The console reader on the mirrored generation, the endpoint, `carlos geo` | |
| @@ -1269,6 +1293,16 @@ Round 6 (`CODEX-FINDINGS-GEOIP-R6.md`): every finding accepted. |
| | 1 | P2 | Accepted, through D2: the tunnel mark is transport-independent, so the hub's cache, the app's headers and the analytics always agree (§0, §8.2, §8.4, §9) | |
| | 2 | P2 | Accepted. A dedicated staging directory, cleared at console startup (§4.2) | |
| +Round 7 (`CODEX-FINDINGS-GEOIP-R7.md`): every finding accepted. |
| + |
| +| # | Sev | Disposition | |
| +|---|---|---| |
| +| R6-1 (partly) | | §16 now says the tunnel mark is transport-independent (D2) | |
| +| 1 | P2 | Accepted. An expired candidate is refused, with no force override, and rechecked before the pointer write (§4.1 step 5) | |
| +| 2 | P2 | Accepted. `Source` falls back to `mmdb` and is never empty (§6) | |
| +| 3 | P2 | Accepted. Attributed sources are never folded into `_other` (§9) | |
| +| 4 | P3 | Accepted. Signatures reconciled with the committed code (§4.1, §6) | |
| + |
| ## Appendix A. The request (#241, verbatim) |
| > **Expose the platform's GeoIP lookup to apps (headers and/or lookup endpoint)** |
diff --git a/internal/console/geoippublish.go b/internal/console/geoippublish.go| index 50935360a..ab564df6e 100644 |
| --- a/internal/console/geoippublish.go |
| +++ b/internal/console/geoippublish.go |
| @@ -45,6 +45,9 @@ type geoipPublisher struct { |
| maxBytes int64 |
| deadline time.Duration |
| now func() time.Time |
| + // expired is D1's rule, a field so a test can apply it to the GeoIP2 |
| + // fixtures (which never expire) without a GeoLite2 file in the repo. |
| + expired func(dbType string, build, now time.Time) bool |
| } |
| type geoipPublishOpts struct { |
| @@ -65,7 +68,7 @@ func newGeoipPublisher(store release.Store, tmpRoot string) *geoipPublisher { |
| } |
| } |
| return &geoipPublisher{store: store, tmpRoot: tmpRoot, maxBytes: geoip.MaxBlobBytes, |
| - deadline: 15 * time.Minute, now: time.Now} |
| + deadline: 15 * time.Minute, now: time.Now, expired: geoip.Expired} |
| } |
| func marshalPointer(p geoip.Pointer) ([]byte, error) { return json.MarshalIndent(p, "", " ") } |
| @@ -92,6 +95,14 @@ func (p *geoipPublisher) Current(ctx context.Context) (geoip.Pointer, string, bo |
| type geoipStaged struct { |
| dir, path, sha string |
| size int64 |
| + // The pointer as it stood when the slot was taken (spec §3 step 1): |
| + // every comparison is against this read, and the pointer write is |
| + // conditional on its etag, so a foreign write during the upload is |
| + // refused rather than overwritten. |
| + cur geoip.Pointer |
| + curEtag string |
| + curPresent bool |
| + curErr error |
| } |
| // Stage reserves the one publish slot BEFORE reading any of src, then |
| @@ -107,6 +118,11 @@ func (p *geoipPublisher) Stage(ctx context.Context, src io.Reader) (*geoipStaged |
| return nil, err |
| } |
| st := &geoipStaged{dir: dir, path: filepath.Join(dir, "db.mmdb")} |
| + st.cur, st.curEtag, st.curPresent, st.curErr = p.Current(ctx) |
| + if st.curErr != nil && !st.curPresent { |
| + p.Abandon(st) |
| + return nil, st.curErr // the store itself failed |
| + } |
| st.sha, st.size, err = copyCapped(ctx, st.path, src, p.maxBytes) |
| if err != nil { |
| p.Abandon(st) |
| @@ -149,10 +165,14 @@ func (p *geoipPublisher) Commit(ctx context.Context, st *geoipStaged, o geoipPub |
| if !hasCountryData(db) { |
| return geoip.Pointer{}, refused("the database has no country data") |
| } |
| - cur, etag, present, curErr := p.Current(ctx) |
| + // D1 at the door (codex R7-1): an already-expired GeoLite2 would replace |
| + // a usable database with one every edge must reject. force never |
| + // overrides it; the check runs again just before the pointer write. |
| + if p.expired(db.Source(), db.BuildEpoch(), p.now()) { |
| + return geoip.Pointer{}, refused("the database is already expired (GeoLite2, 30 days after build)") |
| + } |
| + cur, etag, present, curErr := st.cur, st.curEtag, st.curPresent, st.curErr |
| switch { |
| - case curErr != nil && !present: |
| - return geoip.Pointer{}, curErr // the store itself failed |
| case curErr != nil: |
| // A pointer exists but does not parse: replace it (etag still guards). |
| log.Printf("console: geoip publish: the current pointer is unreadable and will be replaced") |
| @@ -167,9 +187,15 @@ func (p *geoipPublisher) Commit(ctx context.Context, st *geoipStaged, o geoipPub |
| return geoip.Pointer{}, err |
| } |
| defer f.Close() |
| - // A CAS conflict on the blob means a blob with this exact name, so these |
| - // exact bytes, already exists: carry on (spec §3 step 2). |
| - if _, err := release.PutObjectFrom(ctx, p.store, next.Key, f, st.size, ""); err != nil && !errors.Is(err, release.ErrCASConflict) { |
| + // A CAS conflict on the blob means an object already sits at this |
| + // content-addressed name. It is only the same bytes if it hashes to the |
| + // name: a truncated or foreign object there must never be published |
| + // (spec §3 step 2, plan review R1-6). |
| + if _, err := release.PutObjectFrom(ctx, p.store, next.Key, f, st.size, ""); errors.Is(err, release.ErrCASConflict) { |
| + if err := p.verifyExistingBlob(ctx, next.Key, st.sha, st.size); err != nil { |
| + return geoip.Pointer{}, err |
| + } |
| + } else if err != nil { |
| return geoip.Pointer{}, err |
| } |
| body, err := marshalPointer(next) |
| @@ -179,6 +205,9 @@ func (p *geoipPublisher) Commit(ctx context.Context, st *geoipStaged, o geoipPub |
| if !present { |
| etag = "" |
| } |
| + if p.expired(next.DatabaseType, next.Build(), p.now()) { |
| + return geoip.Pointer{}, refused("the database expired while it was being published (GeoLite2, 30 days after build)") |
| + } |
| if _, err := p.store.Put(ctx, geoip.PointerKey, body, etag); err != nil { |
| return geoip.Pointer{}, fmt.Errorf("pointer write: %w", err) |
| } |
| @@ -226,6 +255,25 @@ func (c geoipCtxReader) Read(b []byte) (int, error) { |
| return c.r.Read(b) |
| } |
| +// verifyExistingBlob streams the object at key and checks it is exactly |
| +// sha/size. Rare (a republish of the same file), so the read is acceptable. |
| +func (p *geoipPublisher) verifyExistingBlob(ctx context.Context, key, sha string, size int64) error { |
| + rc, _, err := release.OpenObject(ctx, p.store, key) |
| + if err != nil { |
| + return fmt.Errorf("existing blob unreadable: %w", err) |
| + } |
| + defer rc.Close() |
| + h := sha256.New() |
| + n, err := io.Copy(h, io.LimitReader(rc, size+1)) |
| + if err != nil { |
| + return fmt.Errorf("existing blob unreadable: %w", err) |
| + } |
| + if n != size || hex.EncodeToString(h.Sum(nil)) != sha { |
| + return refused("a different object already sits at this database's blob key") |
| + } |
| + return nil |
| +} |
| + |
| func hasCountryData(db *geo.MMDB) bool { |
| n := 0 |
| for _, a := range geoipProbeAddrs { |
diff --git a/internal/console/geoippublish_test.go b/internal/console/geoippublish_test.go| index c055dbfa2..428f84bbc 100644 |
| --- a/internal/console/geoippublish_test.go |
| +++ b/internal/console/geoippublish_test.go |
| @@ -3,6 +3,8 @@ package console |
| import ( |
| "bytes" |
| "context" |
| + "crypto/sha256" |
| + "encoding/hex" |
| "errors" |
| "os" |
| "path/filepath" |
| @@ -110,7 +112,14 @@ func TestPublishShrinkCheckOnlyForTheSameType(t *testing.T) { |
| if _, err := p.Publish(ctx, bytes.NewReader(same), geoipPublishOpts{Origin: "manual", Force: true}); err != nil { |
| t.Fatalf("forced = %v", err) |
| } |
| - // A different type skips the check entirely. |
| + // A different type skips the check entirely. Re-inflate first, or the |
| + // City fixture (1547 nodes) would pass against Country's real 1704 even |
| + // if the check were wrongly applied across types (plan review R1-20). |
| + raw, etag, _ = s.Get(ctx, geoip.PointerKey) |
| + inflated = bytes.Replace(raw, []byte(`"node_count": 1704`), []byte(`"node_count": 99999`), 1) |
| + if _, err := s.Put(ctx, geoip.PointerKey, inflated, etag); err != nil { |
| + t.Fatal(err) |
| + } |
| if _, err := p.Publish(ctx, bytes.NewReader(fixture(t, "GeoIP2-City-Test.mmdb")), geoipPublishOpts{Origin: "manual"}); err != nil { |
| t.Fatalf("different type = %v", err) |
| } |
| @@ -243,3 +252,103 @@ func TestPublisherStartupClearsAbandonedStaging(t *testing.T) { |
| t.Fatalf("publish after restart: %v", err) |
| } |
| } |
| + |
| +func TestPublishRefusesAWrongObjectAtTheBlobKey(t *testing.T) { |
| + withFixtureProbes(t) |
| + ctx := context.Background() |
| + p, s := newTestPublisher(t) |
| + body := fixture(t, "GeoIP2-City-Test.mmdb") |
| + sum := sha256.Sum256(body) |
| + key := geoip.BlobKey(hex.EncodeToString(sum[:])) |
| + if _, err := s.Put(ctx, key, body[:len(body)/2], ""); err != nil { // a truncated object squatting the name |
| + t.Fatal(err) |
| + } |
| + if _, err := p.Publish(ctx, bytes.NewReader(body), geoipPublishOpts{Origin: "manual"}); !errors.Is(err, errGeoipRefused) { |
| + t.Fatalf("publish over a wrong object = %v, want refused", err) |
| + } |
| + if _, _, err := s.Get(ctx, geoip.PointerKey); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatal("the pointer moved onto a wrong blob") |
| + } |
| +} |
| + |
| +func TestPublishRefusesAnExpiredDatabaseEvenWithForce(t *testing.T) { |
| + withFixtureProbes(t) |
| + ctx := context.Background() |
| + p, s := newTestPublisher(t) |
| + good, err := p.Publish(ctx, bytes.NewReader(fixture(t, "GeoIP2-City-Test.mmdb")), geoipPublishOpts{Origin: "manual"}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + p.expired = func(string, time.Time, time.Time) bool { return true } |
| + _, err = p.Publish(ctx, bytes.NewReader(fixture(t, "GeoIP2-Country-Test.mmdb")), geoipPublishOpts{Origin: "manual", Force: true}) |
| + if !errors.Is(err, errGeoipRefused) { |
| + t.Fatalf("expired candidate with force = %v, want refused (codex R7-1)", err) |
| + } |
| + cur, _, _, _ := p.Current(ctx) |
| + if cur.SHA256 != good.SHA256 { |
| + t.Fatal("a refused expired candidate moved the pointer") |
| + } |
| + if keys := blobKeys(t, s); len(keys) != 1 || keys[0] != good.Key { |
| + t.Fatalf("blobs after refusal = %v, want only the previous good one", keys) |
| + } |
| +} |
| + |
| +func TestCommitRefusesAForeignPointerWrite(t *testing.T) { |
| + withFixtureProbes(t) |
| + ctx := context.Background() |
| + p, s := newTestPublisher(t) |
| + first, err := p.Publish(ctx, bytes.NewReader(fixture(t, "GeoIP2-City-Test.mmdb")), geoipPublishOpts{Origin: "manual"}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + st, err := p.Stage(ctx, bytes.NewReader(fixture(t, "GeoIP2-Country-Test.mmdb"))) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + // Something outside this console rewrites the pointer between staging |
| + // and commit (spec §3 step 3). |
| + raw, etag, _ := s.Get(ctx, geoip.PointerKey) |
| + if _, err := s.Put(ctx, geoip.PointerKey, append(raw, ' '), etag); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := p.Commit(ctx, st, geoipPublishOpts{Origin: "manual", Force: true}); err == nil { |
| + t.Fatal("commit over a foreign pointer write succeeded") |
| + } |
| + cur, _, _, _ := p.Current(ctx) |
| + if cur.SHA256 != first.SHA256 { |
| + t.Fatal("the foreign pointer was overwritten") |
| + } |
| + if !p.mu.TryLock() { |
| + t.Fatal("a failed commit must release the publish slot") |
| + } |
| + p.mu.Unlock() |
| +} |
| + |
| +func TestOffWaitsForAnInFlightPublish(t *testing.T) { |
| + withFixtureProbes(t) |
| + ctx := context.Background() |
| + p, s := newTestPublisher(t) |
| + st, err := p.Stage(ctx, bytes.NewReader(fixture(t, "GeoIP2-City-Test.mmdb"))) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + offDone := make(chan error, 1) |
| + go func() { offDone <- p.Off(ctx) }() |
| + select { |
| + case <-offDone: |
| + t.Fatal("off ran while a publish held the slot") |
| + case <-time.After(50 * time.Millisecond): |
| + } |
| + if _, err := p.Commit(ctx, st, geoipPublishOpts{Origin: "manual"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := <-offDone; err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, _, err := s.Get(ctx, geoip.PointerKey); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatal("off, serialised after the publish, must leave no pointer") |
| + } |
| + if keys := blobKeys(t, s); len(keys) != 0 { |
| + t.Fatalf("blobs after off = %v", keys) |
| + } |
| +} |
diff --git a/internal/geo/mmdb.go b/internal/geo/mmdb.go| index 406ce6de1..d547a79d9 100644 |
| --- a/internal/geo/mmdb.go |
| +++ b/internal/geo/mmdb.go |
| @@ -79,6 +79,11 @@ func sanitizeSource(s string) string { |
| b.WriteRune(c) |
| } |
| } |
| + if b.Len() == 0 { |
| + // Never empty (codex R7-2): a loaded reader's source must survive |
| + // the hop header's grammar, or a local hit becomes "off" downstream. |
| + return "mmdb" |
| + } |
| return b.String() |
| } |
diff --git a/internal/geo/mmdb_test.go b/internal/geo/mmdb_test.go| index 2a95670d5..4fa5a4b7f 100644 |
| --- a/internal/geo/mmdb_test.go |
| +++ b/internal/geo/mmdb_test.go |
| @@ -298,3 +298,12 @@ func TestSourceIsSanitised(t *testing.T) { |
| } |
| _ = time.Now |
| } |
| + |
| +func TestSourceNeverSanitisesToEmpty(t *testing.T) { |
| + if got := sanitizeSource("\n<>!!"); got != "mmdb" { |
| + t.Fatalf("sanitizeSource of only discarded characters = %q, want mmdb", got) |
| + } |
| + if got := sanitizeSource(""); got != "mmdb" { |
| + t.Fatalf("empty type = %q, want mmdb", got) |
| + } |
| +} |
diff --git a/internal/geoip/answer.go b/internal/geoip/answer.go| index cafe12c96..7350d2fc3 100644 |
| --- a/internal/geoip/answer.go |
| +++ b/internal/geoip/answer.go |
| @@ -65,12 +65,20 @@ func FromSnapshot(s geo.Snapshot) Answer { |
| return a |
| } |
| -// StripAll deletes every geo header an app or an edge could receive. |
| +// geoPrefix is the reserved namespace: spec §8.1 strips EVERY header in |
| +// it, not just the five this version writes, so a client cannot hand an app |
| +// an X-Carlos-Geo-City (or any later name) that looks platform-set. |
| +const geoPrefix = "X-Carlos-Geo-" |
| + |
| +// StripAll deletes every X-Carlos-Geo-* header and X-Carlos-Hop-Geo. |
| +// Header keys are canonicalised by net/http, but a map built by hand may |
| +// not be, so the match is case-insensitive. |
| func StripAll(h http.Header) { |
| - for _, k := range appHeaders { |
| - h.Del(k) |
| + for k := range h { |
| + if len(k) >= len(geoPrefix) && strings.EqualFold(k[:len(geoPrefix)], geoPrefix) || strings.EqualFold(k, HeaderHop) { |
| + delete(h, k) |
| + } |
| } |
| - h.Del(HeaderHop) |
| } |
| // WriteApp deletes the app headers and writes this answer's. A miss and |
diff --git a/internal/geoip/answer_test.go b/internal/geoip/answer_test.go| index ce7855d65..9ff42c005 100644 |
| --- a/internal/geoip/answer_test.go |
| +++ b/internal/geoip/answer_test.go |
| @@ -79,3 +79,24 @@ func TestAdoptRefusesWhatItCannotTrust(t *testing.T) { |
| t.Fatalf("partial = %+v", got) |
| } |
| } |
| + |
| +// Plan review R1-1: the whole reserved namespace goes, not just today's names. |
| +func TestStripAllRemovesTheWholeNamespace(t *testing.T) { |
| + h := http.Header{} |
| + h.Set("X-Carlos-Geo-City", "Atlantis") |
| + h.Set("X-Carlos-Geo-Country", "KP") |
| + h["x-carlos-geo-asn"] = []string{"1"} // non-canonical key, as a hand-built map might hold |
| + h.Set(HeaderHop, "hit;src=x") |
| + h.Set("X-Carlos-Version", "keep me") |
| + h.Set("X-Geo-Country", "keep me too") |
| + StripAll(h) |
| + if len(h) != 2 || h.Get("X-Carlos-Version") == "" || h.Get("X-Geo-Country") == "" { |
| + t.Fatalf("after strip = %v", h) |
| + } |
| + h2 := http.Header{} |
| + h2.Set("X-Carlos-Geo-City", "Atlantis") |
| + Capture(h2) |
| + if len(h2) != 0 { |
| + t.Fatalf("Capture must strip the namespace too: %v", h2) |
| + } |
| +} |