Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions api-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,28 @@ paths:
type: number
format: double
example: -82.9988
- name: bbox
in: query
required: false
description: >-
Restrict results to a rectangle, given as minLng,minLat,maxLng,maxLat.
Longitude first, matching GeoJSON and every mapping library, so a map
viewport can be passed straight through. A reversed or malformed box
is rejected with 400 rather than silently widening the search.
schema:
type: string
example: "-83.1,39.9,-82.9,40.1"
- name: polygon
in: query
required: false
description: >-
Restrict results to an arbitrary GeoJSON Polygon, for territories a
rectangle cannot express - a sales area, a delivery zone, a canvassing
route. The ring must be closed, with the last position repeating the
first.
schema:
type: string
example: '{"type":"Polygon","coordinates":[[[-83.1,39.9],[-82.9,39.9],[-82.9,40.1],[-83.1,40.1],[-83.1,39.9]]]}'
- name: radius
in: query
required: false
Expand Down
137 changes: 135 additions & 2 deletions handlers/address_handlers.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package handlers

import (
"encoding/json"
"fmt"
"geocoding-api/models"
"geocoding-api/services"
"math"
"net/http"
"strconv"

"geocoding-api/models"
"geocoding-api/services"

"github.com/labstack/echo/v4"
)

Expand Down Expand Up @@ -37,6 +40,33 @@ func SearchOhioAddressesHandler(c echo.Context) error {
params.Radius = val
}
}

// Territory filters. Unlike the numeric parameters above, a malformed
// value here is reported rather than ignored: silently dropping a bbox
// widens the search to the whole state, and a caller asking for one
// neighbourhood would get 50 arbitrary rows back and no indication why.
if raw := c.QueryParam("bbox"); raw != "" {
box, err := models.ParseBBox(raw)
if err != nil {
return c.JSON(http.StatusBadRequest, map[string]interface{}{
"success": false,
"error": err.Error(),
"example": "bbox=-83.1,39.9,-82.9,40.1",
})
}
params.BBox = box
}

if raw := c.QueryParam("polygon"); raw != "" {
if err := validateGeoJSONPolygon(raw); err != nil {
return c.JSON(http.StatusBadRequest, map[string]interface{}{
"success": false,
"error": err.Error(),
"example": `polygon={"type":"Polygon","coordinates":[[[-83.1,39.9],[-82.9,39.9],[-82.9,40.1],[-83.1,40.1],[-83.1,39.9]]]}`,
})
}
params.Polygon = raw
}
if limit := c.QueryParam("limit"); limit != "" {
if val, err := strconv.Atoi(limit); err == nil {
params.Limit = val
Expand Down Expand Up @@ -80,6 +110,15 @@ func SearchOhioAddressesHandler(c echo.Context) error {
filters["radius_km"] = params.Radius
}
}
if params.BBox != nil {
filters["bbox"] = params.BBox
}
if params.Polygon != "" {
// Echoed as a flag, not as the shape. A territory polygon can run to
// hundreds of vertices, and repeating it would dwarf the results the
// caller asked for.
filters["polygon"] = true
}

return c.JSON(http.StatusOK, models.AddressSearchResponse{
Success: true,
Expand Down Expand Up @@ -189,3 +228,97 @@ func FullTextSearchAddressesHandler(c echo.Context) error {

return c.JSON(http.StatusOK, response)
}

// validateGeoJSONPolygon checks the shape before it reaches PostGIS.
//
// ST_GeomFromGeoJSON raises on malformed input, which would surface as a 500
// on a request that is simply wrong -- a caller's typo should not look like a
// server fault. Checking here also keeps the error specific: "not a Polygon"
// is actionable, "failed to search addresses" is not.
func validateGeoJSONPolygon(raw string) error {
// Read the type before the coordinates. Decoding both at once makes a
// Point fail on its coordinates -- "cannot unmarshal number into
// .coordinates.0 of type [][]float64" -- which tells the caller nothing
// about the actual problem, that they sent the wrong geometry.
var header struct {
Type string `json:"type"`
Coordinates json.RawMessage `json:"coordinates"`
}
if err := json.Unmarshal([]byte(raw), &header); err != nil {
return fmt.Errorf("polygon is not valid GeoJSON: %v", err)
}

switch header.Type {
case "Polygon":
case "":
return fmt.Errorf(`polygon is missing its "type" field; expected {"type":"Polygon","coordinates":[...]}`)
default:
return fmt.Errorf("polygon type is %q; only Polygon is supported", header.Type)
}

var shape struct {
Coordinates [][][]float64
}
if err := json.Unmarshal(header.Coordinates, &shape.Coordinates); err != nil {
return fmt.Errorf("polygon coordinates are not an array of rings: %v", err)
}

if len(shape.Coordinates) == 0 {
return fmt.Errorf("polygon has no rings")
}

// Every ring, not just the exterior one. A Polygon whose first ring is
// well-formed but whose hole is short or unclosed would otherwise pass
// here and be rejected by GEOS inside ST_Intersects -- surfacing as a 500
// on a request that is merely wrong, which is the outcome this function
// exists to prevent.
total := 0
for r, ring := range shape.Coordinates {
where := "exterior ring"
if r > 0 {
where = fmt.Sprintf("hole %d", r)
}

if len(ring) < 4 {
return fmt.Errorf("%s needs at least 4 positions (the last repeating the first), got %d", where, len(ring))
}
total += len(ring)

first, last := ring[0], ring[len(ring)-1]
if len(first) < 2 || len(last) < 2 {
return fmt.Errorf("each position in the %s needs at least a longitude and a latitude", where)
}
if first[0] != last[0] || first[1] != last[1] {
return fmt.Errorf("%s is not closed: the last position must repeat the first", where)
}

for i, pos := range ring {
if len(pos) < 2 {
return fmt.Errorf("%s position %d is missing a coordinate", where, i)
}
if math.IsNaN(pos[0]) || math.IsNaN(pos[1]) || math.IsInf(pos[0], 0) || math.IsInf(pos[1], 0) {
return fmt.Errorf("%s position %d is not a finite coordinate", where, i)
}
if pos[0] < -180 || pos[0] > 180 {
return fmt.Errorf("%s position %d has longitude %g outside -180..180 (GeoJSON is longitude first)", where, i, pos[0])
}
if pos[1] < -90 || pos[1] > 90 {
return fmt.Errorf("%s position %d has latitude %g outside -90..90 (GeoJSON is longitude first)", where, i, pos[1])
}
}
}

// An exact point-in-polygon test runs per candidate row, so cost scales
// with vertices as well as with rows. A shape traced off a map can carry
// tens of thousands of them; bounding it keeps one request from costing
// seconds.
if total > maxPolygonVertices {
return fmt.Errorf("polygon has %d positions, more than the %d allowed; simplify the shape first",
total, maxPolygonVertices)
}

return nil
}

// maxPolygonVertices bounds how detailed a territory may be.
const maxPolygonVertices = 1000
101 changes: 101 additions & 0 deletions handlers/polygon_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package handlers

import (
"fmt"
"strings"
"testing"
)

// A malformed shape must be a 400 naming the problem, not a 500 from GEOS
// deep inside ST_Intersects. Holes are the gap that mattered: validating only
// coordinates[0] let a short or unclosed hole through.
func TestPolygonValidationRejectsMalformedShapes(t *testing.T) {
cases := []struct {
name string
raw string
want string
}{
{
name: "unclosed exterior ring",
raw: `{"type":"Polygon","coordinates":[[[-83.1,39.9],[-82.9,39.9],[-82.9,40.1]]]}`,
want: "exterior ring",
},
{
name: "short hole",
raw: `{"type":"Polygon","coordinates":[` +
`[[-83.1,39.9],[-82.9,39.9],[-82.9,40.1],[-83.1,39.9]],` +
`[[-83.0,40.0],[-82.95,40.0]]]}`,
want: "hole 1",
},
{
name: "unclosed hole",
raw: `{"type":"Polygon","coordinates":[` +
`[[-83.1,39.9],[-82.9,39.9],[-82.9,40.1],[-83.1,39.9]],` +
`[[-83.0,40.0],[-82.95,40.0],[-82.95,40.05],[-83.0,40.01]]]}`,
want: "hole 1",
},
{
// Only catchable when the swap pushes a value out of range. A
// latitude-first pair whose numbers both happen to be legal
// coordinates is indistinguishable from a deliberate query for
// somewhere else on Earth, and the validator does not pretend
// otherwise -- it reports the range, and names the convention so
// the caller can spot the swap themselves.
name: "coordinates out of range",
raw: `{"type":"Polygon","coordinates":[[[39.9,-183.1],[39.9,-82.9],[40.1,-82.9],[39.9,-183.1]]]}`,
want: "longitude first",
},
{
name: "wrong geometry type",
raw: `{"type":"Point","coordinates":[-83.0,40.0]}`,
want: "only Polygon",
},
{
name: "not json",
raw: `not json at all`,
want: "valid GeoJSON",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
err := validateGeoJSONPolygon(tc.raw)
if err == nil {
t.Fatalf("accepted; GEOS would reject it as a 500 instead of a 400")
}
if !strings.Contains(err.Error(), tc.want) {
t.Errorf("error %q does not mention %q, so it does not tell the caller what to fix", err, tc.want)
}
})
}
}

// An exact point-in-polygon test runs per candidate row, so an unbounded
// vertex count lets one request cost seconds.
func TestPolygonVertexCountIsBounded(t *testing.T) {
var b strings.Builder
b.WriteString(`{"type":"Polygon","coordinates":[[`)
n := maxPolygonVertices + 10
for i := 0; i < n; i++ {
if i > 0 {
b.WriteString(",")
}
fmt.Fprintf(&b, "[%f,%f]", -83.0+float64(i)*1e-6, 40.0)
}
b.WriteString(`,[-83.0,40.0]]]}`)

if err := validateGeoJSONPolygon(b.String()); err == nil {
t.Error("a polygon with more than the allowed vertices was accepted")
}
}

// A well-formed polygon with a hole has to still be accepted -- the point of
// validating every ring is correctness, not refusing legitimate shapes.
func TestPolygonWithValidHoleIsAccepted(t *testing.T) {
raw := `{"type":"Polygon","coordinates":[` +
`[[-83.1,39.9],[-82.9,39.9],[-82.9,40.1],[-83.1,40.1],[-83.1,39.9]],` +
`[[-83.05,39.95],[-82.95,39.95],[-82.95,40.05],[-83.05,40.05],[-83.05,39.95]]]}`
if err := validateGeoJSONPolygon(raw); err != nil {
t.Errorf("a valid polygon with a hole was rejected: %v", err)
}
}
76 changes: 74 additions & 2 deletions models/address.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package models

import (
"fmt"
"math"
"strconv"
"strings"
"time"
)

Expand Down Expand Up @@ -71,8 +75,18 @@ type AddressSearchParams struct {
Lat float64 `json:"lat" form:"lat"` // Latitude for proximity search
Lng float64 `json:"lng" form:"lng"` // Longitude for proximity search
Radius float64 `json:"radius" form:"radius"` // Radius in kilometers for proximity search
Limit int `json:"limit" form:"limit"` // Number of results to return (default: 50, max: 500)
Offset int `json:"offset" form:"offset"` // Offset for pagination

// BBox restricts results to a rectangle, as minLng,minLat,maxLng,maxLat --
// the order every mapping library emits, so a caller can pass a viewport
// straight through without reordering it.
BBox *BoundingBox `json:"bbox" form:"bbox"`

// Polygon restricts results to an arbitrary shape, given as GeoJSON. A
// radius is a circle and a bbox is a rectangle; a sales territory, a
// delivery zone or a canvassing walk is neither.
Polygon string `json:"polygon" form:"polygon"`
Limit int `json:"limit" form:"limit"` // Number of results to return (default: 50, max: 500)
Offset int `json:"offset" form:"offset"` // Offset for pagination
}

// AddressSearchResponse represents the response for address search
Expand All @@ -85,3 +99,61 @@ type AddressSearchResponse struct {
Query string `json:"query,omitempty"`
Filters map[string]any `json:"filters,omitempty"`
}

// BoundingBox is a rectangle in WGS84 degrees.
type BoundingBox struct {
MinLng float64 `json:"min_lng"`
MinLat float64 `json:"min_lat"`
MaxLng float64 `json:"max_lng"`
MaxLat float64 `json:"max_lat"`
}

// ParseBBox reads "minLng,minLat,maxLng,maxLat".
//
// Ordering is longitude-first because that is what GeoJSON, Leaflet, MapLibre
// and PostGIS all use. Latitude-first is the common mistake and it is silent:
// a swapped pair inside Ohio's range still parses and just returns nothing, so
// the bounds are validated rather than trusted.
func ParseBBox(raw string) (*BoundingBox, error) {
parts := strings.Split(raw, ",")
if len(parts) != 4 {
return nil, fmt.Errorf("bbox needs 4 comma-separated values (minLng,minLat,maxLng,maxLat), got %d", len(parts))
}

vals := make([]float64, 4)
for i, p := range parts {
v, err := strconv.ParseFloat(strings.TrimSpace(p), 64)
if err != nil {
return nil, fmt.Errorf("bbox value %d is not a number: %q", i+1, strings.TrimSpace(p))
}
vals[i] = v
}

// ParseFloat accepts "NaN" and "Inf", and every comparison against NaN is
// false -- so all three checks below pass and the value flows into
// ST_MakeEnvelope, producing either an empty 200 or a PostGIS error. That
// is exactly the silent-empty-result failure this validation exists to
// stop, so it has to be rejected before the range checks, not by them.
for i, v := range vals {
if math.IsNaN(v) {
return nil, fmt.Errorf("bbox value %d is NaN", i+1)
}
if math.IsInf(v, 0) {
return nil, fmt.Errorf("bbox value %d is infinite", i+1)
}
}

box := &BoundingBox{MinLng: vals[0], MinLat: vals[1], MaxLng: vals[2], MaxLat: vals[3]}

if box.MinLng < -180 || box.MaxLng > 180 || box.MinLat < -90 || box.MaxLat > 90 {
return nil, fmt.Errorf("bbox is outside valid coordinate ranges (longitude -180..180, latitude -90..90)")
}
if box.MinLng >= box.MaxLng {
return nil, fmt.Errorf("bbox min longitude (%g) must be less than max longitude (%g); values are minLng,minLat,maxLng,maxLat", box.MinLng, box.MaxLng)
}
if box.MinLat >= box.MaxLat {
return nil, fmt.Errorf("bbox min latitude (%g) must be less than max latitude (%g); values are minLng,minLat,maxLng,maxLat", box.MinLat, box.MaxLat)
}

return box, nil
}
Loading
Loading