Skip to content

Commit

Permalink
BGDIINF_SB-2238: Improved error handling - avoid 500 errors on bad re…
Browse files Browse the repository at this point in the history
…quest

Avoiding 500 error when the request parameters are invalid.

Only LineString and Point are supported as input geometry. Checking this
avoid later crash.

Removed also broad exception by using correct exception type.

Point and LineString shape `is_valid` is a property and not a function
and do not raise exception but simply return True or False. Before this
kind of invalid shape object where caught later by trying to guess the
srid.
  • Loading branch information
ltshb committed May 5, 2022
1 parent bace43e commit 0e3b762
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 18 deletions.
27 changes: 16 additions & 11 deletions app/helpers/validation/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
logger = logging.getLogger(__name__)
max_content_length = 32 * 1024 * 1024 # 32MB

PROFILE_VALID_GEOMETRY_TYPES = ['LineString', 'Point']


def read_linestring():
# param geom, list of coordinates defining the line on which we want a profile
Expand All @@ -30,22 +32,25 @@ def read_linestring():

if not linestring:
abort(400, "No 'geom' given, cannot create a profile without coordinates")

try:
geom = geojson.loads(linestring, object_hook=geojson.GeoJSON.to_instance)
except ValueError as e:
logger.exception(e)
abort(400, "Error loading geometry in JSON string")
logger.error('Invalid "geom" parameter, it is not geojson: %s', e)
abort(400, "Invalid geom parameter, must be a GEOJSON")

if geom.get('type') not in PROFILE_VALID_GEOMETRY_TYPES:
abort(400, f"geom parameter must be a {'/'.join(PROFILE_VALID_GEOMETRY_TYPES)} GEOJSON")

try:
geom_to_shape = shape(geom)
# pylint: disable=broad-except
except Exception as e:
logger.exception(e)
abort(400, "Error converting JSON to Shape")
try:
geom_to_shape.is_valid
# pylint: disable=broad-except
except Exception:
abort(400, "Invalid Linestring syntax")
except ValueError as e:
logger.error("Failed to transformed GEOJSON to shape: %s", e)
abort(400, "Error converting GEOJSON to Shape")

if not geom_to_shape.is_valid:
abort(400, f"Invalid {geom['type']}")

if len(geom_to_shape.coords) > PROFILE_MAX_AMOUNT_POINTS:
abort(
413,
Expand Down
14 changes: 7 additions & 7 deletions tests/unit_tests/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def test_profile_lv03_layers_none(self, mock_georaster_utils):
params={'geom': '{"type":"LineString","coordinates":[[0,0],[0,0],[0,0]]}'},
expected_status=400
)
self.assert_response_contains(resp, "No 'sr' given and cannot be guessed from 'geom'")
self.assert_response_contains(resp, "Invalid LineString")

def test_profile_lv03_layers_none2(self):
resp = self.get_json_profile(
Expand Down Expand Up @@ -212,7 +212,7 @@ def test_profile_lv03_json_wrong_geom(self, mock_georaster_utils):
resp = self.prepare_mock_and_test_json_profile(
mock_georaster_utils=mock_georaster_utils, params={'geom': 'toto'}, expected_status=400
)
self.assert_response_contains(resp, 'Error loading geometry in JSON string')
self.assert_response_contains(resp, 'Invalid geom parameter, must be a GEOJSON')

@patch('app.routes.georaster_utils')
def test_profile_lv03_json_wrong_shape(self, mock_georaster_utils):
Expand All @@ -221,7 +221,7 @@ def test_profile_lv03_json_wrong_shape(self, mock_georaster_utils):
params={'geom': LINESTRING_WRONG_SHAPE},
expected_status=400
)
self.assert_response_contains(resp, 'Error converting JSON to Shape')
self.assert_response_contains(resp, 'geom parameter must be a LineString/Point GEOJSON')

@patch('app.routes.georaster_utils')
def test_profile_lv03_json_nb_points(self, mock_georaster_utils):
Expand Down Expand Up @@ -305,7 +305,7 @@ def test_profile_lv03_json_invalid_linestring(self, mock_georaster_utils):
params={'geom': '{"type":"LineString","coordinates":[[550050,206550]]}'},
expected_status=400
)
self.assert_response_contains(resp, 'Error converting JSON to Shape')
self.assert_response_contains(resp, 'Error converting GEOJSON to Shape')

@patch('app.routes.georaster_utils')
def test_profile_lv03_json_offset(self, mock_georaster_utils):
Expand Down Expand Up @@ -444,7 +444,7 @@ def test_profile_lv03_cvs_wrong_geom(self, mock_georaster_utils):
resp = self.prepare_mock_and_test_csv_profile(
mock_georaster_utils=mock_georaster_utils, params={'geom': 'toto'}, expected_status=400
)
self.assert_response_contains(resp, 'Error loading geometry in JSON string')
self.assert_response_contains(resp, 'Invalid geom parameter, must be a GEOJSON')

@patch('app.routes.georaster_utils')
def test_profile_lv03_csv_misspelled_shape(self, mock_georaster_utils):
Expand All @@ -453,14 +453,14 @@ def test_profile_lv03_csv_misspelled_shape(self, mock_georaster_utils):
params={'geom': LINESTRING_MISSPELLED_SHAPE},
expected_status=400
)
self.assert_response_contains(resp, 'Error loading geometry in JSON string')
self.assert_response_contains(resp, 'Invalid geom parameter, must be a GEOJSON')

resp = self.prepare_mock_and_test_csv_profile(
mock_georaster_utils=mock_georaster_utils,
params={'geom': LINESTRING_WRONG_SHAPE},
expected_status=400
)
self.assert_response_contains(resp, 'Error converting JSON to Shape')
self.assert_response_contains(resp, 'geom parameter must be a LineString/Point GEOJSON')

@patch('app.routes.georaster_utils')
def test_profile_lv03_csv_callback(self, mock_georaster_utils):
Expand Down

0 comments on commit 0e3b762

Please sign in to comment.