Browse Source

fix: Merge pull request #753 from pelias/improved_scalar_params

improvements for handling non-scalar request params. fixes #744
pull/755/head v3.13.1
Julian Simioni 8 years ago committed by GitHub
parent
commit
84c7a66bfa
  1. 2
      sanitizer/_single_scalar_parameters.js
  2. 2
      sanitizer/reverse.js
  3. 2
      sanitizer/search.js
  4. 2
      sanitizer/structured_geocoding.js
  5. 30
      test/ciao/autocomplete/non_scalar_parameter_array.coffee
  6. 30
      test/ciao/autocomplete/non_scalar_parameter_object.coffee
  7. 30
      test/ciao/reverse/non_scalar_parameter_array.coffee
  8. 2
      test/ciao/reverse/non_scalar_parameter_object.coffee
  9. 2
      test/ciao/search/address_parsing.coffee
  10. 30
      test/ciao/search/non_scalar_parameter_array.coffee
  11. 30
      test/ciao/search/non_scalar_parameter_object.coffee
  12. 9
      test/unit/sanitizer/_single_scalar_parameters.js
  13. 2
      test/unit/sanitizer/nearby.js
  14. 2
      test/unit/sanitizer/reverse.js
  15. 2
      test/unit/sanitizer/search.js
  16. 2
      test/unit/sanitizer/structured_geocoding.js

2
sanitizer/_single_scalar_parameters.js

@ -10,8 +10,10 @@ function sanitize( raw, clean ){
Object.keys(raw).forEach(function(key) { Object.keys(raw).forEach(function(key) {
if (_.isArray(raw[key])) { if (_.isArray(raw[key])) {
messages.errors.push('\'' + key + '\' parameter can only have one value'); messages.errors.push('\'' + key + '\' parameter can only have one value');
delete raw[key];
} else if (_.isObject(raw[key])) { } else if (_.isObject(raw[key])) {
messages.errors.push('\'' + key + '\' parameter must be a scalar'); messages.errors.push('\'' + key + '\' parameter must be a scalar');
delete raw[key];
} }
}); });

2
sanitizer/reverse.js

@ -2,8 +2,8 @@
var type_mapping = require('../helper/type_mapping'); var type_mapping = require('../helper/type_mapping');
var sanitizeAll = require('../sanitizer/sanitizeAll'), var sanitizeAll = require('../sanitizer/sanitizeAll'),
sanitizers = { sanitizers = {
quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'),
singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'),
quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'),
layers: require('../sanitizer/_targets')('layers', type_mapping.layer_mapping), layers: require('../sanitizer/_targets')('layers', type_mapping.layer_mapping),
sources: require('../sanitizer/_targets')('sources', type_mapping.source_mapping), sources: require('../sanitizer/_targets')('sources', type_mapping.source_mapping),
// depends on the layers and sources sanitizers, must be run after them // depends on the layers and sources sanitizers, must be run after them

2
sanitizer/search.js

@ -2,8 +2,8 @@ var type_mapping = require('../helper/type_mapping');
var sanitizeAll = require('../sanitizer/sanitizeAll'), var sanitizeAll = require('../sanitizer/sanitizeAll'),
sanitizers = { sanitizers = {
quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'),
singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'),
quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'),
text: require('../sanitizer/_text'), text: require('../sanitizer/_text'),
iso2_to_iso3: require('../sanitizer/_iso2_to_iso3'), iso2_to_iso3: require('../sanitizer/_iso2_to_iso3'),
size: require('../sanitizer/_size')(/* use defaults*/), size: require('../sanitizer/_size')(/* use defaults*/),

2
sanitizer/structured_geocoding.js

@ -2,8 +2,8 @@ var type_mapping = require('../helper/type_mapping');
var sanitizeAll = require('../sanitizer/sanitizeAll'), var sanitizeAll = require('../sanitizer/sanitizeAll'),
sanitizers = { sanitizers = {
quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'),
singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), singleScalarParameters: require('../sanitizer/_single_scalar_parameters'),
quattroshapes_deprecation: require('../sanitizer/_deprecate_quattroshapes'),
synthesize_analysis: require('../sanitizer/_synthesize_analysis'), synthesize_analysis: require('../sanitizer/_synthesize_analysis'),
iso2_to_iso3: require('../sanitizer/_iso2_to_iso3'), iso2_to_iso3: require('../sanitizer/_iso2_to_iso3'),
size: require('../sanitizer/_size')(/* use defaults*/), size: require('../sanitizer/_size')(/* use defaults*/),

30
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' ]

30
test/ciao/autocomplete/non_scalar_parameter_object.coffee

@ -0,0 +1,30 @@
#> define parameter as object
path: '/v1/autocomplete?text=A&parameter[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' ]

30
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' ]

2
test/ciao/reverse/non_scalar_parameter.coffee → 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&parameter[idx]=value' path: '/v1/reverse?point.lat=1&point.lon=1&parameter[idx]=value'
#? 200 ok #? 200 ok

2
test/ciao/search/address_parsing.coffee

@ -35,7 +35,7 @@ json.geocoding.query['size'].should.eql 10
#? address parsing #? address parsing
json.geocoding.query.parsed_text['number'].should.eql '30' 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['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.confidence.should.eql 1
json.features[0].properties.match_type.should.eql "exact" json.features[0].properties.match_type.should.eql "exact"

30
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' ]

30
test/ciao/search/non_scalar_parameter_object.coffee

@ -0,0 +1,30 @@
#> define parameter as object
path: '/v1/search?text=A&parameter[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' ]

9
test/unit/sanitizer/_single_scalar_parameters.js

@ -18,6 +18,11 @@ module.exports.tests.single_scalar_parameters = function(test, common) {
], ],
warnings: [] warnings: []
}); });
// erroneous params should be deleted to avoid middleware errors
t.false(raw.arrayParameter1);
t.false(raw.arrayParameter2);
t.end(); t.end();
}); });
@ -36,6 +41,10 @@ module.exports.tests.single_scalar_parameters = function(test, common) {
], ],
warnings: [] warnings: []
}); });
// erroneous params should be deleted to avoid middleware errors
t.false(raw.objectParameter1);
t.false(raw.objectParameter2);
t.end(); t.end();
}); });

2
test/unit/sanitizer/nearby.js

@ -30,7 +30,7 @@ module.exports.tests.interface = function(test, common) {
module.exports.tests.sanitizers = function(test, common) { module.exports.tests.sanitizers = function(test, common) {
test('check sanitizer list', function (t) { 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']; 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country', 'categories'];
t.deepEqual(Object.keys(nearby.sanitizer_list), expected); t.deepEqual(Object.keys(nearby.sanitizer_list), expected);
t.end(); t.end();

2
test/unit/sanitizer/reverse.js

@ -36,7 +36,7 @@ module.exports.tests.interface = function(test, common) {
module.exports.tests.sanitizers = function(test, common) { module.exports.tests.sanitizers = function(test, common) {
test('check sanitizer list', function (t) { 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']; 'sources', 'sources_and_layers', 'size', 'private', 'geo_reverse', 'boundary_country'];
t.deepEqual(Object.keys(reverse.sanitizer_list), expected); t.deepEqual(Object.keys(reverse.sanitizer_list), expected);
t.end(); t.end();

2
test/unit/sanitizer/search.js

@ -82,8 +82,8 @@ module.exports.tests.sanitize = function(test, common) {
}); });
var expected_sanitizers = [ var expected_sanitizers = [
'_deprecate_quattroshapes',
'_single_scalar_parameters', '_single_scalar_parameters',
'_deprecate_quattroshapes',
'_text', '_text',
'_iso2_to_iso3', '_iso2_to_iso3',
'_size', '_size',

2
test/unit/sanitizer/structured_geocoding.js

@ -82,8 +82,8 @@ module.exports.tests.sanitize = function(test, common) {
}); });
var expected_sanitizers = [ var expected_sanitizers = [
'_deprecate_quattroshapes',
'_single_scalar_parameters', '_single_scalar_parameters',
'_deprecate_quattroshapes',
'_synthesize_analysis', '_synthesize_analysis',
'_iso2_to_iso3', '_iso2_to_iso3',
'_size', '_size',

Loading…
Cancel
Save