amadan
geo: a malformed GeoIP location is a miss (final review)
A structurally valid .mmdb can carry a nonfinite or off-globe location, and Locate handed it on as a point, so a semantically malformed database could rank regions from nowhere. Nonfinite values, a latitude outside [-90, 90] or a longitude outside [-180, 180] are now a miss (ok=false), with no fall-through to the record's country: a record wrong in one field is not trusted on the next. The poles and the antimeridian stay in.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+59
−1
internal/geo/mmdb.go+19 −1internal/geo/mmdb_test.go+40 −0
diff --git a/internal/geo/mmdb.go b/internal/geo/mmdb.go| index 563618834..c9cc6250b 100644 |
| --- a/internal/geo/mmdb.go |
| +++ b/internal/geo/mmdb.go |
| @@ -2,6 +2,7 @@ package geo |
| import ( |
| "log" |
| + "math" |
| "net" |
| "sync" |
| "time" |
| @@ -85,7 +86,16 @@ func (m *MMDB) Locate(ip net.IP) (Point, bool) { |
| return Point{}, false |
| } |
| if rec.Location.Latitude != nil && rec.Location.Longitude != nil { |
| - return Point{*rec.Location.Latitude, *rec.Location.Longitude}, true |
| + p := Point{*rec.Location.Latitude, *rec.Location.Longitude} |
| + if !onTheGlobe(p) { |
| + // A structurally valid database can still carry a malformed |
| + // location (final review). It is a miss, not a point that |
| + // ranks regions from nowhere, and not a fall-through to the |
| + // record's country: a record wrong in one field is not |
| + // trusted on the next. |
| + return Point{}, false |
| + } |
| + return p, true |
| } |
| if p, ok := CountryPoint(rec.Country.ISOCode); ok { |
| return p, true |
| @@ -93,6 +103,14 @@ func (m *MMDB) Locate(ip net.IP) (Point, bool) { |
| return CountryPoint(rec.RegisteredCountry.ISOCode) |
| } |
| +// onTheGlobe is false for nonfinite coordinates, a latitude outside |
| +// [-90, 90] or a longitude outside [-180, 180]. Both bounds are inclusive: |
| +// the poles and the antimeridian are places. |
| +func onTheGlobe(p Point) bool { |
| + finite := func(f float64) bool { return !math.IsNaN(f) && !math.IsInf(f, 0) } |
| + return finite(p.Lat) && finite(p.Lon) && p.Lat >= -90 && p.Lat <= 90 && p.Lon >= -180 && p.Lon <= 180 |
| +} |
| + |
| var cgnat = &net.IPNet{IP: net.IPv4(100, 64, 0, 0), Mask: net.CIDRMask(10, 32)} |
| // Locatable is false for addresses that say nothing about where a person |
diff --git a/internal/geo/mmdb_test.go b/internal/geo/mmdb_test.go| index d8cdeeb33..a6e8dcf15 100644 |
| --- a/internal/geo/mmdb_test.go |
| +++ b/internal/geo/mmdb_test.go |
| @@ -4,6 +4,7 @@ import ( |
| "bytes" |
| "errors" |
| "log" |
| + "math" |
| "net" |
| "os" |
| "path/filepath" |
| @@ -87,6 +88,45 @@ func TestMMDBLookupErrorsNeverLogTheAddress(t *testing.T) { |
| } |
| } |
| +// Final review: a structurally valid database can still carry a malformed |
| +// location. Nonfinite values and coordinates off the globe are a miss |
| +// (ok=false), never a point that ranks regions from nowhere, and never a |
| +// fall-through to the record's country either: a record that lies about |
| +// one field is not trusted on the next. |
| +func TestLocateRejectsMalformedCoordinates(t *testing.T) { |
| + at := func(lat, lon float64) *MMDB { |
| + return &MMDB{lookup: func(_ net.IP, result any) error { |
| + rec := result.(*mmdbRecord) |
| + rec.Location.Latitude, rec.Location.Longitude = &lat, &lon |
| + rec.Country.ISOCode = "GB" |
| + return nil |
| + }} |
| + } |
| + bad := map[string][2]float64{ |
| + "NaN latitude": {math.NaN(), 0}, |
| + "NaN longitude": {0, math.NaN()}, |
| + "+Inf latitude": {math.Inf(1), 0}, |
| + "-Inf longitude": {0, math.Inf(-1)}, |
| + "latitude above 90": {90.0001, 0}, |
| + "latitude below -90": {-91, 0}, |
| + "longitude above 180": {0, 180.5}, |
| + "longitude below -180": {0, -181}, |
| + } |
| + for name, c := range bad { |
| + if p, ok := at(c[0], c[1]).Locate(net.ParseIP("81.2.69.142")); ok { |
| + t.Errorf("%s: located %v, want a miss", name, p) |
| + } |
| + } |
| + // The edges of the globe are on it. |
| + for name, c := range map[string][2]float64{ |
| + "north pole": {90, 0}, "south pole": {-90, 0}, "antimeridian east": {0, 180}, "antimeridian west": {0, -180}, |
| + } { |
| + if p, ok := at(c[0], c[1]).Locate(net.ParseIP("81.2.69.142")); !ok || p != (Point{c[0], c[1]}) { |
| + t.Errorf("%s: (%v, %v), want %v", name, p, ok, Point{c[0], c[1]}) |
| + } |
| + } |
| +} |
| + |
| // Review Focus 2: non-public addresses never reach the reader — proved |
| // through Locate itself, with a counting lookup in place of the reader. |
| func TestLocateNeverLooksUpNonPublicAddresses(t *testing.T) { |