fix(scripts): stop the frontmatter field regex backtracking quadratically - #1206
Conversation
There was a problem hiding this comment.
The fix holds. Two quantifiers competing for the same whitespace run is exactly the shape that goes quadratic, and moving the trimming into trim() removes the competition rather than just shifting it.
Equivalence checks I confirmed independently:
\wwithoutuoriis exactly[A-Za-z0-9_], so the key group is unchanged.String.prototype.trim()strips WhiteSpace plus LineTerminator, which is the same set regex\smatches, U+00A0 and U+FEFF included.- Trimming still happens before the quote stripping, so
title: "x"still yieldsx. FIELDhas one call site, on lines already split by/\r?\n/, so the single-line assumption the rewrite relies on is actually enforced.
The pre-existing replace(/^['"]|['"]$/g, '') still strips an unpaired quote (title: "x gives x), but that is untouched by this PR and not something to fix here.
One nit inline about a trailing-character case where (.*) is slightly stricter than the old pattern. Not blocking.
|
|
||
| const FRONTMATTER = /^---\r?\n([\s\S]*?)\r?\n---\r?\n/; | ||
| const FIELD = /^([A-Za-z_][A-Za-z0-9_]*):\s*(.*?)\s*$/; | ||
| const FIELD = /^([A-Za-z_]\w*):(.*)$/; |
There was a problem hiding this comment.
Nit: (.*)$ is not quite equivalent to the old \s*(.*?)\s*$, because . excludes \r,
and
while \s includes the first two. On a line ending in one of those (title:a\r), the old pattern matched and stored a; the new one cannot match at all, so the field silently disappears and validation reports a missing title.
Not reachable through the current call path, as far as I can tell: the block is split on /\r?\n/, and a CR-only file never matches FRONTMATTER in the first place, so it takes something like a doubled \r\r\n to get there. Still, [\s\S] costs nothing and is just as linear (greedy, runs straight to the end, $ matches in one step):
| const FIELD = /^([A-Za-z_]\w*):(.*)$/; | |
| const FIELD = /^([A-Za-z_]\w*):([\s\S]*)$/; |
|
Right, and it contradicted my own "no difference" claim: the alphabet I fuzzed had no line terminator in it. Rerun with CR, U+2028 and U+2029 included: |
Closes #1205.
and the value is trimmed with
String.prototype.trim()where it is stored.The old pattern had two ways to consume the same whitespace: the lazy
(.*?)and the trailing\s*$. On a run of spaces followed by anything else, each one-character extension of.*?sent\s*across the rest of the run before$failed, so the work grew with the square of the run. The new pattern has one quantifier after the colon and nothing competing with it, and the trimming moves to code that cannot backtrack.It is only a safe rewrite because of how the function is called: the frontmatter block is split on
\r?\nfirst, soFIELDonly ever sees a single line and.*runs straight to$.\wwithout theuflag is exactly[A-Za-z0-9_], andtrim()strips the same set\smatches, including the no-break space and the BOM.Verified
a:x+ n spaces +y: 11, 48 and 176 ms at n = 5000, 10000 and 20000 before (doubling n quadruples it), 0.0 ms after at every size._,:, spaces, tabs,, quotes and non-ASCII: no difference.docs/site/: 169 files, identical frontmatter.node scripts/validate-site-docs.mjs: 169 pages validated, as before.The same function also exists in a local
FerrFlow-Docscheckout, but that repository no longer exists on GitHub: the docs are back underdocs/site/here and reach the site through@ferrflow/doc. Nothing to fix there.