From 0e3b762a705c6f5ed651e128b7163357124295c5 Mon Sep 17 00:00:00 2001 From: Brice Schaffner Date: Thu, 5 May 2022 13:46:34 +0200 Subject: [PATCH] BGDIINF_SB-2238: Improved error handling - avoid 500 errors on bad request 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. --- app/helpers/validation/profile.py | 27 ++++++++++++++++----------- tests/unit_tests/test_profile.py | 14 +++++++------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/app/helpers/validation/profile.py b/app/helpers/validation/profile.py index 1d713f0c..57e84218 100644 --- a/app/helpers/validation/profile.py +++ b/app/helpers/validation/profile.py @@ -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 @@ -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, diff --git a/tests/unit_tests/test_profile.py b/tests/unit_tests/test_profile.py index 637da4b4..d7ff25a0 100644 --- a/tests/unit_tests/test_profile.py +++ b/tests/unit_tests/test_profile.py @@ -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( @@ -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): @@ -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): @@ -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): @@ -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): @@ -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):