allowReserved not validated properly. #158

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

@@ -5,6 +5,7 @@ import {
   OpenApiRequestMetadata,
   ValidationSchema,
 } from '../../framework/types';
+import * as url from 'url';
 import { validationError } from '../util';
 import { dereferenceParameter, normalizeParameter } from './util';
 import * as mediaTypeParser from 'media-typer';
@@ -14,6 +15,8 @@ type SchemaObject = OpenAPIV3.SchemaObject;
 type ReferenceObject = OpenAPIV3.ReferenceObject;
 type ParameterObject = OpenAPIV3.ParameterObject;
 
+const RESERVED_CHARS = /[\:\/\?#\[\]@!\$&\'()\*\+,;=]/;
+
 const ARRAY_DELIMITER = {
   form: ',',
   spaceDelimited: ' ',
@@ -56,6 +59,10 @@ export class RequestParameterMutator {
    */
   public modifyRequest(req: OpenApiRequest): void {
     const { parameters } = (<OpenApiRequestMetadata>req.openapi).schema;
+    const rawQuery = this.parseQueryStringUndecoded(
+      url.parse(req.originalUrl).query,
+    );
+
     parameters.forEach(p => {
       const parameter = dereferenceParameter(this._apiDocs, p);
       const { name, schema } = normalizeParameter(parameter);
@@ -64,6 +71,10 @@ export class RequestParameterMutator {
       const i = req.originalUrl.indexOf('?');
       const queryString = req.originalUrl.substr(i + 1);
 
+      if (parameter.in === 'query' && !parameter.allowReserved) {
+        this.validateReservedCharacters(name, rawQuery);
+      }
+
       if (parameter.content) {
         this.handleContent(req, name, parameter);
       } else if (parameter.in === 'query' && this.isObjectOrXOf(schema)) {
@@ -260,7 +271,32 @@ export class RequestParameterMutator {
   ): void {
     if (!delimiter) {
       const message = `Parameter 'style' has incorrect value '${parameter.style}' for [${parameter.name}]`;
-      throw validationError(400, this.path, message);
+      throw validationError(400, `.query.${parameter.name}`, message);
     }
   }
+
+  private validateReservedCharacters(
+    name: string,
+    pairs: { [key: string]: string[] },
+  ) {
+    const vs = pairs[name];
+    if (!vs) return;
+    for (const v of vs) {
+      if (v?.match(RESERVED_CHARS)) {
+        const message = `Parameter '${name}' must be url encoded. It's value may not contain reserved characters.`;
+        throw validationError(400, `.query.${name}`, message);
+      }
+    }
+  }
+
+  private parseQueryStringUndecoded(qs: string) {
+    if (!qs) return {};
+    const q = qs.replace('?', '');
+    return q.split('&').reduce((m, p) => {
+      const [k, v] = p.split('=');
+      m[k] = m[k] ?? [];
+      m[k].push(v);
+      return m;
+    }, {});
+  }
 }

test/common/app.common.ts

@@ -44,13 +44,15 @@ export function routes(app) {
   app.get(`${basePath}/pets`, function(req: Request, res: Response): void {
     res.json({
       test: 'hi',
-      ...req.body,
+      body: req.body,
+      query: req.query,
     });
   });
 
   app.post(`${basePath}/pets`, function(req: Request, res: Response): void {
     res.json({
-      ...req.body,
+      body: req.body,
+      query: req.query,
       id: 'new-id',
     });
   });

test/nullable.spec.ts

@@ -52,7 +52,7 @@ describe(packageJson.name, () => {
       })
       .expect(200)
       .then(r => {
-        expect(r.body.tag).to.equal('my default value');
+        expect(r.body.body.tag).to.equal('my default value');
       }));
 
   it('should fail if required and not provided (nullable true)', async () =>

test/openapi.spec.ts

@@ -81,18 +81,40 @@ describe(packageJson.name, () => {
             expect(e[0].message).to.equal('should be >= 5');
           }));
 
-      it('should return 200 when JSON in query param', async () =>
+      it('should return 400 when non-urlencoded JSON in query param', async () =>
         request(apps[i])
           .get(`${basePath}/pets`)
           .query(`limit=10&test=one&testJson={"foo": "bar"}`)
           .set('Accept', 'application/json')
           .expect('Content-Type', /json/)
+          .then(r => {
+            expect(r.body)
+              .to.have.property('message')
+              .that.equals(
+                "Parameter 'testJson' must be url encoded. It's value may not contain reserved characters.",
+              );
+          }));
+
+      it('should return 200 when JSON in query param', async () =>
+        request(apps[i])
+          .get(`${basePath}/pets`)
+          .query(
+            `limit=10&test=one&testJson=${encodeURIComponent(
+              '{"foo": "bar"}',
+            )}`,
+          )
+          .set('Accept', 'application/json')
+          .expect('Content-Type', /json/)
           .expect(200));
 
       it('should return 400 when improper JSON in query param', async () =>
         request(apps[i])
           .get(`${basePath}/pets`)
-          .query(`limit=10&test=one&testJson={"foo": "test"}`)
+          .query({
+            limit: 10,
+            test: 'one',
+            testJson: { foo: 'test' },
+          })
           .set('Accept', 'application/json')
           .expect('Content-Type', /json/)
           .expect(400)
@@ -105,22 +127,56 @@ describe(packageJson.name, () => {
             );
           }));
 
-      it('should return 200 when separated array in query param', async () =>
+      it('should return 400 when comma separated array in query param', async () =>
+        request(apps[i])
+          .get(`${basePath}/pets`)
+          .query({
+            limit: 10,
+            test: 'one',
+            testArray: 'foo,bar,baz',
+          })
+          .set('Accept', 'application/json')
+          .expect('Content-Type', /json/)
+          .expect(200));
+
+      it('should return 400 when comma separated array in query param is not url encoded', async () =>
         request(apps[i])
           .get(`${basePath}/pets`)
           .query(`limit=10&test=one&testArray=foo,bar,baz`)
           .set('Accept', 'application/json')
           .expect('Content-Type', /json/)
+          .expect(400)
+          .then(r => {
+            expect(r.body)
+              .to.have.property('message')
+              .that.equals(
+                "Parameter 'testArray' must be url encoded. It's value may not contain reserved characters.",
+              );
+          }));
+
+      it('should return 200 when separated array in query param', async () =>
+        request(apps[i])
+          .get(`${basePath}/pets`)
+          .query(
+            `limit=10&test=one&testArray=${encodeURIComponent('foo,bar,baz')}`,
+          )
+          .set('Accept', 'application/json')
+          .expect('Content-Type', /json/)
           .expect(200));
 
       it('should return 400 when improper separated array in query param', async () =>
         request(apps[i])
           .get(`${basePath}/pets`)
-          .query(`limit=10&test=one&testArray=foo,bar,test`)
+          .query({
+            limit: 10,
+            test: 'one',
+            testArray: 'foo,bar,test',
+          })
           .set('Accept', 'application/json')
           .expect('Content-Type', /json/)
           .expect(400)
           .then(r => {
+            console.log(r.body);
             const e = r.body.errors;
             expect(e).to.have.length(1);
             expect(e[0].path).to.contain('testArray');

test/query.params.spec.ts

@@ -16,7 +16,9 @@ describe(packageJson.name, () => {
         `${app.basePath}`,
         express
           .Router()
-          .post(`/pets/nullable`, (req, res) => res.json(req.body)),
+          .post(`/pets/nullable`, (req, res) => res.json(req.body))
+          .get(`/no_reserved`, (req, res) => res.json(req.body))
+          .get(`/allow_reserved`, (req, res) => res.json(req.body)),
       ),
     );
   });
@@ -86,8 +88,26 @@ describe(packageJson.name, () => {
       })
       .expect(200));
 
-      it("should succeed when query param 'name' has empty value and sets allowEmptyValue: true", async () =>
-      request(app)
-        .get(`${app.basePath}/pets?name=&tags=one&limit=10&breed=german_shepherd&owner_name=carmine`)
-        .expect(200));
+  it("should succeed when query param 'name' has empty value and sets allowEmptyValue: true", async () =>
+    request(app)
+      .get(
+        `${app.basePath}/pets?name=&tags=one&limit=10&breed=german_shepherd&owner_name=carmine`,
+      )
+      .expect(200));
+
+  it('should not allow reserved characters', async () =>
+    request(app)
+      .get(`${app.basePath}/no_reserved?value=ThisHas$ReservedChars!`)
+      .expect(400)
+      .then(r => {
+        const body = r.body;
+        expect(body.message).equals(
+          "Parameter 'value' must be url encoded. It's value may not contain reserved characters.",
+        );
+      }));
+
+  it('should may allow reserved characters when allowedReserved: true', async () =>
+    request(app)
+      .get(`${app.basePath}/allow_reserved?value=ThisHas$ReservedChars!`)
+      .expect(200));
 });

test/resources/query.params.yaml

@@ -30,6 +30,46 @@ servers:
         enum:
           - v1
 paths:
+  /no_reserved:
+    get:
+      parameters:
+        - name: value
+          in: query
+          required: true
+          schema:
+            type: string
+      responses:
+        '200':
+          description: success
+  /allow_reserved:
+    get:
+      parameters:
+       -  name: value
+          in: query
+          required: true
+          schema:
+            type: string
+          allowReserved: true
+      responses:
+        '200':
+          description: success
+
+  /allow_reserved/array_explode:
+    get:
+      parameters:
+       -  name: value
+          in: query
+          required: true
+          schema:
+            type: array
+            items:
+              type: string
+          explode: true
+          allowReserved: true
+      responses:
+        '200':
+          description: success
+
   /pets:
     get:
       description: |
@@ -45,6 +85,7 @@ paths:
             type: array
             items:
               type: string
+          # allowReserved: true
         - name: limit
           in: query
           description: maximum number of results to return

tsconfig.json

@@ -9,11 +9,10 @@
     "resolveJsonModule": true,
     "typeRoots": ["./node_modules/@types", "./typings"]
   },
-  "exclude": ["node_modules", "a_reference"],
+  "exclude": ["node_modules"],
   "include": [
     "typings/**/*.d.ts",
     "src/**/*.ts",
-    "src/framework/modded.express.mung.ts",
-    "a_reference/query.parser.ts"
+    "src/framework/modded.express.mung.ts"
   ]
 }