Bound response cache memory and enforce result-size limits
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 10
- Forks
- 1
- PR merge metrics
- No merged PRs in 30d
Description
Problem
Three independent defects combine into a single denial-of-service path against the response cache and the database. They are filed together because fixing any one in isolation leaves the path open.
1. The response cache is bounded by entry count, not bytes
server.go:56
memory.AdapterWithCapacity(int(math.Pow(1000, 2))), // 1000^3 = gigabyte
The comment is wrong twice over. math.Pow(1000, 2) is 1,000,000, not 1000³; and in the pinned victorspringer/http-cache version, AdapterWithCapacity sets the maximum number of cached responses, not a byte budget. The library's byte-capacity option (AdapterWithStorageCapacity) is not used, and cache.ClientWithMaxBodySize is not set, so body size is unlimited.
The effective configuration permits one million cached entries of unbounded size, each holding a full uncompressed response body for a one-hour TTL. Several endpoints serve large GeoJSON payloads. The adapter also pre-allocates two maps sized to capacity at startup, and its LRU eviction is a linear scan under a write lock, so a full cache is a latency hazard as well as a memory one.
2. No maximum limit, and /bom/causes has no default limit at all
internal/datasets/bom/causes.go:165internal/datasets/bom/bills.go:307internal/datasets/pinkertons/pinkertons.go:287internal/datasets/bom/christenings.go:152
No endpoint enforces an upper bound on limit; ?limit=100000000 is accepted verbatim. Pinkertons rejects only values less than or equal to zero.
/bom/causes is worse: it emits a LIMIT clause only when the client supplies one, so a bare GET /bom/causes returns the entire bom.causes_of_death table joined across two more tables, with a COUNT(*) OVER() window over the whole result set.
Each response is then held in memory three times: the accumulated result slice, the full json.Marshal buffer, and the cache middleware's stored copy.
3. nocache is an unauthenticated cache-bypass
server.go:67
cache.ClientWithRefreshKey("nocache"),
When this key appears in the query string, the middleware releases the stored entry and re-executes the handler. Any anonymous caller can purge any URL's cache entry and force a full database round-trip, repeatedly.
Risk
Combined, these remove the cache as a protective buffer and turn cheap requests into unbounded full-table scans on demand. Cache keys include the full query string, so junk parameters mint unlimited distinct entries; with unbounded body size, resident memory is limited only by available RAM.
Acceptance criteria
- Set
memory.AdapterWithStorageCapacityto an explicit byte budget andcache.ClientWithMaxBodySizeto a per-response ceiling. - Reduce
AdapterWithCapacityto a realistic entry count and correct the misleading comment. - Add a shared limit helper in
internal/paramstaking a default and a maximum, returning400 Bad Requestabove the ceiling. - Apply the helper to the BOM bills, BOM causes, BOM christenings, and Pinkertons handlers.
- Give
/bom/causesa default limit so an unparameterized request cannot return the full table. - Decide the fate of
ClientWithRefreshKey("nocache"): remove it, or gate it behind a shared secret checked before the cache middleware runs. - Add tests covering the limit ceiling and the absent-limit default.
Notes
Removing or gating the refresh key conflicts with the final acceptance criterion of #126, which asks the cache benchmarks to exercise a ?nocache variant. Decide this issue first; if the key changes, the benchmark needs another mechanism for its uncached arm.
Found during the repository audit recorded in AUDIT.md (H-1, H-2, H-3).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the cache configuration in server.go and the handlers named in internal/datasets/bom/causes.go, bills.go, christenings.go, and internal/datasets/pinkertons/pinkertons.go. Review the shared cache and parameter APIs before deciding how to handle nocache and the benchmark conflict in #126. Done means bounded cache and result sizes, safe default and maximum limits, and tests for the ceiling and absent-limit behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend, databases, performance, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 32/100