fixes 1064: type: object and explode: false in query parameter is not working as expected (#1066)

Author: cdimascio

src/middlewares/openapi.request.validator.ts

@@ -1,5 +1,4 @@
 import Ajv, { ValidateFunction } from 'ajv';
-import { parse } from 'qs';
 import { NextFunction, RequestHandler, Response } from 'express';
 import { createRequestAjv } from '../framework/ajv';
 import {
@@ -147,8 +146,6 @@ export class RequestValidator {
         writable: true,
         value: req.query,
       });
-      // TODO should be in RequestParameterMutator?
-      req.query = this.normalizeQueryFields(req.query);
       const schemaProperties = validator.allSchemaProperties;
       const mutator = new RequestParameterMutator(
         this.ajv,
@@ -291,28 +288,6 @@ export class RequestValidator {
       }
     }
   }
-
-  /**
-   * Mutates and normalizes the req.query object by parsing braket notation query string key values pairs
-   * into its corresponding key=<json-object> and update req.query with the parsed value
-   * for instance, req.query that equals { filter[name]: test} is translated into { filter: { name: 'test' }, where 
-   * the query string field is set as filter and its value is the full javascript object (translated from bracket notation)
-   * @param keys
-   * @returns
-   */
-  private normalizeQueryFields(query: { [key: string]: any }): {
-    [key: string]: any;
-  } {
-    Object.keys(query).forEach((key) => {
-      const bracketNotation = key.includes('[');
-      if (bracketNotation) {
-        const normalizedKey = key.split('[')[0];
-        query[normalizedKey] = parse(`${key}=${query[key]}`)[normalizedKey];
-        delete query[key];
-      }
-    });
-    return query;
-  }
 }
 
 class Validator {

src/middlewares/parsers/req.parameter.mutator.ts

@@ -11,6 +11,7 @@ import * as url from 'url';
 import { dereferenceParameter, normalizeParameter } from './util';
 import * as mediaTypeParser from 'media-typer';
 import * as contentTypeParser from 'content-type';
+import { parse } from 'qs';
 
 type SchemaObject = OpenAPIV3.SchemaObject;
 type ReferenceObject = OpenAPIV3.ReferenceObject;
@@ -67,6 +68,8 @@ export class RequestParameterMutator {
       url.parse(req.originalUrl).query,
     );
 
+    req.query = this.handleBracketNotationQueryFields(req.query);
+
     (parameters || []).forEach((p) => {
       const parameter = dereferenceParameter(this._apiDocs, p);
       const { name, schema } = normalizeParameter(this.ajv, parameter);
@@ -96,14 +99,27 @@ export class RequestParameterMutator {
       if (parameter.content) {
         this.handleContent(req, name, parameter);
       } else if (parameter.in === 'query' && this.isObjectOrXOf(schema)) {
+        // handle bracket notation and mutates query param
+        
+
         if (style === 'form' && explode) {
           this.parseJsonAndMutateRequest(req, parameter.in, name);
           this.handleFormExplode(req, name, <SchemaObject>schema, parameter);
         } else if (style === 'deepObject') {
           this.handleDeepObject(req, queryString, name, schema);
+        } else if (style === 'form' && !explode && schema.type === 'object') {
+          const value = req.query[name];
+          if (typeof value === 'string') {
+            const kvPairs = this.csvToKeyValuePairs(value);
+            if (kvPairs) {
+              req.query[name] = kvPairs;
+              return;
+            }
+          }
+          this.parseJsonAndMutateRequest(req, parameter.in, name);
         } else {
           this.parseJsonAndMutateRequest(req, parameter.in, name);
-        }
+      }
       } else if (type === 'array' && !explode) {
         const delimiter = ARRAY_DELIMITER[parameter.style];
         this.validateArrayDelimiter(delimiter, parameter);
@@ -411,4 +427,52 @@ export class RequestParameterMutator {
       return m;
     }, new Map<string, string[]>());
   }
+
+  private csvToKeyValuePairs(csvString: string): Record<string, string> | undefined {
+    const hasBrace = csvString.split('{').length > 1;
+    const items = csvString.split(',');
+    
+    if (hasBrace) {
+      // if it has a brace, we assume its JSON and skip creating k v pairs
+      // TODO improve json check, but ensure its cheap
+      return;
+    }
+  
+    if (items.length % 2 !== 0) {
+      // if the number of elements is not event, 
+      // then we do not have k v pairs, so return undefined
+      return;
+    }
+
+    const result = {};
+    
+    for (let i = 0; i < items.length - 1; i += 2) {
+      result[items[i]] = items[i + 1];
+    }
+    
+    return result;
+  }
+
+  /**
+   * Mutates and normalizes the req.query object by parsing braket notation query string key values pairs
+   * into its corresponding key=<json-object> and update req.query with the parsed value
+   * for instance, req.query that equals { filter[name]: test} is translated into { filter: { name: 'test' }, where 
+   * the query string field is set as filter and its value is the full javascript object (translated from bracket notation)
+   * @param keys
+   * @returns
+   */
+  private handleBracketNotationQueryFields(query: { [key: string]: any }): {
+    [key: string]: any;
+  } {
+    Object.keys(query).forEach((key) => {
+      const bracketNotation = key.includes('[');
+      if (bracketNotation) {
+        const normalizedKey = key.split('[')[0];
+        query[normalizedKey] = parse(`${key}=${query[key]}`)[normalizedKey];
+        delete query[key];
+      }
+    });
+    return query;
+  }
+  
 }

test/query.object.explode.spec.ts

@@ -0,0 +1,59 @@
+import { expect } from 'chai';
+import * as path from 'path';
+import * as request from 'supertest';
+import { createApp } from './common/app';
+import { AppWithServer } from './common/app.common';
+
+describe('query object with explode:false', () => {
+  let app: AppWithServer;
+
+  before(async () => {
+    const apiSpec = path.join('test', 'resources', 'query.object.explode.yaml');
+    app = await createApp(
+      {
+        apiSpec,
+        validateRequests: true,
+        validateResponses: false,
+      },
+      3005,
+      (app) => {
+        app.get(`${app.basePath}/users`, (req, res) => {
+          res.json(req.query);
+        });
+      },
+    );
+  });
+
+  after(() => {
+    app.server.close();
+  });
+
+  it('should correctly parse query parameter with style:form and explode:false', async () => {
+    return request(app)
+      .get(`${app.basePath}/users`)
+      .query('id=role,admin,firstName,Alex')
+      .expect(200)
+      .then((r) => {
+        console.log(r.body);
+        expect(r.body.id).to.deep.equal({
+          role: 'admin',
+          firstName: 'Alex',
+        });
+      });
+  });
+
+  it('should correctly parse query parameter with style:form and explode:false using url encoded values', async () => {
+    return request(app)
+      .get(
+        `${app.basePath}/users?id=%7B%22role%22%3A%22admin%22%2C%22firstName%22%3A%22Alex%22%7D`,
+      )
+      .expect(200)
+      .then((r) => {
+        console.log(r.body);
+        expect(r.body.id).to.deep.equal({
+          role: 'admin',
+          firstName: 'Alex',
+        });
+      });
+  });
+});

test/resources/query.object.explode.yaml

@@ -0,0 +1,28 @@
+openapi: 3.0.0
+info:
+  title: Query Object Explode Test
+  version: '1'
+servers:
+  - url: /v1
+paths:
+  /users:
+    get:
+      parameters:
+        - name: id
+          in: query
+          required: true
+          style: form
+          explode: false
+          schema:
+            type: object
+            properties:
+              role:
+                type: string
+              firstName:
+                type: string
+            required:
+              - role
+              - firstName
+      responses:
+        200:
+          description: 'Success'