From f087a5badc4204079781feceec4fbfba6d19802a Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 2 Dec 2016 18:29:16 +0100 Subject: [PATCH] improvements for handling non-scalar request params. fixes #744 --- sanitizer/_single_scalar_parameters.js | 2 ++ sanitizer/reverse.js | 2 +- sanitizer/search.js | 2 +- sanitizer/structured_geocoding.js | 2 +- .../non_scalar_parameter_array.coffee | 30 +++++++++++++++++++ .../non_scalar_parameter_object.coffee | 30 +++++++++++++++++++ .../reverse/non_scalar_parameter_array.coffee | 30 +++++++++++++++++++ ...fee => non_scalar_parameter_object.coffee} | 2 +- test/ciao/search/address_parsing.coffee | 2 +- .../search/non_scalar_parameter_array.coffee | 30 +++++++++++++++++++ .../search/non_scalar_parameter_object.coffee | 30 +++++++++++++++++++ .../sanitizer/_single_scalar_parameters.js | 9 ++++++ test/unit/sanitizer/nearby.js | 2 +- test/unit/sanitizer/reverse.js | 2 +- test/unit/sanitizer/search.js | 2 +- test/unit/sanitizer/structured_geocoding.js | 2 +- 16 files changed, 170 insertions(+), 9 deletions(-) create mode 100644 test/ciao/autocomplete/non_scalar_parameter_array.coffee create mode 100644 test/ciao/autocomplete/non_scalar_parameter_object.coffee create mode 100644 test/ciao/reverse/non_scalar_parameter_array.coffee rename test/ciao/reverse/{non_scalar_parameter.coffee => non_scalar_parameter_object.coffee} (96%) create mode 100644 test/ciao/search/non_scalar_parameter_array.coffee create mode 100644 test/ciao/search/non_scalar_parameter_object.coffee diff --git a/sanitizer/_single_scalar_parameters.js b/sanitizer/_single_scalar_parameters.js index 6bf2fd55..2b06cc65 100644 --- a/sanitizer/_single_scalar_parameters.js +++ b/sanitizer/_single_scalar_parameters.js @@ -10,8 +10,10 @@ function sanitize( raw, clean ){ Object.keys(raw).forEach(function(key) { if (_.isArray(raw[key])) { messages.errors.push('\'' + key + '\' parameter can only have one value'); + delete raw[key]; } else if (_.isObject(raw[key])) { messages.errors.push('\'' + key + '\' parameter must be a scalar'); + delete raw[key]; } }); diff --git a/sanitizer/reverse.js b/sanitizer/reverse.js index 90795a19..285ab56c 100644 --- a/sanitizer/reverse.js +++ b/sanitizer/reverse.js @@ -2,8 +2,8 @@ var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitizer/sanitizeAll'), sanitizers = { - quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), + quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), layers: require('../sanitizer/_targets')('layers', type_mapping.layer_mapping), sources: require('../sanitizer/_targets')('sources', type_mapping.source_mapping), // depends on the layers and sources sanitizers, must be run after them diff --git a/sanitizer/search.js b/sanitizer/search.js index 450f6b62..d99a926e 100644 --- a/sanitizer/search.js +++ b/sanitizer/search.js @@ -2,8 +2,8 @@ var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitizer/sanitizeAll'), sanitizers = { - quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), + quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), text: require('../sanitizer/_text'), iso2_to_iso3: require('../sanitizer/_iso2_to_iso3'), size: require('../sanitizer/_size')(/* use defaults*/), diff --git a/sanitizer/structured_geocoding.js b/sanitizer/structured_geocoding.js index a5a95de9..ebd55a56 100644 --- a/sanitizer/structured_geocoding.js +++ b/sanitizer/structured_geocoding.js @@ -2,8 +2,8 @@ var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitizer/sanitizeAll'), sanitizers = { - quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), + quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'), synthesize_analysis: require('../sanitizer/_synthesize_analysis'), iso2_to_iso3: require('../sanitizer/_iso2_to_iso3'), size: require('../sanitizer/_size')(/* use defaults*/), diff --git a/test/ciao/autocomplete/non_scalar_parameter_array.coffee b/test/ciao/autocomplete/non_scalar_parameter_array.coffee new file mode 100644 index 00000000..081cbb60 --- /dev/null +++ b/test/ciao/autocomplete/non_scalar_parameter_array.coffee @@ -0,0 +1,30 @@ + +#> define two sources +path: '/v1/autocomplete?text=A&sources=A&sources=B' + +#? 200 ok +response.statusCode.should.be.equal 400 +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 [ '\'sources\' parameter can only have one value' ] diff --git a/test/ciao/autocomplete/non_scalar_parameter_object.coffee b/test/ciao/autocomplete/non_scalar_parameter_object.coffee new file mode 100644 index 00000000..5c7e74af --- /dev/null +++ b/test/ciao/autocomplete/non_scalar_parameter_object.coffee @@ -0,0 +1,30 @@ + +#> define parameter as object +path: '/v1/autocomplete?text=A¶meter[idx]=value' + +#? 200 ok +response.statusCode.should.be.equal 400 +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' ] diff --git a/test/ciao/reverse/non_scalar_parameter_array.coffee b/test/ciao/reverse/non_scalar_parameter_array.coffee new file mode 100644 index 00000000..01792042 --- /dev/null +++ b/test/ciao/reverse/non_scalar_parameter_array.coffee @@ -0,0 +1,30 @@ + +#> define two sources +path: '/v1/reverse?point.lat=1&point.lon=1&sources=A&sources=B' + +#? 200 ok +response.statusCode.should.be.equal 400 +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 [ '\'sources\' parameter can only have one value' ] diff --git a/test/ciao/reverse/non_scalar_parameter.coffee b/test/ciao/reverse/non_scalar_parameter_object.coffee similarity index 96% rename from test/ciao/reverse/non_scalar_parameter.coffee rename to test/ciao/reverse/non_scalar_parameter_object.coffee index 757761e4..97f5fe6d 100644 --- a/test/ciao/reverse/non_scalar_parameter.coffee +++ b/test/ciao/reverse/non_scalar_parameter_object.coffee @@ -1,5 +1,5 @@ -#> set size +#> define parameter as object path: '/v1/reverse?point.lat=1&point.lon=1¶meter[idx]=value' #? 200 ok diff --git a/test/ciao/search/address_parsing.coffee b/test/ciao/search/address_parsing.coffee index 74b4d75e..2293ffa6 100644 --- a/test/ciao/search/address_parsing.coffee +++ b/test/ciao/search/address_parsing.coffee @@ -35,7 +35,7 @@ json.geocoding.query['size'].should.eql 10 #? address parsing json.geocoding.query.parsed_text['number'].should.eql '30' json.geocoding.query.parsed_text['street'].should.eql 'w 26th st' -json.geocoding.query.parsed_text['state'].should.eql 'ny' +json.geocoding.query.parsed_text['state'].should.eql 'NY' json.features[0].properties.confidence.should.eql 1 json.features[0].properties.match_type.should.eql "exact" diff --git a/test/ciao/search/non_scalar_parameter_array.coffee b/test/ciao/search/non_scalar_parameter_array.coffee new file mode 100644 index 00000000..9d5161fc --- /dev/null +++ b/test/ciao/search/non_scalar_parameter_array.coffee @@ -0,0 +1,30 @@ + +#> define two sources +path: '/v1/search?text=A&sources=A&sources=B' + +#? 200 ok +response.statusCode.should.be.equal 400 +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 [ '\'sources\' parameter can only have one value' ] diff --git a/test/ciao/search/non_scalar_parameter_object.coffee b/test/ciao/search/non_scalar_parameter_object.coffee new file mode 100644 index 00000000..80294db4 --- /dev/null +++ b/test/ciao/search/non_scalar_parameter_object.coffee @@ -0,0 +1,30 @@ + +#> define parameter as object +path: '/v1/search?text=A¶meter[idx]=value' + +#? 200 ok +response.statusCode.should.be.equal 400 +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' ] diff --git a/test/unit/sanitizer/_single_scalar_parameters.js b/test/unit/sanitizer/_single_scalar_parameters.js index 4337d6d1..c3322703 100644 --- a/test/unit/sanitizer/_single_scalar_parameters.js +++ b/test/unit/sanitizer/_single_scalar_parameters.js @@ -18,6 +18,11 @@ module.exports.tests.single_scalar_parameters = function(test, common) { ], warnings: [] }); + + // erroneous params should be deleted to avoid middleware errors + t.false(raw.arrayParameter1); + t.false(raw.arrayParameter2); + t.end(); }); @@ -36,6 +41,10 @@ module.exports.tests.single_scalar_parameters = function(test, common) { ], warnings: [] }); + + // erroneous params should be deleted to avoid middleware errors + t.false(raw.objectParameter1); + t.false(raw.objectParameter2); t.end(); }); diff --git a/test/unit/sanitizer/nearby.js b/test/unit/sanitizer/nearby.js index 666944d0..4b5507e0 100644 --- a/test/unit/sanitizer/nearby.js +++ b/test/unit/sanitizer/nearby.js @@ -30,7 +30,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitizers = function(test, common) { test('check sanitizer list', function (t) { - var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', + var expected = ['singleScalarParameters', 'quattroshapes_deprecation', 'layers', 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country', 'categories']; t.deepEqual(Object.keys(nearby.sanitizer_list), expected); t.end(); diff --git a/test/unit/sanitizer/reverse.js b/test/unit/sanitizer/reverse.js index 96cec51a..b3bbdb95 100644 --- a/test/unit/sanitizer/reverse.js +++ b/test/unit/sanitizer/reverse.js @@ -36,7 +36,7 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.sanitizers = function(test, common) { test('check sanitizer list', function (t) { - var expected = ['quattroshapes_deprecation', 'singleScalarParameters', 'layers', + var expected = ['singleScalarParameters', 'quattroshapes_deprecation', 'layers', 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country']; t.deepEqual(Object.keys(reverse.sanitizer_list), expected); t.end(); diff --git a/test/unit/sanitizer/search.js b/test/unit/sanitizer/search.js index fa60e58a..e2c01f8d 100644 --- a/test/unit/sanitizer/search.js +++ b/test/unit/sanitizer/search.js @@ -82,8 +82,8 @@ module.exports.tests.sanitize = function(test, common) { }); var expected_sanitizers = [ - '_deprecate_quattroshapes', '_single_scalar_parameters', + '_deprecate_quattroshapes', '_text', '_iso2_to_iso3', '_size', diff --git a/test/unit/sanitizer/structured_geocoding.js b/test/unit/sanitizer/structured_geocoding.js index 3d6c606e..ef9711b3 100644 --- a/test/unit/sanitizer/structured_geocoding.js +++ b/test/unit/sanitizer/structured_geocoding.js @@ -82,8 +82,8 @@ module.exports.tests.sanitize = function(test, common) { }); var expected_sanitizers = [ - '_deprecate_quattroshapes', '_single_scalar_parameters', + '_deprecate_quattroshapes', '_synthesize_analysis', '_iso2_to_iso3', '_size',