fix(swift): remove catastrophic backtracking in Vapor route regex (#1547)
The arg-list group `(?:[^,()]+,\s*)*` was ambiguous: the trailing `\s*` and the next iteration's `[^,()]+` could both claim the same run of spaces, so a `.METHOD(...)` call with many comma-separated args that never reaches `use:` forced an exponential search. Measured on `app.get(arg0: value0, ...)`: 40ms at 20 args, 647ms at 24, 41.7s at 30, and no result after 120s at 60. Anchoring each repetition at a comma (`(?:[^,()]+,)*\s*`) makes the split unique — `,` is outside the char class, so there is nothing to re-partition. Same input is now 0.09ms at 1000 args. Match behaviour is unchanged: all four capture groups are identical on 18 hand-written Vapor route shapes (no args, single/multi path segments, `X.parameter`, multi-line calls, Environment.get non-matches) and on 200k fuzzed inputs. Fixes #1544
This commit is contained in:
@@ -1472,6 +1472,47 @@ func boot(routes: RoutesBuilder) throws {
|
|||||||
const { nodes } = vaporResolver.extract!('configure.swift', src);
|
const { nodes } = vaporResolver.extract!('configure.swift', src);
|
||||||
expect(nodes).toHaveLength(0);
|
expect(nodes).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// A `.METHOD(...)` call with many comma-separated args and no `use:` used to
|
||||||
|
// make the route regex backtrack exponentially (60 args hung for minutes).
|
||||||
|
it('does not backtrack exponentially on a long arg list without use:', () => {
|
||||||
|
const args = Array.from({ length: 60 }, (_, i) => `arg${i}: value${i}`).join(', ');
|
||||||
|
const src = `app.get(${args})\n`;
|
||||||
|
const start = performance.now();
|
||||||
|
const { nodes } = vaporResolver.extract!('routes.swift', src);
|
||||||
|
const elapsed = performance.now() - start;
|
||||||
|
expect(nodes).toHaveLength(0);
|
||||||
|
expect(elapsed).toBeLessThan(250);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still parses every Vapor route shape after the arg-list rewrite', () => {
|
||||||
|
const src = `
|
||||||
|
admin.get(use: self.list)
|
||||||
|
app.get("users", use: listUsers)
|
||||||
|
router.post("users", User.parameter, "edit", use: UserController.edit)
|
||||||
|
app.patch(":id" , "meta" , use: update)
|
||||||
|
app.get(
|
||||||
|
"multi",
|
||||||
|
"line",
|
||||||
|
use: multiLine
|
||||||
|
)
|
||||||
|
`;
|
||||||
|
const { nodes, references } = vaporResolver.extract!('routes.swift', src);
|
||||||
|
expect(nodes.map((n) => n.name)).toEqual([
|
||||||
|
'GET /',
|
||||||
|
'GET /users',
|
||||||
|
'POST /users/edit',
|
||||||
|
'PATCH /:id/meta',
|
||||||
|
'GET /multi/line',
|
||||||
|
]);
|
||||||
|
expect(references.map((r) => r.referenceName)).toEqual([
|
||||||
|
'list',
|
||||||
|
'listUsers',
|
||||||
|
'edit',
|
||||||
|
'update',
|
||||||
|
'multiLine',
|
||||||
|
]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
import { reactResolver } from '../src/resolution/frameworks/react';
|
import { reactResolver } from '../src/resolution/frameworks/react';
|
||||||
|
|||||||
@@ -367,7 +367,17 @@ export const vaporResolver: FrameworkResolver = {
|
|||||||
// (`BlogUser.parameter`, `:id`, a path constant) so accept any comma-separated
|
// (`BlogUser.parameter`, `:id`, a path constant) so accept any comma-separated
|
||||||
// args before `use:` — the label keeps only the string parts. `use:`
|
// args before `use:` — the label keeps only the string parts. `use:`
|
||||||
// discriminates a real route from Environment.get("X")/req.parameters.get("X").
|
// discriminates a real route from Environment.get("X")/req.parameters.get("X").
|
||||||
const routeRegex = /\b(\w+)\.(get|post|put|patch|delete|head|options)\s*\(\s*((?:[^,()]+,\s*)*)use:\s*([A-Za-z_][\w.]*)/g;
|
// Each arg repetition must end at a comma, and `,` is outside the char class,
|
||||||
|
// so the split is unique and matching stays linear. The earlier
|
||||||
|
// `(?:[^,()]+,\s*)*` was ambiguous — the trailing `\s*` and the next
|
||||||
|
// iteration's `[^,()]+` could both claim the same spaces — which backtracked
|
||||||
|
// exponentially on a long arg list that never reaches `use:`.
|
||||||
|
// The tail is `\s*` rather than a lazy `[^,()]*?` on purpose: both are
|
||||||
|
// linear, but the lazy form drops the "`use:` is preceded by a comma"
|
||||||
|
// requirement and widens the match set — `req.get(foo.use: bar)` would then
|
||||||
|
// be indexed as a route (groups `["req","get","foo.","bar"]`) where both
|
||||||
|
// this pattern and the original match nothing.
|
||||||
|
const routeRegex = /\b(\w+)\.(get|post|put|patch|delete|head|options)\s*\(\s*((?:[^,()]+,)*\s*)use:\s*([A-Za-z_][\w.]*)/g;
|
||||||
let match: RegExpExecArray | null;
|
let match: RegExpExecArray | null;
|
||||||
while ((match = routeRegex.exec(safe)) !== null) {
|
while ((match = routeRegex.exec(safe)) !== null) {
|
||||||
const [, receiver, method, segsStr, handlerExpr] = match;
|
const [, receiver, method, segsStr, handlerExpr] = match;
|
||||||
|
|||||||
Reference in New Issue
Block a user