Merge pull request #206 from cdimascio/allow-empty-value

handle allow empty value properly
cdimascio · Dec 30, 2019 · 461e960 · 461e960
2 parents 72428c2 + ac1734d
commit 461e960
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 8 deletions.
diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts
@@ -115,7 +115,7 @@ export class RequestValidator {
       mutator.modifyRequest(req);
 
       if (!this.requestOpts.allowUnknownQueryParameters) {
-        this.rejectUnknownQueryParams(
+        this.processQueryParam(
           req.query,
           schema.properties.query,
           securityQueryParam,
@@ -141,21 +141,27 @@ export class RequestValidator {
     };
   }
 
-  private rejectUnknownQueryParams(
-    query,
-    schema,
-    whiteList: string[] = [],
-  ): void {
+  private processQueryParam(query, schema, whiteList: string[] = []) {
     if (!schema.properties) return;
     const knownQueryParams = new Set(Object.keys(schema.properties));
     whiteList.forEach(item => knownQueryParams.add(item));
     const queryParams = Object.keys(query);
+    const allowedEmpty = schema.allowEmptyValue;
     for (const q of queryParams) {
-      if (!knownQueryParams.has(q)) {
+      if (
+        !this.requestOpts.allowUnknownQueryParameters &&
+        !knownQueryParams.has(q)
+      ) {
+        throw validationError(
+          400,
+          `.query.${q}`,
+          `Unknown query parameter '${q}'`,
+        );
+      } else if (!allowedEmpty?.has(q) && (query[q] === '' || null)) {
         throw validationError(
           400,
           `.query.${q}`,
-          `Unknown query parameter ${q}`,
+          `Empty value found for query parameter '${q}'`,
         );
       }
     }

diff --git a/src/middlewares/parsers/schema.parse.ts b/src/middlewares/parsers/schema.parse.ts
@@ -47,6 +47,12 @@ export class ParametersSchemaParser {
       }
 
       schemas[reqField].properties[name] = schema;
+      if (reqField === 'query' && parameter.allowEmptyValue) {
+        if (!schemas[reqField].allowEmptyValue) {
+          schemas[reqField].allowEmptyValue = new Set<string>();
+        }
+        schemas[reqField].allowEmptyValue.add(name);
+      }
       if (parameter.required) {
         if (!schemas[reqField].required) {
           schemas[reqField].required = [];

diff --git a/test/query.params.allow.unknown.spec.ts b/test/query.params.allow.unknown.spec.ts
@@ -32,6 +32,7 @@ describe(packageJson.name, () => {
     request(app)
       .get(`${app.basePath}/pets`)
       .query({
+        name: 'max',
         tags: 'one,two,three',
         limit: 10,
         breed: 'german_shepherd',
@@ -43,6 +44,7 @@ describe(packageJson.name, () => {
     request(app)
       .get(`${app.basePath}/pets`)
       .query({
+        name: 'max',
         tags: 'one,two,three',
         limit: 10,
         breed: 'german_shepherd',

diff --git a/test/query.params.spec.ts b/test/query.params.spec.ts
@@ -29,6 +29,7 @@ describe(packageJson.name, () => {
     request(app)
       .get(`${app.basePath}/pets`)
       .query({
+        name: 'max',
         tags: 'one,two,three',
         limit: 10,
         breed: 'german_shepherd',
@@ -40,6 +41,7 @@ describe(packageJson.name, () => {
     request(app)
       .get(`${app.basePath}/pets`)
       .query({
+        name: 'max',
         tags: 'one,two,three',
         limit: 10,
         breed: 'german_shepherd',
@@ -50,4 +52,42 @@ describe(packageJson.name, () => {
       .then(r => {
         expect(r.body.errors).to.be.an('array');
       }));
+
+  it('should not allow empty query param value', async () =>
+    request(app)
+      .get(`${app.basePath}/pets`)
+      .query({
+        name: 'max',
+        tags: 'one,two,three',
+        limit: 10,
+        breed: '',
+        owner_name: 'carmine',
+      })
+      .expect(400)
+      .then(r => {
+        expect(r.body)
+          .to.have.property('message')
+          .that.equals("Empty value found for query parameter 'breed'");
+        expect(r.body.errors)
+          .to.be.an('array')
+          .with.length(1);
+        expect(r.body.errors[0].path).to.equal('.query.breed');
+      }));
+
+  it('should allow empty query param value with allowEmptyValue: true', async () =>
+    request(app)
+      .get(`${app.basePath}/pets`)
+      .query({
+        name: '',
+        tags: 'one,two,three',
+        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));
 });
diff --git a/test/resources/query.params.yaml b/test/resources/query.params.yaml
@@ -35,6 +35,7 @@ paths:
       description: |
         Returns all pets from the system that the user has access tp
       parameters:
+        - $ref: '#/components/parameters/name'
         - name: tags
           in: query
           description: tags to filter by
@@ -73,6 +74,14 @@ paths:
                   $ref: '#/components/schemas/Pet'
 components:
   parameters:
+    name:
+      name: name
+      in: query
+      description: name
+      required: true
+      schema:
+        type: string
+      allowEmptyValue: true
     owner_name:
       name: owner_name
       in: query