Merge pull request #107 from geoadmin/bug-BGDIINF_SB-2238-crash-with-…

…invalid-data

BGDIINF_SB-2238: Improved error handling - avoid 500 errors on bad request

app/helpers/validation/__init__.py

@@ -2,7 +2,10 @@
 
 from shapely.geometry import Polygon
 
+from flask import abort
+
 from app.helpers.helpers import float_raise_nan
+from app.settings import VALID_SRID
 
 bboxes = {
     2056:
@@ -38,3 +41,13 @@ def srs_guesser(geom):
                 sr = epsg
                 break
     return sr
+
+
+def validate_sr(sr):
+    if sr not in VALID_SRID:
+        abort(
+            400,
+            "Please provide a valid number for the spatial reference system model: "
+            f"{', '.join(map(str, VALID_SRID))}"
+        )
+    return sr

app/helpers/validation/height.py

@@ -3,7 +3,6 @@
 from flask import abort
 
 from app.helpers.helpers import float_raise_nan
-from app.settings import VALID_SRID
 
 
 def validate_lon_lat(lon, lat):
@@ -22,13 +21,3 @@ def validate_lon_lat(lon, lat):
         abort(400, "Please provide numerical values for the parameter 'northing'/'lat'")
 
     return lon, lat
-
-
-def validate_sr(sr):
-    if sr not in VALID_SRID:
-        abort(
-            400,
-            "Please provide a valid number for the spatial reference system model: "
-            f"{','.join(map(str, VALID_SRID))}"
-        )
-    return sr

app/helpers/validation/profile.py

@@ -10,10 +10,13 @@
 from app.helpers.profile_helpers import PROFILE_DEFAULT_AMOUNT_POINTS
 from app.helpers.profile_helpers import PROFILE_MAX_AMOUNT_POINTS
 from app.helpers.validation import srs_guesser
+from app.helpers.validation import validate_sr
 
 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 +33,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,
@@ -102,11 +108,7 @@ def read_spatial_reference(linestring):
             abort(400, "No 'sr' given and cannot be guessed from 'geom'")
         spatial_reference = sr
 
-    if spatial_reference not in (21781, 2056):
-        abort(
-            400,
-            "Please provide a valid number for the spatial reference system model 21781 or 2056"
-        )
+    validate_sr(spatial_reference)
     return spatial_reference
 
 

app/routes.py

@@ -22,8 +22,8 @@
 from app.helpers.route import prefix_route
 from app.helpers.validation import bboxes
 from app.helpers.validation import srs_guesser
+from app.helpers.validation import validate_sr
 from app.helpers.validation.height import validate_lon_lat
-from app.helpers.validation.height import validate_sr
 # add route prefix
 from app.statistics.statistics import load_json
 from app.statistics.statistics import prepare_data

tests/unit_tests/test_profile.py

@@ -109,7 +109,7 @@ def test_profile_invalid_sr_json_valid(self, mock_georaster_utils):
         self.assert_response_contains(
             resp,
             "Please provide a valid number for the spatial reference "
-            "system model 21781 or 2056"
+            "system model: 21781, 2056"
         )
 
     @patch('app.routes.georaster_utils')
@@ -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):