From 670c673419028a626adb525efb241849d253266f Mon Sep 17 00:00:00 2001 From: Lily He Date: Fri, 28 Jul 2017 15:59:47 -0400 Subject: [PATCH] call runAllChecks for nearby, reverse, and place sanitizing --- sanitizer/nearby.js | 10 +++++++--- sanitizer/place.js | 14 +++++++++----- sanitizer/reverse.js | 20 ++++++++++++-------- test/unit/sanitizer/nearby.js | 5 +++-- test/unit/sanitizer/place.js | 15 ++++++++------- test/unit/sanitizer/reverse.js | 29 +++++++++++++++-------------- 6 files changed, 54 insertions(+), 39 deletions(-) diff --git a/sanitizer/nearby.js b/sanitizer/nearby.js index d518b997..46f1ae2c 100644 --- a/sanitizer/nearby.js +++ b/sanitizer/nearby.js @@ -4,10 +4,10 @@ var reverseSanitizers = require('./reverse').sanitizer_list; // add categories to the sanitizer list var sanitizers = _.merge({}, reverseSanitizers, { - categories: require('../sanitizer/_categories') + categories: require('../sanitizer/_categories')() }); -var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; +var sanitize = sanitizeAll.runAllChecks; // export sanitize for testing module.exports.sanitize = sanitize; @@ -15,7 +15,11 @@ module.exports.sanitizer_list = sanitizers; // middleware module.exports.middleware = function( req, res, next ){ - sanitize( req, function( err, clean ){ + sanitize(req, sanitizers, ( err, clean ) => { + if( err ){ + res.status(400); // 400 Bad Request + return next(err); + } next(); }); }; diff --git a/sanitizer/place.js b/sanitizer/place.js index 3a6d4408..363fa7ef 100644 --- a/sanitizer/place.js +++ b/sanitizer/place.js @@ -1,20 +1,24 @@ var sanitizeAll = require('../sanitizer/sanitizeAll'), sanitizers = { - singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), - ids: require('../sanitizer/_ids'), + singleScalarParameters: require('../sanitizer/_single_scalar_parameters')(), + ids: require('../sanitizer/_ids')(), private: require('../sanitizer/_flag_bool')('private', false) }; -var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; +var sanitize = sanitizeAll.runAllChecks; // export sanitize for testing module.exports.sanitize = sanitize; module.exports.sanitizer_list = sanitizers; // middleware -module.exports.middleware = function( req, res, next ){ - sanitize( req, function( err, clean ){ +module.exports.middleware = function(req, res, next){ + sanitize(req, sanitizers, ( err, clean ) => { + if( err ){ + res.status(400); // 400 Bad Request + return next(err); + } next(); }); }; diff --git a/sanitizer/reverse.js b/sanitizer/reverse.js index e5ef3272..0a02f9a8 100644 --- a/sanitizer/reverse.js +++ b/sanitizer/reverse.js @@ -2,20 +2,20 @@ var type_mapping = require('../helper/type_mapping'); var sanitizeAll = require('../sanitizer/sanitizeAll'), sanitizers = { - singleScalarParameters: require('../sanitizer/_single_scalar_parameters'), - 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 - sources_and_layers: require('../sanitizer/_sources_and_layers'), - geonames_deprecation: require('../sanitizer/_geonames_deprecation'), + sources_and_layers: require('../sanitizer/_sources_and_layers')(), + geonames_deprecation: require('../sanitizer/_geonames_deprecation')(), size: require('../sanitizer/_size')(/* use defaults*/), private: require('../sanitizer/_flag_bool')('private', false), - geo_reverse: require('../sanitizer/_geo_reverse'), - boundary_country: require('../sanitizer/_boundary_country') + geo_reverse: require('../sanitizer/_geo_reverse')(), + boundary_country: require('../sanitizer/_boundary_country')() }; -var sanitize = function(req, cb) { sanitizeAll(req, sanitizers, cb); }; +var sanitize = sanitizeAll.runAllChecks; // export sanitize for testing module.exports.sanitize = sanitize; @@ -23,7 +23,11 @@ module.exports.sanitizer_list = sanitizers; // middleware module.exports.middleware = function( req, res, next ){ - sanitize( req, function( err, clean ){ + sanitize(req, sanitizers, ( err, clean ) => { + if( err ){ + res.status(400); // 400 Bad Request + return next(err); + } next(); }); }; diff --git a/test/unit/sanitizer/nearby.js b/test/unit/sanitizer/nearby.js index 72be019a..f816ea62 100644 --- a/test/unit/sanitizer/nearby.js +++ b/test/unit/sanitizer/nearby.js @@ -3,6 +3,7 @@ var nearby = require('../../../sanitizer/nearby'); var defaults = require('../../../query/reverse_defaults'); var sanitize = nearby.sanitize; var middleware = nearby.middleware; +var sanitizer_list = nearby.sanitizer_list; var defaultClean = { 'point.lat': 0, 'point.lon': 0, @@ -17,7 +18,7 @@ module.exports.tests = {}; module.exports.tests.interface = function(test, common) { test('sanitize interface', function(t) { t.equal(typeof sanitize, 'function', 'sanitize is a function'); - t.equal(sanitize.length, 2, 'sanitize interface'); + t.equal(sanitize.length, 3, 'sanitize interface takes one argument: req'); t.end(); }); test('middleware interface', function(t) { @@ -52,7 +53,7 @@ module.exports.tests.middleware_success = function(test, common) { module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('SANTIZE /nearby ' + name, testFunction); + return tape('SANITIZE /nearby ' + name, testFunction); } for( var testCase in module.exports.tests ){ diff --git a/test/unit/sanitizer/place.js b/test/unit/sanitizer/place.js index b7a4b6b8..ecbe295e 100644 --- a/test/unit/sanitizer/place.js +++ b/test/unit/sanitizer/place.js @@ -1,6 +1,7 @@ var place = require('../../../sanitizer/place'), sanitize = place.sanitize, middleware = place.middleware, + sanitizer_list = place.sanitizer_list, defaultClean = { ids: [ { source: 'geonames', layer: 'venue', id: '123' } ], private: false }; // these are the default values you would expect when no input params are specified. @@ -9,7 +10,7 @@ module.exports.tests = {}; module.exports.tests.interface = function(test, common) { test('sanitize interface', function(t) { t.equal(typeof sanitize, 'function', 'sanitize is a function'); - t.equal(sanitize.length, 2, 'sanitize interface'); + t.equal(sanitize.length, 3, 'sanitize interface takes one arg: req'); t.end(); }); test('middleware interface', function(t) { @@ -32,7 +33,7 @@ module.exports.tests.sanitize_private = function(test, common) { invalid_values.forEach(function(value) { test('invalid private param ' + value, function(t) { var req = { query: { ids:'geonames:venue:123', 'private': value } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' ); t.equal(req.clean.private, false, 'default private set (to false)'); @@ -45,7 +46,7 @@ module.exports.tests.sanitize_private = function(test, common) { valid_values.forEach(function(value) { test('valid private param ' + value, function(t) { var req = { query: { ids:'geonames:venue:123', 'private': value } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' ); t.equal(req.clean.private, true, 'private set to true'); @@ -58,7 +59,7 @@ module.exports.tests.sanitize_private = function(test, common) { valid_false_values.forEach(function(value) { test('test setting false explicitly ' + value, function(t) { var req = { query: { ids:'geonames:venue:123', 'private': value } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' ); t.equal(req.clean.private, false, 'private set to false'); @@ -69,7 +70,7 @@ module.exports.tests.sanitize_private = function(test, common) { test('test default behavior', function(t) { var req = { query: { ids:'geonames:venue:123' } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.deepEqual( req.errors, [], 'no errors' ); t.deepEqual( req.warnings, [], 'no warnings' ); t.equal(req.clean.private, false, 'private set to false'); @@ -81,7 +82,7 @@ module.exports.tests.sanitize_private = function(test, common) { module.exports.tests.invalid_params = function(test, common) { test('no params', function(t) { var req = { query: {} }; - sanitize( req, function(){ + sanitize(req, sanitizer_list, () => { t.equal( req.errors[0], 'invalid param \'ids\': length must be >0', 'error for missing `ids` param'); t.deepEqual( req.warnings, [], 'no warnings' ); t.end(); @@ -105,7 +106,7 @@ module.exports.tests.middleware_success = function(test, common) { module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('SANTIZE /place ' + name, testFunction); + return tape('SANITIZE /place ' + name, testFunction); } for( var testCase in module.exports.tests ){ diff --git a/test/unit/sanitizer/reverse.js b/test/unit/sanitizer/reverse.js index c7125743..16f05d39 100644 --- a/test/unit/sanitizer/reverse.js +++ b/test/unit/sanitizer/reverse.js @@ -4,6 +4,7 @@ var reverse = require('../../../sanitizer/reverse'), sanitize = reverse.sanitize, middleware = reverse.middleware, + sanitizer_list = reverse.sanitizer_list, defaults = require('../../../query/reverse_defaults'), defaultError = 'missing param \'lat\'', defaultClean = { 'point.lat': 0, @@ -23,7 +24,7 @@ module.exports.tests = {}; module.exports.tests.interface = function(test, common) { test('sanitize interface', function(t) { t.equal(typeof sanitize, 'function', 'sanitize is a function'); - t.equal(sanitize.length, 2, 'sanitize interface'); + t.equal(sanitize.length, 3, 'sanitize interface takes one param: req'); t.end(); }); test('middleware interface', function(t) { @@ -52,7 +53,7 @@ module.exports.tests.sanitize_lat = function(test, common) { test('invalid lat', function(t) { lats.invalid.forEach( function( lat ){ var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.errors[0], 'invalid param \'point.lat\': must be >-90 and <90', lat + ' is an invalid latitude'); t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); @@ -62,7 +63,7 @@ module.exports.tests.sanitize_lat = function(test, common) { test('valid lat', function(t) { lats.valid.forEach( function( lat ){ var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { var expected_lat = parseFloat( lat ); t.deepEqual(req.errors, [], 'no errors'); }); @@ -72,7 +73,7 @@ module.exports.tests.sanitize_lat = function(test, common) { test('missing lat', function(t) { lats.missing.forEach( function( lat ){ var req = { query: { 'point.lat': lat, 'point.lon': 0 } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.errors[0], 'missing param \'point.lat\'', 'latitude is a required field'); t.deepEqual(req.clean, emptyClean, 'clean only has default values set'); }); @@ -89,7 +90,7 @@ module.exports.tests.sanitize_lon = function(test, common) { test('valid lon', function(t) { lons.valid.forEach( function( lon ){ var req = { query: { 'point.lat': 0, 'point.lon': lon } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { var expected_lon = parseFloat( lon ); t.deepEqual(req.errors, [], 'no errors'); }); @@ -102,7 +103,7 @@ module.exports.tests.sanitize_lon = function(test, common) { // @todo: why is lat set? var expected = { 'point.lat': 0, private: false, size: 10 }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.errors[0], 'missing param \'point.lon\'', 'longitude is a required field'); t.deepEqual(req.clean, expected, 'clean only has default values set'); }); @@ -114,21 +115,21 @@ module.exports.tests.sanitize_lon = function(test, common) { module.exports.tests.sanitize_size = function(test, common) { test('invalid size value', function(t) { var req = { query: { size: 'a', 'point.lat': 0, 'point.lon': 0 } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.clean.size, 10, 'default size set'); t.end(); }); }); test('below min size value', function(t) { var req = { query: { size: -100, 'point.lat': 0, 'point.lon': 0 } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.clean.size, 1, 'min size set'); t.end(); }); }); test('above max size value', function(t) { var req = { query: { size: 9999, 'point.lat': 0, 'point.lon': 0 } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.clean.size, 40, 'max size set'); t.end(); }); @@ -140,7 +141,7 @@ module.exports.tests.sanitize_private = function(test, common) { invalid_values.forEach(function(value) { test('invalid private param ' + value, function(t) { var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.clean.private, false, 'default private set (to false)'); t.end(); }); @@ -151,7 +152,7 @@ module.exports.tests.sanitize_private = function(test, common) { valid_values.forEach(function(value) { test('valid private param ' + value, function(t) { var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.clean.private, true, 'private set to true'); t.end(); }); @@ -162,7 +163,7 @@ module.exports.tests.sanitize_private = function(test, common) { valid_false_values.forEach(function(value) { test('test setting false explicitly ' + value, function(t) { var req = { query: { 'point.lat': 0, 'point.lon': 0, 'private': value } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.clean.private, false, 'private set to false'); t.end(); }); @@ -171,7 +172,7 @@ module.exports.tests.sanitize_private = function(test, common) { test('test default behavior', function(t) { var req = { query: { 'point.lat': 0, 'point.lon': 0 } }; - sanitize(req, function(){ + sanitize(req, sanitizer_list, () => { t.equal(req.clean.private, false, 'private set to false'); t.end(); }); @@ -193,7 +194,7 @@ module.exports.tests.middleware_success = function(test, common) { module.exports.all = function (tape, common) { function test(name, testFunction) { - return tape('SANTIZE /reverse ' + name, testFunction); + return tape('SANITIZE /reverse ' + name, testFunction); } for( var testCase in module.exports.tests ){