From 0953971df44bd6d2ed31753cd85640af6e5b7431 Mon Sep 17 00:00:00 2001 From: Carmine DiMascio Date: Wed, 18 Dec 2019 19:29:29 -0500 Subject: [PATCH] Revert "Merge pull request #142 from ckeboss/better-query-param-support_136" This reverts commit 793b9989a29428814503841afd8538fabb48939d, reversing changes made to 20a9c6ef8e1c5184b22cab886083957dccf73315. --- package-lock.json | 15 ---- package.json | 1 - src/middlewares/openapi.request.validator.ts | 67 +------------- test/common/app.common.ts | 93 ++++---------------- test/openapi.spec.ts | 17 ++-- test/query.params.spec.ts | 38 +------- test/resources/query.params.yaml | 32 +------ 7 files changed, 30 insertions(+), 233 deletions(-) diff --git a/package-lock.json b/package-lock.json index e568e919..52e70287 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1458,15 +1458,6 @@ "integrity": "sha1-6td0q+5y4gQJQzoGY2YCPdaIekE=", "dev": true }, - "get-port": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/get-port/-/get-port-5.0.0.tgz", - "integrity": "sha512-imzMU0FjsZqNa6BqOjbbW6w5BivHIuQKopjpPqcnx0AVHJQKCxK1O+Ab3OrVXhrekqfVMjwA9ZYu062R+KcIsQ==", - "dev": true, - "requires": { - "type-fest": "^0.3.0" - } - }, "get-stream": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-4.1.0.tgz", @@ -3701,12 +3692,6 @@ "integrity": "sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g==", "dev": true }, - "type-fest": { - "version": "0.3.1", - "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.3.1.tgz", - "integrity": "sha512-cUGJnCdr4STbePCgqNFbpVNCepa+kAVohJs1sLhxzdH+gnEoOd8VhbYa7pD3zZYGiURWM2xzEII3fQcRizDkYQ==", - "dev": true - }, "type-is": { "version": "1.6.18", "resolved": "https://registry.npmjs.org/type-is/-/type-is-1.6.18.tgz", diff --git a/package.json b/package.json index c4bb1d66..63265826 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,6 @@ "coveralls": "^3.0.5", "deasync": "^0.1.16", "express": "^4.17.1", - "get-port": "^5.0.0", "mocha": "^6.2.0", "morgan": "^1.9.1", "nodemon": "^2.0.0", diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts index d934618e..9260cddb 100644 --- a/src/middlewares/openapi.request.validator.ts +++ b/src/middlewares/openapi.request.validator.ts @@ -128,12 +128,9 @@ export class RequestValidator { const validator = this.ajv.compile(schema); return (req: OpenApiRequest, res: Response, next: NextFunction): void => { - - const queryParamsToValidate = this.parseQueryParamsFromURL(req.originalUrl); - if (!this._requestOpts.allowUnknownQueryParameters) { this.rejectUnknownQueryParams( - queryParamsToValidate, + req.query, schema.properties.query, securityQueryParameter, ); @@ -155,27 +152,6 @@ export class RequestValidator { * https://swagger.io/docs/specification/describing-parameters/#schema-vs-content */ parameters.parseJson.forEach(item => { - if (item.reqField === 'query' && queryParamsToValidate[item.name]) { - if (queryParamsToValidate[item.name] === req[item.reqField][item.name]) { - queryParamsToValidate[item.name] = req[item.reqField][item.name] = JSON.parse( - queryParamsToValidate[item.name], - ); - - /** - * The query param we parse and the query param express - * parsed are the same value, so we assign them the same - * parsed value. - */ - return; - } - /** - * They query params are not the same, so we parse the - * `queryParamsToValidate` and don't return. - */ - queryParamsToValidate[item.name] = JSON.parse( - queryParamsToValidate[item.name], - ); - } if (req[item.reqField]?.[item.name]) { try { req[item.reqField][item.name] = JSON.parse( @@ -195,18 +171,6 @@ export class RequestValidator { * filter=foo%20bar%20baz */ parameters.parseArray.forEach(item => { - if (item.reqField === 'query' && queryParamsToValidate[item.name]) { - if (queryParamsToValidate[item.name] === req[item.reqField][item.name]) { - queryParamsToValidate[item.name] = req[item.reqField][item.name] = queryParamsToValidate[item.name].split( - item.delimiter, - ); - - return; - } - queryParamsToValidate[item.name] = queryParamsToValidate[item.name].split( - item.delimiter, - ); - } if (req[item.reqField]?.[item.name]) { req[item.reqField][item.name] = req[item.reqField][item.name].split( item.delimiter, @@ -218,13 +182,6 @@ export class RequestValidator { * forcing convert to array if scheme describes param as array + explode */ parameters.parseArrayExplode.forEach(item => { - if ( - item.reqField === 'query' && - queryParamsToValidate[item.name] && - !(queryParamsToValidate[item.name] instanceof Array) - ) { - queryParamsToValidate[item.name] = [queryParamsToValidate[item.name]]; - } if ( req[item.reqField]?.[item.name] && !(req[item.reqField][item.name] instanceof Array) @@ -235,7 +192,6 @@ export class RequestValidator { const reqToValidate = { ...req, - query: queryParamsToValidate, cookies: req.cookies ? { ...req.cookies, ...req.signedCookies } : undefined, @@ -461,25 +417,4 @@ export class RequestValidator { return { schema, parseJson, parseArray, parseArrayExplode }; } - - private parseQueryParamsFromURL(url: string): object { - const queryIndex = url.indexOf('?'); - const queryString = (queryIndex >= 0) ? url.slice(queryIndex + 1) : ''; - const searchParams = new URLSearchParams(queryString); - const queryParamsToValidate: object = {}; - - searchParams.forEach((value, key) => { - if (queryParamsToValidate[key]) { - if (queryParamsToValidate[key] instanceof Array) { - queryParamsToValidate[key].push(value) - } else { - queryParamsToValidate[key] = [queryParamsToValidate[key], value] - } - } else { - queryParamsToValidate[key] = value - } - }); - - return queryParamsToValidate; - } } diff --git a/test/common/app.common.ts b/test/common/app.common.ts index d7fe4e64..5a3f5f9b 100644 --- a/test/common/app.common.ts +++ b/test/common/app.common.ts @@ -17,38 +17,22 @@ export function routes(app) { const basePath = app.basePath; const router1 = express .Router() - .post('/', function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + .post('/', function(req, res, next) { res.json({ name: `${req.method}: /router_1`, }); }) - .get('/', function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + .get('/', function(req, res, next) { res.json({ name: `${req.method}: /router_1`, }); }) - .get('/:id', function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + .get('/:id', function(req, res, next) { res.json({ name: `${req.method}: /router_1/${req.params.id}`, }); }) - .get('/:id/best/:bid', function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + .get('/:id/best/:bid', function(req, res, next) { res.json({ name: `${req.method}: /router_1/${req.params.id}/best/${req.params.bid}`, }); @@ -56,64 +40,37 @@ export function routes(app) { app.use(`${basePath}/router_1`, router1); - app.get(`${basePath}/pets`, function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + app.get(`${basePath}/pets`, function(req, res, next) { res.json({ test: 'hi', ...req.body, }); }); - app.post(`${basePath}/pets`, function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + app.post(`${basePath}/pets`, function(req, res, next) { res.json({ ...req.body, id: 'new-id', }); }); - app.get(`${basePath}/pets/with-required-date-filter`, function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { - res.json({ - test: 'hi', - ...req.body, - }); - }); - - app.get(`${basePath}/pets/:id`, function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + app.get(`${basePath}/pets/:id`, function(req, res, next) { res.json({ id: req.params.id, }); }); - app.get(`${basePath}/pets/:id/attributes`, function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + app.get(`${basePath}/pets/:id/attributes`, function(req, res, next) { res.json({ id: req.params.id, }); }); app.get(`${basePath}/pets/:id/attributes/:attribute_id`, function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + req, + res, + next, + ) { res.json({ id: req.params.id, attribute_id: req.params.attribute_id, @@ -121,30 +78,22 @@ export function routes(app) { }); app.post(`${basePath}/route_defined_in_express_not_openapi`, function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + req, + res, + next, + ) { res.json({ id: req.params.id, }); }); - app.get('/not_under_an_openapi_basepath', function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + app.get('/not_under_an_openapi_basepath', function(req, res, next) { res.json({ id: '/not_under_an_openapi_basepath', }); }); - app.post('/v1/pets/:id/photos', function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + app.post('/v1/pets/:id/photos', function(req, res, next) { // req.file is the `avatar` file // req.body will hold the text fields, if there were any const files = req.files; @@ -153,11 +102,7 @@ export function routes(app) { metadata: req.body.metadata, }); }); - app.post('/v1/pets_charset', function( - req: express.Request, - res: express.Response, - next: express.NextFunction, - ): void { + app.post('/v1/pets_charset', function (req: Request, res: any) { // req.file is the `avatar` file // req.body will hold the text fields, if there were any res.json({ diff --git a/test/openapi.spec.ts b/test/openapi.spec.ts index 12f6c34a..0944f96a 100644 --- a/test/openapi.spec.ts +++ b/test/openapi.spec.ts @@ -1,7 +1,6 @@ import * as path from 'path'; import { expect } from 'chai'; import * as request from 'supertest'; -import * as getPort from 'get-port'; import { createApp } from './common/app'; import * as packageJson from '../package.json'; @@ -12,15 +11,13 @@ describe(packageJson.name, () => { before(() => { const apiSpecPath = path.join('test', 'resources', 'openapi.yaml'); const apiSpecJson = require('./resources/openapi.json'); - return Promise.all([getPort(), getPort()]).then(ports => { - return Promise.all([ - createApp({ apiSpec: apiSpecPath }, ports[0]), - createApp({ apiSpec: apiSpecJson }, ports[1]), - ]).then(([a1, a2]) => { - apps.push(a1); - apps.push(a2); - basePath = (a1).basePath; - }); + return Promise.all([ + createApp({ apiSpec: apiSpecPath }, 3001), + createApp({ apiSpec: apiSpecJson }, 3002), + ]).then(([a1, a2]) => { + apps.push(a1); + apps.push(a2); + basePath = (a1).basePath; }); }); diff --git a/test/query.params.spec.ts b/test/query.params.spec.ts index 69cc9289..1539b828 100644 --- a/test/query.params.spec.ts +++ b/test/query.params.spec.ts @@ -30,55 +30,21 @@ describe(packageJson.name, () => { request(app) .get(`${app.basePath}/pets`) .query({ - breed: 'german_shepherd', - 'filter[date]': '2000-02-29', - limit: 10, - owner_name: 'carmine', tags: 'one,two,three', - }) - .expect(200)); - - it('should pass with query params containing []', async () => - request(app) - .get(`${app.basePath}/pets`) - .query({ - breed: 'german_shepherd', - 'filter[date]': '2000-02-29', limit: 10, - owner_name: 'carmine', - }) - .expect(200)); - - it('should fail with invalid query params containing []', async () => - request(app) - .get(`${app.basePath}/pets`) - .query({ breed: 'german_shepherd', - 'filter[date]': 'not-a-date', - limit: 10, owner_name: 'carmine', }) - .expect(400) - .then(r => { - expect(r.body.errors).to.be.an('array'); - })); - - it('should pass with required query params containing []', async () => - request(app) - .get(`${app.basePath}/pets/with-required-date-filter`) - .query({ - 'filter[date]': '2000-02-29', - }) .expect(200)); it('should fail if unknown query param is specified', async () => request(app) .get(`${app.basePath}/pets`) .query({ - breed: 'german_shepherd', + tags: 'one,two,three', limit: 10, + breed: 'german_shepherd', owner_name: 'carmine', - tags: 'one,two,three', unknown_prop: 'test', }) .expect(400) diff --git a/test/resources/query.params.yaml b/test/resources/query.params.yaml index 683498bb..d6e62dd4 100644 --- a/test/resources/query.params.yaml +++ b/test/resources/query.params.yaml @@ -33,16 +33,8 @@ paths: /pets: get: description: | - Returns all pets from the system that the user has access to + Returns all pets from the system that the user has access tp parameters: - - name: filter[date] - in: query - description: date to filter by - required: false - style: form - schema: - type: string - format: date - name: tags in: query description: tags to filter by @@ -79,28 +71,6 @@ paths: type: array items: $ref: '#/components/schemas/Pet' - /pets/with-required-date-filter: - get: - description: | - Returns all pets from the system that the user has access to with required date filter - parameters: - - name: filter[date] - in: query - description: date to filter by - required: true - style: form - schema: - type: string - format: date - responses: - '200': - description: pet response - content: - application/json: - schema: - type: array - items: - $ref: '#/components/schemas/Pet' components: parameters: owner_name: