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

@@ -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
 

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

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] == ""