From bf42290c7cdaba59b72f0227048ffa3e67b13e05 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 14 Sep 2015 16:07:53 +0200 Subject: [PATCH 1/4] refactor sanitizers: first pass --- package.json | 2 +- sanitiser/_categories.js | 35 ++++----- sanitiser/_details.js | 35 +++------ sanitiser/_geo_common.js | 6 +- sanitiser/_geo_reverse.js | 37 +++------- sanitiser/_geo_search.js | 37 +++------- sanitiser/_id.js | 129 +++++++++++++++++---------------- sanitiser/_layers.js | 62 +++++++--------- sanitiser/_sanitize.js | 17 ----- sanitiser/_size.js | 55 +++++++++----- sanitiser/_source.js | 59 +++++++-------- sanitiser/_text.js | 41 ++++++----- sanitiser/place.js | 5 +- sanitiser/reverse.js | 18 ++--- sanitiser/sanitizeAll.js | 26 +++++++ sanitiser/search.js | 8 +- test/unit/sanitiser/_source.js | 51 ++++++++----- test/unit/sanitiser/search.js | 8 ++ 18 files changed, 318 insertions(+), 313 deletions(-) delete mode 100644 sanitiser/_sanitize.js create mode 100644 sanitiser/sanitizeAll.js diff --git a/package.json b/package.json index d2853d8c..4e35f83d 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "dependencies": { "addressit": "1.3.0", "async": "^0.9.0", + "check-types": "^3.3.1", "cluster2": "git://github.com/missinglink/cluster2.git#node_zero_twelve", "express": "^4.8.8", "express-http-proxy": "^0.6.0", @@ -43,7 +44,6 @@ "geojson-extent": "^0.3.1", "geolib": "^2.0.18", "geopipes-elasticsearch-backend": "^0.2.0", - "is-object": "^1.0.1", "markdown": "0.5.0", "microtime": "1.4.0", "morgan": "1.5.2", diff --git a/sanitiser/_categories.js b/sanitiser/_categories.js index f29c9fd0..3f465186 100644 --- a/sanitiser/_categories.js +++ b/sanitiser/_categories.js @@ -1,36 +1,33 @@ -var isObject = require('is-object'); +var check = require('check-types'); // validate inputs, convert types and apply defaults -function sanitize( req ){ +function sanitize( unclean, clean ){ - var clean = req.clean || {}; - var params= req.query; - - // ensure the input params are a valid object - if( !isObject( params ) ){ - params = {}; - } + // error & warning messages + var messages = { errors: [], warnings: [] }; // default case (no categories specified in GET params) - if('string' !== typeof params.categories || !params.categories.length){ - clean.categories = []; - } - else { - // parse GET params - clean.categories = params.categories.split(',') + clean.categories = []; + + // if categories string has been set + if( check.unemptyString( unclean.categories ) ){ + + // map input categories to valid format + clean.categories = unclean.categories.split(',') .map(function (cat) { return cat.toLowerCase().trim(); // lowercase inputs }) .filter( function( cat ) { return ( cat.length > 0 ); }); - } - // pass validated params to next middleware - req.clean = clean; + if( !clean.categories.length ){ + messages.warnings.push( 'invalid \'categories\': no valid category strings found'); + } + } - return { 'error': false }; + return messages; } diff --git a/sanitiser/_details.js b/sanitiser/_details.js index 6dc2b119..7f621959 100644 --- a/sanitiser/_details.js +++ b/sanitiser/_details.js @@ -1,36 +1,25 @@ -var isObject = require('is-object'); + +var check = require('check-types'); +var DEFAULT_DETAILS_BOOL = true; // validate inputs, convert types and apply defaults -function sanitize( req, default_value ){ - - var clean = req.clean || {}; - var params= req.query; +function sanitize( unclean, clean ){ - if (default_value === undefined) { - default_value = true; - } - - default_value = !!default_value; + // error & warning messages + var messages = { errors: [], warnings: [] }; - // ensure the input params are a valid object - if( !isObject( params ) ){ - params = {}; - } - - if (params.details !== undefined) { - clean.details = isTruthy(params.details); + if( !check.undefined( unclean.details ) ){ + clean.details = isTruthy( unclean.details ); } else { - clean.details = default_value; + clean.details = DEFAULT_DETAILS_BOOL; } - req.clean = clean; - - return {'error':false}; - + return messages; } +// be lenient with 'truthy' values function isTruthy(val) { - if (typeof val === 'string') { + if( check.string( val ) ){ return ['true', '1', 'yes', 'y'].indexOf(val) !== -1; } diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js index 2bf8c6a5..faf99fd0 100644 --- a/sanitiser/_geo_common.js +++ b/sanitiser/_geo_common.js @@ -12,12 +12,12 @@ var util = require( 'util' ); * @param {object} clean * @param {string} param */ -function sanitize_bbox( clean, param ) { - if( !param ) { +function sanitize_bbox( unclean, clean ) { + if( !unclean.bbox ) { return; } - var bboxArr = param.split( ',' ); + var bboxArr = unclean.bbox.split( ',' ); if( Array.isArray( bboxArr ) && bboxArr.length === 4 ) { var bbox = bboxArr.map(parseFloat); diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js index f613e54f..7b3cad58 100644 --- a/sanitiser/_geo_reverse.js +++ b/sanitiser/_geo_reverse.js @@ -1,39 +1,26 @@ -var isObject = require('is-object'); + var geo_common = require ('./_geo_common'); +var LAT_LON_IS_REQUIRED = true, + CIRCLE_IS_REQUIRED = false, + CIRCLE_MUST_BE_COMPLETE = false; // validate inputs, convert types and apply defaults -module.exports = function sanitize( req ){ - var clean = req.clean || {}; - var params = req.query; - var latlon_is_required = true; - var circle_is_required = false; - var circle_must_be_complete = false; +module.exports = function sanitize( unclean, clean ){ - // ensure the input params are a valid object - if( !isObject( params ) ){ - params = {}; - } - - if( !isObject( params.point ) ){ - params.point = {}; - } + // error & warning messages + var messages = { errors: [], warnings: [] }; try { - geo_common.sanitize_coord( 'lat', clean, params['point.lat'], latlon_is_required ); - geo_common.sanitize_coord( 'lon', clean, params['point.lon'], latlon_is_required ); + geo_common.sanitize_coord( 'lat', clean, unclean['point.lat'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'lon', clean, unclean['point.lon'], LAT_LON_IS_REQUIRED ); // boundary.circle.* is not mandatory, and only specifying radius is fine, // as point.lat/lon will be used to fill those values by default - geo_common.sanitize_boundary_circle( clean, params, circle_is_required, circle_must_be_complete); + geo_common.sanitize_boundary_circle( clean, unclean, CIRCLE_IS_REQUIRED, CIRCLE_MUST_BE_COMPLETE); } catch (err) { - return { - 'error': true, - 'message': err.message - }; + messages.errors.push( err.message ); } - req.clean = clean; - - return { 'error': false }; + return messages; }; diff --git a/sanitiser/_geo_search.js b/sanitiser/_geo_search.js index 1f1fbeb2..c3ff6a76 100644 --- a/sanitiser/_geo_search.js +++ b/sanitiser/_geo_search.js @@ -1,38 +1,21 @@ -var isObject = require('is-object'); + var geo_common = require ('./_geo_common'); +var LAT_LON_IS_REQUIRED = false; // validate inputs, convert types and apply defaults -module.exports = function sanitize( req ){ - var clean = req.clean || {}; - var params = req.query; - var latlon_is_required = false; - - // ensure the input params are a valid object - if( !isObject( params ) ){ - params = {}; - } +module.exports = function sanitize( unclean, clean ){ - if( !isObject( params.focus ) ){ - params.focus = {}; - } - - if( !isObject( params.focus.point ) ){ - params.focus.point = {}; - } + // error & warning messages + var messages = { errors: [], warnings: [] }; try { - geo_common.sanitize_coord( 'lat', clean, params['focus.point.lat'], latlon_is_required ); - geo_common.sanitize_coord( 'lon', clean, params['focus.point.lon'], latlon_is_required ); - geo_common.sanitize_bbox(clean, params.bbox); + geo_common.sanitize_coord( 'lat', clean, unclean['focus.point.lat'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'lon', clean, unclean['focus.point.lon'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_bbox(unclean, clean); } catch (err) { - return { - 'error': true, - 'message': err.message - }; + messages.errors.push( err.message ); } - req.clean = clean; - - return { 'error': false }; + return messages; }; diff --git a/sanitiser/_id.js b/sanitiser/_id.js index 78ff6b1f..296b6907 100644 --- a/sanitiser/_id.js +++ b/sanitiser/_id.js @@ -1,72 +1,79 @@ -var isObject = require('is-object'); + +var check = require('check-types'), + types = require('../query/types'); + +var ID_DELIM = ':'; // validate inputs, convert types and apply defaults // id generally looks like 'geoname:4163334' (type:id) // so, both type and id are required fields. -function sanitize( req ){ - req.clean = req.clean || {}; - var params = req.query; - var types = require('../query/types'); - var delim = ':'; - - // ensure params is a valid object - if( !isObject( params ) ){ - params = {}; - } - - var errormessage = function(fieldname, message) { - return { - 'error': true, - 'message': message || ('invalid param \''+ fieldname + '\': text length, must be >0') - }; - }; - - if(('string' === typeof params.id && !params.id.length) || params.id === undefined){ - return errormessage('id'); - } - - if( params && params.id && params.id.length ){ - req.clean.ids = []; - params.ids = Array.isArray(params.id) ? params.id : [params.id]; - - // de-dupe - params.ids = params.ids.filter(function(item, pos) { - return params.ids.indexOf(item) === pos; - }); +function errorMessage(fieldname, message) { + return message || 'invalid param \''+ fieldname + '\': text length, must be >0'; +} + +function sanitize( unclean, clean ){ - for (var i=0; i MAX_SIZE ){ + // set the max size + messages.warnings.push('out-of-range integer \'size\', using MAX_SIZE'); + clean.size = MAX_SIZE; + } + else if( _size < MIN_SIZE ){ + // set the min size + messages.warnings.push('out-of-range integer \'size\', using MIN_SIZE'); + clean.size = MIN_SIZE; + } + else { + // set the input size + clean.size = _size; + } + + } + return messages; } // export function diff --git a/sanitiser/_source.js b/sanitiser/_source.js index f8f89a68..4c87967a 100644 --- a/sanitiser/_source.js +++ b/sanitiser/_source.js @@ -1,44 +1,45 @@ -var isObject = require('is-object'); -var sources_map = require( '../query/sources' ); -var all_sources = Object.keys(sources_map); -function sanitize( req ) { - req.clean = req.clean || {}; - var params = req.query; +var check = require('check-types'), + sources_map = require( '../query/sources' ); - req.clean.types = req.clean.types || {}; +var ALL_SOURCES = Object.keys(sources_map), + ALL_SOURCES_JOINED = ALL_SOURCES.join(','); - // ensure the input params are a valid object - if( !isObject( params ) ){ - params = {}; - } +function sanitize( unclean, clean ) { + + // error & warning messages + var messages = { errors: [], warnings: [] }; + + // init clean.types (if not already init) + clean.types = clean.types || {}; // default case (no layers specified in GET params) // don't even set the from_layers key in this case - if('string' !== typeof params.source || !params.source.length){ - return { error: false }; - } + if( check.unemptyString( unclean.source ) ){ - var sources = params.source.split(','); + var sources = unclean.source.split(','); - var invalid_sources = sources.filter(function(source) { - return all_sources.indexOf(source) === -1; - }); + var invalid_sources = sources.filter(function(source) { + return ALL_SOURCES.indexOf(source) === -1; + }); - if (invalid_sources.length > 0) { - return { - error: true, - msg: '`' + invalid_sources[0] + '` is an invalid source parameter. Valid options: ' + all_sources.join(', ') - }; - } + if( invalid_sources.length > 0 ){ + invalid_sources.forEach( function( invalid ){ + messages.errors.push('\'' + invalid + '\' is an invalid source parameter. Valid options: ' + ALL_SOURCES_JOINED); + }); + } + + else { + var types = sources.reduce(function(acc, source) { + return acc.concat(sources_map[source]); + }, []); - var types = sources.reduce(function(acc, source) { - return acc.concat(sources_map[source]); - }, []); + clean.types.from_source = types; + } - req.clean.types.from_source = types; + } - return { error: false }; + return messages; } module.exports = sanitize; diff --git a/sanitiser/_text.js b/sanitiser/_text.js index 41f38d6c..e3268a54 100644 --- a/sanitiser/_text.js +++ b/sanitiser/_text.js @@ -1,32 +1,33 @@ -var isObject = require('is-object'); -var query_parser = require('../helper/query_parser'); + +var check = require('check-types'), + query_parser = require('../helper/query_parser'); // validate texts, convert types and apply defaults -function sanitize( req ){ - req.clean = req.clean || {}; - var params= req.query; +function sanitize( unclean, clean ){ - // ensure the text params are a valid object - if( !isObject( params ) ){ - params = {}; - } + // error & warning messages + var messages = { errors: [], warnings: [] }; - // text text - if('string' !== typeof params.text || !params.text.length){ - return { - 'error': true, - 'message': 'invalid param \'text\': text length, must be >0' - }; + // invalid input 'text' + if( !check.unemptyString( unclean.text ) ){ + messages.errors.push('invalid param \'text\': text length, must be >0'); } - req.clean.text = params.text; + // valid input 'text' + else { - req.clean.parsed_text = query_parser.get_parsed_address(params.text); + // valid text + clean.text = unclean.text; - req.clean.types = req.clean.layers || {}; - req.clean.types.from_address_parsing = query_parser.get_layers(params.text); + // parse text with query parser + clean.parsed_text = query_parser.get_parsed_address(clean.text); + + // try to set layers from query parser results + clean.types = clean.layers || {}; + clean.types.from_address_parsing = query_parser.get_layers(clean.text); + } - return { 'error': false }; + return messages; } // export function diff --git a/sanitiser/place.js b/sanitiser/place.js index 352777a6..fd3faba0 100644 --- a/sanitiser/place.js +++ b/sanitiser/place.js @@ -1,10 +1,11 @@ -var _sanitize = require('../sanitiser/_sanitize'), + +var sanitizeAll = require('../sanitiser/sanitizeAll'), sanitizers = { id: require('../sanitiser/_id'), details: require('../sanitiser/_details') }; -var sanitize = function(req, cb) { _sanitize(req, sanitizers, cb); }; +var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; // export sanitize for testing module.exports.sanitize = sanitize; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index edb8da7e..56f4f8c8 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -1,17 +1,15 @@ -var _sanitize = require('../sanitiser/_sanitize'), - sanitiser = { - latlonzoom: require('../sanitiser/_geo_reverse'), + +var sanitizeAll = require('../sanitiser/sanitizeAll'), + sanitizers = { layers: require('../sanitiser/_layers'), - suorce: require('../sanitiser/_source'), - details: require('../sanitiser/_details'), size: require('../sanitiser/_size'), - categories: function ( req ) { - var categories = require('../sanitiser/_categories'); - return categories(req); - } + source: require('../sanitiser/_source'), + details: require('../sanitiser/_details'), + geo_reverse: require('../sanitiser/_geo_reverse'), + categories: require('../sanitiser/_categories') }; -var sanitize = function(req, cb) { _sanitize(req, sanitiser, cb); }; +var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; // export sanitize for testing module.exports.sanitize = sanitize; diff --git a/sanitiser/sanitizeAll.js b/sanitiser/sanitizeAll.js new file mode 100644 index 00000000..886a64c3 --- /dev/null +++ b/sanitiser/sanitizeAll.js @@ -0,0 +1,26 @@ + +function sanitize( req, sanitizers, cb ){ + + // init an object to store clean + // (sanitized) input parameters + req.clean = {}; + + // source of input parameters + // (in this case from the GET querystring params) + var params = req.query || {}; + + for (var s in sanitizers) { + var sanity = sanitizers[s]( params, req.clean ); + + // errors + if( sanity.errors.length ){ + return cb( sanity.errors[0] ); + } + } + + return cb( undefined, req.clean ); + +} + +// export function +module.exports = sanitize; \ No newline at end of file diff --git a/sanitiser/search.js b/sanitiser/search.js index 19263732..c0cb059b 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -1,15 +1,15 @@ -var _sanitize = require('../sanitiser/_sanitize'), +var sanitizeAll = require('../sanitiser/sanitizeAll'), sanitizers = { text: require('../sanitiser/_text'), - size: require('../sanitiser/_size'), layers: require('../sanitiser/_layers'), + size: require('../sanitiser/_size'), source: require('../sanitiser/_source'), details: require('../sanitiser/_details'), - latlonzoom: require('../sanitiser/_geo_search') + geo_search: require('../sanitiser/_geo_search') }; -var sanitize = function(req, cb) { _sanitize(req, sanitizers, cb); }; +var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; // export sanitize for testing module.exports.sanitize = sanitize; diff --git a/test/unit/sanitiser/_source.js b/test/unit/sanitiser/_source.js index 235e18c6..991accd5 100644 --- a/test/unit/sanitiser/_source.js +++ b/test/unit/sanitiser/_source.js @@ -7,13 +7,15 @@ module.exports.tests = {}; module.exports.tests.no_sources = function(test, common) { test('source is not set', function(t) { var req = { - query: { } + query: { }, + clean: { } }; - var response = sanitize(req); + var response = sanitize(req.query, req.clean); t.deepEqual(req.clean.types, {}, 'clean.types should be empty object'); - t.deepEqual(response, success_response, 'no error returned'); + t.deepEqual(response.errors, [], 'no error returned'); + t.deepEqual(response.warnings, [], 'no warnings returned'); t.end(); }); @@ -21,13 +23,15 @@ module.exports.tests.no_sources = function(test, common) { var req = { query: { source: '' - } + }, + clean: { } }; - var response = sanitize(req); + var response = sanitize(req.query, req.clean); t.deepEqual(req.clean.types, {}, 'clean.types should be empty object'); - t.deepEqual(response, success_response, 'no error returned'); + t.deepEqual(response.errors, [], 'no error returned'); + t.deepEqual(response.warnings, [], 'no warnings returned'); t.end(); }); }; @@ -37,13 +41,15 @@ module.exports.tests.valid_sources = function(test, common) { var req = { query: { source: 'geonames' - } + }, + clean: { } }; - var response = sanitize(req); + var response = sanitize(req.query, req.clean); t.deepEqual(req.clean.types, { from_source: ['geoname'] }, 'clean.types should contain from_source entry with geonames'); - t.deepEqual(response, success_response, 'no error returned'); + t.deepEqual(response.errors, [], 'no error returned'); + t.deepEqual(response.warnings, [], 'no warnings returned'); t.end(); }); @@ -51,16 +57,18 @@ module.exports.tests.valid_sources = function(test, common) { var req = { query: { source: 'openstreetmap' - } + }, + clean: { } }; var expected_types = { from_source: ['osmaddress', 'osmnode', 'osmway'] }; - var response = sanitize(req); + var response = sanitize(req.query, req.clean); t.deepEqual(req.clean.types, expected_types, 'clean.types should contain from_source entry with multiple entries for openstreetmap'); - t.deepEqual(response, success_response, 'no error returned'); + t.deepEqual(response.errors, [], 'no error returned'); + t.deepEqual(response.warnings, [], 'no warnings returned'); t.end(); }); @@ -68,17 +76,19 @@ module.exports.tests.valid_sources = function(test, common) { var req = { query: { source: 'openstreetmap,openaddresses' - } + }, + clean: { } }; var expected_types = { from_source: ['osmaddress', 'osmnode', 'osmway', 'openaddresses'] }; - var response = sanitize(req); + var response = sanitize(req.query, req.clean); t.deepEqual(req.clean.types, expected_types, 'clean.types should contain from_source entry with multiple entries for openstreetmap and openadresses'); - t.deepEqual(response, success_response, 'no error returned'); + t.deepEqual(response.errors, [], 'no error returned'); + t.deepEqual(response.warnings, [], 'no warnings returned'); t.end(); }); }; @@ -88,14 +98,17 @@ module.exports.tests.invalid_sources = function(test, common) { var req = { query: { source: 'notasource' - } + }, + clean: { } }; var expected_response = { - error: true, - msg: '`notasource` is an invalid source parameter. Valid options: geonames, openaddresses, quattroshapes, openstreetmap' + errors: [ + '\'notasource\' is an invalid source parameter. Valid options: geonames,openaddresses,quattroshapes,openstreetmap' + ], + warnings: [] }; - var response = sanitize(req); + var response = sanitize(req.query, req.clean); t.deepEqual(response, expected_response, 'error with message returned'); t.deepEqual(req.clean.types, { }, 'clean.types should remain empty'); diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 744c7aa1..adbcbb32 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -291,6 +291,7 @@ module.exports.tests.sanitize_layers = function(test, common) { test('poi (alias) layer', function(t) { var poi_layers = ['geoname','osmnode','osmway']; sanitize({ layers: 'poi', text: 'test' }, function( err, clean ){ + t.equal(err, undefined); t.deepEqual(clean.types.from_layers, poi_layers, 'poi layers set'); t.end(); }); @@ -298,6 +299,7 @@ module.exports.tests.sanitize_layers = function(test, common) { test('admin (alias) layer', function(t) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; sanitize({ layers: 'admin', text: 'test' }, function( err, clean ){ + t.equal(err, undefined); t.deepEqual(clean.types.from_layers, admin_layers, 'admin layers set'); t.end(); }); @@ -305,6 +307,7 @@ module.exports.tests.sanitize_layers = function(test, common) { test('address (alias) layer', function(t) { var address_layers = ['osmaddress','openaddresses']; sanitize({ layers: 'address', text: 'test' }, function( err, clean ){ + t.equal(err, undefined); t.deepEqual(clean.types.from_layers, address_layers, 'types from layers set'); t.deepEqual(clean.types.from_address_parser, _text.allLayers, 'address parser uses default layers'); t.end(); @@ -314,6 +317,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var poi_layers = ['geoname','osmnode','osmway']; var reg_layers = ['admin0', 'admin1']; sanitize({ layers: 'poi,admin0,admin1', text: 'test' }, function( err, clean ){ + t.equal(err, undefined); t.deepEqual(clean.types.from_layers, reg_layers.concat(poi_layers), 'poi + regular layers'); t.end(); }); @@ -322,6 +326,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var admin_layers = ['admin0','admin1','admin2','neighborhood','locality','local_admin']; var reg_layers = ['geoname', 'osmway']; sanitize({ layers: 'admin,geoname,osmway', text: 'test' }, function( err, clean ){ + t.equal(err, undefined); t.deepEqual(clean.types.from_layers, reg_layers.concat(admin_layers), 'admin + regular layers set'); t.end(); }); @@ -330,6 +335,7 @@ module.exports.tests.sanitize_layers = function(test, common) { var address_layers = ['osmaddress','openaddresses']; var reg_layers = ['geoname', 'osmway']; sanitize({ layers: 'address,geoname,osmway', text: 'test' }, function( err, clean ){ + t.equal(err, undefined); t.deepEqual(clean.types.from_layers, reg_layers.concat(address_layers), 'address + regular layers set'); t.end(); }); @@ -337,6 +343,7 @@ module.exports.tests.sanitize_layers = function(test, common) { test('alias layer plus regular layers (no duplicates)', function(t) { var poi_layers = ['geoname','osmnode','osmway']; sanitize({ layers: 'poi,geoname,osmnode', text: 'test' }, function( err, clean ){ + t.equal(err, undefined); t.deepEqual(clean.types.from_layers, poi_layers, 'poi layers found (no duplicates)'); t.end(); }); @@ -344,6 +351,7 @@ module.exports.tests.sanitize_layers = function(test, common) { test('multiple alias layers (no duplicates)', function(t) { var alias_layers = ['geoname','osmnode','osmway','admin0','admin1','admin2','neighborhood','locality','local_admin']; sanitize({ layers: 'poi,admin', text: 'test' }, function( err, clean ){ + t.equal(err, undefined); t.deepEqual(clean.types.from_layers, alias_layers, 'all layers found (no duplicates)'); t.end(); }); From 052904e2e56df43709b084c4783bb60c040ea7c4 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 14 Sep 2015 16:23:29 +0200 Subject: [PATCH 2/4] documentation typo --- sanitiser/_geo_common.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js index faf99fd0..2ed92656 100644 --- a/sanitiser/_geo_common.js +++ b/sanitiser/_geo_common.js @@ -9,8 +9,8 @@ var util = require( 'util' ); * bbox = left, bottom, right, top * bbox = min Longitude, min Latitude, max Longitude, max Latitude * + * @param {object} unclean * @param {object} clean - * @param {string} param */ function sanitize_bbox( unclean, clean ) { if( !unclean.bbox ) { From e532c2a88a41fce7f0268e05bf24e999f871fb99 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 14 Sep 2015 16:26:04 +0200 Subject: [PATCH 3/4] improved error checking --- sanitiser/_geo_common.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js index 2ed92656..b53e13c3 100644 --- a/sanitiser/_geo_common.js +++ b/sanitiser/_geo_common.js @@ -1,7 +1,8 @@ /** * helper sanitiser methods for geo parameters */ -var util = require( 'util' ); +var util = require('util'), + check = require('check-types'); /** * Parse and validate bbox parameter @@ -13,7 +14,7 @@ var util = require( 'util' ); * @param {object} clean */ function sanitize_bbox( unclean, clean ) { - if( !unclean.bbox ) { + if( !check.unemptyString( unclean.bbox ) ) { return; } From 10242a8682ddb1e8d4315ca0e2cbe1af55b2cf96 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Mon, 14 Sep 2015 17:59:48 +0200 Subject: [PATCH 4/4] rename unclean->raw --- sanitiser/_categories.js | 6 +++--- sanitiser/_details.js | 6 +++--- sanitiser/_geo_common.js | 8 ++++---- sanitiser/_geo_reverse.js | 8 ++++---- sanitiser/_geo_search.js | 8 ++++---- sanitiser/_id.js | 26 +++++++++++++------------- sanitiser/_layers.js | 6 +++--- sanitiser/_size.js | 4 ++-- sanitiser/_source.js | 6 +++--- sanitiser/_text.js | 6 +++--- 10 files changed, 42 insertions(+), 42 deletions(-) diff --git a/sanitiser/_categories.js b/sanitiser/_categories.js index 3f465186..e76e8767 100644 --- a/sanitiser/_categories.js +++ b/sanitiser/_categories.js @@ -2,7 +2,7 @@ var check = require('check-types'); // validate inputs, convert types and apply defaults -function sanitize( unclean, clean ){ +function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; @@ -11,10 +11,10 @@ function sanitize( unclean, clean ){ clean.categories = []; // if categories string has been set - if( check.unemptyString( unclean.categories ) ){ + if( check.unemptyString( raw.categories ) ){ // map input categories to valid format - clean.categories = unclean.categories.split(',') + clean.categories = raw.categories.split(',') .map(function (cat) { return cat.toLowerCase().trim(); // lowercase inputs }) diff --git a/sanitiser/_details.js b/sanitiser/_details.js index 7f621959..e17c656c 100644 --- a/sanitiser/_details.js +++ b/sanitiser/_details.js @@ -3,13 +3,13 @@ var check = require('check-types'); var DEFAULT_DETAILS_BOOL = true; // validate inputs, convert types and apply defaults -function sanitize( unclean, clean ){ +function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; - if( !check.undefined( unclean.details ) ){ - clean.details = isTruthy( unclean.details ); + if( !check.undefined( raw.details ) ){ + clean.details = isTruthy( raw.details ); } else { clean.details = DEFAULT_DETAILS_BOOL; } diff --git a/sanitiser/_geo_common.js b/sanitiser/_geo_common.js index b53e13c3..71b728cb 100644 --- a/sanitiser/_geo_common.js +++ b/sanitiser/_geo_common.js @@ -10,15 +10,15 @@ var util = require('util'), * bbox = left, bottom, right, top * bbox = min Longitude, min Latitude, max Longitude, max Latitude * - * @param {object} unclean + * @param {object} raw * @param {object} clean */ -function sanitize_bbox( unclean, clean ) { - if( !check.unemptyString( unclean.bbox ) ) { +function sanitize_bbox( raw, clean ) { + if( !check.unemptyString( raw.bbox ) ) { return; } - var bboxArr = unclean.bbox.split( ',' ); + var bboxArr = raw.bbox.split( ',' ); if( Array.isArray( bboxArr ) && bboxArr.length === 4 ) { var bbox = bboxArr.map(parseFloat); diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js index 7b3cad58..4b259199 100644 --- a/sanitiser/_geo_reverse.js +++ b/sanitiser/_geo_reverse.js @@ -5,18 +5,18 @@ var LAT_LON_IS_REQUIRED = true, CIRCLE_MUST_BE_COMPLETE = false; // validate inputs, convert types and apply defaults -module.exports = function sanitize( unclean, clean ){ +module.exports = function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; try { - geo_common.sanitize_coord( 'lat', clean, unclean['point.lat'], LAT_LON_IS_REQUIRED ); - geo_common.sanitize_coord( 'lon', clean, unclean['point.lon'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'lat', clean, raw['point.lat'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'lon', clean, raw['point.lon'], LAT_LON_IS_REQUIRED ); // boundary.circle.* is not mandatory, and only specifying radius is fine, // as point.lat/lon will be used to fill those values by default - geo_common.sanitize_boundary_circle( clean, unclean, CIRCLE_IS_REQUIRED, CIRCLE_MUST_BE_COMPLETE); + geo_common.sanitize_boundary_circle( clean, raw, CIRCLE_IS_REQUIRED, CIRCLE_MUST_BE_COMPLETE); } catch (err) { messages.errors.push( err.message ); diff --git a/sanitiser/_geo_search.js b/sanitiser/_geo_search.js index c3ff6a76..cc2fafcb 100644 --- a/sanitiser/_geo_search.js +++ b/sanitiser/_geo_search.js @@ -3,15 +3,15 @@ var geo_common = require ('./_geo_common'); var LAT_LON_IS_REQUIRED = false; // validate inputs, convert types and apply defaults -module.exports = function sanitize( unclean, clean ){ +module.exports = function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; try { - geo_common.sanitize_coord( 'lat', clean, unclean['focus.point.lat'], LAT_LON_IS_REQUIRED ); - geo_common.sanitize_coord( 'lon', clean, unclean['focus.point.lon'], LAT_LON_IS_REQUIRED ); - geo_common.sanitize_bbox(unclean, clean); + geo_common.sanitize_coord( 'lat', clean, raw['focus.point.lat'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_coord( 'lon', clean, raw['focus.point.lon'], LAT_LON_IS_REQUIRED ); + geo_common.sanitize_bbox(raw, clean); } catch (err) { messages.errors.push( err.message ); diff --git a/sanitiser/_id.js b/sanitiser/_id.js index 296b6907..77d21fa8 100644 --- a/sanitiser/_id.js +++ b/sanitiser/_id.js @@ -12,21 +12,21 @@ function errorMessage(fieldname, message) { return message || 'invalid param \''+ fieldname + '\': text length, must be >0'; } -function sanitize( unclean, clean ){ +function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; - // 'unclean.id' can be an array!? - var uncleanIds = check.array( unclean.id ) ? unclean.id : [ unclean.id ]; + // 'raw.id' can be an array!? + var rawIds = check.array( raw.id ) ? raw.id : [ raw.id ]; // de-dupe ids - uncleanIds = uncleanIds.filter(function(item, pos) { - return uncleanIds.indexOf( item ) === pos; + rawIds = rawIds.filter(function(item, pos) { + return rawIds.indexOf( item ) === pos; }); // ensure all elements are valid non-empty strings - uncleanIds = uncleanIds.filter( function( uc ){ + rawIds = rawIds.filter( function( uc ){ if( !check.unemptyString( uc ) ){ messages.errors.push( errorMessage('id') ); return false; @@ -37,12 +37,12 @@ function sanitize( unclean, clean ){ // init 'clean.ids' clean.ids = []; - // cycle through unclean ids and set those which are valid - uncleanIds.forEach( function( uncleanId ){ + // cycle through raw ids and set those which are valid + rawIds.forEach( function( rawId ){ - var param_index = uncleanId.indexOf(ID_DELIM); - var type = uncleanId.substring(0, param_index ); - var id = uncleanId.substring(param_index + 1); + var param_index = rawId.indexOf(ID_DELIM); + var type = rawId.substring(0, param_index ); + var id = rawId.substring(param_index + 1); // basic format/ presence of ':' if(param_index === -1) { @@ -53,11 +53,11 @@ function sanitize( unclean, clean ){ // id text if( !check.unemptyString( id ) ){ - messages.errors.push( errorMessage( uncleanId ) ); + messages.errors.push( errorMessage( rawId ) ); } // type text if( !check.unemptyString( type ) ){ - messages.errors.push( errorMessage( uncleanId ) ); + messages.errors.push( errorMessage( rawId ) ); } // type text must be one of the types if( types.indexOf( type ) === -1 ){ diff --git a/sanitiser/_layers.js b/sanitiser/_layers.js index 7c0edc4d..b7922557 100644 --- a/sanitiser/_layers.js +++ b/sanitiser/_layers.js @@ -9,7 +9,7 @@ var ALIAS_LAYERS = ['poi', 'admin', 'address'], ALIAS_TYPES_JOINED = ALIAS_TYPES.join(','); // validate inputs, convert types and apply defaults -function sanitize( unclean, clean ){ +function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; @@ -19,10 +19,10 @@ function sanitize( unclean, clean ){ // default case (no layers specified in GET params) // don't even set the from_layers key in this case - if( check.unemptyString( unclean.layers ) ){ + if( check.unemptyString( raw.layers ) ){ // parse GET params - var layers = unclean.layers.split(',').map( function( layer ){ + var layers = raw.layers.split(',').map( function( layer ){ return layer.toLowerCase(); // lowercase inputs }); diff --git a/sanitiser/_size.js b/sanitiser/_size.js index 34bc1b37..eb93444b 100644 --- a/sanitiser/_size.js +++ b/sanitiser/_size.js @@ -4,13 +4,13 @@ var MIN_SIZE = 1, DEFAULT_SIZE = 10; // validate inputs, convert types and apply defaults -function sanitize( unclean, clean ){ +function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; // coercions - var _size = parseInt( unclean.size, 10 ); + var _size = parseInt( raw.size, 10 ); // invalid numeric input // @todo: this can be removed now as queries have default sizes? diff --git a/sanitiser/_source.js b/sanitiser/_source.js index 4c87967a..88d620fc 100644 --- a/sanitiser/_source.js +++ b/sanitiser/_source.js @@ -5,7 +5,7 @@ var check = require('check-types'), var ALL_SOURCES = Object.keys(sources_map), ALL_SOURCES_JOINED = ALL_SOURCES.join(','); -function sanitize( unclean, clean ) { +function sanitize( raw, clean ) { // error & warning messages var messages = { errors: [], warnings: [] }; @@ -15,9 +15,9 @@ function sanitize( unclean, clean ) { // default case (no layers specified in GET params) // don't even set the from_layers key in this case - if( check.unemptyString( unclean.source ) ){ + if( check.unemptyString( raw.source ) ){ - var sources = unclean.source.split(','); + var sources = raw.source.split(','); var invalid_sources = sources.filter(function(source) { return ALL_SOURCES.indexOf(source) === -1; diff --git a/sanitiser/_text.js b/sanitiser/_text.js index e3268a54..2f1f96af 100644 --- a/sanitiser/_text.js +++ b/sanitiser/_text.js @@ -3,13 +3,13 @@ var check = require('check-types'), query_parser = require('../helper/query_parser'); // validate texts, convert types and apply defaults -function sanitize( unclean, clean ){ +function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; // invalid input 'text' - if( !check.unemptyString( unclean.text ) ){ + if( !check.unemptyString( raw.text ) ){ messages.errors.push('invalid param \'text\': text length, must be >0'); } @@ -17,7 +17,7 @@ function sanitize( unclean, clean ){ else { // valid text - clean.text = unclean.text; + clean.text = raw.text; // parse text with query parser clean.parsed_text = query_parser.get_parsed_address(clean.text);