fix(drift): match path segments that mix literal text with parameters - #2994
fix(drift): match path segments that mix literal text with parameters#2994ariesclark wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 78d0b0b The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -0,0 +1,58 @@ | |||
| import { compileOpenApiPath } from '../utils/http.js'; | |||
|
|
|||
| describe('compileOpenApiPath', () => { | |||
There was a problem hiding this comment.
Many thanks for the fix — the matching logic is correct, but the generated regexes need a performance improvement before this can merge.
Each parameter compiles to a lazy ([^/]+?) group, so when a segment holds several parameters, the regex engine tries every possible way to split a non-matching value between them — super-linear backtracking. Measured on this branch, a single exec call:
- {a}:{b}:{c}.json against a 5 KB non-matching path: ~2 s
- {a}:{b}:{c}:{d}:{e}.json against the same input: >120 s (had to kill the process)
Since exec is synchronous, this blocks the event loop — drift just hangs, no error, no timeout. Captures routinely contain long junk paths (scanner traffic, malformed URLs), so this is reachable in normal use, and main isn't affected today because it never emits more than one group per segment.
Could you rework the compilation so matching stays linear no matter how many parameters a segment holds? Here's a test that pins the requirement — it currently fails at ~2,000 ms:
it('rejects a long non-matching value without backtracking blow-up', () => {
const { regex } = compileOpenApiPath('/v1/{a}:{b}:{c}.json');
const longPath = '/v1/' + 'a:'.repeat(2500) + 'a';
const startedAt = performance.now();
expect(regex.exec(longPath)).toBeNull();
expect(performance.now() - startedAt).toBeLessThan(100);
});
Please also add this test which ensures that we don't introduce solution that may break trailing literal handling:
it('lets a parameter before a trailing literal contain the literal separator char', () => {
const { regex } = compileOpenApiPath('/files/{name}.json');
expect(regex.exec('/files/report.v1.json')?.[1]).toBe('report.v1');
});
Matching behavior shouldn't change — e.g. /files/{name}.json must still match /files/report.v1.json with name = report.v1, so simply excluding the separator characters from the parameter's character class isn't enough.
What/Why/How?
Note
This was heavily assisted by Anthropic's Opus 5. I've reviewed the code to the best of my ability, but if there's any obvious issues I didn't catch, let me know.
A path template whose segment mixes literal text with parameters never matched a request.
compileOpenApiPathtested each segment with/^\{([^}]+)\}$/, which recognizes only a segment that is entirely one parameter. Anything else, such as two parameters or a parameter beside literal text, fell through toescapeRegexand became a literal. So/instances/{worldId}:{instanceId}matched only a URL containing that raw text, which no request carries.driftreported those requests as undocumented andcoverageleft them out of its figures.The compiler now scans each segment for
\{([^}]+)\}, escapes the literal runs between matches, and emits([^/]+?)per parameter. Non-greedy is deliberate: with two parameters around a separator, the first must stop at the first occurrence rather than swallow to the last.Specificity scoring gains a middle tier. A pure literal still scores
+2and a bare parameter0, with+1for a segment holding both. The counter reads a per-segmentparamsBeforesnapshot, becauseparamsaccumulates across the whole template.Reference
None.
Testing
compile-openapi-path.test.tscovers single-parameter segments, the/boundary, two parameters in one segment, a multi-parameter segment with a following suffix, the first-separator split when the trailing value holds another separator, score ordering, and pure literals.Against a 140-exchange capture,
drift's false "undocumented endpoint" reports went from 3 to 0 andcoveragerose from 87 to 90 operations.driftnow validates the three newly matched responses.Check yourself
Security
The change only widens which documented paths a recorded request can match, and adds no new input handling.
Note
Low Risk
Localized regex compilation fix with broad test coverage; it only expands which documented paths match traffic and does not add new input surfaces.
Overview
compileOpenApiPathno longer treats only whole-segment{param}templates as parameters. Segments like/instances/{worldId}:{instanceId}are compiled by scanning each{name}, escaping literal runs, and emitting non-greedy([^/]+?)captures so the first parameter stops at the first separator.driftandcoveragecan therefore match recorded traffic to those documented routes instead of flagging them undocumented or omitting them from coverage. Specificity scoring adds +1 for mixed literal/parameter segments (between pure literals +2 and bare parameters 0).New unit tests in
compile-openapi-path.test.tscover multi-parameter segments, separator boundaries, suffix paths, and score ordering. Changeset documents the patch for@redocly/cli.Reviewed by Cursor Bugbot for commit f7237de. Bugbot is set up for automated code reviews on this repo. Configure here.