Tighten location request validation
This commit is contained in:
@@ -1,4 +1,5 @@
|
|||||||
import json
|
import json
|
||||||
|
import logging
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, Request
|
from fastapi import APIRouter, Depends, Request
|
||||||
from fastapi.responses import PlainTextResponse, Response
|
from fastapi.responses import PlainTextResponse, Response
|
||||||
@@ -10,6 +11,8 @@ from app.schemas.location import LocationRecordRequest
|
|||||||
from app.services.location import record_location
|
from app.services.location import record_location
|
||||||
|
|
||||||
router = APIRouter(tags=["location"])
|
router = APIRouter(tags=["location"])
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
BAD_REQUEST_MESSAGE = "bad request"
|
||||||
|
|
||||||
|
|
||||||
@router.post("/location/record")
|
@router.post("/location/record")
|
||||||
@@ -18,11 +21,15 @@ async def create_location_record(request: Request, db: Session = Depends(get_db)
|
|||||||
raw_payload = await request.body()
|
raw_payload = await request.body()
|
||||||
data = json.loads(raw_payload)
|
data = json.loads(raw_payload)
|
||||||
payload = LocationRecordRequest.model_validate(data)
|
payload = LocationRecordRequest.model_validate(data)
|
||||||
|
record_location(db, payload)
|
||||||
except json.JSONDecodeError as exc:
|
except json.JSONDecodeError as exc:
|
||||||
return PlainTextResponse(str(exc), status_code=400)
|
logger.warning("Rejected location request due to invalid JSON: %s", exc)
|
||||||
|
return PlainTextResponse(BAD_REQUEST_MESSAGE, status_code=400)
|
||||||
except ValidationError as exc:
|
except ValidationError as exc:
|
||||||
return PlainTextResponse(str(exc), status_code=400)
|
logger.warning("Rejected location request due to payload validation failure: %s", exc)
|
||||||
|
return PlainTextResponse(BAD_REQUEST_MESSAGE, status_code=400)
|
||||||
|
except ValueError as exc:
|
||||||
|
logger.warning("Rejected location request due to invalid numeric input: %s", exc)
|
||||||
|
return PlainTextResponse(BAD_REQUEST_MESSAGE, status_code=400)
|
||||||
|
|
||||||
record_location(db, payload)
|
|
||||||
return Response(status_code=200)
|
return Response(status_code=200)
|
||||||
|
|
||||||
|
|||||||
@@ -5,7 +5,6 @@ class LocationRecordRequest(BaseModel):
|
|||||||
person: str
|
person: str
|
||||||
latitude: str
|
latitude: str
|
||||||
longitude: str
|
longitude: str
|
||||||
altitude: str = ""
|
altitude: str | None = None
|
||||||
|
|
||||||
model_config = ConfigDict(extra="forbid")
|
model_config = ConfigDict(extra="forbid")
|
||||||
|
|
||||||
|
|||||||
@@ -7,13 +7,20 @@ from app.models.location import Location
|
|||||||
from app.schemas.location import LocationRecordRequest
|
from app.schemas.location import LocationRecordRequest
|
||||||
|
|
||||||
|
|
||||||
def _parse_float_compat(value: str) -> float:
|
def _parse_optional_float_compat(value: str | None) -> float:
|
||||||
try:
|
try:
|
||||||
return float(value)
|
return float(value)
|
||||||
except (TypeError, ValueError):
|
except (TypeError, ValueError):
|
||||||
return 0.0
|
return 0.0
|
||||||
|
|
||||||
|
|
||||||
|
def _parse_required_float(value: str, field_name: str) -> float:
|
||||||
|
try:
|
||||||
|
return float(value)
|
||||||
|
except (TypeError, ValueError) as exc:
|
||||||
|
raise ValueError(f"Invalid numeric value for {field_name}") from exc
|
||||||
|
|
||||||
|
|
||||||
def _utc_now_rfc3339() -> str:
|
def _utc_now_rfc3339() -> str:
|
||||||
now = datetime.now(timezone.utc).replace(microsecond=0)
|
now = datetime.now(timezone.utc).replace(microsecond=0)
|
||||||
return now.isoformat().replace("+00:00", "Z")
|
return now.isoformat().replace("+00:00", "Z")
|
||||||
@@ -26,11 +33,10 @@ def record_location(session: Session, payload: LocationRecordRequest) -> None:
|
|||||||
.values(
|
.values(
|
||||||
person=payload.person,
|
person=payload.person,
|
||||||
datetime=_utc_now_rfc3339(),
|
datetime=_utc_now_rfc3339(),
|
||||||
latitude=_parse_float_compat(payload.latitude),
|
latitude=_parse_required_float(payload.latitude, "latitude"),
|
||||||
longitude=_parse_float_compat(payload.longitude),
|
longitude=_parse_required_float(payload.longitude, "longitude"),
|
||||||
altitude=_parse_float_compat(payload.altitude),
|
altitude=_parse_optional_float_compat(payload.altitude),
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
session.execute(stmt)
|
session.execute(stmt)
|
||||||
session.commit()
|
session.commit()
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,13 @@
|
|||||||
|
|
||||||
本文档说明 `location recorder` 在 Python 项目中的当前数据库接管策略,以及 legacy SQLite 接管 runbook。
|
本文档说明 `location recorder` 在 Python 项目中的当前数据库接管策略,以及 legacy SQLite 接管 runbook。
|
||||||
|
|
||||||
|
当前 Python 版本的 `POST /location/record` 请求校验策略是:
|
||||||
|
|
||||||
|
- `latitude` 和 `longitude` 为必填,缺失或无法解析成合法数值时返回 `400 bad request`
|
||||||
|
- `altitude` 为可选,缺失或非法时按 `0` 处理
|
||||||
|
- unknown field 仍返回 `400 bad request`
|
||||||
|
- 对 caller 的错误响应保持简洁,不直接暴露底层校验细节;详细原因只写日志
|
||||||
|
|
||||||
## Legacy 事实基线
|
## Legacy 事实基线
|
||||||
|
|
||||||
当前 legacy SQLite 中 `location` 表的真实 schema 为:
|
当前 legacy SQLite 中 `location` 表的真实 schema 为:
|
||||||
|
|||||||
+102
-4
@@ -88,17 +88,115 @@ def test_location_record_endpoint_rejects_unknown_fields(location_client) -> Non
|
|||||||
)
|
)
|
||||||
|
|
||||||
assert response.status_code == 400
|
assert response.status_code == 400
|
||||||
|
assert response.text == "bad request"
|
||||||
|
assert "extra" not in response.text
|
||||||
|
assert "ValidationError" not in response.text
|
||||||
|
|
||||||
|
|
||||||
def test_location_record_endpoint_keeps_legacy_lenient_number_parsing(location_client) -> None:
|
def test_location_record_endpoint_rejects_missing_latitude(location_client) -> None:
|
||||||
client, engine = location_client
|
client, _ = location_client
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
"/location/record",
|
||||||
|
json={
|
||||||
|
"person": "tianyu",
|
||||||
|
"longitude": "4.56",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 400
|
||||||
|
assert response.text == "bad request"
|
||||||
|
assert "latitude" not in response.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_location_record_endpoint_rejects_missing_longitude(location_client) -> None:
|
||||||
|
client, _ = location_client
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
"/location/record",
|
||||||
|
json={
|
||||||
|
"person": "tianyu",
|
||||||
|
"latitude": "1.23",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 400
|
||||||
|
assert response.text == "bad request"
|
||||||
|
assert "longitude" not in response.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_location_record_endpoint_rejects_invalid_latitude(location_client) -> None:
|
||||||
|
client, _ = location_client
|
||||||
|
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/location/record",
|
"/location/record",
|
||||||
json={
|
json={
|
||||||
"person": "tianyu",
|
"person": "tianyu",
|
||||||
"latitude": "bad-lat",
|
"latitude": "bad-lat",
|
||||||
|
"longitude": "4.56",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 400
|
||||||
|
assert response.text == "bad request"
|
||||||
|
assert "bad-lat" not in response.text
|
||||||
|
assert "latitude" not in response.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_location_record_endpoint_rejects_invalid_longitude(location_client) -> None:
|
||||||
|
client, _ = location_client
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
"/location/record",
|
||||||
|
json={
|
||||||
|
"person": "tianyu",
|
||||||
|
"latitude": "1.23",
|
||||||
"longitude": "bad-long",
|
"longitude": "bad-long",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 400
|
||||||
|
assert response.text == "bad request"
|
||||||
|
assert "bad-long" not in response.text
|
||||||
|
assert "longitude" not in response.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_location_record_endpoint_defaults_missing_altitude_to_zero(location_client) -> None:
|
||||||
|
client, engine = location_client
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
"/location/record",
|
||||||
|
json={
|
||||||
|
"person": "tianyu",
|
||||||
|
"latitude": "1.23",
|
||||||
|
"longitude": "4.56",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
|
||||||
|
with engine.connect() as conn:
|
||||||
|
row = conn.execute(
|
||||||
|
text(
|
||||||
|
"SELECT latitude, longitude, altitude "
|
||||||
|
"FROM location ORDER BY datetime DESC LIMIT 1"
|
||||||
|
)
|
||||||
|
).one()
|
||||||
|
|
||||||
|
assert row.latitude == pytest.approx(1.23)
|
||||||
|
assert row.longitude == pytest.approx(4.56)
|
||||||
|
assert row.altitude == pytest.approx(0.0)
|
||||||
|
|
||||||
|
|
||||||
|
def test_location_record_endpoint_defaults_invalid_altitude_to_zero(location_client) -> None:
|
||||||
|
client, engine = location_client
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
"/location/record",
|
||||||
|
json={
|
||||||
|
"person": "tianyu",
|
||||||
|
"latitude": "1.23",
|
||||||
|
"longitude": "4.56",
|
||||||
"altitude": "bad-alt",
|
"altitude": "bad-alt",
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
@@ -113,8 +211,8 @@ def test_location_record_endpoint_keeps_legacy_lenient_number_parsing(location_c
|
|||||||
)
|
)
|
||||||
).one()
|
).one()
|
||||||
|
|
||||||
assert row.latitude == pytest.approx(0.0)
|
assert row.latitude == pytest.approx(1.23)
|
||||||
assert row.longitude == pytest.approx(0.0)
|
assert row.longitude == pytest.approx(4.56)
|
||||||
assert row.altitude == pytest.approx(0.0)
|
assert row.altitude == pytest.approx(0.0)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user