diff --git a/controller/place.js b/controller/place.js index 45dac807..97461e71 100644 --- a/controller/place.js +++ b/controller/place.js @@ -1,10 +1,18 @@ var service = { mget: require('../service/mget') }; function setup( backend ){ + // allow overriding of dependencies backend = backend || require('../src/backend'); function controller( req, res, next ){ + + // do not run controller when a request + // validation error has occurred. + if( req.errors && req.errors.length ){ + return next(); + } + var query = req.clean.ids.map( function(id) { return { _index: 'pelias', diff --git a/controller/search.js b/controller/search.js index 18f18393..2495c37a 100644 --- a/controller/search.js +++ b/controller/search.js @@ -8,6 +8,12 @@ function setup( backend, query ){ function controller( req, res, next ){ + // do not run controller when a request + // validation error has occurred. + if( req.errors && req.errors.length ){ + return next(); + } + // backend command var cmd = { index: 'pelias', diff --git a/middleware/geocodeJSON.js b/middleware/geocodeJSON.js index 8293190a..01caa01c 100644 --- a/middleware/geocodeJSON.js +++ b/middleware/geocodeJSON.js @@ -14,11 +14,6 @@ function setup(peliasConfig) { function convertToGeocodeJSON(peliasConfig, req, res, next) { - // do nothing if no result data set - if (!res || !res.data) { - return next(); - } - res.body = { geocoding: {} }; // REQUIRED. A semver.org compliant version number. Describes the version of @@ -38,8 +33,8 @@ function convertToGeocodeJSON(peliasConfig, req, res, next) { res.body.geocoding.query = req.clean; // OPTIONAL. Warnings and errors. - addMessages(res, 'warnings', res.body.geocoding); - addMessages(res, 'errors', res.body.geocoding); + addMessages(req, 'warnings', res.body.geocoding); + addMessages(req, 'errors', res.body.geocoding); // OPTIONAL // Freeform @@ -49,15 +44,14 @@ function convertToGeocodeJSON(peliasConfig, req, res, next) { res.body.geocoding.timestamp = new Date().getTime(); // convert docs to geojson and merge with geocoding block - extend(res.body, geojsonify(res.data)); + extend(res.body, geojsonify(res.data || [])); next(); } -function addMessages(results, msgType, geocoding) { - if (results.hasOwnProperty(msgType)) { - geocoding.messages = geocoding.messages || {}; - geocoding.messages[msgType] = results[msgType]; +function addMessages(req, msgType, geocoding) { + if (req.hasOwnProperty(msgType) && req[msgType].length) { + geocoding[msgType] = req[msgType]; } } diff --git a/sanitiser/_geo_reverse.js b/sanitiser/_geo_reverse.js index 4b259199..da2bb79c 100644 --- a/sanitiser/_geo_reverse.js +++ b/sanitiser/_geo_reverse.js @@ -14,6 +14,13 @@ module.exports = function sanitize( raw, clean ){ 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 ); + // remove both if only one is set + // @todo: clean this up! + if( !clean.hasOwnProperty('lat') || !clean.hasOwnProperty('lon') ){ + delete clean.lat; + delete clean.lon; + } + // 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, raw, CIRCLE_IS_REQUIRED, CIRCLE_MUST_BE_COMPLETE); diff --git a/sanitiser/_id.js b/sanitiser/_id.js index ff6834ca..4d9c1c71 100644 --- a/sanitiser/_id.js +++ b/sanitiser/_id.js @@ -18,8 +18,18 @@ function sanitize( raw, clean ){ // error & warning messages var messages = { errors: [], warnings: [] }; - // 'raw.id' can be an array!? - var rawIds = check.array( raw.id ) ? _.unique( raw.id ) : [ raw.id ]; + // 'raw.id' can be an array + var rawIds = check.array( raw.id ) ? _.unique( raw.id ) : []; + + // 'raw.id' can be a string + if( check.unemptyString( raw.id ) ){ + rawIds.push( raw.id ); + } + + // no ids provided + if( !rawIds.length ){ + messages.errors.push( errorMessage('id') ); + } // ensure all elements are valid non-empty strings rawIds = rawIds.filter( function( uc ){ @@ -48,25 +58,27 @@ function sanitize( raw, clean ){ } // id text - if( !check.unemptyString( id ) ){ + else if( !check.unemptyString( id ) ){ messages.errors.push( errorMessage( rawId ) ); } // type text - if( !check.unemptyString( type ) ){ + else if( !check.unemptyString( type ) ){ messages.errors.push( errorMessage( rawId ) ); } // type text must be one of the types - if( !_.contains( types, type ) ){ + else if( !_.contains( types, type ) ){ messages.errors.push( errorMessage('type', type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']') ); } - // add valid id to 'clean.ids' array - clean.ids.push({ - id: id, - type: type - }); + else { + clean.ids.push({ + id: id, + type: type + }); + } + }); return messages; diff --git a/sanitiser/place.js b/sanitiser/place.js index 8b5e6034..44deb267 100644 --- a/sanitiser/place.js +++ b/sanitiser/place.js @@ -14,10 +14,6 @@ module.exports.sanitiser_list = sanitizers; // middleware module.exports.middleware = function( req, res, next ){ sanitize( req, function( err, clean ){ - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } next(); }); }; diff --git a/sanitiser/reverse.js b/sanitiser/reverse.js index 532b3652..49c93f2e 100644 --- a/sanitiser/reverse.js +++ b/sanitiser/reverse.js @@ -19,10 +19,6 @@ module.exports.sanitiser_list = sanitizers; // middleware module.exports.middleware = function( req, res, next ){ sanitize( req, function( err, clean ){ - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } next(); }); }; diff --git a/sanitiser/sanitizeAll.js b/sanitiser/sanitizeAll.js index 886a64c3..1c2dd2af 100644 --- a/sanitiser/sanitizeAll.js +++ b/sanitiser/sanitizeAll.js @@ -1,10 +1,16 @@ +var check = require('check-types'); + function sanitize( req, sanitizers, cb ){ // init an object to store clean // (sanitized) input parameters req.clean = {}; + // init erros and warnings arrays + req.errors = []; + req.warnings = []; + // source of input parameters // (in this case from the GET querystring params) var params = req.query || {}; @@ -12,14 +18,21 @@ function sanitize( req, sanitizers, cb ){ for (var s in sanitizers) { var sanity = sanitizers[s]( params, req.clean ); - // errors + // if errors occurred then set them + // on the req object. if( sanity.errors.length ){ - return cb( sanity.errors[0] ); + req.errors = req.errors.concat( sanity.errors ); + } + + // if warnings occurred then set them + // on the req object. + if( sanity.warnings.length ){ + req.warnings = req.warnings.concat( sanity.warnings ); } } - - return cb( undefined, req.clean ); + // @todo remove these args, they do not need to be passed out + return cb( undefined, req.clean ); } // export function diff --git a/sanitiser/search.js b/sanitiser/search.js index 9f22def9..ded714b4 100644 --- a/sanitiser/search.js +++ b/sanitiser/search.js @@ -20,10 +20,6 @@ module.exports.sanitiser_list = sanitizers; // middleware module.exports.middleware = function( req, res, next ){ sanitize( req, function( err, clean ){ - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } next(); }); }; diff --git a/test/unit/sanitiser/place.js b/test/unit/sanitiser/place.js index 812744a8..75fb87ff 100644 --- a/test/unit/sanitiser/place.js +++ b/test/unit/sanitiser/place.js @@ -1,6 +1,8 @@ +// @todo: refactor this test, it's pretty messy, brittle and hard to follow + var place = require('../../../sanitiser/place'), - _sanitize = place.sanitize, + sanitize = place.sanitize, middleware = place.middleware, types = require('../../../query/types'), delimiter = ':', @@ -11,12 +13,14 @@ var place = require('../../../sanitiser/place'), var type = input.split(delimiter)[0]; return type + ' is invalid. It must be one of these values - [' + types.join(', ') + ']'; }, defaultClean = { ids: [ { id: '123', type: 'geoname' } ], private: false }, - sanitize = function(query, cb) { _sanitize({'query':query}, cb); }, inputs = { valid: [ 'geoname:1', 'osmnode:2', 'admin0:53', 'osmway:44', 'geoname:5' ], invalid: [ ':', '', '::', 'geoname:', ':234', 'gibberish:23' ] }; +// these are the default values you would expect when no input params are specified. +var emptyClean = { ids: [], private: false }; + module.exports.tests = {}; module.exports.tests.interface = function(test, common) { @@ -33,33 +37,35 @@ module.exports.tests.interface = function(test, common) { }; module.exports.tests.sanitize_id = function(test, common) { - test('invalid input', function(t) { + test('id: invalid input', function(t) { inputs.invalid.forEach( function( input ){ - sanitize({ id: input }, function( err, clean ){ - switch (err) { + var req = { query: { id: input } }; + sanitize(req, function( ){ + switch (req.errors[0]) { case defaultError: - t.equal(err, defaultError, input + ' is invalid input'); break; + t.equal(req.errors[0], defaultError, input + ' is invalid input'); break; case defaultLengthError(input): - t.equal(err, defaultLengthError(input), input + ' is invalid (missing id/type)'); break; + t.equal(req.errors[0], defaultLengthError(input), input + ' is invalid (missing id/type)'); break; case defaultFormatError: - t.equal(err, defaultFormatError, input + ' is invalid (invalid format)'); break; + t.equal(req.errors[0], defaultFormatError, input + ' is invalid (invalid format)'); break; case defaultMissingTypeError(input): - t.equal(err, defaultMissingTypeError(input), input + ' is an unknown type'); break; + t.equal(req.errors[0], defaultMissingTypeError(input), input + ' is an unknown type'); break; default: break; } - t.equal(clean, undefined, 'clean not set'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); }); t.end(); }); - test('valid input', function(t) { + test('id: valid input', function(t) { inputs.valid.forEach( function( input ){ var input_parts = input.split(delimiter); var expected = { ids: [ { id: input_parts[1], type: input_parts[0] } ], private: false }; - sanitize({ id: input }, function( err, clean ){ - t.equal(err, undefined, 'no error (' + input + ')' ); - t.deepEqual(clean, expected, 'clean set correctly (' + input + ')'); + var req = { query: { id: input } }; + sanitize(req, function(){ + t.deepEqual( req.errors, [], 'no error (' + input + ')' ); + t.deepEqual( req.clean, expected, 'clean set correctly (' + input + ')'); }); }); t.end(); @@ -68,26 +74,26 @@ module.exports.tests.sanitize_id = function(test, common) { module.exports.tests.sanitize_ids = function(test, common) { - test('invalid input', function(t) { - sanitize({ id: inputs.invalid }, function( err, clean ){ - var input = inputs.invalid[0]; // since it breaks on the first invalid element - switch (err) { - case defaultError: - t.equal(err, defaultError, input + ' is invalid input'); break; - case defaultLengthError(input): - t.equal(err, defaultLengthError(input), input + ' is invalid (missing id/type)'); break; - case defaultFormatError: - t.equal(err, defaultFormatError, input + ' is invalid (invalid format)'); break; - case defaultMissingTypeError(input): - t.equal(err, defaultMissingTypeError(input), input + ' is an unknown type'); break; - default: break; - } - t.equal(clean, undefined, 'clean not set'); + test('ids: invalid input', function(t) { + var req = { query: { id: inputs.invalid } }; + var expected = [ + 'invalid param \'id\': text length, must be >0', + 'invalid param \':\': text length, must be >0', + 'invalid param \'::\': text length, must be >0', + 'invalid param \'geoname:\': text length, must be >0', + 'invalid param \':234\': text length, must be >0', + 'gibberish is invalid. It must be one of these values - ' + + '[geoname, osmnode, osmway, admin0, admin1, admin2, neighborhood, ' + + 'locality, local_admin, osmaddress, openaddresses]' + ]; + sanitize(req, function(){ + t.deepEqual(req.errors, expected); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); t.end(); }); - test('valid input', function(t) { + test('ids: valid input', function(t) { var expected={}; expected.ids = []; inputs.valid.forEach( function( input ){ @@ -95,9 +101,10 @@ module.exports.tests.sanitize_ids = function(test, common) { expected.ids.push({ id: input_parts[1], type: input_parts[0] }); }); expected.private = false; - sanitize({ id: inputs.valid }, function( err, clean ){ - t.equal(err, undefined, 'no error' ); - t.deepEqual(clean, expected, 'clean set correctly'); + var req = { query: { id: inputs.valid } }; + sanitize(req, function(){ + t.deepEqual( req.errors, [], 'no errors' ); + t.deepEqual( req.clean, expected, 'clean set correctly' ); }); t.end(); }); @@ -107,8 +114,11 @@ module.exports.tests.sanitize_private = function(test, common) { var invalid_values = [null, -1, 123, NaN, 'abc']; invalid_values.forEach(function(value) { test('invalid private param ' + value, function(t) { - sanitize({ id:'geoname:123', 'private': value}, function( err, clean ){ - t.equal(clean.private, false, 'default private set (to false)'); + var req = { query: { id:'geoname:123', 'private': value } }; + sanitize(req, function(){ + t.deepEqual( req.errors, [], 'no errors' ); + t.deepEqual( req.warnings, [], 'no warnings' ); + t.equal(req.clean.private, false, 'default private set (to false)'); t.end(); }); }); @@ -117,8 +127,11 @@ module.exports.tests.sanitize_private = function(test, common) { var valid_values = ['true', true, 1]; valid_values.forEach(function(value) { test('valid private param ' + value, function(t) { - sanitize({ id:'geoname:123', 'private': value }, function( err, clean ){ - t.equal(clean.private, true, 'private set to true'); + var req = { query: { id:'geoname:123', 'private': value } }; + sanitize(req, function(){ + t.deepEqual( req.errors, [], 'no errors' ); + t.deepEqual( req.warnings, [], 'no warnings' ); + t.equal(req.clean.private, true, 'private set to true'); t.end(); }); }); @@ -127,16 +140,22 @@ module.exports.tests.sanitize_private = function(test, common) { var valid_false_values = ['false', false, 0]; valid_false_values.forEach(function(value) { test('test setting false explicitly ' + value, function(t) { - sanitize({ id:'geoname:123', 'private': value }, function( err, clean ){ - t.equal(clean.private, false, 'private set to false'); + var req = { query: { id:'geoname:123', 'private': value } }; + sanitize(req, function(){ + t.deepEqual( req.errors, [], 'no errors' ); + t.deepEqual( req.warnings, [], 'no warnings' ); + t.equal(req.clean.private, false, 'private set to false'); t.end(); }); }); }); test('test default behavior', function(t) { - sanitize({ id:'geoname:123' }, function( err, clean ){ - t.equal(clean.private, false, 'private set to false'); + var req = { query: { id:'geoname:123' } }; + sanitize(req, function(){ + t.deepEqual( req.errors, [], 'no errors' ); + t.deepEqual( req.warnings, [], 'no warnings' ); + t.equal(req.clean.private, false, 'private set to false'); t.end(); }); }); @@ -144,10 +163,12 @@ module.exports.tests.sanitize_private = function(test, common) { module.exports.tests.de_dupe = function(test, common) { var expected = { ids: [ { id: '1', type: 'geoname' }, { id: '2', type: 'osmnode' } ], private: false }; + var req = { query: { id: ['geoname:1', 'osmnode:2', 'geoname:1'] } }; test('duplicate ids', function(t) { - sanitize( { id: ['geoname:1', 'osmnode:2', 'geoname:1'] }, function( err, clean ){ - t.equal(err, undefined, 'no error' ); - t.deepEqual(clean, expected, 'clean set correctly'); + sanitize( req, function(){ + t.deepEqual( req.errors, [], 'no errors' ); + t.deepEqual( req.warnings, [], 'no warnings' ); + t.deepEqual(req.clean, expected, 'clean set correctly'); t.end(); }); }); @@ -155,31 +176,21 @@ module.exports.tests.de_dupe = function(test, common) { module.exports.tests.invalid_params = function(test, common) { test('invalid params', function(t) { - sanitize( undefined, function( err, clean ){ - t.equal(err, defaultError, 'handle invalid params gracefully'); + var req = { query: {} }; + sanitize( req, function(){ + t.equal( req.errors[0], defaultError, 'handle invalid params gracefully'); + t.deepEqual( req.warnings, [], 'no warnings' ); t.end(); }); }); }; -module.exports.tests.middleware_failure = function(test, common) { - test('middleware failure', function(t) { - var res = { status: function( code ){ - t.equal(code, 400, 'status set'); - }}; - var next = function( message ){ - t.equal(message, defaultError); - t.end(); - }; - middleware( {}, res, next ); - }); -}; - module.exports.tests.middleware_success = function(test, common) { test('middleware success', function(t) { - var req = { query: { id: 'geoname' + delimiter + '123' }}; - var next = function( message ){ - t.equal(message, undefined, 'no error message set'); + var req = { query: { id: 'geoname:123' }}; + var next = function(){ + t.deepEqual( req.errors, [], 'no errors' ); + t.deepEqual( req.warnings, [], 'no warnings' ); t.deepEqual(req.clean, defaultClean); t.end(); }; diff --git a/test/unit/sanitiser/reverse.js b/test/unit/sanitiser/reverse.js index cd72eecd..5fb346b7 100644 --- a/test/unit/sanitiser/reverse.js +++ b/test/unit/sanitiser/reverse.js @@ -1,6 +1,8 @@ +// @todo: refactor this test, it's pretty messy, brittle and hard to follow + var reverse = require('../../../sanitiser/reverse'), - _sanitize = reverse.sanitize, + sanitize = reverse.sanitize, middleware = reverse.middleware, defaultError = 'missing param \'lat\'', defaultClean = { lat:0, @@ -11,8 +13,11 @@ var reverse = require('../../../sanitiser/reverse'), private: false, categories: [], boundary: { } - }, - sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; + }; + +// these are the default values you would expect when no input params are specified. +// @todo: why is this different from $defaultClean? +var emptyClean = { boundary: {}, categories: [], private: false, size: 10, types: {} }; module.exports.tests = {}; @@ -45,29 +50,32 @@ module.exports.tests.sanitize_lat = function(test, common) { }; test('invalid lat', function(t) { lats.invalid.forEach( function( lat ){ - sanitize({ 'point.lat': lat, 'point.lon': 0 }, function( err, clean ){ - t.equal(err, 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); - t.equal(clean, undefined, 'clean not set'); + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.errors[0], 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); }); t.end(); }); test('valid lat', function(t) { lats.valid.forEach( function( lat ){ - sanitize({ 'point.lat': lat, 'point.lon': 0 }, function( err, clean ){ + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lat = parseFloat( lat ); - t.equal(err, undefined, 'no error'); - t.equal(clean.lat, parseFloat(lat), 'clean set correctly (' + lat + ')'); + t.deepEqual(req.errors, [], 'no errors'); + t.equal(req.clean.lat, parseFloat(lat), 'clean set correctly (' + lat + ')'); }); }); t.end(); }); test('missing lat', function(t) { lats.missing.forEach( function( lat ){ - sanitize({ 'point.lat': lat, 'point.lon': 0 }, function( err, clean ){ - t.equal(err, 'missing param \'lat\'', 'latitude is a required field'); - t.equal(clean, undefined, 'clean not set'); + var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.errors[0], 'missing param \'lat\'', 'latitude is a required field'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); }); t.end(); @@ -81,43 +89,50 @@ module.exports.tests.sanitize_lon = function(test, common) { }; test('valid lon', function(t) { lons.valid.forEach( function( lon ){ - sanitize({ 'point.lat': 0, 'point.lon': lon }, function( err, clean ){ + var req = { query: { 'point.lat': 0, 'point.lon': lon } }; + sanitize(req, function(){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lon = parseFloat( lon ); - t.equal(err, undefined, 'no error'); - t.equal(clean.lon, parseFloat(lon), 'clean set correctly (' + lon + ')'); + t.deepEqual(req.errors, [], 'no errors'); + t.equal(req.clean.lon, parseFloat(lon), 'clean set correctly (' + lon + ')'); }); }); t.end(); }); test('missing lon', function(t) { lons.missing.forEach( function( lon ){ - sanitize({ 'point.lat': 0, 'point.lon': lon }, function( err, clean ){ - t.equal(err, 'missing param \'lon\'', 'longitude is a required field'); - t.equal(clean, undefined, 'clean not set'); + var req = { query: { 'point.lat': 0, 'point.lon': lon } }; + + // @todo: why is lat set? + var expected = { boundary: {}, categories: [], lat: 0, private: false, size: 10, types: {} }; + sanitize(req, function(){ + t.equal(req.errors[0], 'missing param \'lon\'', 'longitude is a required field'); + t.deepEqual(req.clean, expected, 'clean only has default values set'); }); }); t.end(); }); }; - module.exports.tests.sanitize_size = function(test, common) { test('invalid size value', function(t) { - sanitize({ size: 'a', 'point.lat': 0, 'point.lon': 0 }, function( err, clean ){ - t.equal(clean.size, 10, 'default size set'); + var req = { query: { size: 'a', 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 10, 'default size set'); t.end(); }); }); test('below min size value', function(t) { - sanitize({ size: -100, 'point.lat': 0, 'point.lon': 0 }, function( err, clean ){ - t.equal(clean.size, 1, 'min size set'); + var req = { query: { size: -100, 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 1, 'min size set'); t.end(); }); }); test('above max size value', function(t) { - sanitize({ size: 9999, 'point.lat': 0, 'point.lon': 0 }, function( err, clean ){ - t.equal(clean.size, 40, 'max size set'); + var req = { query: { size: 9999, 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 40, 'max size set'); t.end(); }); }); @@ -127,8 +142,9 @@ module.exports.tests.sanitize_private = function(test, common) { var invalid_values = [null, -1, 123, NaN, 'abc']; invalid_values.forEach(function(value) { test('invalid private param ' + value, function(t) { - sanitize({ 'point.lat': 0, 'point.lon': 0, 'private': value}, function( err, clean ){ - t.equal(clean.private, false, 'default private set (to false)'); + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'default private set (to false)'); t.end(); }); }); @@ -137,8 +153,9 @@ module.exports.tests.sanitize_private = function(test, common) { var valid_values = ['true', true, 1, '1']; valid_values.forEach(function(value) { test('valid private param ' + value, function(t) { - sanitize({ 'point.lat': 0, 'point.lon': 0, 'private': value }, function( err, clean ){ - t.equal(clean.private, true, 'private set to true'); + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, true, 'private set to true'); t.end(); }); }); @@ -147,78 +164,67 @@ module.exports.tests.sanitize_private = function(test, common) { var valid_false_values = ['false', false, 0]; valid_false_values.forEach(function(value) { test('test setting false explicitly ' + value, function(t) { - sanitize({ 'point.lat': 0, 'point.lon': 0, 'private': value }, function( err, clean ){ - t.equal(clean.private, false, 'private set to false'); + var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); t.end(); }); }); }); test('test default behavior', function(t) { - sanitize({ 'point.lat': 0, 'point.lon': 0 }, function( err, clean ){ - t.equal(clean.private, false, 'private set to false'); + var req = { query: { 'point.lat': 0, 'point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); t.end(); }); }); }; module.exports.tests.sanitize_categories = function(test, common) { - var queryParams = { 'point.lat': 0, 'point.lon': 0 }; + var req = { query: { 'point.lat': 0, 'point.lon': 0 } }; test('unspecified', function(t) { - queryParams.categories = undefined; - sanitize(queryParams, function( err, clean ){ - t.deepEqual(clean.categories, defaultClean.categories, 'default to empty categories array'); + req.query.categories = undefined; + sanitize(req, function(){ + t.deepEqual(req.clean.categories, defaultClean.categories, 'default to empty categories array'); t.end(); }); }); test('single category', function(t) { - queryParams.categories = 'food'; - sanitize(queryParams, function( err, clean ){ - t.deepEqual(clean.categories, ['food'], 'category set'); + req.query.categories = 'food'; + sanitize(req, function(){ + t.deepEqual(req.clean.categories, ['food'], 'category set'); t.end(); }); }); test('multiple categories', function(t) { - queryParams.categories = 'food,education,nightlife'; - sanitize(queryParams, function( err, clean ){ - t.deepEqual(clean.categories, ['food', 'education', 'nightlife'], 'categories set'); + req.query.categories = 'food,education,nightlife'; + sanitize(req, function(){ + t.deepEqual(req.clean.categories, ['food', 'education', 'nightlife'], 'categories set'); t.end(); }); }); test('whitespace and empty strings', function(t) { - queryParams.categories = 'food, , nightlife ,'; - sanitize(queryParams, function( err, clean ){ - t.deepEqual(clean.categories, ['food', 'nightlife'], 'categories set'); + req.query.categories = 'food, , nightlife ,'; + sanitize(req, function(){ + t.deepEqual(req.clean.categories, ['food', 'nightlife'], 'categories set'); t.end(); }); }); test('all empty strings', function(t) { - queryParams.categories = ', , ,'; - sanitize(queryParams, function( err, clean ){ - t.deepEqual(clean.categories, defaultClean.categories, 'empty strings filtered out'); + req.query.categories = ', , ,'; + sanitize(req, function(){ + t.deepEqual(req.clean.categories, defaultClean.categories, 'empty strings filtered out'); t.end(); }); }); }; -module.exports.tests.middleware_failure = function(test, common) { - test('middleware failure', function(t) { - var res = { status: function( code ){ - t.equal(code, 400, 'status set'); - }}; - var next = function( message ){ - t.equals(message, defaultError); - t.end(); - }; - middleware( {}, res, next ); - }); -}; - module.exports.tests.middleware_success = function(test, common) { test('middleware success', function(t) { var req = { query: { 'point.lat': 0, 'point.lon': 0 }}; - var next = function( message ){ - t.equal(message, undefined, 'no error message set'); + var next = function(){ + t.deepEqual(req.errors, [], 'no error message set'); t.deepEqual(req.clean, defaultClean); t.end(); }; diff --git a/test/unit/sanitiser/search.js b/test/unit/sanitiser/search.js index 3a085a55..6e234e50 100644 --- a/test/unit/sanitiser/search.js +++ b/test/unit/sanitiser/search.js @@ -3,7 +3,7 @@ var search = require('../../../sanitiser/search'), _text = require('../sanitiser/_text'), parser = require('../../../helper/query_parser'), defaultParsed = _text.defaultParsed, - _sanitize = search.sanitize, + sanitize = search.sanitize, middleware = search.middleware, defaultError = 'invalid param \'text\': text length, must be >0', defaultClean = { text: 'test', @@ -11,8 +11,11 @@ var search = require('../../../sanitiser/search'), }, size: 10, parsed_text: defaultParsed, - }, - sanitize = function(query, cb) { _sanitize({'query':query}, cb); }; + }; + +// these are the default values you would expect when no input params are specified. +// @todo: why is this different from $defaultClean? +var emptyClean = { boundary: {}, categories: [], private: false, size: 10, types: {} }; module.exports.tests = {}; @@ -41,9 +44,10 @@ module.exports.tests.sanitize_invalid_text = function(test, common) { test('invalid text', function(t) { var invalid = [ '', 100, null, undefined, new Date() ]; invalid.forEach( function( text ){ - sanitize({ text: text }, function( err, clean ){ - t.equal(err, 'invalid param \'text\': text length, must be >0', text + ' is an invalid text'); - t.equal(clean, undefined, 'clean not set'); + var req = { query: { text: text } }; + sanitize(req, function(){ + t.equal(req.errors[0], 'invalid param \'text\': text length, must be >0', text + ' is an invalid text'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); }); t.end(); @@ -52,22 +56,25 @@ module.exports.tests.sanitize_invalid_text = function(test, common) { module.exports.tests.sanitise_valid_text = function(test, common) { test('valid short text', function(t) { - sanitize({ text: 'a' }, function( err, clean ){ - t.equal(err, undefined, 'no error'); + var req = { query: { text: 'a' } }; + sanitize(req, function(){ + t.equal(req.errors[0], undefined, 'no error'); }); t.end(); }); test('valid not-quite-as-short text', function(t) { - sanitize({ text: 'aa' }, function( err, clean ){ - t.equal(err, undefined, 'no error'); + var req = { query: { text: 'aa' } }; + sanitize(req, function(){ + t.equal(req.errors[0], undefined, 'no error'); }); t.end(); }); test('valid longer text', function(t) { - sanitize({ text: 'aaaaaaaa' }, function( err, clean ){ - t.equal(err, undefined, 'no error'); + var req = { query: { text: 'aaaaaaaa' } }; + sanitize(req, function(){ + t.equal(req.errors[0], undefined, 'no error'); }); t.end(); }); @@ -78,13 +85,14 @@ module.exports.tests.sanitize_text_with_delim = function(test, common) { test('valid texts with a comma', function(t) { texts.forEach( function( text ){ - sanitize({ text: text }, function( err, clean ){ + var req = { query: { text: text } }; + sanitize(req, function(){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.text = text; expected.parsed_text = parser.get_parsed_address(text); - t.equal(err, undefined, 'no error'); - t.equal(clean.parsed_text.name, expected.parsed_text.name, 'clean name set correctly'); + t.equal(req.errors[0], undefined, 'no error'); + t.equal(req.clean.parsed_text.name, expected.parsed_text.name, 'clean name set correctly'); }); }); @@ -94,8 +102,9 @@ module.exports.tests.sanitize_text_with_delim = function(test, common) { module.exports.tests.sanitize_private_no_value = function(test, common) { test('default private should be set to true', function(t) { - sanitize({ text: 'test' }, function( err, clean ){ - t.equal(clean.private, false, 'private set to false'); + var req = { query: { text: 'test' } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); }); t.end(); }); @@ -103,8 +112,9 @@ module.exports.tests.sanitize_private_no_value = function(test, common) { module.exports.tests.sanitize_private_explicit_true_value = function(test, common) { test('explicit private should be set to true', function(t) { - sanitize({ text: 'test', private: true }, function( err, clean ){ - t.equal(clean.private, true, 'private set to true'); + var req = { query: { text: 'test', private: true } }; + sanitize(req, function(){ + t.equal(req.clean.private, true, 'private set to true'); }); t.end(); }); @@ -112,8 +122,9 @@ module.exports.tests.sanitize_private_explicit_true_value = function(test, commo module.exports.tests.sanitize_private_explicit_false_value = function(test, common) { test('explicit private should be set to false', function(t) { - sanitize({ text: 'test', private: false }, function( err, clean ){ - t.equal(clean.private, false, 'private set to false'); + var req = { query: { text: 'test', private: false } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); }); t.end(); }); @@ -126,19 +137,21 @@ module.exports.tests.sanitize_lat = function(test, common) { }; test('invalid lat', function(t) { lats.invalid.forEach( function( lat ){ - sanitize({ text: 'test', 'focus.point.lat': lat, 'focus.point.lon': 0 }, function( err, clean ){ - t.equal(err, 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); - t.equal(clean, undefined, 'clean not set'); + var req = { query: { text: 'test', 'focus.point.lat': lat, 'focus.point.lon': 0 } }; + sanitize(req, function(){ + t.equal(req.errors[0], 'invalid param \'lat\': must be >-90 and <90', lat + ' is an invalid latitude'); + t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); }); t.end(); }); test('valid lat', function(t) { lats.valid.forEach( function( lat ){ - sanitize({ text: 'test', 'focus.point.lat': lat, 'focus.point.lon': 0 }, function( err, clean ){ + var req = { query: { text: 'test', 'focus.point.lat': lat, 'focus.point.lon': 0 } }; + sanitize(req, function(){ var expected_lat = parseFloat( lat ); - t.equal(err, undefined, 'no error'); - t.deepEqual(clean.lat, expected_lat, 'clean lat set correctly (' + lat + ')'); + t.equal(req.errors[0], undefined, 'no error'); + t.equal(req.clean.lat, expected_lat, 'clean lat set correctly (' + lat + ')'); }); }); t.end(); @@ -151,11 +164,12 @@ module.exports.tests.sanitize_lon = function(test, common) { }; test('valid lon', function(t) { lons.valid.forEach( function( lon ){ - sanitize({ text: 'test', 'focus.point.lat': 0, 'focus.point.lon': lon }, function( err, clean ){ + var req = { query: { text: 'test', 'focus.point.lat': 0, 'focus.point.lon': lon } }; + sanitize(req, function(){ var expected = JSON.parse(JSON.stringify( defaultClean )); expected.lon = parseFloat( lon ); - t.equal(err, undefined, 'no error'); - t.deepEqual(clean.lon, expected.lon, 'clean set correctly (' + lon + ')'); + t.equal(req.errors[0], undefined, 'no error'); + t.equal(req.clean.lon, expected.lon, 'clean set correctly (' + lon + ')'); }); }); t.end(); @@ -164,26 +178,29 @@ module.exports.tests.sanitize_lon = function(test, common) { module.exports.tests.sanitize_optional_geo = function(test, common) { test('no lat/lon', function(t) { - sanitize({ text: 'test' }, function( err, clean ){ - t.equal(err, undefined, 'no error'); - t.equal(clean.lat, undefined, 'clean set without lat'); - t.equal(clean.lon, undefined, 'clean set without lon'); + var req = { query: { text: 'test' } }; + sanitize(req, function(){ + t.equal(req.errors[0], undefined, 'no error'); + t.equal(req.clean.lat, undefined, 'clean set without lat'); + t.equal(req.clean.lon, undefined, 'clean set without lon'); }); t.end(); }); test('no lat', function(t) { - sanitize({ text: 'test', 'focus.point.lon': 0 }, function( err, clean ){ + var req = { query: { text: 'test', 'focus.point.lon': 0 } }; + sanitize(req, function(){ var expected_lon = 0; - t.equal(err, undefined, 'no error'); - t.deepEqual(clean.lon, expected_lon, 'clean set correctly (without any lat)'); + t.equal(req.errors[0], undefined, 'no error'); + t.deepEqual(req.clean.lon, expected_lon, 'clean set correctly (without any lat)'); }); t.end(); }); test('no lon', function(t) { - sanitize({ text: 'test', 'focus.point.lat': 0 }, function( err, clean ){ + var req = { query: { text: 'test', 'focus.point.lat': 0 } }; + sanitize(req, function(){ var expected_lat = 0; - t.equal(err, undefined, 'no error'); - t.deepEqual(clean.lat, expected_lat, 'clean set correctly (without any lon)'); + t.equal(req.errors[0], undefined, 'no error'); + t.deepEqual(req.clean.lat, expected_lat, 'clean set correctly (without any lon)'); }); t.end(); }); @@ -219,18 +236,20 @@ module.exports.tests.sanitize_bbox = function(test, common) { }; test('invalid bbox', function(t) { bboxes.invalid.forEach( function( bbox ){ - sanitize({ text: 'test', bbox: bbox }, function( err, clean ){ - t.equal(err, undefined, 'no error'); - t.equal(clean.bbox, undefined, 'falling back on 50km distance from centroid'); + var req = { query: { text: 'test', bbox: bbox } }; + sanitize(req, function(){ + t.equal(req.errors[0], undefined, 'no error'); + t.equal(req.clean.bbox, undefined, 'falling back on 50km distance from centroid'); }); }); t.end(); }); test('valid bbox', function(t) { bboxes.valid.forEach( function( bbox ){ - sanitize({ text: 'test', bbox: bbox }, function( err, clean ){ + var req = { query: { text: 'test', bbox: bbox } }; + sanitize(req, function(){ var bboxArray = bbox.split(',').map(function(i) { - return parseInt(i); + return parseInt(i, 10); }); var expected_bbox = { right: Math.max(bboxArray[0], bboxArray[2]), @@ -238,8 +257,8 @@ module.exports.tests.sanitize_bbox = function(test, common) { left: Math.min(bboxArray[0], bboxArray[2]), bottom: Math.min(bboxArray[1], bboxArray[3]) }; - t.equal(err, undefined, 'no error'); - t.deepEqual(clean.bbox, expected_bbox, 'clean set correctly (' + bbox + ')'); + t.equal(req.errors[0], undefined, 'no error'); + t.deepEqual(req.clean.bbox, expected_bbox, 'clean set correctly (' + bbox + ')'); }); }); t.end(); @@ -248,20 +267,23 @@ module.exports.tests.sanitize_bbox = function(test, common) { module.exports.tests.sanitize_size = function(test, common) { test('invalid size value', function(t) { - sanitize({ size: 'a', text: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.equal(clean.size, 10, 'default size set'); + var req = { query: { size: 'a', text: 'test', lat: 0, lon: 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 10, 'default size set'); t.end(); }); }); test('below min size value', function(t) { - sanitize({ size: -100, text: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.equal(clean.size, 1, 'min size set'); + var req = { query: { size: -100, text: 'test', lat: 0, lon: 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 1, 'min size set'); t.end(); }); }); test('above max size value', function(t) { - sanitize({ size: 9999, text: 'test', lat: 0, lon: 0 }, function( err, clean ){ - t.equal(clean.size, 40, 'max size set'); + var req = { query: { size: 9999, text: 'test', lat: 0, lon: 0 } }; + sanitize(req, function(){ + t.equal(req.clean.size, 40, 'max size set'); t.end(); }); }); @@ -271,8 +293,9 @@ module.exports.tests.sanitize_private = function(test, common) { var invalid_values = [null, -1, 123, NaN, 'abc']; invalid_values.forEach(function(value) { test('invalid private param ' + value, function(t) { - sanitize({ text: 'test', lat: 0, lon: 0, 'private': value }, function( err, clean ){ - t.equal(clean.private, false, 'default private set (to false)'); + var req = { query: { text: 'test', lat: 0, lon: 0, 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'default private set (to false)'); t.end(); }); }); @@ -281,8 +304,9 @@ module.exports.tests.sanitize_private = function(test, common) { var valid_values = ['true', true, 1, '1']; valid_values.forEach(function(value) { test('valid private ' + value, function(t) { - sanitize({ text: 'test', 'private': value}, function( err, clean ){ - t.equal(clean.private, true, 'private set to true'); + var req = { query: { text: 'test', 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, true, 'private set to true'); t.end(); }); }); @@ -291,16 +315,18 @@ module.exports.tests.sanitize_private = function(test, common) { var valid_false_values = ['false', false, 0, '0']; valid_false_values.forEach(function(value) { test('test setting false explicitly ' + value, function(t) { - sanitize({ text: 'test', 'private': value }, function( err, clean ){ - t.equal(clean.private, false, 'private set to false'); + var req = { query: { text: 'test', 'private': value } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); t.end(); }); }); }); test('test default behavior', function(t) { - sanitize({ text: 'test' }, function( err, clean ){ - t.equal(clean.private, false, 'private set to false'); + var req = { query: { text: 'test' } }; + sanitize(req, function(){ + t.equal(req.clean.private, false, 'private set to false'); t.end(); }); }); @@ -308,31 +334,19 @@ module.exports.tests.sanitize_private = function(test, common) { module.exports.tests.invalid_params = function(test, common) { test('invalid text params', function(t) { - sanitize( undefined, function( err, clean ){ - t.equal(err, defaultError, 'handle invalid params gracefully'); + var req = { query: {} }; + sanitize( req, function(){ + t.equal(req.errors[0], defaultError, 'handle invalid params gracefully'); t.end(); }); }); }; -module.exports.tests.middleware_failure = function(test, common) { - test('middleware failure', function(t) { - var res = { status: function( code ){ - t.equal(code, 400, 'status set'); - }}; - var next = function( message ){ - t.equal(message, defaultError); - t.end(); - }; - middleware( {}, res, next ); - }); -}; - module.exports.tests.middleware_success = function(test, common) { test('middleware success', function(t) { var req = { query: { text: 'test' }}; var next = function( message ){ - t.equal(message, undefined, 'no error message set'); + t.deepEqual(req.errors, [], 'no error messages set'); t.end(); }; middleware( req, undefined, next );