Skip to content

Commit

Permalink
version 5.2.1
Browse files Browse the repository at this point in the history
  • Loading branch information
carmine committed Aug 5, 2024
1 parent 8e66d3f commit aace73c
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 35 deletions.
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "express-openapi-validator",
"version": "5.2.0",
"version": "5.3.1",
"description": "Automatically validate API requests and responses with OpenAPI 3 and Express.",
"main": "dist/index.js",
"scripts": {
Expand Down
65 changes: 45 additions & 20 deletions src/middlewares/parsers/req.parameter.mutator.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { Request } from 'express';
import Ajv from 'ajv';
import {
OpenAPIV3,
BadRequest,
OpenApiRequest,
OpenApiRequestMetadata,
OpenAPIV3,
ValidationSchema,
BadRequest,
} from '../../framework/types';
import * as url from 'url';
import { dereferenceParameter, normalizeParameter } from './util';
Expand Down Expand Up @@ -57,7 +57,7 @@ export class RequestParameterMutator {
}

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

if (parameter.in === 'query' && !parameter.allowReserved && parameter.explode === true) {
if (parameter.in === 'query' && !parameter.allowReserved && !!parameter.explode) { //} && !!parameter.explode) {
this.validateReservedCharacters(name, rawQuery);
}
if (parameter.in === 'query' && !parameter.allowReserved && !parameter.explode) { //} && !!parameter.explode) {
this.validateReservedCharacters(name, rawQuery, true);
}

if (parameter.content) {
this.handleContent(req, name, parameter);
Expand All @@ -94,15 +97,7 @@ export class RequestParameterMutator {
} else if (type === 'array' && !explode) {
const delimiter = ARRAY_DELIMITER[parameter.style];
this.validateArrayDelimiter(delimiter, parameter);
if (parameter.in === "query") {
const field = REQUEST_FIELDS[parameter.in];
const vs = rawQuery.get(name);
if (vs) {
req[field][name] = vs[0].split(delimiter).map(v => decodeURIComponent(v));
}
} else {
this.parseJsonArrayAndMutateRequest(req, parameter.in, name, delimiter);
}
this.parseJsonArrayAndMutateRequest(req, parameter.in, name, delimiter, rawQuery);
} else if (type === 'array' && explode) {
this.explodeJsonArrayAndMutateRequest(req, parameter.in, name);
} else if (style === 'form' && explode) {
Expand Down Expand Up @@ -195,8 +190,8 @@ export class RequestParameterMutator {
const properties = hasXOf
? xOfProperties(schema)
: type === 'object'
? Object.keys(schema.properties ?? {})
: [];
? Object.keys(schema.properties ?? {})
: [];

this.explodedJsonObjectAndMutateRequest(
req,
Expand Down Expand Up @@ -248,23 +243,49 @@ export class RequestParameterMutator {
}
}

/**
* used for !explode array parameters
* @param req
* @param $in
* @param name
* @param delimiter
* @param rawQuery
* @private
*/
private parseJsonArrayAndMutateRequest(
req: Request,
$in: string,
name: string,
delimiter: string,
rawQuery: Map<string, string[]>,
): void {
/**
* array deserialization
* array deserialization for query and params
* filter=foo,bar,baz
* filter=foo|bar|baz
* filter=foo%20bar%20baz
*/
const field = REQUEST_FIELDS[$in];
const rawValues = []
if (['query'].includes($in)) {
// perhaps split query from params
rawValues.concat(rawQuery.get(name) ?? []);
}

let i = 0;
if (req[field]?.[name]) {
if (Array.isArray(req[field][name])) return;
const value = req[field][name].split(delimiter);
req[field][name] = value;
const rawValue = rawValues[i++];
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
// Currently, rawValue is only populated for query params
// if the raw value contains a delimiter, decode manually
// parse the decode value and update req[field][name]
const manuallyDecodedValues = rawValue.split(delimiter).map(v => decodeURIComponent(v));
req[field][name] = manuallyDecodedValues;
} else {
req[field][name] = value;
}
}
}

Expand Down Expand Up @@ -342,13 +363,17 @@ export class RequestParameterMutator {
private validateReservedCharacters(
name: string,
pairs: Map<string, string[]>,
allowComma: boolean = false,
) {
const vs = pairs.get(name);
if (!vs) return;
for (const v of vs) {
if (v?.match(RESERVED_CHARS)) {
const message = `Parameter '${name}' must be url encoded. Its value may not contain reserved characters.`;
throw new BadRequest({ path: `/query/${name}`, message: message });
const svs = allowComma ? v.split(',') : [v];
for (const sv of svs) {
if (sv?.match(RESERVED_CHARS)) {
const message = `Parameter '${name}' must be url encoded. Its value may not contain reserved characters.`;
throw new BadRequest({ path: `/query/${name}`, message: message });
}
}
}
}
Expand Down
30 changes: 21 additions & 9 deletions test/openapi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,38 +135,50 @@ describe(packageJson.name, () => {
);
}));

it('should return 400 when comma separated array in query param', async () =>
it('should return 200 when comma separated array in query param - no allow reserved (default)', async () =>
// query params default is style: form, explode: true - false, also comma separated list
// testArrayNoExplode is set to form, explode false
request(apps[i])
.get(`${basePath}/pets?limit=10&test=one&testArrayNoExplode2=categoryId%3AIN%3A%5B1%2C2%2C3%2C4%2C5%5D,itemId%3AEQ%3A2`)
.set('Accept', 'application/json')
.expect('Content-Type', /json/)
.expect(200))

it('should return 200 when comma separated array in query param', async () =>
// query params default is style: form, explode: true - false, also comma separated list
// testArrayNoExplode is set to form, explode false
request(apps[i])
.get(`${basePath}/pets`)
.query({
limit: 10,
test: 'one',
testArray: 'foo,bar,baz',
testArrayNoExplode: 'foo,bar,baz',
})
.set('Accept', 'application/json')
.expect('Content-Type', /json/)
.expect(200));

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

it('should return 200 when separated array in query param', async () =>
request(apps[i])
.get(`${basePath}/pets`)
.query(
`limit=10&test=one&testArray=${encodeURIComponent('foo,bar,baz')}`,
`limit=10&test=one&testArrayNoExplode=${encodeURIComponent('foo,bar,baz')}`,
)
.set('Accept', 'application/json')
.expect('Content-Type', /json/)
Expand All @@ -178,15 +190,15 @@ describe(packageJson.name, () => {
.query({
limit: 10,
test: 'one',
testArray: 'foo,bar,test',
testArrayNoExplode: 'foo,bar,test',
})
.set('Accept', 'application/json')
.expect('Content-Type', /json/)
.expect(400)
.then(r => {
const e = r.body.errors;
expect(e).to.have.length(1);
expect(e[0].path).to.contain('testArray');
expect(e[0].path).to.contain('testArrayNoExplode');
expect(e[0].message).to.equal(
'must be equal to one of the allowed values: foo, bar, baz',
);
Expand Down Expand Up @@ -388,7 +400,7 @@ describe(packageJson.name, () => {
.query({
limit: 10,
test: 'one',
testArray: ['unknown_value'],
testArrayNoExplode: ['unknown_value'],
})
.expect(400)
.then(r => {
Expand Down
15 changes: 14 additions & 1 deletion test/resources/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
}
},
{
"name": "testArray",
"name": "testArrayNoExplode",
"in": "query",
"description": "Array in query param",
"schema": {
Expand All @@ -128,6 +128,19 @@
"style": "form",
"explode": false
},
{
"name": "testArrayNoExplode2",
"in": "query",
"description": "Array in query param",
"schema": {
"type": "array",
"items": {
"type": "string"
}
},
"style": "form",
"explode": false
},
{
"name": "testArrayExplode",
"in": "query",
Expand Down
13 changes: 11 additions & 2 deletions test/resources/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ paths:
enum:
- bar
- baz
- name: testArray
- name: testArrayNoExplode
in: query
description: Array in query param
schema:
Expand All @@ -94,6 +94,15 @@ paths:
- baz
style: form
explode: false
- name: testArrayNoExplode2
in: query
description: Array in query param
schema:
type: array
items:
type: string
style: form
explode: false
- name: testArrayExplode
in: query
description: Array explode in query param
Expand All @@ -106,7 +115,7 @@ paths:
- bar
- baz
style: form
explode: true
# explode: true
responses:
'200':
description: pet response
Expand Down

0 comments on commit aace73c

Please sign in to comment.