diff --git a/lookup.py b/lookup.py index 9531063..93877e9 100755 --- a/lookup.py +++ b/lookup.py @@ -3,6 +3,7 @@ import inspect import json +import math import random import re import sys @@ -315,7 +316,7 @@ def parse_coordinate(coord_str: str) -> float: (?P[NSEW])? """ - match = re.match(dms_pattern, coord_str, re.VERBOSE) + match = re.fullmatch(dms_pattern, coord_str, re.VERBOSE) if match: degrees = int(match.group("degrees")) minutes = int(match.group("minutes")) @@ -337,15 +338,15 @@ def parse_coordinate(coord_str: str) -> float: def validate_coordinates(lat: float, lon: float) -> str | None: """Validate latitude and longitude ranges. Returns error message if invalid.""" - if lat < -90 or lat > 90: + if not math.isfinite(lat) or lat < -90 or lat > 90: return "Latitude must be between -90 and 90 degrees" - if lon < -180 or lon > 180: + if not math.isfinite(lon) or lon < -180 or lon > 180: return "Longitude must be between -180 and 180 degrees" return None @app.route("/") -def index() -> str | Response: +def index() -> str | Response | tuple[Response, int] | tuple[Response, int, dict[str, str]]: """Index page.""" t0 = time() database.session.execute(text("SELECT 1")) @@ -359,8 +360,6 @@ def index() -> str | Response: samples = sorted(geocode.samples, key=lambda row: row[2]) return render_template("index.html", samples=samples) - lat, lon = float(lat_str), float(lon_str) - try: lat = parse_coordinate(lat_str) lon = parse_coordinate(lon_str) @@ -370,10 +369,10 @@ def index() -> str | Response: error="Invalid coordinate format. " + "Please use decimal degrees (e.g., 56.099600) " + "or DMS format (e.g., 56°5'58.56\"N)", - ) + ), 400 if error_msg := validate_coordinates(lat, lon): - return jsonify(coords={"lat": lat, "lon": lon}, error=error_msg) + return jsonify(coords={"lat": lat_str, "lon": lon_str}, error=error_msg), 400 needs_commons = request.args.get("needs_commons", "true").lower() != "false" try: diff --git a/tests/test_coordinates.py b/tests/test_coordinates.py new file mode 100644 index 0000000..61fea43 --- /dev/null +++ b/tests/test_coordinates.py @@ -0,0 +1,62 @@ +"""Coordinate validation at the HTTP boundary.""" + +from unittest.mock import Mock + +import pytest + +import lookup + + +@pytest.mark.parametrize("parameter", ["lat", "lon"]) +@pytest.mark.parametrize( + "value", + [ + "", + "-4.177.'),(\"abcd", + "55.7644)\"'.,(abcd", + "55.7644 AND EXTRACTVALUE(1,CONCAT(0x7e,1))", + "55.7644 AND 1=CAST('abc' AS NUMERIC)", + "56°5'58.56\"N trailing text", + "nan", + "inf", + "-inf", + "1e999", + "181", + ], +) +def test_invalid_coordinates_return_400(monkeypatch, parameter, value): + """Malformed coordinates never reach the geocoding lookup.""" + monkeypatch.setattr(lookup.database.session, "execute", Mock()) + geocode_lookup = Mock() + monkeypatch.setattr(lookup, "lat_lon_to_wikidata", geocode_lookup) + args = {"lat": "55.7644", "lon": "-4.177", parameter: value} + + response = lookup.app.test_client().get("/", query_string=args) + + assert response.status_code == 400 + assert response.get_json()["error"] + geocode_lookup.assert_not_called() + + +@pytest.mark.parametrize( + "lat, lon, expected", + [ + ("55.7644", "-4.177", (55.7644, -4.177)), + ('56°6\'0"N', '3°30\'0"W', (56.1, -3.5)), + ("-90", "180", (-90.0, 180.0)), + ], +) +def test_valid_coordinates_reach_lookup(monkeypatch, lat, lon, expected): + """Decimal, DMS, and boundary coordinates retain their behavior.""" + monkeypatch.setattr(lookup.database.session, "execute", Mock()) + monkeypatch.setattr(lookup, "logging_enabled", False) + geocode_lookup = Mock(return_value={"result": {"ok": True}}) + monkeypatch.setattr(lookup, "lat_lon_to_wikidata", geocode_lookup) + + response = lookup.app.test_client().get( + "/", query_string={"lat": lat, "lon": lon, "needs_commons": "false"} + ) + + assert response.status_code == 200 + assert response.get_json() == {"ok": True} + geocode_lookup.assert_called_once_with(*expected, needs_commons=False)