Skip to content

Commit fc0fc1a

Browse files
webb-bentomkralidis
authored andcommitted
Safely handle single feature SQL requests (#2288)
* Update test_mysql_provider.py * Update sql.py * Fix import order * Update sql.py Fix assertion fix flake8
1 parent 22b8d49 commit fc0fc1a

File tree

3 files changed

+98
-3
lines changed

3 files changed

+98
-3
lines changed

pygeoapi/provider/sql.py

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@
6464
from sqlalchemy.exc import (
6565
ConstraintColumnNotFoundError,
6666
InvalidRequestError,
67-
OperationalError
67+
OperationalError,
68+
SQLAlchemyError
6869
)
6970
from sqlalchemy.ext.automap import automap_base
7071
from sqlalchemy.orm import Session, load_only
@@ -312,8 +313,12 @@ def get(self, identifier, crs_transform_spec=None, **kwargs):
312313
# Execute query within self-closing database Session context
313314
with Session(self._engine) as session:
314315
# Retrieve data from database as feature
315-
item = session.get(self.table_model, identifier)
316-
if item is None:
316+
try:
317+
item = session.get(self.table_model, identifier)
318+
# Ensure that item is not None
319+
assert item is not None
320+
except (AssertionError, SQLAlchemyError) as e:
321+
LOGGER.debug(e, exc_info=True)
317322
msg = f'No such item: {self.id_field}={identifier}.'
318323
raise ProviderItemNotFoundError(msg)
319324
crs_transform_out = get_transform_from_spec(crs_transform_spec)
@@ -827,3 +832,67 @@ def _get_bbox_filter(self, bbox: list[float]):
827832
func.ST_GeomFromText(polygon_wkt), geom_column
828833
)
829834
return bbox_filter
835+
836+
def get(self, identifier, crs_transform_spec=None, **kwargs):
837+
"""
838+
Query the provider for a specific
839+
feature id e.g: /collections/hotosm_bdi_waterways/items/13990765
840+
841+
:param identifier: feature id
842+
:param crs_transform_spec: `CrsTransformSpec` instance, optional
843+
844+
:returns: GeoJSON FeatureCollection
845+
"""
846+
LOGGER.debug(f'Get item by ID: {identifier}')
847+
848+
# Execute query within self-closing database Session context
849+
with Session(self._engine) as session:
850+
# Retrieve data from database as feature
851+
try:
852+
item = session.get(self.table_model, identifier)
853+
# Ensure that item is not None
854+
assert item is not None
855+
# Ensure returned row has exact match
856+
feature_id = getattr(item, self.id_field)
857+
assert str(feature_id) == identifier
858+
except (AssertionError, SQLAlchemyError) as e:
859+
LOGGER.debug(e, exc_info=True)
860+
msg = f'No such item: {self.id_field}={identifier}.'
861+
raise ProviderItemNotFoundError(msg)
862+
crs_transform_out = get_transform_from_spec(crs_transform_spec)
863+
feature = self._sqlalchemy_to_feature(item, crs_transform_out)
864+
865+
# Drop non-defined properties
866+
if self.properties:
867+
props = feature['properties']
868+
dropping_keys = deepcopy(props).keys()
869+
for item in dropping_keys:
870+
if item not in self.properties:
871+
props.pop(item)
872+
873+
# Add fields for previous and next items
874+
id_field = getattr(self.table_model, self.id_field)
875+
prev_item = (
876+
session.query(self.table_model)
877+
.order_by(id_field.desc())
878+
.filter(id_field < feature_id)
879+
.first()
880+
)
881+
next_item = (
882+
session.query(self.table_model)
883+
.order_by(id_field.asc())
884+
.filter(id_field > feature_id)
885+
.first()
886+
)
887+
feature['prev'] = (
888+
getattr(prev_item, self.id_field)
889+
if prev_item is not None
890+
else feature_id
891+
)
892+
feature['next'] = (
893+
getattr(next_item, self.id_field)
894+
if next_item is not None
895+
else feature_id
896+
)
897+
898+
return feature

tests/provider/test_mysql_provider.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,19 @@ def test_query_skip_geometry(config):
166166
assert feature['geometry'] is None
167167

168168

169+
def test_get_with_injection(config):
170+
"""Testing query for injection attack string"""
171+
p = MySQLProvider(config)
172+
feature = p.get('1')
173+
assert feature.get('type') == 'Feature'
174+
175+
with pytest.raises(ProviderItemNotFoundError):
176+
p.get('1; DROP TABLE location;')
177+
178+
with pytest.raises(ProviderItemNotFoundError):
179+
p.get('1<script>alert("We love pygeoapi")</script>')
180+
181+
169182
def test_get_not_existing_item_raise_exception(config):
170183
"""Testing query for a not existing object"""
171184
p = MySQLProvider(config)

tests/provider/test_postgresql_provider.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,19 @@ def test_get_simple(config, id_, prev, next_):
354354
assert result['next'] == next_
355355

356356

357+
def test_get_with_injection(config):
358+
"""Testing query for injection attack string"""
359+
p = PostgreSQLProvider(config)
360+
feature = p.get('29701937')
361+
assert feature.get('type') == 'Feature'
362+
363+
with pytest.raises(ProviderItemNotFoundError):
364+
p.get('29701937; DROP TABLE location;')
365+
366+
with pytest.raises(ProviderItemNotFoundError):
367+
p.get('29701937<script>alert("We love pygeoapi")</script>')
368+
369+
357370
def test_get_with_config_properties(config):
358371
"""
359372
Test that get is restricted by properties in the config.

0 commit comments

Comments
 (0)