Skip to content

Commit 749fc4b

Browse files
committed
handle malformed percent-encoding in UriTemplate.match()
1 parent e2a569c commit 749fc4b

File tree

2 files changed

+63
-13
lines changed

2 files changed

+63
-13
lines changed

src/shared/uriTemplate.ts

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export class UriTemplate {
105105
return expr
106106
.slice(operator.length)
107107
.split(',')
108-
.map(name => name.replace('*', '').trim())
108+
.map(name => name.replace(/\*/g, '').trim())
109109
.filter(name => name.length > 0);
110110
}
111111

@@ -256,10 +256,11 @@ export class UriTemplate {
256256
match(uri: string): Variables | null {
257257
UriTemplate.validateLength(uri, MAX_TEMPLATE_LENGTH, 'URI');
258258

259-
// Split URI into path and query parts
260-
const queryIndex = uri.indexOf('?');
261-
const pathPart = queryIndex === -1 ? uri : uri.slice(0, queryIndex);
262-
const queryPart = queryIndex === -1 ? '' : uri.slice(queryIndex + 1);
259+
// Check whether any literal string segment in the template contains a
260+
// '?'. If so, the template author has written a manual query-string
261+
// prefix (e.g. `/path?fixed=1{&var}`) and we cannot simply split the
262+
// URI at its first '?' — the path regex itself needs to consume past it.
263+
const hasLiteralQuery = this.parts.some(part => typeof part === 'string' && part.includes('?'));
263264

264265
// Build regex pattern for path (non-query) parts
265266
let pattern = '^';
@@ -282,18 +283,36 @@ export class UriTemplate {
282283
}
283284
}
284285

285-
pattern += '$';
286-
UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
287-
const regex = new RegExp(pattern);
288-
const match = pathPart.match(regex);
289-
290-
if (!match) return null;
286+
let match: RegExpMatchArray | null;
287+
let queryPart: string;
288+
289+
if (hasLiteralQuery) {
290+
// Match the path regex against the full URI without a trailing
291+
// anchor, then treat everything after the match as the remaining
292+
// query string to parse for {?...}/{&...} expressions.
293+
UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
294+
const regex = new RegExp(pattern);
295+
match = uri.match(regex);
296+
if (!match) return null;
297+
queryPart = uri.slice(match[0].length).replace(/^&/, '');
298+
} else {
299+
// Split URI into path and query parts at the first '?'
300+
const queryIndex = uri.indexOf('?');
301+
const pathPart = queryIndex === -1 ? uri : uri.slice(0, queryIndex);
302+
queryPart = queryIndex === -1 ? '' : uri.slice(queryIndex + 1);
303+
304+
pattern += '$';
305+
UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
306+
const regex = new RegExp(pattern);
307+
match = pathPart.match(regex);
308+
if (!match) return null;
309+
}
291310

292311
const result: Variables = {};
293312

294313
for (const [i, { name, exploded }] of names.entries()) {
295314
const value = match[i + 1]!;
296-
const cleanName = name.replace('*', '');
315+
const cleanName = name.replace(/\*/g, '');
297316
result[cleanName] = exploded && value.includes(',') ? value.split(',') : value;
298317
}
299318

@@ -311,7 +330,7 @@ export class UriTemplate {
311330
}
312331

313332
for (const { name, exploded } of queryParts) {
314-
const cleanName = name.replace('*', '');
333+
const cleanName = name.replace(/\*/g, '');
315334
const value = queryParams.get(cleanName);
316335

317336
if (value === undefined) continue;

test/shared/uriTemplate.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,37 @@ describe('UriTemplate', () => {
249249
expect(match).toEqual({ q: 'hello world' });
250250
expect(template.variableNames).toEqual(['q']);
251251
});
252+
253+
it('should not throw on malformed percent-encoding in query parameters', () => {
254+
const template = new UriTemplate('/search{?q}');
255+
expect(template.match('/search?q=100%')).toEqual({ q: '100%' });
256+
expect(template.match('/search?q=%ZZ')).toEqual({ q: '%ZZ' });
257+
});
258+
259+
it('should match templates with a literal ? followed by {&...} continuation', () => {
260+
const template = new UriTemplate('/path?static=1{&dynamic}');
261+
const match = template.match('/path?static=1&dynamic=hello');
262+
expect(match).toEqual({ dynamic: 'hello' });
263+
expect(template.variableNames).toEqual(['dynamic']);
264+
});
265+
266+
it('should match templates with literal ? when continuation param is absent', () => {
267+
const template = new UriTemplate('/path?static=1{&dynamic}');
268+
const match = template.match('/path?static=1');
269+
expect(match).toEqual({});
270+
});
271+
272+
it('should match templates with literal ? and multiple continuation params', () => {
273+
const template = new UriTemplate('/api?v=2{&key,page}');
274+
const match = template.match('/api?v=2&page=3&key=abc');
275+
expect(match).toEqual({ key: 'abc', page: '3' });
276+
});
277+
278+
it('should match path variables combined with literal ? and {&...}', () => {
279+
const template = new UriTemplate('/api/{version}?format=json{&key}');
280+
const match = template.match('/api/v1?format=json&key=secret');
281+
expect(match).toEqual({ version: 'v1', key: 'secret' });
282+
});
252283
});
253284

254285
describe('security and edge cases', () => {

0 commit comments

Comments
 (0)