Merge pull request #142 from ckeboss/better-query-param-support_136

Support a wider range of query param names
cdimascio · Dec 18, 2019 · 793b998 · 793b998
2 parents 20a9c6e + 5b2b5ab
commit 793b998
Show file tree

Hide file tree

Showing 7 changed files with 233 additions and 30 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -56,6 +56,7 @@
     "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
@@ -128,9 +128,12 @@ 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(
-          req.query,
+          queryParamsToValidate,
           schema.properties.query,
           securityQueryParameter,
         );
@@ -152,6 +155,27 @@ 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(
@@ -171,6 +195,18 @@ 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,
@@ -182,6 +218,13 @@ 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)
@@ -192,6 +235,7 @@ export class RequestValidator {
 
       const reqToValidate = {
         ...req,
+        query: queryParamsToValidate,
         cookies: req.cookies
           ? { ...req.cookies, ...req.signedCookies }
           : undefined,
@@ -417,4 +461,25 @@ 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
@@ -17,83 +17,134 @@ export function routes(app) {
   const basePath = app.basePath;
   const router1 = express
     .Router()
-    .post('/', function(req, res, next) {
+    .post('/', function(
+      req: express.Request,
+      res: express.Response,
+      next: express.NextFunction,
+    ): void {
       res.json({
         name: `${req.method}: /router_1`,
       });
     })
-    .get('/', function(req, res, next) {
+    .get('/', function(
+      req: express.Request,
+      res: express.Response,
+      next: express.NextFunction,
+    ): void {
       res.json({
         name: `${req.method}: /router_1`,
       });
     })
-    .get('/:id', function(req, res, next) {
+    .get('/:id', function(
+      req: express.Request,
+      res: express.Response,
+      next: express.NextFunction,
+    ): void {
       res.json({
         name: `${req.method}: /router_1/${req.params.id}`,
       });
     })
-    .get('/:id/best/:bid', function(req, res, next) {
+    .get('/:id/best/:bid', function(
+      req: express.Request,
+      res: express.Response,
+      next: express.NextFunction,
+    ): void {
       res.json({
         name: `${req.method}: /router_1/${req.params.id}/best/${req.params.bid}`,
       });
     });
 
   app.use(`${basePath}/router_1`, router1);
 
-  app.get(`${basePath}/pets`, function(req, res, next) {
+  app.get(`${basePath}/pets`, function(
+    req: express.Request,
+    res: express.Response,
+    next: express.NextFunction,
+  ): void {
     res.json({
       test: 'hi',
       ...req.body,
     });
   });
 
-  app.post(`${basePath}/pets`, function(req, res, next) {
+  app.post(`${basePath}/pets`, function(
+    req: express.Request,
+    res: express.Response,
+    next: express.NextFunction,
+  ): void {
     res.json({
       ...req.body,
       id: 'new-id',
     });
   });
 
-  app.get(`${basePath}/pets/:id`, function(req, res, next) {
+  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 {
     res.json({
       id: req.params.id,
     });
   });
 
-  app.get(`${basePath}/pets/:id/attributes`, function(req, res, next) {
+  app.get(`${basePath}/pets/:id/attributes`, function(
+    req: express.Request,
+    res: express.Response,
+    next: express.NextFunction,
+  ): void {
     res.json({
       id: req.params.id,
     });
   });
 
   app.get(`${basePath}/pets/:id/attributes/:attribute_id`, function(
-    req,
-    res,
-    next,
-  ) {
+    req: express.Request,
+    res: express.Response,
+    next: express.NextFunction,
+  ): void {
     res.json({
       id: req.params.id,
       attribute_id: req.params.attribute_id,
     });
   });
 
   app.post(`${basePath}/route_defined_in_express_not_openapi`, function(
-    req,
-    res,
-    next,
-  ) {
+    req: express.Request,
+    res: express.Response,
+    next: express.NextFunction,
+  ): void {
     res.json({
       id: req.params.id,
     });
   });
 
-  app.get('/not_under_an_openapi_basepath', function(req, res, next) {
+  app.get('/not_under_an_openapi_basepath', function(
+    req: express.Request,
+    res: express.Response,
+    next: express.NextFunction,
+  ): void {
     res.json({
       id: '/not_under_an_openapi_basepath',
     });
   });
 
-  app.post('/v1/pets/:id/photos', function(req, res, next) {
+  app.post('/v1/pets/:id/photos', function(
+    req: express.Request,
+    res: express.Response,
+    next: express.NextFunction,
+  ): void {
     // req.file is the `avatar` file
     // req.body will hold the text fields, if there were any
     const files = req.files;
@@ -102,7 +153,11 @@ export function routes(app) {
       metadata: req.body.metadata,
     });
   });
-  app.post('/v1/pets_charset', function (req: Request, res: any) {
+  app.post('/v1/pets_charset', function(
+    req: express.Request,
+    res: express.Response,
+    next: express.NextFunction,
+  ): void {
     // 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
@@ -1,6 +1,7 @@
 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';
 
@@ -11,13 +12,15 @@ describe(packageJson.name, () => {
   before(() => {
     const apiSpecPath = path.join('test', 'resources', 'openapi.yaml');
     const apiSpecJson = require('./resources/openapi.json');
-    return Promise.all([
-      createApp({ apiSpec: apiSpecPath }, 3001),
-      createApp({ apiSpec: apiSpecJson }, 3002),
-    ]).then(([a1, a2]) => {
-      apps.push(a1);
-      apps.push(a2);
-      basePath = (<any>a1).basePath;
+    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 = (<any>a1).basePath;
+      });
     });
   });