From 828cd1ef52c07fe495d82c9e225d277dbdabccf3 Mon Sep 17 00:00:00 2001 From: Ray Vincent Date: Wed, 27 Sep 2023 03:29:41 -0700 Subject: [PATCH] Normalize request body ContentTypes Fixes #862 --- src/middlewares/openapi.request.validator.ts | 2 +- src/middlewares/openapi.response.validator.ts | 5 +- src/middlewares/parsers/body.parse.ts | 29 +++++-- src/middlewares/util.ts | 81 +++++++++++++------ test/common/app.common.ts | 11 +++ test/headers.spec.ts | 53 ++++++++++++ test/resources/openapi.yaml | 24 ++++++ 7 files changed, 167 insertions(+), 38 deletions(-) diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts index a2d1dc38..4a9179c6 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 715a1dfa..6166ca5a 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 = 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 e0aa76b3..324aa313 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 7ca7aa2d..6f794b05 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 = {}; 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']) { + 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 { diff --git a/test/common/app.common.ts b/test/common/app.common.ts index ba841856..9503237e 100644 --- a/test/common/app.common.ts +++ b/test/common/app.common.ts @@ -116,4 +116,15 @@ 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'], + accept: req.headers['accept'], + id: 'new-id', + }); + }); } diff --git a/test/headers.spec.ts b/test/headers.spec.ts index dae8ff82..151258da 100644 --- a/test/headers.spec.ts +++ b/test/headers.spec.ts @@ -70,5 +70,58 @@ describe(packageJson.name, () => { tag: 'cat', }) .expect(200)); + + it('should succeed in sending a content-type: "application/json; version=1"', async () => + 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 succeed in sending a content-type: "application/json;version=1', async () => + request(app) + .post(`${app.basePath}/pets_content_types`) + .set('Content-Type', 'application/json;version=1') + .set('Accept', 'application/json;param1=1') + .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 a path/method that\'s already been cached', async () => + 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 43e0c256..f73748c4 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: