Merge pull request #289 from pelias/272-error-on-duplicate-parameter-names

272 error on duplicate parameter names

sanitiser/_single_scalar_parameters.js

@@ -0,0 +1,23 @@
+
+var _ = require('lodash'),
+    check = require('check-types');
+
+// validate inputs
+function sanitize( raw, clean ){
+  // error & warning messages
+  var messages = { errors: [], warnings: [] };
+
+  Object.keys(raw).forEach(function(key) {
+    if (_.isArray(raw[key])) {
+      messages.errors.push('\'' + key + '\' parameter can only have one value');
+    } else if (_.isObject(raw[key])) {
+      messages.errors.push('\'' + key + '\' parameter must be a scalar');
+    }
+
+  });
+
+  return messages;
+}
+
+// export function
+module.exports = sanitize;

sanitiser/autocomplete.js

@@ -1,5 +1,6 @@
 var sanitizeAll = require('../sanitiser/sanitizeAll'),
     sanitizers = {
+      singleScalarParameters: require('../sanitiser/_single_scalar_parameters'),
       text: require('../sanitiser/_text'),
       size: require('../sanitiser/_size'),
       private: require('../sanitiser/_flag_bool')('private', false),

sanitiser/place.js

@@ -1,6 +1,7 @@
 
 var sanitizeAll = require('../sanitiser/sanitizeAll'),
     sanitizers = {
+      singleScalarParameters: require('../sanitiser/_single_scalar_parameters'),
       ids: require('../sanitiser/_ids'),
       private: require('../sanitiser/_flag_bool')('private', false)
     };

sanitiser/reverse.js

@@ -1,6 +1,7 @@
 
 var sanitizeAll = require('../sanitiser/sanitizeAll'),
     sanitizers = {
+      singleScalarParameters: require('../sanitiser/_single_scalar_parameters'),
       layers: require('../sanitiser/_targets')('layers', require('../query/layers')),
       sources: require('../sanitiser/_targets')('sources', require('../query/sources')),
       size: require('../sanitiser/_size'),

sanitiser/search.js

@@ -1,6 +1,7 @@
 
 var sanitizeAll = require('../sanitiser/sanitizeAll'),
     sanitizers = {
+      singleScalarParameters: require('../sanitiser/_single_scalar_parameters'),
       text: require('../sanitiser/_text'),
       size: require('../sanitiser/_size'),
       layers: require('../sanitiser/_targets')('layers', require( '../query/layers' )),

test/ciao/reverse/duplicate_parameter_name.coffee

@@ -0,0 +1,30 @@
+
+#> set size
+path: '/v1/reverse?point.lat=1&point.lon=1&param=value1&param=value2'
+
+#? 200 ok
+response.statusCode.should.be.equal 200
+response.should.have.header 'charset', 'utf8'
+response.should.have.header 'content-type', 'application/json; charset=utf-8'
+
+#? valid geocoding block
+should.exist json.geocoding
+should.exist json.geocoding.version
+should.exist json.geocoding.attribution
+should.exist json.geocoding.query
+should.exist json.geocoding.engine
+should.exist json.geocoding.engine.name
+should.exist json.geocoding.engine.author
+should.exist json.geocoding.engine.version
+should.exist json.geocoding.timestamp
+
+#? valid geojson
+json.type.should.be.equal 'FeatureCollection'
+json.features.should.be.instanceof Array
+
+#? expected warnings
+should.not.exist json.geocoding.warnings
+
+#? expected errors
+should.exist json.geocoding.errors
+json.geocoding.errors.should.eql [ '\'param\' parameter can only have one value' ]

test/ciao/reverse/non_scalar_parameter.coffee

@@ -0,0 +1,30 @@
+
+#> set size
+path: '/v1/reverse?point.lat=1&point.lon=1&parameter[idx]=value'
+
+#? 200 ok
+response.statusCode.should.be.equal 200
+response.should.have.header 'charset', 'utf8'
+response.should.have.header 'content-type', 'application/json; charset=utf-8'
+
+#? valid geocoding block
+should.exist json.geocoding
+should.exist json.geocoding.version
+should.exist json.geocoding.attribution
+should.exist json.geocoding.query
+should.exist json.geocoding.engine
+should.exist json.geocoding.engine.name
+should.exist json.geocoding.engine.author
+should.exist json.geocoding.engine.version
+should.exist json.geocoding.timestamp
+
+#? valid geojson
+json.type.should.be.equal 'FeatureCollection'
+json.features.should.be.instanceof Array
+
+#? expected warnings
+should.not.exist json.geocoding.warnings
+
+#? expected errors
+should.exist json.geocoding.errors
+json.geocoding.errors.should.eql [ '\'parameter\' parameter must be a scalar' ]

test/unit/run.js

@@ -30,6 +30,7 @@ var tests = [
   require('./middleware/distance'),
   require('./middleware/confidenceScoreReverse'),
   require('./sanitiser/_size'),
+  require('./sanitiser/_single_scalar_parameters'),
 ];
 
 tests.map(function(t) {

test/unit/sanitiser/_single_scalar_parameters.js

@@ -0,0 +1,60 @@
+var sanitize = require('../../../sanitiser/_single_scalar_parameters');
+
+module.exports.tests = {};
+
+module.exports.tests.single_scalar_parameters = function(test, common) {
+  test('all duplicate parameters should have error messages returned', function(t) {
+    var raw = {
+      arrayParameter1: ['value1', 'value2'],
+      scalarParameter: 'value',
+      arrayParameter2: ['value3']
+    };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+    t.deepEquals(errorsAndWarnings, {
+      errors: [
+        '\'arrayParameter1\' parameter can only have one value',
+        '\'arrayParameter2\' parameter can only have one value',
+      ],
+      warnings: []
+    });
+    t.end();
+  });
+
+  test('object parameters should have error messages returned', function(t) {
+    var raw = {
+      objectParameter1: { key1: 'value1', key2: 'value2'},
+      scalarParameter: 'value',
+      objectParameter2: { }
+    };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+    t.deepEquals(errorsAndWarnings, {
+      errors: [
+        '\'objectParameter1\' parameter must be a scalar',
+        '\'objectParameter2\' parameter must be a scalar'
+      ],
+      warnings: []
+    });
+    t.end();
+  });
+
+  test('request with all scalar parameters should return empty errors', function(t) {
+    var raw = { scalarParameter1: 'value1', scalarParameter2: 2, scalarParameter3: true };
+    var clean = {};
+    var errorsAndWarnings = sanitize(raw, clean);
+    t.deepEquals(errorsAndWarnings, { errors: [], warnings: [] });
+    t.end();
+  });
+
+};
+
+module.exports.all = function (tape, common) {
+  function test(name, testFunction) {
+    return tape('SANTIZE _single_scalar_parameters ' + name, testFunction);
+  }
+
+  for( var testCase in module.exports.tests ){
+    module.exports.tests[testCase](test, common);
+  }
+};

test/unit/sanitiser/autocomplete.js

@@ -4,7 +4,7 @@ module.exports.tests = {};
 
 module.exports.tests.sanitisers = function(test, common) {
   test('check sanitiser list', function (t) {
-    var expected = ['text', 'size', 'private', 'geo_autocomplete' ];
+    var expected = ['singleScalarParameters', 'text', 'size', 'private', 'geo_autocomplete' ];
     t.deepEqual(Object.keys(autocomplete.sanitiser_list), expected);
     t.end();
   });

test/unit/sanitiser/place.js

@@ -19,6 +19,14 @@ module.exports.tests.interface = function(test, common) {
   });
 };
 
+module.exports.tests.sanitisers = function(test, common) {
+  test('check sanitiser list', function (t) {
+    var expected = ['singleScalarParameters', 'ids', 'private' ];
+    t.deepEqual(Object.keys(place.sanitiser_list), expected);
+    t.end();
+  });
+};
+
 module.exports.tests.sanitize_private = function(test, common) {
   var invalid_values = [null, -1, 123, NaN, 'abc'];
   invalid_values.forEach(function(value) {

test/unit/sanitiser/reverse.js

@@ -36,7 +36,7 @@ module.exports.tests.interface = function(test, common) {
 
 module.exports.tests.sanitisers = function(test, common) {
   test('check sanitiser list', function (t) {
-    var expected = ['layers', 'sources', 'size', 'private', 'geo_reverse', 'boundary_country'];
+    var expected = ['singleScalarParameters', 'layers', 'sources', 'size', 'private', 'geo_reverse', 'boundary_country'];
     t.deepEqual(Object.keys(reverse.sanitiser_list), expected);
     t.end();
   });

test/unit/sanitiser/search.js

@@ -25,7 +25,7 @@ module.exports.tests.interface = function(test, common) {
 
 module.exports.tests.sanitisers = function(test, common) {
   test('check sanitiser list', function (t) {
-    var expected = ['text', 'size', 'layers', 'sources', 'private', 'geo_search', 'boundary_country' ];
+    var expected = ['singleScalarParameters', 'text', 'size', 'layers', 'sources', 'private', 'geo_search', 'boundary_country' ];
     t.deepEqual(Object.keys(search.sanitiser_list), expected);
     t.end();
   });
@@ -33,7 +33,7 @@ module.exports.tests.sanitisers = function(test, common) {
 
 module.exports.tests.sanitize_invalid_text = function(test, common) {
   test('invalid text', function(t) {
-    var invalid = [ '', 100, null, undefined, new Date() ];
+    var invalid = [ '', 100, null, undefined ];
     invalid.forEach( function( text ){
       var req = { query: { text: text } };
       sanitize(req, function(){