From a5afc786380d2456bec88be7726e444c8a8e5fba Mon Sep 17 00:00:00 2001 From: Zoom Date: Wed, 2 Sep 2026 14:28:27 +0400 Subject: [PATCH] stdlib: pegs: fix crashes, raise EInvalidPeg instead (#26149) Fixes for a few crashes, including a runtime underflow, I stumbled upon while trying some more _inventive_ patterns. * `primary()`: unary `*` applied to an operand that can match empty input (!>.*), 'a'?) now pegError()s at pattern-parse time instead of AssertionDefect "unreachable" from `*` * `getCharSet()`: unknown builtin/malformed escapes inside charsets ([^\n], [z-\n]) propagate `tkInvalid` instead of reading `tok.literal[len-1]` of an empty string (IndexDefect) * rawMatch pkCapture: `{}` with no previous capture is a no-op instead of a runtime underflow defect --- changelog.md | 7 +++++++ lib/pure/pegs.nim | 19 ++++++++++++++----- tests/stdlib/tpegs.nim | 42 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/changelog.md b/changelog.md index c38267aa3e..dc1cf67111 100644 --- a/changelog.md +++ b/changelog.md @@ -100,6 +100,13 @@ parameter and result types, not just their source-level shape. Use - `std/uri`: The `?` operator now appends query parameters to an existing query string instead of replacing it. Fixes [#19782](https://github.com/nim-lang/Nim/issues/19782). - `std/jsonutils`: `fromJson` now throws an exception when converting to `array`/`seq` if the JSON isn't an array instead of silently failing +- `std/pegs` no longer crashes on some patterns: repetition of an expression + that can match the empty input (e.g. ``('a'?)*``) is now valid (the matcher + terminates on zero-length matches) instead of aborting with + `AssertionDefect`; unknown builtin escapes inside character classes + (e.g. ``[^\n]``) raise `EInvalidPeg` instead of `IndexDefect`. An empty + capture `{}` with no previous capture is now a no-op instead of + underflowing the matcher's capture array. ## Language changes diff --git a/lib/pure/pegs.nim b/lib/pure/pegs.nim index eaece6fa12..ed2ec47753 100644 --- a/lib/pure/pegs.nim +++ b/lib/pure/pegs.nim @@ -235,8 +235,6 @@ func `?`*(a: Peg): Peg {.rtl, extern: "npegsOptional".} = func `*`*(a: Peg): Peg {.rtl, extern: "npegsGreedyRep".} = ## constructs a "greedy repetition" for the PEG `a` case a.kind - of pkGreedyRep, pkGreedyRepChar, pkGreedyRepSet, pkGreedyAny, pkOption: - raiseAssert "unreachable" # produces endless loop! of pkChar: result = Peg(kind: pkGreedyRepChar, ch: a.ch) of pkCharChoice: @@ -244,6 +242,9 @@ func `*`*(a: Peg): Peg {.rtl, extern: "npegsGreedyRep".} = of pkAny, pkAnyRune: result = Peg(kind: pkGreedyAny) else: + # Note that `a` may match the empty input (e.g. `?a`): the matcher + # breaks out of the repetition loop on a zero-length match, so this + # does not produce an endless loop. result = Peg(kind: pkGreedyRep, sons: @[a]) func `!*`*(a: Peg): Peg {.rtl, extern: "npegsSearch".} = @@ -835,9 +836,11 @@ template matchOrParse(mopProc: untyped) = of pkCapture: enter(pkCapture, s, p, start) if p.sons.len == 0 or p.sons[0].kind == pkEmpty: - # empty capture removes last match - dec(c.ml) - c.matches[c.ml] = (0, 0) + # empty capture removes last match; if there is no previous capture, + # treat it as a no-op instead of underflowing the matches array: + if c.ml > 0: + dec(c.ml) + c.matches[c.ml] = (0, 0) result = 0 # match of length 0 else: var idx = c.ml # reserve a slot for the subpattern @@ -1625,6 +1628,9 @@ func getCharSet(c: var PegLexer, tok: var Token) = c.bufpos = pos getEscapedChar(c, tok) pos = c.bufpos + if tok.kind == tkInvalid: + # unknown builtin or malformed escape: propagate the error + break ch = tok.literal[tok.literal.len-1] of '\C', '\L', '\0': tok.kind = tkInvalid @@ -1648,6 +1654,9 @@ func getCharSet(c: var PegLexer, tok: var Token) = c.bufpos = pos getEscapedChar(c, tok) pos = c.bufpos + if tok.kind == tkInvalid: + # unknown builtin or malformed escape: propagate the error + break ch2 = tok.literal[tok.literal.len-1] of '\C', '\L', '\0': tok.kind = tkInvalid diff --git a/tests/stdlib/tpegs.nim b/tests/stdlib/tpegs.nim index 04774aa35b..e16ad5e7be 100644 --- a/tests/stdlib/tpegs.nim +++ b/tests/stdlib/tpegs.nim @@ -347,3 +347,45 @@ call() pegsTest() static: pegsTest() + +block: # pegs shouldn't crash for invalid inputs but raise EInvalidPeg + var captures: array[20, string] + + # star of an expression that can match the empty input used to abort with + # AssertionDefect("unreachable") from `*`; the matcher breaks out of the + # repetition loop on a zero-length match, so these are valid and terminate: + doAssert "aaa".match(peg"(! '>' .)* * $") + doAssert "aaa".match(peg"'a'? * $") + doAssert "aaa".match(peg".* * $") + doAssert "aaa".match(peg"('a'*)* $") + doAssert "aaa".match(peg"{a*} * $") + doAssert "aaa".match(peg"'a'? + $") + doAssert "aaa".match(peg"'a'* * $") + doAssert "aaa".match(peg"[a-c]* * $") + # the zero-length break in the matcher's repetition loop is load-bearing + # here; pin the empty-input behavior and the not-matching path that + # forces the break (a regression to zero-width looping would hang these): + doAssert "".match(peg"'a'? * $") + doAssert "".match(peg".* * $") + var greedyCaps: array[8, string] + doAssert "aaa".match(peg"{a?} *", greedyCaps) + doAssert greedyCaps[0] == "a" and greedyCaps[1] == "a" and + greedyCaps[2] == "a" + doAssert not "b".match(peg"^('a'?)* $") + doAssert not "aab".match(peg"^('a'?)* $") + # the internal DSL `*` proc takes the same code path: + doAssert match("a", sequence(startAnchor(), *(?term("a")), endAnchor())) + # `$` representations remain parseable with identical meaning: + doAssert $peg"'a'? *" == "'a'?*" + doAssert $peg"('a' 'b')? *" == "('a' 'b')?*" + + # an unknown builtin escape inside a character class doesn't crash with + # IndexDefect in getCharSet + doAssertRaises(EInvalidPeg): discard peg"[^\n]" + doAssertRaises(EInvalidPeg): discard peg"[z-\n]" + + # an empty capture with no previous capture doesn't underflow c.matches + doAssert "abc".match(peg"^{}") + # documented behavior (deleting the last capture) still works: + doAssert "ab".match(peg"{[a-z]} {}", captures) + doAssert captures[0] == ""