Skip to content

Commit 828cd1e

Browse files
rayvincent2rayvincent2
committed
Normalize request body ContentTypes
Fixes #862
1 parent bb8d6b8 commit 828cd1e

File tree

7 files changed

+167
-38
lines changed

7 files changed

+167
-38
lines changed

‎src/middlewares/openapi.request.validator.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export class RequestValidator {
6767
const reqSchema = openapi.schema;
6868
// cache middleware by combining method, path, and contentType
6969
const contentType = ContentType.from(req);
70-
const contentTypeKey = contentType.equivalents()[0] ?? 'not_provided';
70+
const contentTypeKey = contentType.normalize() ?? 'not_provided';
7171
// use openapi.expressRoute as path portion of key
7272
const key = `${req.method}-${path}-${contentTypeKey}`;
7373

‎src/middlewares/openapi.response.validator.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,7 @@ export class ResponseValidator {
9999
): { [key: string]: ValidateFunction } {
100100
// get the request content type - used only to build the cache key
101101
const contentTypeMeta = ContentType.from(req);
102-
const contentType =
103-
(contentTypeMeta.contentType?.indexOf('multipart') > -1
104-
? contentTypeMeta.equivalents()[0]
105-
: contentTypeMeta.contentType) ?? 'not_provided';
102+
const contentType = contentTypeMeta.normalize() ?? 'not_provided';
106103

107104
const openapi = <OpenApiRequestMetadata>req.openapi;
108105
const key = `${req.method}-${openapi.expressRoute}-${contentType}`;

‎src/middlewares/parsers/body.parse.ts

+22-7
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,24 @@ export class BodySchemaParser {
3131
if (!requestBody?.content) return {};
3232

3333
let content = null;
34-
for (const type of contentType.equivalents()) {
35-
content = requestBody.content[type];
36-
if (content) break;
34+
let requestBodyTypes = Object.keys(requestBody.content);
35+
for (const type of requestBodyTypes) {
36+
let openApiContentType = ContentType.fromString(type);
37+
if (contentType.normalize() == openApiContentType.normalize()) {
38+
content = requestBody.content[type];
39+
break;
40+
}
41+
}
42+
43+
if (!content) {
44+
const equivalentContentTypes = contentType.equivalents();
45+
for (const type of requestBodyTypes) {
46+
let openApiContentType = ContentType.fromString(type);
47+
if (equivalentContentTypes.find((type2) => openApiContentType.normalize() === type2.normalize())) {
48+
content = requestBody.content[type];
49+
break;
50+
}
51+
}
3752
}
3853

3954
if (!content) {
@@ -49,7 +64,7 @@ export class BodySchemaParser {
4964

5065
const [type] = requestContentType.split('/', 1);
5166

52-
if (new RegExp(`^${type}\/.+$`).test(contentType.contentType)) {
67+
if (new RegExp(`^${type}\/.+$`).test(contentType.normalize())) {
5368
content = requestBody.content[requestContentType];
5469
break;
5570
}
@@ -58,14 +73,14 @@ export class BodySchemaParser {
5873

5974
if (!content) {
6075
// check if required is false, if so allow request when no content type is supplied
61-
const contentNotProvided = contentType.contentType === 'not_provided';
62-
if ((contentType.contentType === undefined || contentNotProvided) && requestBody.required === false) {
76+
const contentNotProvided = contentType.normalize() === 'not_provided';
77+
if ((contentType.normalize() === undefined || contentNotProvided) && requestBody.required === false) {
6378
return {};
6479
}
6580
const msg =
6681
contentNotProvided
6782
? 'media type not specified'
68-
: `unsupported media type ${contentType.contentType}`;
83+
: `unsupported media type ${contentType.normalize()}`;
6984
throw new UnsupportedMediaType({ path: path, message: msg });
7085
}
7186
return content.schema ?? {};

‎src/middlewares/util.ts

+55-26
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,29 @@ import { Request } from 'express';
33
import { ValidationError } from '../framework/types';
44

55
export class ContentType {
6-
public readonly contentType: string = null;
76
public readonly mediaType: string = null;
8-
public readonly charSet: string = null;
9-
public readonly withoutBoundary: string = null;
107
public readonly isWildCard: boolean;
8+
public readonly parameters: { charset?: string, boundary?: string } & Record<string, string> = {};
119
private constructor(contentType: string | null) {
12-
this.contentType = contentType;
1310
if (contentType) {
14-
this.withoutBoundary = contentType
15-
.replace(/;\s{0,}boundary.*/, '')
16-
.toLowerCase();
17-
this.mediaType = this.withoutBoundary.split(';')[0].toLowerCase().trim();
18-
this.charSet = this.withoutBoundary.split(';')[1]?.toLowerCase();
19-
this.isWildCard = RegExp(/^[a-z]+\/\*$/).test(this.contentType);
20-
if (this.charSet) {
21-
this.charSet = this.charSet.toLowerCase().trim();
11+
const parameterRegExp = /;\s*([^=]+)=([^;]+)/g;
12+
const paramMatches = contentType.matchAll(parameterRegExp)
13+
if (paramMatches) {
14+
this.parameters = {};
15+
for (let match of paramMatches) {
16+
const key = match[1].toLowerCase();
17+
let value = match[2];
18+
19+
if (key === 'charset') {
20+
// charset parameter is case insensitive
21+
// @see [rfc2046, Section 4.1.2](https://www.rfc-editor.org/rfc/rfc2046#section-4.1.2)
22+
value = value.toLowerCase();
23+
}
24+
this.parameters[key] = value;
25+
};
2226
}
27+
this.mediaType = contentType.split(';')[0].toLowerCase().trim();
28+
this.isWildCard = RegExp(/^[a-z]+\/\*$/).test(contentType);
2329
}
2430
}
2531
public static from(req: Request): ContentType {
@@ -30,12 +36,30 @@ export class ContentType {
3036
return new ContentType(type);
3137
}
3238

33-
public equivalents(): string[] {
34-
if (!this.withoutBoundary) return [];
35-
if (this.charSet) {
36-
return [this.mediaType, `${this.mediaType}; ${this.charSet}`];
39+
public equivalents(): ContentType[] {
40+
const types: ContentType[] = [];
41+
if (!this.mediaType) {
42+
return types;
43+
}
44+
types.push(new ContentType(this.mediaType));
45+
46+
if (!this.parameters['charset']) {
47+
types.push(new ContentType(`${this.normalize(['charset'])}; charset=utf-8`));
3748
}
38-
return [this.withoutBoundary, `${this.mediaType}; charset=utf-8`];
49+
return types;
50+
}
51+
52+
public normalize(excludeParams: string[] = ['boundary']) {
53+
let parameters = '';
54+
Object.keys(this.parameters)
55+
.sort()
56+
.forEach((key) => {
57+
if (!excludeParams.includes(key)) {
58+
parameters += `; ${key}=${this.parameters[key]}`
59+
}
60+
});
61+
if (this.mediaType)
62+
return this.mediaType + parameters;
3963
}
4064
}
4165

@@ -105,23 +129,28 @@ export const findResponseContent = function (
105129
accepts: string[],
106130
expectedTypes: string[],
107131
): string {
108-
const expectedTypesSet = new Set(expectedTypes);
132+
const expectedTypesMap = new Map();
133+
for(let type of expectedTypes) {
134+
expectedTypesMap.set(ContentType.fromString(type).normalize(), type);
135+
}
136+
109137
// if accepts are supplied, try to find a match, and use its validator
110138
for (const accept of accepts) {
111139
const act = ContentType.fromString(accept);
112-
if (act.contentType === '*/*') {
140+
const normalizedCT = act.normalize();
141+
if (normalizedCT === '*/*') {
113142
return expectedTypes[0];
114-
} else if (expectedTypesSet.has(act.contentType)) {
115-
return act.contentType;
116-
} else if (expectedTypesSet.has(act.mediaType)) {
143+
} else if (expectedTypesMap.has(normalizedCT)) {
144+
return normalizedCT;
145+
} else if (expectedTypesMap.has(act.mediaType)) {
117146
return act.mediaType;
118147
} else if (act.isWildCard) {
119148
// wildcard of type application/*
120-
const [type] = act.contentType.split('/', 1);
149+
const [type] = normalizedCT.split('/', 1);
121150

122-
for (const expectedType of expectedTypesSet) {
123-
if (new RegExp(`^${type}\/.+$`).test(expectedType)) {
124-
return expectedType;
151+
for (const expectedType of expectedTypesMap) {
152+
if (new RegExp(`^${type}\/.+$`).test(expectedType[0])) {
153+
return expectedType[1];
125154
}
126155
}
127156
} else {

‎test/common/app.common.ts

+11
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,15 @@ export function routes(app) {
116116
id: 'new-id',
117117
});
118118
});
119+
120+
app.post('/v1/pets_content_types', function(req: Request, res: Response): void {
121+
// req.file is the `avatar` file
122+
// req.body will hold the text fields, if there were any
123+
res.json({
124+
...req.body,
125+
contentType: req.headers['content-type'],
126+
accept: req.headers['accept'],
127+
id: 'new-id',
128+
});
129+
});
119130
}

‎test/headers.spec.ts

+53
Original file line numberDiff line numberDiff line change
@@ -70,5 +70,58 @@ describe(packageJson.name, () => {
7070
tag: 'cat',
7171
})
7272
.expect(200));
73+
74+
it('should succeed in sending a content-type: "application/json; version=1"', async () =>
75+
request(app)
76+
.post(`${app.basePath}/pets_content_types`)
77+
.set('Content-Type', 'application/json; version=1')
78+
.set('Accept', 'application/json')
79+
.send({
80+
name: 'myPet',
81+
tag: 'cat',
82+
})
83+
.expect(200)
84+
.expect((res) => {
85+
expect(res.body.contentType).to.equal('application/json; version=1')
86+
})
87+
);
88+
89+
it('should throw a 415 error for unsupported "application/json; version=2" content type', async () =>
90+
request(app)
91+
.post(`${app.basePath}/pets_content_types`)
92+
.set('Content-Type', 'application/json; version=2')
93+
.set('Accept', 'application/json')
94+
.send({
95+
name: 'myPet',
96+
tag: 'cat',
97+
})
98+
.expect(415));
99+
100+
it('should succeed in sending a content-type: "application/json;version=1', async () =>
101+
request(app)
102+
.post(`${app.basePath}/pets_content_types`)
103+
.set('Content-Type', 'application/json;version=1')
104+
.set('Accept', 'application/json;param1=1')
105+
.send({
106+
name: 'myPet',
107+
tag: 'cat',
108+
})
109+
.expect(200)
110+
.expect((res) => {
111+
expect(res.body.contentType).to.equal('application/json;version=1')
112+
})
113+
);
114+
115+
it('should throw a 415 error for a path/method that\'s already been cached', async () =>
116+
request(app)
117+
.post(`${app.basePath}/pets_content_types`)
118+
.set('Content-Type', 'application/json; version=3')
119+
.set('Accept', 'application/json')
120+
.send({
121+
name: 'myPet',
122+
tag: 'cat',
123+
})
124+
.expect(415));
125+
73126
});
74127
});

‎test/resources/openapi.yaml

+24
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,30 @@ paths:
362362
application/json; charset=utf-8:
363363
schema:
364364
$ref: '#/components/schemas/Error'
365+
/pets_content_types:
366+
post:
367+
description: Creates a new pet in the store. Duplicates are allowed
368+
operationId: addPet
369+
requestBody:
370+
description: Pet to add to the store
371+
required: true
372+
content:
373+
application/json; version=1:
374+
schema:
375+
$ref: '#/components/schemas/NewPet'
376+
responses:
377+
'200':
378+
description: pet response
379+
content:
380+
application/json:
381+
schema:
382+
$ref: '#/components/schemas/Pet'
383+
default:
384+
description: unexpected error
385+
content:
386+
application/json; charset=utf-8:
387+
schema:
388+
$ref: '#/components/schemas/Error'
365389

366390
components:
367391
parameters:

0 commit comments

Comments
 (0)