Skip to content

Commit 319b303

Browse files
cdimasciocdimascio
authored
fix(#1071): Fix query param validation for literal bracket notation in param names (#1084)
* fix(#1071): Fix query param validation for literal bracket notation in param names * fix(#1071): Fix query param validation for literal bracket notation in param names * fix query mutation for express 4 and 5 * cache literal params * cleanup daad code * cleanup this * lint / format fixes
1 parent 122ac9d commit 319b303

File tree

2 files changed

+241
-37
lines changed

2 files changed

+241
-37
lines changed

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

Lines changed: 111 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ export class RequestParameterMutator {
100100
this.handleContent(req, name, parameter);
101101
} else if (parameter.in === 'query' && this.isObjectOrXOf(schema)) {
102102
// handle bracket notation and mutates query param
103-
104103

105104
if (style === 'form' && explode) {
106105
this.parseJsonAndMutateRequest(req, parameter.in, name);
@@ -119,7 +118,7 @@ export class RequestParameterMutator {
119118
this.parseJsonAndMutateRequest(req, parameter.in, name);
120119
} else {
121120
this.parseJsonAndMutateRequest(req, parameter.in, name);
122-
}
121+
}
123122
} else if (type === 'array' && !explode) {
124123
const delimiter = ARRAY_DELIMITER[parameter.style];
125124
this.validateArrayDelimiter(delimiter, parameter);
@@ -428,78 +427,153 @@ export class RequestParameterMutator {
428427
}, new Map<string, string[]>());
429428
}
430429

431-
private csvToKeyValuePairs(csvString: string): Record<string, string> | undefined {
430+
private csvToKeyValuePairs(
431+
csvString: string,
432+
): Record<string, string> | undefined {
432433
const hasBrace = csvString.split('{').length > 1;
433434
const items = csvString.split(',');
434-
435+
435436
if (hasBrace) {
436437
// if it has a brace, we assume its JSON and skip creating k v pairs
437438
// TODO improve json check, but ensure its cheap
438439
return;
439440
}
440-
441+
441442
if (items.length % 2 !== 0) {
442-
// if the number of elements is not event,
443+
// if the number of elements is not event,
443444
// then we do not have k v pairs, so return undefined
444445
return;
445446
}
446447

447448
const result = {};
448-
449+
449450
for (let i = 0; i < items.length - 1; i += 2) {
450451
result[items[i]] = items[i + 1];
451452
}
452-
453+
453454
return result;
454455
}
455456

456457
/**
457458
* Handles query parameters with bracket notation.
458-
* - If the parameter in the OpenAPI spec has literal brackets in its name (e.g., 'filter[name]'),
459-
* it will be treated as a literal parameter name.
460-
* - Otherwise, it will be parsed as a nested object using qs.
461459
* @param query The query parameters object to process
462460
* @returns The processed query parameters object
463461
*/
464462
private handleBracketNotationQueryFields(query: { [key: string]: any }): {
465463
[key: string]: any;
466464
} {
467-
// Get the OpenAPI parameters for the current request
468-
const openApiParams = (query._openapi?.schema?.parameters || []) as ParameterObject[];
469-
470-
// Create a Set of parameter names that have literal brackets in the spec
471-
const literalBracketParams = new Set<string>(
472-
openApiParams
473-
.filter(p => p.in === 'query' && p.name.includes('[') && p.name.endsWith(']'))
474-
.map(p => p.name)
475-
);
465+
const handler = new BracketNotationHandler(query);
466+
return handler.process();
467+
}
468+
}
469+
470+
/**
471+
* Handles parsing of query parameters with bracket notation.
472+
* - If a parameter in the OpenAPI spec has literal brackets in its name (e.g., 'filter[name]'),
473+
* it will be treated as a literal parameter name.
474+
* - Otherwise, it will be parsed as a nested object using qs.
475+
*/
476+
class BracketNotationHandler {
477+
// Cache for literal bracket parameters per endpoint
478+
private static readonly literalBracketParamsCache = new Map<
479+
string,
480+
Set<string>
481+
>();
482+
483+
constructor(private readonly query: { [key: string]: any }) {}
484+
485+
/**
486+
* Process the query parameters to handle bracket notation
487+
*/
488+
public process(): { [key: string]: any } {
489+
const literalBracketParams = this.getLiteralBracketParams();
476490

477-
// Create a new object to avoid mutating the original during iteration
478-
const result: { [key: string]: any } = { ...query };
479-
491+
const query = this.query;
480492
Object.keys(query).forEach((key) => {
481493
// Only process keys that contain brackets
482-
if (key.includes('[') && key.endsWith(']')) {
483-
if (literalBracketParams.has(key)) {
484-
// If the parameter is defined with literal brackets in the spec, preserve it as-is
485-
result[key] = query[key];
486-
} else {
487-
// Otherwise, use qs.parse to handle it as a nested object
494+
if (key.includes('[')) {
495+
// Only process keys that do not contain literal bracket notation
496+
if (!literalBracketParams.has(key)) {
497+
// Use qs.parse to handle it as a nested object
488498
const normalizedKey = key.split('[')[0];
489499
const parsed = parse(`${key}=${query[key]}`);
490-
500+
491501
// Use the parsed value for the normalized key
492502
if (parsed[normalizedKey] !== undefined) {
493-
result[normalizedKey] = parsed[normalizedKey];
503+
// If we already have a value for this key, merge the objects
504+
if (
505+
query[normalizedKey] &&
506+
typeof query[normalizedKey] === 'object' &&
507+
typeof parsed[normalizedKey] === 'object' &&
508+
!Array.isArray(parsed[normalizedKey])
509+
) {
510+
query[normalizedKey] = {
511+
...query[normalizedKey],
512+
...parsed[normalizedKey],
513+
};
514+
} else {
515+
query[normalizedKey] = parsed[normalizedKey];
516+
}
494517
}
495-
496-
// Remove the original bracketed key
497-
delete result[key];
518+
519+
// Remove the original bracketed key from the query
520+
delete query[key];
498521
}
499522
}
500523
});
501-
502-
return result;
524+
525+
return query;
526+
}
527+
528+
/**
529+
* Gets a cache key for the current request's OpenAPI schema
530+
* Combines path, method, and operation ID to create a key
531+
*/
532+
private getCacheKey(): string | null {
533+
const schema = this.query._openapi?.schema;
534+
if (!schema) return null;
535+
536+
// Use all available identifiers to ensure uniqueness
537+
const path = schema.path ?? '';
538+
const method = schema.method ?? '';
539+
const operationId = schema.operationId ?? '';
540+
541+
// Combine all parts with a consistent separator
542+
return `${path}|${method}|${operationId}`;
543+
}
544+
545+
/**
546+
* Gets the set of parameter names that should be treated as literal bracket notation
547+
*/
548+
private getLiteralBracketParams(): Set<string> {
549+
const cacheKey = this.getCacheKey();
550+
551+
if (
552+
cacheKey &&
553+
BracketNotationHandler.literalBracketParamsCache.has(cacheKey)
554+
) {
555+
return BracketNotationHandler.literalBracketParamsCache.get(cacheKey)!;
556+
}
557+
558+
// Get the OpenAPI parameters for the current request
559+
const openApiParams = (this.query._openapi?.schema?.parameters ||
560+
[]) as ParameterObject[];
561+
562+
// Create a Set of parameter names that have literal brackets in the spec
563+
const literalBracketParams = new Set<string>(
564+
openApiParams
565+
.filter((p) => p.in === 'query' && p.name.includes('['))
566+
.map((p) => p.name),
567+
);
568+
569+
// Cache the result for future requests to this endpoint
570+
if (cacheKey) {
571+
BracketNotationHandler.literalBracketParamsCache.set(
572+
cacheKey,
573+
literalBracketParams,
574+
);
575+
}
576+
577+
return literalBracketParams;
503578
}
504-
505579
}

‎test/query.bracket.notation.spec.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import { expect } from 'chai';
2+
import { describe, it } from 'mocha';
3+
import { RequestParameterMutator } from '../src/middlewares/parsers/req.parameter.mutator';
4+
import Ajv from 'ajv';
5+
import { OpenAPIV3 } from '../src/framework/types';
6+
7+
type DocumentV3 = OpenAPIV3.DocumentV3;
8+
9+
describe('RequestParameterMutator - handleBracketNotationQueryFields', () => {
10+
const ajv = new Ajv();
11+
const mockApiDoc: DocumentV3 = {
12+
openapi: '3.0.0',
13+
info: { title: 'Test', version: '1.0.0' },
14+
paths: {}
15+
};
16+
17+
const createMockRequest = (query: any) => ({
18+
query,
19+
_openapi: {
20+
schema: {
21+
parameters: [
22+
// This simulates the parameters defined in the OpenAPI spec
23+
{ name: 'filter[name]', in: 'query', schema: { type: 'string' } },
24+
{ name: 'user', in: 'query', schema: { type: 'object' } }
25+
]
26+
}
27+
}
28+
});
29+
30+
const testValidationSchema = {
31+
body: {},
32+
query: {
33+
type: 'object',
34+
properties: {
35+
'filter[name]': { type: 'string' },
36+
'user': { type: 'object' },
37+
'user[name]': { type: 'string' },
38+
'user[age]': { type: 'string' },
39+
'simple': { type: 'string' },
40+
'another': { type: 'string' }
41+
},
42+
additionalProperties: true
43+
},
44+
headers: {},
45+
params: {},
46+
cookies: {}
47+
};
48+
49+
it('should preserve literal bracket notation when defined in the spec', () => {
50+
// Arrange
51+
const req = createMockRequest({
52+
'filter[name]': 'test',
53+
_openapi: { schema: { parameters: [{ name: 'filter[name]', in: 'query' }] } }
54+
});
55+
const mutator = new RequestParameterMutator(ajv, mockApiDoc, '/test', testValidationSchema);
56+
57+
// Act
58+
const result = mutator['handleBracketNotationQueryFields'](req.query);
59+
60+
// Assert
61+
expect(result).to.have.property('filter[name]', 'test');
62+
expect(result).to.not.have.property('filter');
63+
});
64+
65+
it('should parse bracket notation as nested object when not defined in spec', () => {
66+
// Arrange
67+
const req = createMockRequest({
68+
'user[name]': 'John',
69+
'user[age]': '30',
70+
_openapi: { schema: { parameters: [{ name: 'user', in: 'query', schema: { type: 'object' } }] } }
71+
});
72+
const mutator = new RequestParameterMutator(ajv, mockApiDoc, '/test', testValidationSchema);
73+
74+
// Act
75+
const result = mutator['handleBracketNotationQueryFields'](req.query);
76+
77+
// Assert
78+
expect(result).to.have.property('user');
79+
expect(result.user).to.deep.equal({ name: 'John', age: '30' });
80+
expect(result).to.not.have.property('user[name]');
81+
expect(result).to.not.have.property('user[age]');
82+
});
83+
84+
it('should handle mixed literal and nested bracket notation', () => {
85+
// Arrange
86+
const req = createMockRequest({
87+
'filter[name]': 'test',
88+
'user[age]': '30',
89+
_openapi: {
90+
schema: {
91+
parameters: [
92+
{ name: 'filter[name]', in: 'query', schema: { type: 'string' } },
93+
{ name: 'user', in: 'query', schema: { type: 'object' } }
94+
]
95+
}
96+
}
97+
});
98+
const mutator = new RequestParameterMutator(ajv, mockApiDoc, '/test', testValidationSchema);
99+
100+
// Act
101+
const result = mutator['handleBracketNotationQueryFields'](req.query);
102+
103+
// Assert
104+
expect(result).to.have.property('filter[name]', 'test');
105+
expect(result).to.have.property('user');
106+
expect(result.user).to.deep.equal({ age: '30' });
107+
expect(result).to.not.have.property('filter');
108+
expect(result).to.not.have.property('user[age]');
109+
});
110+
111+
it('should not modify parameters without brackets', () => {
112+
// Arrange
113+
const req = createMockRequest({
114+
simple: 'value',
115+
another: 'test',
116+
_openapi: { schema: { parameters: [] } }
117+
});
118+
const mutator = new RequestParameterMutator(ajv, mockApiDoc, '/test', testValidationSchema);
119+
testValidationSchema
120+
// Act
121+
const result = mutator['handleBracketNotationQueryFields'](req.query);
122+
123+
// Assert
124+
expect(result).to.deep.equal({
125+
simple: 'value',
126+
another: 'test',
127+
_openapi: { schema: { parameters: [] } }
128+
});
129+
});
130+
});

0 commit comments

Comments
 (0)