Merge pull request #52 from Workpop/querystring-hasownproperty-fix

validation: fix add support validating query objects that do not extend Object.prototype
mac- · Dec 1, 2016 · 9f3b09d · 9f3b09d
2 parents b6f3272 + 0328070
commit 9f3b09d
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 84 deletions.
diff --git a/lib/RouteSchemaManager.js b/lib/RouteSchemaManager.js
@@ -104,6 +104,29 @@ var RouteSchemaManager = function(options) {
 			return schemas;
 		},
 
+		convertValueFromStringToType = function(value, type) {
+			if (typeof(value) !== 'string' || type === 'string') {
+				return value;
+			}
+			if (type === 'integer' || type === 'number') {
+				// fastest (and more reliable) way to convert strings to numbers
+				var convertedVal = 1 * value;
+				// make sure that if our schema calls for an integer, that there is no decimal
+				if (convertedVal || convertedVal === 0 && (type === 'number' || (value.indexOf('.') === -1))) {
+					return convertedVal;
+				}
+			}
+			else if (type === 'boolean') {
+				if (value === 'true') {
+					return true;
+				}
+				else if (value === 'false') {
+					return false;
+				}
+			}
+			return value;
+		},
+
 		convertPropertyTypesToMatchSchema = function(object, schema, forceArrayConversion) {
 			// in some cases (query params), we want to force a value to be an array that contains that value,
 			// if the schema expects an array of strings, numbers, integers, or booleans
@@ -114,7 +137,7 @@ var RouteSchemaManager = function(options) {
 			var i, prop;
 			if (schema.type === 'object' && typeof(object) === 'object' && schema.properties) {
 				for (prop in schema.properties) {
-					if (schema.properties.hasOwnProperty(prop) && object.hasOwnProperty(prop)) {
+					if (schema.properties.hasOwnProperty(prop) && Object.hasOwnProperty.call(object, prop)) {
 						object[prop] = convertPropertyTypesToMatchSchema(object[prop], schema.properties[prop], forceArrayConversion);
 					}
 				}
@@ -135,34 +158,11 @@ var RouteSchemaManager = function(options) {
 			}
 		},
 
-		convertValueFromStringToType = function(value, type) {
-			if (typeof(value) !== 'string' || type === 'string') {
-				return value;
-			}
-			if (type === 'integer' || type === 'number') {
-				// fastest (and more reliable) way to convert strings to numbers
-				var convertedVal = 1 * value;
-				// make sure that if our schema calls for an integer, that there is no decimal
-				if (convertedVal || convertedVal === 0 && (type === 'number' || (value.indexOf('.') === -1))) {
-					return convertedVal;
-				}
-			}
-			else if (type === 'boolean') {
-				if (value === 'true') {
-					return true;
-				}
-				else if (value === 'false') {
-					return false;
-				}
-			}
-			return value;
-		},
-
 		convertArraysInQueryString = function(queryObj) {
 			var prop, newProp, idx,
 				arraySyntaxRegex = /\[\d+\]$/;
 			for (prop in queryObj) {
-				if (queryObj.hasOwnProperty(prop)) {
+				if (Object.hasOwnProperty.call(queryObj, prop)) {
 					if (arraySyntaxRegex.test(prop)) {
 						newProp = prop.substring(0, prop.lastIndexOf('['));
 						queryObj[newProp] = queryObj[newProp] || [];
@@ -284,4 +284,4 @@ var RouteSchemaManager = function(options) {
 	};
 };
 
-module.exports = RouteSchemaManager;
+module.exports = RouteSchemaManager;
diff --git a/lib/SwaggerManager.js b/lib/SwaggerManager.js
@@ -30,22 +30,19 @@ var SwaggerManager = function(options) {
 			});
 		},
 
+		swaggerParamTypeMap = {
+			path: 'path',
+			query: 'query',
+			payload: 'body',
+			headers: 'header'
+		},
+
 		getRoutesGroupedByPath = function(routes) {
 			return _.groupBy(routes, function(item) {
 				return item.path;
 			});
 		},
 
-		getApplicationVersion = function() {
-			var executingFile = process.argv[1];
-			var packageLoc = findPackageJson(executingFile.replace(/\/[^\/]+?$/g, ''));
-
-			if (packageLoc) {
-				return require(packageLoc).version;
-			}
-			return 'unknown';
-		},
-
 		findPackageJson = function(startingDirectory) {
 			if (!startingDirectory) {
 				return false;
@@ -56,6 +53,16 @@ var SwaggerManager = function(options) {
 			return findPackageJson(startingDirectory.replace(/\/[^\/]+?$/g, ''));
 		},
 
+		getApplicationVersion = function() {
+			var executingFile = process.argv[1];
+			var packageLoc = findPackageJson(executingFile.replace(/\/[^\/]+?$/g, ''));
+
+			if (packageLoc) {
+				return require(packageLoc).version;
+			}
+			return 'unknown';
+		},
+
 		getSwaggerParams = function(route, type, operationNickname) {
 			var params = [], prop, param;
 			if (swaggerParamTypeMap[type] &&
@@ -128,6 +135,18 @@ var SwaggerManager = function(options) {
 			return params;
 		},
 
+		setOperationConsumes = function (payloadParameters, route, operation) {
+			if (!payloadParameters.length) { //if no payload parameters, no consumes attribute
+				return;
+			}
+
+			if (route.settings.payload && route.settings.payload.allow && route.settings.payload.parse) {
+				operation.consumes = route.settings.payload.allow;
+			} else {
+				operation.consumes = consumesDefaults;
+			}
+		},
+
 		getSwaggerOperationForRoute = function(route, resourceType, path) {
 			var pathParts = path.split('/'),
 				regex = /^\{.+\}$/,
@@ -201,18 +220,6 @@ var SwaggerManager = function(options) {
 			return operation;
 		},
 
-		setOperationConsumes = function (payloadParameters, route, operation) {
-			if (!payloadParameters.length) { //if no payload parameters, no consumes attribute
-				return;
-			}
-
-			if (route.settings.payload && route.settings.payload.allow && route.settings.payload.parse) {
-				operation.consumes = route.settings.payload.allow;
-			} else {
-				operation.consumes = consumesDefaults;
-			}
-		},
-
 		getModelForRoute = function(route, modelKind, modelGetter){
 			var plugins = route.settings.plugins;
 			var model;
@@ -276,12 +283,6 @@ var SwaggerManager = function(options) {
 			});
 		},
 
-		swaggerParamTypeMap = {
-			path: 'path',
-			query: 'query',
-			payload: 'body',
-			headers: 'header'
-		},
 		defaultResourceListingModel = {
 			apiVersion: options.apiVersion || getApplicationVersion().split('.')[0],
 			swaggerVersion: '1.2',

diff --git a/test/RouteSchemaManager.tests.js b/test/RouteSchemaManager.tests.js
@@ -1,4 +1,5 @@
 // headings were generated by http://patorjk.com/software/taag/#p=display&f=Colossal&t=ValidateResponse
+var querystring = require('querystring');
 var assert = require('assert'),
 	RouteSchemaManager = require('../lib/RouteSchemaManager'),
 	rsmConfig = {
@@ -286,15 +287,10 @@ describe('RouteSchemaManager Unit Tests', function() {
 
 		it('should validate query params for route with no query schema successfully', function() {
 			var routeSchemaManager = new RouteSchemaManager(rsmConfig),
+				query = querystring.parse('string=fnord&array[0]=fnord1&array[1]=fnord2'),
 				mockRequest = {
 					_route: mockRoute2,
-					query: {
-						string: 'fnord',
-						array: [
-							'fnord1',
-							'fnord2'
-						]
-					}
+					query: query
 				};
 			routeSchemaManager.initializeRoutes(mockRoute1.server.info.uri, mockRoutes);
 			var report = routeSchemaManager.validateQuery(mockRequest);
@@ -303,12 +299,10 @@ describe('RouteSchemaManager Unit Tests', function() {
 
 		it('should validate query params successfully while converting properties to arrays if defined as such by schema', function() {
 			var routeSchemaManager = new RouteSchemaManager(rsmConfig),
+				query = querystring.parse('string=fnord&array=fnord1'),
 				mockRequest = {
 					_route: mockRoute1,
-					query: {
-						string: 'fnord',
-						array: 'fnord1'
-					}
+					query: query
 				};
 			routeSchemaManager.initializeRoutes(mockRoute1.server.info.uri, mockRoutes);
 			var report = routeSchemaManager.validateQuery(mockRequest);
@@ -318,13 +312,10 @@ describe('RouteSchemaManager Unit Tests', function() {
 		it('should validate query params successfully while converting properties to arrays (from objects) if defined as such by schema', function() {
 
 			var routeSchemaManager = new RouteSchemaManager(rsmConfig),
+				query = querystring.parse('string=fnord&array[0]=fnord1&array[1]=fnord2'),
 				mockRequest = {
 					_route: mockRoute1,
-					query: {
-						string: 'fnord',
-						'array[0]': 'fnord1',
-						'array[1]': 'fnord2'
-					}
+					query: query
 				};
 			routeSchemaManager.initializeRoutes(mockRoute1.server.info.uri, mockRoutes);
 			var report = routeSchemaManager.validateQuery(mockRequest);
@@ -334,12 +325,10 @@ describe('RouteSchemaManager Unit Tests', function() {
 		it('should validate query params successfully while converting properties to booleans (true) if defined as such by schema', function() {
 
 			var routeSchemaManager = new RouteSchemaManager(rsmConfig),
+				query = querystring.parse('string=fnord&bool=true'),
 				mockRequest = {
 					_route: mockRoute1,
-					query: {
-						string: 'fnord',
-						bool: 'true'
-					}
+					query: query
 				};
 			routeSchemaManager.initializeRoutes(mockRoute1.server.info.uri, mockRoutes);
 			var report = routeSchemaManager.validateQuery(mockRequest);
@@ -348,12 +337,10 @@ describe('RouteSchemaManager Unit Tests', function() {
 
 		it('should validate query params successfully while converting properties to booleans (false) if defined as such by schema', function() {
 			var routeSchemaManager = new RouteSchemaManager(rsmConfig),
+				query = querystring.parse('string=fnord&bool=false'),
 				mockRequest = {
 					_route: mockRoute1,
-					query: {
-						string: 'fnord',
-						bool: 'false'
-					}
+					query: query
 				};
 			routeSchemaManager.initializeRoutes(mockRoute1.server.info.uri, mockRoutes);
 			var report = routeSchemaManager.validateQuery(mockRequest);
@@ -363,20 +350,17 @@ describe('RouteSchemaManager Unit Tests', function() {
 		it('should not validate query params successfully while avoiding the conversion of properties to different types when the types cannot be coerced to the type defined in the schema', function() {
 
 			var routeSchemaManager = new RouteSchemaManager(rsmConfig),
+				query = querystring.parse('string=fnord&array[fnord]=1&bool=truefalse'),
 				mockRequest = {
 					_route: mockRoute1,
-					query: {
-						string: 'fnord',
-						array: { fnord: 1 },
-						bool: 'truefalse'
-					}
+					query: query
 				};
 			routeSchemaManager.initializeRoutes(mockRoute1.server.info.uri, mockRoutes);
 			var report = routeSchemaManager.validateQuery(mockRequest);
 			assert(!report.valid, 'query obj should not be valid');
 		});
 
-		it('should fail validation of query params', function() {
+		it('should fail validation of query params if parsed object has incorrect type', function() {
 
 			var routeSchemaManager = new RouteSchemaManager(rsmConfig),
 				mockRequest = {
@@ -390,6 +374,7 @@ describe('RouteSchemaManager Unit Tests', function() {
 			assert(!report.valid, 'query obj should not be valid');
 			assert(report.errors, 'errors obj should be valid');
 		});
+
 	});
 
 /*