From 80a3a259ef40bdefa7b51fe25b4410d579a586b0 Mon Sep 17 00:00:00 2001 From: Lily He Date: Mon, 31 Jul 2017 18:00:33 -0400 Subject: [PATCH] remove callback in sanitizeAll.runAllChecks for sync processing --- sanitizer/autocomplete.js | 9 +--- sanitizer/nearby.js | 9 +--- sanitizer/place.js | 9 +--- sanitizer/reverse.js | 9 +--- sanitizer/sanitizeAll.js | 33 +++++++-------- sanitizer/search.js | 9 +--- sanitizer/search_fallback.js | 9 +--- sanitizer/structured_geocoding.js | 10 ++--- test/unit/sanitizer/sanitizeAll.js | 68 ++++++++++++------------------ 9 files changed, 56 insertions(+), 109 deletions(-) diff --git a/sanitizer/autocomplete.js b/sanitizer/autocomplete.js index 55315e41..85fcec86 100644 --- a/sanitizer/autocomplete.js +++ b/sanitizer/autocomplete.js @@ -20,12 +20,7 @@ module.exports.middleware = (_api_pelias_config) => { }; return ( req, res, next ) => { - sanitizeAll.runAllChecks(req, sanitizers, ( err, clean ) => { - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } - next(); - }); + sanitizeAll.runAllChecks(req, sanitizers); + next(); }; }; diff --git a/sanitizer/nearby.js b/sanitizer/nearby.js index 46f1ae2c..00efefc4 100644 --- a/sanitizer/nearby.js +++ b/sanitizer/nearby.js @@ -15,11 +15,6 @@ module.exports.sanitizer_list = sanitizers; // middleware module.exports.middleware = function( req, res, next ){ - sanitize(req, sanitizers, ( err, clean ) => { - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } - next(); - }); + sanitizeAll.runAllChecks(req, sanitizers); + next(); }; diff --git a/sanitizer/place.js b/sanitizer/place.js index 363fa7ef..6800b06b 100644 --- a/sanitizer/place.js +++ b/sanitizer/place.js @@ -14,11 +14,6 @@ module.exports.sanitizer_list = sanitizers; // middleware module.exports.middleware = function(req, res, next){ - sanitize(req, sanitizers, ( err, clean ) => { - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } - next(); - }); + sanitizeAll.runAllChecks(req, sanitizers); + next(); }; diff --git a/sanitizer/reverse.js b/sanitizer/reverse.js index 0a02f9a8..91094812 100644 --- a/sanitizer/reverse.js +++ b/sanitizer/reverse.js @@ -23,11 +23,6 @@ module.exports.sanitizer_list = sanitizers; // middleware module.exports.middleware = function( req, res, next ){ - sanitize(req, sanitizers, ( err, clean ) => { - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } - next(); - }); + sanitizeAll.runAllChecks(req, sanitizers); + next(); }; diff --git a/sanitizer/sanitizeAll.js b/sanitizer/sanitizeAll.js index c21c78ff..af6e1510 100644 --- a/sanitizer/sanitizeAll.js +++ b/sanitizer/sanitizeAll.js @@ -1,8 +1,5 @@ 'use strict'; - -const async = require('async'); - -function sanitize( req, sanitizers, cb ){ +function sanitize( req, sanitizers ){ // init an object to store clean (sanitized) input parameters if not initialized req.clean = req.clean || {}; @@ -29,11 +26,11 @@ function sanitize( req, sanitizers, cb ){ req.warnings = req.warnings.concat( sanity.warnings ); } } - return cb( undefined, req.clean ); } // Adds to goodParameters every acceptable parameter passed through API call -function checkParameters(req, sanitizers, cb) { +function checkParameters( req, sanitizers ) { + req.warnings = req.warnings || []; // source of input parameters // (in this case from the GET querystring params) const params = req.query || {}; @@ -41,9 +38,9 @@ function checkParameters(req, sanitizers, cb) { for (let s in sanitizers) { - // checks if there is a function that returns valid params + // checks if function exists if (typeof sanitizers[s].expected === 'function'){ - /** func returns {array} ex: [{ name: 'text' }] */ + /** expected() returns {array} ex: [{ name: 'text' }] */ for (let t in sanitizers[s].expected()) { /** {object} prop */ const prop = sanitizers[s].expected()[t]; @@ -54,21 +51,21 @@ function checkParameters(req, sanitizers, cb) { } } } - // If there are any unexpected parameters, add a warning to messages - for (let p in params) { - if (!goodParameters.hasOwnProperty(p)){ - req.warnings = req.warnings.concat('Invalid Parameter: ' + p); + // If there are any unexpected parameters & goodParameters isn't empty, + // add a warning message + if (Object.keys(goodParameters).length !== 0) { + for (let p in params) { + if (!goodParameters.hasOwnProperty(p)){ + req.warnings = req.warnings.concat('Invalid Parameter: ' + p); + } } } - return cb( undefined, req.clean ); } // runs both sanitize and checkParameters functions in async parallel -function runAllChecks (req, sanitizers, cb) { - async.parallel([ - sanitize.bind(null, req, sanitizers), - checkParameters.bind(null, req, sanitizers) - ], cb); +function runAllChecks (req, sanitizers) { + sanitize(req, sanitizers); + checkParameters(req, sanitizers); } // export function diff --git a/sanitizer/search.js b/sanitizer/search.js index c36a4d14..9db5f900 100644 --- a/sanitizer/search.js +++ b/sanitizer/search.js @@ -21,13 +21,8 @@ module.exports.middleware = (_api_pelias_config) => { }; return ( req, res, next ) => { - sanitizeAll.runAllChecks(req, sanitizers, ( err, clean ) => { - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } - next(); - }); + sanitizeAll.runAllChecks(req, sanitizers); + next(); }; }; diff --git a/sanitizer/search_fallback.js b/sanitizer/search_fallback.js index 628ee4b8..6c8297be 100644 --- a/sanitizer/search_fallback.js +++ b/sanitizer/search_fallback.js @@ -23,12 +23,7 @@ module.exports.middleware = function( req, res, next ){ } // calls to sanitize the input // omits check if parameters are valid since it only calls _text_addressit - sanitizeAll.sanitize(req, sanitizers, ( err, clean ) => { - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } - next(); - }); + sanitizeAll.sanitize(req, sanitizers); + next(); }; diff --git a/sanitizer/structured_geocoding.js b/sanitizer/structured_geocoding.js index f5c95afc..b84d843c 100644 --- a/sanitizer/structured_geocoding.js +++ b/sanitizer/structured_geocoding.js @@ -22,13 +22,9 @@ module.exports.middleware = (_api_pelias_config) => { }; return ( req, res, next ) => { - sanitizeAll.runAllChecks(req, sanitizers, ( err, clean ) => { - if( err ){ - res.status(400); // 400 Bad Request - return next(err); - } - next(); - }); + sanitizeAll.runAllChecks(req, sanitizers); + next(); + }; }; diff --git a/test/unit/sanitizer/sanitizeAll.js b/test/unit/sanitizer/sanitizeAll.js index 1355fed3..f74e7efb 100644 --- a/test/unit/sanitizer/sanitizeAll.js +++ b/test/unit/sanitizer/sanitizeAll.js @@ -35,10 +35,9 @@ module.exports.tests.all = function(test, common) { warnings: ['warning 1', 'warning 2', 'warning 3'] }; - sanitizeAll.sanitize(req, sanitizers, function (){ - t.deepEquals(req, expected_req); - t.end(); - }); + sanitizeAll.runAllChecks(req, sanitizers); + t.deepEquals(req, expected_req); + t.end(); }); @@ -82,11 +81,9 @@ module.exports.tests.all = function(test, common) { warnings: ['pre-existing warning', 'warning 1', 'warning 2', 'warning 3'] }; - sanitizeAll.sanitize(req, sanitizers, function () { - t.deepEquals(req, expected_req); - t.end(); - }); - + sanitizeAll.runAllChecks(req, sanitizers); + t.deepEquals(req, expected_req); + t.end(); }); test('req.query should be passed to individual sanitizers when available', function(t) { @@ -97,7 +94,7 @@ module.exports.tests.all = function(test, common) { }; var sanitizers = { 'first': { - sanitize: function(params) { + sanitize: function (params) { req.clean.query = params; return { errors: [], @@ -120,11 +117,9 @@ module.exports.tests.all = function(test, common) { warnings: [] }; - sanitizeAll.sanitize(req, sanitizers, function () { - t.deepEquals(req, expected_req); - t.end(); - }); - + sanitizeAll.runAllChecks(req, sanitizers); + t.deepEquals(req, expected_req); + t.end(); }); test('an empty object should be passed to individual sanitizers when req.query is unavailable', function(t) { @@ -152,11 +147,9 @@ module.exports.tests.all = function(test, common) { warnings: [] }; - sanitizeAll.sanitize(req, sanitizers, function () { - t.deepEquals(req, expected_req); - t.end(); - }); - + sanitizeAll.runAllChecks(req, sanitizers); + t.deepEquals(req, expected_req); + t.end(); }); test('unexpected parameters should throw warning', function(t) { @@ -178,12 +171,10 @@ module.exports.tests.all = function(test, common) { } }; - sanitizeAll.checkParameters(req, sanitizers, function () { - t.equals(req.errors.length, 0); - t.deepEquals(req.warnings[0], 'Invalid Parameter: unknown_value'); - t.end(); - }); - + sanitizeAll.checkParameters(req, sanitizers); + t.equals(req.errors.length, 0); + t.deepEquals(req.warnings[0], 'Invalid Parameter: unknown_value'); + t.end(); }); test('expected parameters should not throw warning', function(t) { @@ -205,14 +196,14 @@ module.exports.tests.all = function(test, common) { } }; - sanitizeAll.checkParameters(req, sanitizers, function () { - t.equals(req.errors.length, 0); - t.equals(req.warnings.length, 0); - t.end(); - }); + sanitizeAll.checkParameters(req, sanitizers); + t.equals(req.errors.length, 0); + t.equals(req.warnings.length, 0); + t.end(); + }); - test('runAllChecks calls both sanitize and expectedParameters function', function(t) { + test('sanitizer without expected() should not validate parameters', function(t) { var req = { query: { value: 'query' @@ -227,12 +218,6 @@ module.exports.tests.all = function(test, common) { errors: [], warnings: ['warning 1'] }; - }, - expected: function _expected () { - // add value as a valid parameter - return [{ - name: 'value' - }]; } } }; @@ -250,10 +235,9 @@ module.exports.tests.all = function(test, common) { warnings: ['warning 1'] }; - sanitizeAll.runAllChecks(req, sanitizers, function () { - t.deepEquals(req, expected_req); - t.end(); - }); + sanitizeAll.runAllChecks(req, sanitizers); + t.deepEquals(req, expected_req); + t.end(); }); };