Skip to content

fix(geoloc): cap the bounding box area on GET /geoloc/entrances - #1813

Merged
ClemRz merged 2 commits into
developfrom
fix/geoloc-entrances-bbox-area-cap
Sep 22, 2026
Merged

ClemRz merged 2 commits into
developfrom
fix/geoloc-entrances-bbox-area-cap

Conversation

@ClemRz

@ClemRz ClemRz commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Rejects bounding boxes larger than 35 000 km² on GET /api/v1/geoloc/entrances with a 400 BBOX_AREA_EXCEEDED, before any query runs.

Why

  • One request could stall the whole API. An unbounded bounding box matches ~134 000 entrances. Postgres returns that set in ~1.3s, but the endpoint took 5–40s — the rest is Node marshalling and JSON-serialising 134k rows × 25 columns synchronously, on a single event loop (numberOfWorkers = 1).
  • The blocking is measurable on unrelated routes. Bucketing other endpoints by how many large geoloc requests shared the same minute: 0 large → p95 713ms; 3–5 large → p95 4441ms, with medians unchanged. Tail moves 6×, medians flat — head-of-line blocking, not database contention.
  • It was firing the Sev1 alert. Slow responses make clients give up; App Service counts aborted connections toward the Http5xx platform metric. 193 aborts in 4 days, 86 on this endpoint, while the application returned zero 500s.
  • Rate limiting does not cover it. rateLimiter.js allows 200 requests per 10 minutes per IP; the observed 3–5 world-scale requests per minute sits far inside that budget.
  • No production client is affected. All three callers verified in grottocenter-front: the web map fetches one z12 tile per request (67–95 km², ~370–520× headroom), MapMassif is gated on zoom ≥ 13 (~1 300 km²), and duplicate detection uses a 1 km radius (~4 km²). Server-side, grottocenter.org (12 530 requests) and the mobile app (1 224) have zero requests over the cap.

What

  • api/utils/computeBoundingBoxAreaKm2.js — exact spherical rectangle area, pure and synchronous.
  • GeoLocService.validateBoundingBoxArea() — returns null | { code, message }, mirroring MassifService.validatePolygon, and logs one warn on rejection (log-response.js would otherwise record a 400 only at verbose).
  • One call in find-entrances.js, placed before the massif lookup so an oversized box never pays for a database round trip first. Consequence: an oversized box plus an unknown massif id now returns 400 rather than 404 — pinned by a test.
  • Swagger: the 400 response, and a note that inverted longitudes are normalised rather than wrapped.
  • Tests: a unit suite and 7 fast-check properties for the helper, a service block, 10 route tests, and a regression block asserting the other 11 geoloc route names still accept world bounds.

Two details worth a reviewer's attention:

The cap is deliberately not in checkAndGetCoordinatesParams. That function is the first statement of all eight geoloc controllers, and the web app requests world bounds from four of the others on every map page load. Putting the check there would break the production map.

The area is not wrap-aware, on purpose. The query filters with planar ST_Within(point_geom, ST_MakeEnvelope(...)). Verified on PostGIS 3.4.3: ST_MakeEnvelope(170,-10,-170,10,4326) yields POLYGON((170 -10,170 10,-170 10,-170 -10,170 -10)), and ST_Within accepts POINT(0 0) while rejecting POINT(175 0) — a 340°-wide box of ~83.6M km², not a 20° strip. A wrap-aware Δλ would understate the most expensive requests by 17× and let them through. The area is also in km² rather than degrees², because the polar useNearbyEntrances box is 6.5 degrees² but only ~3 km² of real surface and must keep working.

Related

Follow-ups, not in this PR

  • MapMassif.jsx calls .wrap() per corner, so panning a massif map across ±180 produces |Δλ| ≈ 359.6° → 400. Degrades gracefully (markers vanish, no crash) and needs a massif straddling the antimeridian. Frontend issue.
  • Coordinate params are never validated as numeric, and ?sw_lat= coerces to 0. Affects all eight geoloc endpoints.
  • work_mem sort spill on PUBLIC_ENTRANCES_IN_BOUNDS — this cap shrinks the sort input and may remove it without server tuning.

The POST /massifs 400 description still cited 8 000 km², the value in
force before d33eb56 raised MassifService.MAX_AREA_KM2 to 35 000.
PUT /massifs/{id} was already correct.
An unbounded bounding box matched ~134 000 entrances. Postgres returned
that set in ~1.3s, but the endpoint took 5-40s: the remaining time was
Node marshalling and serialising 134k rows x 25 columns synchronously on
a single event loop, so every other request queued behind it. Bucketing
unrelated endpoints by concurrent large geoloc requests showed p95 rising
from 713ms to 4441ms with medians flat - head-of-line blocking, not
database contention. Clients then gave up, and App Service counted the
aborted connections toward Http5xx, firing the Sev1 alert.

- Reject a bounding box over 35 000 km² with 400 BBOX_AREA_EXCEEDED
- Compute the area in JS, not PostGIS: ST_Area(::geography) raises
  "Antipodal edge detected" on world bounds, the exact input to reject
- Use absolute, non-wrap-aware deltas, matching ST_MakeEnvelope's min/max
  normalisation and the planar ST_Within predicate the query uses; an
  inverted longitude covers the complement, not a 20° strip
- Check before the massif lookup, so an oversized box never pays for a
  database round trip before being rejected
- Keep the cap out of checkAndGetCoordinatesParams: all eight geoloc
  controllers share it, and four other endpoints serve world bounds to
  the map on every page load
- Shrink three route-test bounding boxes that exceeded the new limit

Closes #1810
@ClemRz
ClemRz requested a review from Paul-AUB September 21, 2026 23:33
@ClemRz ClemRz self-assigned this Sep 21, 2026

@Paul-AUB Paul-AUB left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the critical request path, bounding-box calculation, API contract, and focused test coverage. No must-fix issues found.

@ClemRz
ClemRz merged commit a499cc8 into develop Sep 22, 2026
5 checks passed
@ClemRz
ClemRz deleted the fix/geoloc-entrances-bbox-area-cap branch September 22, 2026 15:49

This branch was successfully deployed

1 active deployment
build — 8e976b6a Deployed Sep 21, 2026 by ClemRz via build-test #3863
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(api): unbounded bbox on GET /api/v1/geoloc/entrances blocks the event loop and degrades every route

2 participants