Merge pull request #1207 from pelias/handle-out-of-bounds-bbox

Improve invalid bounding box value handling

sanitizer/_geo_common.js

@@ -2,7 +2,6 @@
  * helper sanitizer methods for geo parameters
  */
 var groups = require('./_groups'),
-    util = require('util'),
     check = require('check-types'),
     wrap = require('./wrap'),
     _ = require('lodash');
@@ -34,23 +33,45 @@ function sanitize_rect( key_prefix, clean, raw, bbox_is_required ) {
   // and not present
   if (!bbox_present) { return; }
 
-  // check each property individually. now that it is known a bbox is present,
+  sanitize_bbox_min_max(raw, key_prefix);
-  // all properties must exist, so pass the true flag for coord_is_required
+  sanitize_bbox_bounds(raw, key_prefix);
+
+  // use sanitize_coord to set values in `clean`
   properties.forEach(function(prop) {
     sanitize_coord(prop, clean, raw, true);
   });
+}
+
+  // validate max is greater than min for lat and lon
+function sanitize_bbox_min_max(raw, key_prefix) {
+  ['lat', 'lon'].forEach(function(dimension) {
+    const max = parseFloat(raw[`${key_prefix}.max_${dimension}`]);
+    const min = parseFloat(raw[`${key_prefix}.min_${dimension}`]);
 
-  var min_lat = parseFloat( raw[key_prefix + '.' + 'min_lat'] );
+    if (max <= min) {
-  var max_lat = parseFloat( raw[key_prefix + '.' + 'max_lat'] );
+      throw new Error(`${key_prefix}.min_${dimension} (${min}) must be less than ${key_prefix}.max_${dimension} (${max})`);
-  if (min_lat > max_lat) {
-    throw new Error( util.format( 'min_lat is larger than max_lat in \'%s\'', key_prefix ) );
     }
+  });
+}
 
-  var min_lon = parseFloat( raw[key_prefix + '.' + 'min_lon'] );
+// validate lat/lon values are within bounds
-  var max_lon = parseFloat( raw[key_prefix + '.' + 'max_lon'] );
+function sanitize_bbox_bounds(raw, key_prefix) {
-  if (min_lon > max_lon) {
+  const bounds = [ { dimension: 'lat', range: 90},
-    throw new Error( util.format( 'min_lon is larger than max_lon in \'%s\'', key_prefix ) );
+                   { dimension: 'lon', range: 180}];
+
+  bounds.forEach(function(bound) {
+    const values = {
+      max: parseFloat(raw[`${key_prefix}.max_${bound.dimension}`]),
+      min: parseFloat(raw[`${key_prefix}.min_${bound.dimension}`])
+    };
+
+    ['min', 'max'].forEach(function(prefix) {
+      if (Math.abs(values[prefix]) > bound.range) {
+        const key =`${key_prefix}.${prefix}_${bound.dimension}`;
+        throw new Error(`${key} value ${values[prefix]} is outside range -${bound.range},${bound.range}`);
       }
+    });
+  });
 }
 
 /**
@@ -127,7 +148,7 @@ function sanitize_coord( key, clean, raw, latlon_is_required ) {
     clean[key] = parsedValue;
   }
   else if (latlon_is_required) {
-    throw new Error( util.format( 'missing param \'%s\'', key ) );
+    throw new Error(`missing param '${key}'`);
   }
 }
 

test/unit/sanitizer/_geo_common.js

@@ -241,13 +241,12 @@ module.exports.tests.rect = function(test, common) {
     };
     var mandatory = false;
 
+    t.throws( function(){
       sanitize.sanitize_rect( 'boundary.rect', clean, params, mandatory );
-    t.equal(clean['boundary.rect.min_lat'], params['boundary.rect.min_lat'], 'min_lat approved');
+    },/boundary.rect.min_lat \(0\) must be less than boundary.rect.max_lat \(0\)/);
-    t.equal(clean['boundary.rect.max_lat'], params['boundary.rect.max_lat'], 'min_lat approved');
-    t.equal(clean['boundary.rect.min_lon'], params['boundary.rect.min_lon'], 'min_lat approved');
-    t.equal(clean['boundary.rect.max_lon'], params['boundary.rect.max_lon'], 'min_lat approved');
     t.end();
   });
+
   test('invalid rect - partially specified', function (t) {
     var clean = {};
     var params = {
@@ -295,7 +294,55 @@ module.exports.tests.rect = function(test, common) {
 
     t.throws( function() {
       sanitize.sanitize_rect( 'boundary.rect', clean, params, mandatory );
+    },/boundary.rect.min_lat \(52.7945\) must be less than boundary.rect.max_lat \(52.2387\)/, 'should error when min >= max');
+    t.end();
   });
+
+  test('invalid rect - max/min equal', function (t) {
+    var clean = {};
+    var params = {
+      'boundary.rect.max_lat': 52,
+      'boundary.rect.max_lon': 14,
+      'boundary.rect.min_lat': 52,
+      'boundary.rect.min_lon': 12
+    };
+    var mandatory = false;
+
+    t.throws( function() {
+      sanitize.sanitize_rect( 'boundary.rect', clean, params, mandatory );
+    });
+    t.end();
+  });
+
+  test('invalid rect - out of range latitude', function(t) {
+    var clean = {};
+    var params = {
+      'boundary.rect.max_lat': 352.2387,
+      'boundary.rect.max_lon': 14.1367,
+      'boundary.rect.min_lat': 52.7945,
+      'boundary.rect.min_lon': 12.6398
+    };
+    var mandatory = false;
+
+    t.throws( function() {
+      sanitize.sanitize_rect( 'boundary.rect', clean, params, mandatory );
+    }, /boundary.rect.max_lat value 352.2387 is outside range -90,90/, 'should throw error on boundary.rect.max_lat value');
+    t.end();
+  });
+
+  test('invalid rect - out of range longitude', function(t) {
+    var clean = {};
+    var params = {
+      'boundary.rect.max_lat': 52.2387,
+      'boundary.rect.max_lon': 14.1367,
+      'boundary.rect.min_lat': 12.7945,
+      'boundary.rect.min_lon': -200.6398
+    };
+    var mandatory = false;
+
+    t.throws( function() {
+      sanitize.sanitize_rect( 'boundary.rect', clean, params, mandatory );
+    }, /boundary.rect.min_lon value -200.6398 is outside range -180,180/, 'should throw error on boundary.rect.min_lon');
     t.end();
   });
 };