Skip to content

Commit aace73c

Browse files
author
carmine
committed
version 5.2.1
1 parent 8e66d3f commit aace73c

File tree

6 files changed

+94
-35
lines changed

6 files changed

+94
-35
lines changed

‎package-lock.json

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "express-openapi-validator",
3-
"version": "5.2.0",
3+
"version": "5.3.1",
44
"description": "Automatically validate API requests and responses with OpenAPI 3 and Express.",
55
"main": "dist/index.js",
66
"scripts": {

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

+45-20
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { Request } from 'express';
22
import Ajv from 'ajv';
33
import {
4-
OpenAPIV3,
4+
BadRequest,
55
OpenApiRequest,
66
OpenApiRequestMetadata,
7+
OpenAPIV3,
78
ValidationSchema,
8-
BadRequest,
99
} from '../../framework/types';
1010
import * as url from 'url';
1111
import { dereferenceParameter, normalizeParameter } from './util';
@@ -57,7 +57,7 @@ export class RequestParameterMutator {
5757
}
5858

5959
/**
60-
* Modifies an incoing request object by applying the openapi schema
60+
* Modifies an incoming request object by applying the openapi schema
6161
* req values may be parsed/mutated as a JSON object, JSON Exploded Object, JSON Array, or JSON Exploded Array
6262
* @param req
6363
*/
@@ -76,9 +76,12 @@ export class RequestParameterMutator {
7676
const i = req.originalUrl.indexOf('?');
7777
const queryString = req.originalUrl.substr(i + 1);
7878

79-
if (parameter.in === 'query' && !parameter.allowReserved && parameter.explode === true) {
79+
if (parameter.in === 'query' && !parameter.allowReserved && !!parameter.explode) { //} && !!parameter.explode) {
8080
this.validateReservedCharacters(name, rawQuery);
8181
}
82+
if (parameter.in === 'query' && !parameter.allowReserved && !parameter.explode) { //} && !!parameter.explode) {
83+
this.validateReservedCharacters(name, rawQuery, true);
84+
}
8285

8386
if (parameter.content) {
8487
this.handleContent(req, name, parameter);
@@ -94,15 +97,7 @@ export class RequestParameterMutator {
9497
} else if (type === 'array' && !explode) {
9598
const delimiter = ARRAY_DELIMITER[parameter.style];
9699
this.validateArrayDelimiter(delimiter, parameter);
97-
if (parameter.in === "query") {
98-
const field = REQUEST_FIELDS[parameter.in];
99-
const vs = rawQuery.get(name);
100-
if (vs) {
101-
req[field][name] = vs[0].split(delimiter).map(v => decodeURIComponent(v));
102-
}
103-
} else {
104-
this.parseJsonArrayAndMutateRequest(req, parameter.in, name, delimiter);
105-
}
100+
this.parseJsonArrayAndMutateRequest(req, parameter.in, name, delimiter, rawQuery);
106101
} else if (type === 'array' && explode) {
107102
this.explodeJsonArrayAndMutateRequest(req, parameter.in, name);
108103
} else if (style === 'form' && explode) {
@@ -195,8 +190,8 @@ export class RequestParameterMutator {
195190
const properties = hasXOf
196191
? xOfProperties(schema)
197192
: type === 'object'
198-
? Object.keys(schema.properties ?? {})
199-
: [];
193+
? Object.keys(schema.properties ?? {})
194+
: [];
200195

201196
this.explodedJsonObjectAndMutateRequest(
202197
req,
@@ -248,23 +243,49 @@ export class RequestParameterMutator {
248243
}
249244
}
250245

246+
/**
247+
* used for !explode array parameters
248+
* @param req
249+
* @param $in
250+
* @param name
251+
* @param delimiter
252+
* @param rawQuery
253+
* @private
254+
*/
251255
private parseJsonArrayAndMutateRequest(
252256
req: Request,
253257
$in: string,
254258
name: string,
255259
delimiter: string,
260+
rawQuery: Map<string, string[]>,
256261
): void {
257262
/**
258-
* array deserialization
263+
* array deserialization for query and params
259264
* filter=foo,bar,baz
260265
* filter=foo|bar|baz
261266
* filter=foo%20bar%20baz
262267
*/
263268
const field = REQUEST_FIELDS[$in];
269+
const rawValues = []
270+
if (['query'].includes($in)) {
271+
// perhaps split query from params
272+
rawValues.concat(rawQuery.get(name) ?? []);
273+
}
274+
275+
let i = 0;
264276
if (req[field]?.[name]) {
265277
if (Array.isArray(req[field][name])) return;
266278
const value = req[field][name].split(delimiter);
267-
req[field][name] = value;
279+
const rawValue = rawValues[i++];
280+
if (rawValue?.includes(delimiter)) { // TODO add && !allowReserved to improve performance. When allowReserved is true, commas are common and we do not need to do this extra work
281+
// Currently, rawValue is only populated for query params
282+
// if the raw value contains a delimiter, decode manually
283+
// parse the decode value and update req[field][name]
284+
const manuallyDecodedValues = rawValue.split(delimiter).map(v => decodeURIComponent(v));
285+
req[field][name] = manuallyDecodedValues;
286+
} else {
287+
req[field][name] = value;
288+
}
268289
}
269290
}
270291

@@ -342,13 +363,17 @@ export class RequestParameterMutator {
342363
private validateReservedCharacters(
343364
name: string,
344365
pairs: Map<string, string[]>,
366+
allowComma: boolean = false,
345367
) {
346368
const vs = pairs.get(name);
347369
if (!vs) return;
348370
for (const v of vs) {
349-
if (v?.match(RESERVED_CHARS)) {
350-
const message = `Parameter '${name}' must be url encoded. Its value may not contain reserved characters.`;
351-
throw new BadRequest({ path: `/query/${name}`, message: message });
371+
const svs = allowComma ? v.split(',') : [v];
372+
for (const sv of svs) {
373+
if (sv?.match(RESERVED_CHARS)) {
374+
const message = `Parameter '${name}' must be url encoded. Its value may not contain reserved characters.`;
375+
throw new BadRequest({ path: `/query/${name}`, message: message });
376+
}
352377
}
353378
}
354379
}

‎test/openapi.spec.ts

+21-9
Original file line numberDiff line numberDiff line change
@@ -135,38 +135,50 @@ describe(packageJson.name, () => {
135135
);
136136
}));
137137

138-
it('should return 400 when comma separated array in query param', async () =>
138+
it('should return 200 when comma separated array in query param - no allow reserved (default)', async () =>
139+
// query params default is style: form, explode: true - false, also comma separated list
140+
// testArrayNoExplode is set to form, explode false
141+
request(apps[i])
142+
.get(`${basePath}/pets?limit=10&test=one&testArrayNoExplode2=categoryId%3AIN%3A%5B1%2C2%2C3%2C4%2C5%5D,itemId%3AEQ%3A2`)
143+
.set('Accept', 'application/json')
144+
.expect('Content-Type', /json/)
145+
.expect(200))
146+
147+
it('should return 200 when comma separated array in query param', async () =>
148+
// query params default is style: form, explode: true - false, also comma separated list
149+
// testArrayNoExplode is set to form, explode false
139150
request(apps[i])
140151
.get(`${basePath}/pets`)
141152
.query({
142153
limit: 10,
143154
test: 'one',
144-
testArray: 'foo,bar,baz',
155+
testArrayNoExplode: 'foo,bar,baz',
145156
})
146157
.set('Accept', 'application/json')
147158
.expect('Content-Type', /json/)
148159
.expect(200));
149160

150-
it('should return 400 when comma separated array in query param is not url encoded', async () =>
161+
// Note with version <=5.2.0, this test was incorrectly returning 400 for testArray (explode true), rather than testArrayNoE
162+
it('should return 400 when comma separated array in query param is not url encoded and explode is set to true', async () =>
151163
request(apps[i])
152164
.get(`${basePath}/pets`)
153-
.query(`limit=10&test=one&testArray=foo,bar,baz`)
165+
.query(`limit=10&test=one&testArrayExplode=foo,bar,baz`)
154166
.set('Accept', 'application/json')
155167
.expect('Content-Type', /json/)
156168
.expect(400)
157169
.then(r => {
158170
expect(r.body)
159171
.to.have.property('message')
160172
.that.equals(
161-
"Parameter 'testArray' must be url encoded. Its value may not contain reserved characters.",
173+
"Parameter 'testArrayExplode' must be url encoded. Its value may not contain reserved characters.",
162174
);
163175
}));
164176

165177
it('should return 200 when separated array in query param', async () =>
166178
request(apps[i])
167179
.get(`${basePath}/pets`)
168180
.query(
169-
`limit=10&test=one&testArray=${encodeURIComponent('foo,bar,baz')}`,
181+
`limit=10&test=one&testArrayNoExplode=${encodeURIComponent('foo,bar,baz')}`,
170182
)
171183
.set('Accept', 'application/json')
172184
.expect('Content-Type', /json/)
@@ -178,15 +190,15 @@ describe(packageJson.name, () => {
178190
.query({
179191
limit: 10,
180192
test: 'one',
181-
testArray: 'foo,bar,test',
193+
testArrayNoExplode: 'foo,bar,test',
182194
})
183195
.set('Accept', 'application/json')
184196
.expect('Content-Type', /json/)
185197
.expect(400)
186198
.then(r => {
187199
const e = r.body.errors;
188200
expect(e).to.have.length(1);
189-
expect(e[0].path).to.contain('testArray');
201+
expect(e[0].path).to.contain('testArrayNoExplode');
190202
expect(e[0].message).to.equal(
191203
'must be equal to one of the allowed values: foo, bar, baz',
192204
);
@@ -388,7 +400,7 @@ describe(packageJson.name, () => {
388400
.query({
389401
limit: 10,
390402
test: 'one',
391-
testArray: ['unknown_value'],
403+
testArrayNoExplode: ['unknown_value'],
392404
})
393405
.expect(400)
394406
.then(r => {

‎test/resources/openapi.json

+14-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@
111111
}
112112
},
113113
{
114-
"name": "testArray",
114+
"name": "testArrayNoExplode",
115115
"in": "query",
116116
"description": "Array in query param",
117117
"schema": {
@@ -128,6 +128,19 @@
128128
"style": "form",
129129
"explode": false
130130
},
131+
{
132+
"name": "testArrayNoExplode2",
133+
"in": "query",
134+
"description": "Array in query param",
135+
"schema": {
136+
"type": "array",
137+
"items": {
138+
"type": "string"
139+
}
140+
},
141+
"style": "form",
142+
"explode": false
143+
},
131144
{
132145
"name": "testArrayExplode",
133146
"in": "query",

‎test/resources/openapi.yaml

+11-2
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ paths:
8181
enum:
8282
- bar
8383
- baz
84-
- name: testArray
84+
- name: testArrayNoExplode
8585
in: query
8686
description: Array in query param
8787
schema:
@@ -94,6 +94,15 @@ paths:
9494
- baz
9595
style: form
9696
explode: false
97+
- name: testArrayNoExplode2
98+
in: query
99+
description: Array in query param
100+
schema:
101+
type: array
102+
items:
103+
type: string
104+
style: form
105+
explode: false
97106
- name: testArrayExplode
98107
in: query
99108
description: Array explode in query param
@@ -106,7 +115,7 @@ paths:
106115
- bar
107116
- baz
108117
style: form
109-
explode: true
118+
# explode: true
110119
responses:
111120
'200':
112121
description: pet response

0 commit comments

Comments
 (0)