Repository: cdimascio/express-openapi-validator (ts)

Issue:
Request Body content-types don't consider custom parameters
**Describe the bug**
There are a few issues that come from not having the request body content types resolved by their `equivalents` instead of by sorting the params and normalizing the content-type before comparing against the available openapi content types.

**To Reproduce**/**Actual behavior**/**Examples and context**
Considering this openapi document:
```yaml
...
paths:
  /pets_content_types:
    post:
      description: Creates a new pet in the store.  Duplicates are allowed
      requestBody:
        description: Pet to add to the store
        required: true
        content:
          application/json; version=1:
            schema:
              $ref: '#/components/schemas/NewPet'
      responses:
        '200':
          description: pet response
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Pet'
        default:
          description: unexpected error
          content:
            application/json; charset=utf-8:
              schema:
                $ref: '#/components/schemas/Error'
```

If the first request has `application/json; version=1` for the request body's content type, then openapi-validator will generate a cached validator which includes the content type used.  The problem is that the cache key doesn't include the request-body params.
```
POST /pets_content_types HTTP/1.1
Content-Type: application/json; version=1
Accept: application/json

{
  "name": "curly",
  "tag": "bald"
}
```

This request would result in a validator cached as `GET-/pets_content_types-application/json`.  Subsequent requests with unsupported versions now use the same key and may return 200 instead of 415 because the requestBody content-type is in-fact not supported.

**Expected behavior**
Every request with a different requestBody content-type should be analyzed separately.  If the cache is used, then the content types must be normalized to ensure that any parameters provided in different orders create the same cache key.

All of these should also result in different cache keys so each have a chance to return a 415 status code.  Currently, they all result in the same cache key.
- application/json; version=1
- application/json; param1=1
- application/json

Likewise, both of these content-types should result in the same cache key
- application/json; version=1; charset=utf-8; param1=1
- application/json; param1=1; charset=utf-8;  version=1

Pull request:
Normalize request body ContentTypes
Fixes #862 

First I tackled ensuring that any ContentType type could be normalized so we could perform easy comparisons between what an HTTP request may send vs what the openapi document is expecting.  By normalizing as well, I was able to easily ensure that cache keys for validators don't have any conflicts.

Tests have been added to showcase the working solution.

Patch:
diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts
index d570ab0..47748cb 100644
--- a/src/middlewares/openapi.request.validator.ts
+++ b/src/middlewares/openapi.request.validator.ts
@@ -67,7 +67,7 @@ export class RequestValidator {
     const reqSchema = openapi.schema;
     // cache middleware by combining method, path, and contentType
     const contentType = ContentType.from(req);
-    const contentTypeKey = contentType.equivalents()[0] ?? 'not_provided';
+    const contentTypeKey = contentType.normalize() ?? 'not_provided';
     // use openapi.expressRoute as path portion of key
     const key = `${req.method}-${path}-${contentTypeKey}`;
 
diff --git a/src/middlewares/openapi.response.validator.ts b/src/middlewares/openapi.response.validator.ts
index 715a1df..6166ca5 100644
--- a/src/middlewares/openapi.response.validator.ts
+++ b/src/middlewares/openapi.response.validator.ts
@@ -99,10 +99,7 @@ export class ResponseValidator {
   ): { [key: string]: ValidateFunction } {
     // get the request content type - used only to build the cache key
     const contentTypeMeta = ContentType.from(req);
-    const contentType =
-      (contentTypeMeta.contentType?.indexOf('multipart') > -1
-        ? contentTypeMeta.equivalents()[0]
-        : contentTypeMeta.contentType) ?? 'not_provided';
+    const contentType = contentTypeMeta.normalize() ?? 'not_provided';
 
     const openapi = <OpenApiRequestMetadata>req.openapi;
     const key = `${req.method}-${openapi.expressRoute}-${contentType}`;
diff --git a/src/middlewares/parsers/body.parse.ts b/src/middlewares/parsers/body.parse.ts
index e0aa76b..324aa31 100644
--- a/src/middlewares/parsers/body.parse.ts
+++ b/src/middlewares/parsers/body.parse.ts
@@ -31,9 +31,24 @@ export class BodySchemaParser {
     if (!requestBody?.content) return {};
 
     let content = null;
-    for (const type of contentType.equivalents()) {
-      content = requestBody.content[type];
-      if (content) break;
+    let requestBodyTypes = Object.keys(requestBody.content);
+    for (const type of requestBodyTypes) {
+      let openApiContentType = ContentType.fromString(type);
+      if (contentType.normalize() == openApiContentType.normalize()) {
+        content = requestBody.content[type];
+        break;
+      }
+    }
+
+    if (!content) {
+      const equivalentContentTypes = contentType.equivalents();
+      for (const type of requestBodyTypes) {
+        let openApiContentType = ContentType.fromString(type);
+        if (equivalentContentTypes.find((type2) => openApiContentType.normalize() === type2.normalize())) {
+          content = requestBody.content[type];
+          break;
+        }
+      }
     }
 
     if (!content) {
@@ -49,7 +64,7 @@ export class BodySchemaParser {
 
         const [type] = requestContentType.split('/', 1);
 
-        if (new RegExp(`^${type}\/.+$`).test(contentType.contentType)) {
+        if (new RegExp(`^${type}\/.+$`).test(contentType.normalize())) {
           content = requestBody.content[requestContentType];
           break;
         }
@@ -58,14 +73,14 @@ export class BodySchemaParser {
 
     if (!content) {
       // check if required is false, if so allow request when no content type is supplied
-      const contentNotProvided = contentType.contentType === 'not_provided';
-      if ((contentType.contentType === undefined || contentNotProvided) && requestBody.required === false) {
+      const contentNotProvided = contentType.normalize() === 'not_provided';
+      if ((contentType.normalize() === undefined || contentNotProvided) && requestBody.required === false) {
         return {};
       }
       const msg =
         contentNotProvided
           ? 'media type not specified'
-          : `unsupported media type ${contentType.contentType}`;
+          : `unsupported media type ${contentType.normalize()}`;
       throw new UnsupportedMediaType({ path: path, message: msg });
     }
     return content.schema ?? {};
diff --git a/src/middlewares/util.ts b/src/middlewares/util.ts
index 7ca7aa2..0fde570 100644
--- a/src/middlewares/util.ts
+++ b/src/middlewares/util.ts
@@ -3,23 +3,29 @@ import { Request } from 'express';
 import { ValidationError } from '../framework/types';
 
 export class ContentType {
-  public readonly contentType: string = null;
   public readonly mediaType: string = null;
-  public readonly charSet: string = null;
-  public readonly withoutBoundary: string = null;
   public readonly isWildCard: boolean;
+  public readonly parameters: { charset?: string, boundary?: string } & Record<string, string> = {};
   private constructor(contentType: string | null) {
-    this.contentType = contentType;
     if (contentType) {
-      this.withoutBoundary = contentType
-        .replace(/;\s{0,}boundary.*/, '')
-        .toLowerCase();
-      this.mediaType = this.withoutBoundary.split(';')[0].toLowerCase().trim();
-      this.charSet = this.withoutBoundary.split(';')[1]?.toLowerCase();
-      this.isWildCard = RegExp(/^[a-z]+\/\*$/).test(this.contentType);
-      if (this.charSet) {
-        this.charSet = this.charSet.toLowerCase().trim();
+      const parameterRegExp = /;\s*([^=]+)=([^;]+)/g;
+      const paramMatches = contentType.matchAll(parameterRegExp)
+      if (paramMatches) {
+        this.parameters = {};
+        for (let match of paramMatches) {
+          const key = match[1].toLowerCase();
+          let value = match[2];
+
+          if (key === 'charset') {
+            // charset parameter is case insensitive
+            // @see [rfc2046, Section 4.1.2](https://www.rfc-editor.org/rfc/rfc2046#section-4.1.2)
+            value = value.toLowerCase();
+          }
+          this.parameters[key] = value;
+        };
       }
+      this.mediaType = contentType.split(';')[0].toLowerCase().trim();
+      this.isWildCard = RegExp(/^[a-z]+\/\*$/).test(contentType);
     }
   }
   public static from(req: Request): ContentType {
@@ -30,12 +36,30 @@ export class ContentType {
     return new ContentType(type);
   }
 
-  public equivalents(): string[] {
-    if (!this.withoutBoundary) return [];
-    if (this.charSet) {
-      return [this.mediaType, `${this.mediaType}; ${this.charSet}`];
+  public equivalents(): ContentType[] {
+    const types: ContentType[] = [];
+    if (!this.mediaType) {
+      return types;
+    }
+    types.push(new ContentType(this.mediaType));
+
+    if (!this.parameters['charset']) {
+      types.push(new ContentType(`${this.normalize(['charset'])}; charset=utf-8`));
     }
-    return [this.withoutBoundary, `${this.mediaType}; charset=utf-8`];
+    return types;
+  }
+
+  public normalize(excludeParams: string[] = ['boundary']): string {
+    let parameters = '';
+    Object.keys(this.parameters)
+      .sort()
+      .forEach((key) => {
+        if (!excludeParams.includes(key)) {
+          parameters += `; ${key}=${this.parameters[key]}`                  
+        }
+      });
+    if (this.mediaType)
+      return this.mediaType + parameters;
   }
 }
 
@@ -105,23 +129,28 @@ export const findResponseContent = function (
   accepts: string[],
   expectedTypes: string[],
 ): string {
-  const expectedTypesSet = new Set(expectedTypes);
+  const expectedTypesMap = new Map();
+  for(let type of expectedTypes) {
+    expectedTypesMap.set(ContentType.fromString(type).normalize(), type);
+  }
+  
   // if accepts are supplied, try to find a match, and use its validator
   for (const accept of accepts) {
     const act = ContentType.fromString(accept);
-    if (act.contentType === '*/*') {
+    const normalizedCT = act.normalize();
+    if (normalizedCT === '*/*') {
       return expectedTypes[0];
-    } else if (expectedTypesSet.has(act.contentType)) {
-      return act.contentType;
-    } else if (expectedTypesSet.has(act.mediaType)) {
+    } else if (expectedTypesMap.has(normalizedCT)) {
+      return normalizedCT;
+    } else if (expectedTypesMap.has(act.mediaType)) {
       return act.mediaType;
     } else if (act.isWildCard) {
       // wildcard of type application/*
-      const [type] = act.contentType.split('/', 1);
+      const [type] = normalizedCT.split('/', 1);
 
-      for (const expectedType of expectedTypesSet) {
-        if (new RegExp(`^${type}\/.+$`).test(expectedType)) {
-          return expectedType;
+      for (const expectedType of expectedTypesMap) {
+        if (new RegExp(`^${type}\/.+$`).test(expectedType[0])) {
+          return expectedType[1];
         }
       }
     } else {


Test patch:
diff --git a/test/common/app.common.ts b/test/common/app.common.ts
index ba84185..523948e 100644
--- a/test/common/app.common.ts
+++ b/test/common/app.common.ts
@@ -116,4 +116,14 @@ export function routes(app) {
       id: 'new-id',
     });
   });
+
+  app.post('/v1/pets_content_types', function(req: Request, res: Response): void {
+    // req.file is the `avatar` file
+    // req.body will hold the text fields, if there were any
+    res.json({
+      ...req.body,
+      contentType: req.headers['content-type'],
+      id: 'new-id',
+    });
+  });
 }
diff --git a/test/headers.spec.ts b/test/headers.spec.ts
index dae8ff8..7c3ec86 100644
--- a/test/headers.spec.ts
+++ b/test/headers.spec.ts
@@ -70,5 +70,69 @@ describe(packageJson.name, () => {
           tag: 'cat',
         })
         .expect(200));
+
+    it('should succeed in sending a content-type: "application/json; version=1" in multiple ways', async () =>{
+      await request(app)
+        .post(`${app.basePath}/pets_content_types`)
+        .set('Content-Type', 'application/json; version=1')
+        .set('Accept', 'application/json')
+        .send({
+          name: 'myPet',
+          tag: 'cat',
+        })
+        .expect(200)
+        .expect((res) => {
+          expect(res.body.contentType).to.equal('application/json; version=1')
+        });
+
+      await request(app)
+        .post(`${app.basePath}/pets_content_types`)
+        .set('Content-Type', 'application/json;version=1')
+        .set('Accept', 'application/json')
+        .send({
+          name: 'myPet',
+          tag: 'cat',
+        })
+        .expect(200)
+        .expect((res) => {
+          expect(res.body.contentType).to.equal('application/json;version=1')
+        });
+    });
+        
+    it('should throw a 415 error for unsupported "application/json; version=2" content type', async () =>
+      request(app)
+        .post(`${app.basePath}/pets_content_types`)
+        .set('Content-Type', 'application/json; version=2')
+        .set('Accept', 'application/json')
+        .send({
+          name: 'myPet',
+          tag: 'cat',
+        })
+        .expect(415));
+
+    it('should throw a 415 error for a path/method which has previously succeeded using different request body content types', async () => {
+      await request(app)
+        .post(`${app.basePath}/pets_content_types`)
+        .set('Content-Type', 'application/json;version=1')
+        .set('Accept', 'application/json')
+        .send({
+          name: 'myPet',
+          tag: 'cat',
+        })
+        .expect(200)
+        .expect((res) => {
+          expect(res.body.contentType).to.equal('application/json;version=1')
+        });
+      
+      await request(app)
+        .post(`${app.basePath}/pets_content_types`)
+        .set('Content-Type', 'application/json; version=3')
+        .set('Accept', 'application/json')
+        .send({
+          name: 'myPet',
+          tag: 'cat',
+        })
+        .expect(415);
+    });
   });
 });
diff --git a/test/resources/openapi.yaml b/test/resources/openapi.yaml
index 43e0c25..f73748c 100644
--- a/test/resources/openapi.yaml
+++ b/test/resources/openapi.yaml
@@ -362,6 +362,30 @@ paths:
             application/json; charset=utf-8:
               schema:
                 $ref: '#/components/schemas/Error'
+  /pets_content_types:
+    post:
+      description: Creates a new pet in the store.  Duplicates are allowed
+      operationId: addPet
+      requestBody:
+        description: Pet to add to the store
+        required: true
+        content:
+          application/json; version=1:
+            schema:
+              $ref: '#/components/schemas/NewPet'
+      responses:
+        '200':
+          description: pet response
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Pet'
+        default:
+          description: unexpected error
+          content:
+            application/json; charset=utf-8:
+              schema:
+                $ref: '#/components/schemas/Error'
 
 components:
   parameters: