uschindler opened a new pull request, #16423: URL: https://github.com/apache/lucene/pull/16423
...so we fail withParseException instead of StackOverflowError or LinkageErrors when the expression is too complex. ### Description Any application that compiles caller-influenced expressions through `JavascriptCompiler` inherits an undeclared crash path. Because `StackOverflowError` is a `VirtualMachineError`, generic `try/catch (ParseException)` or even `catch (Exception)` around `compile()` does not contain it; only an explicit `catch (StackOverflowError)` / `catch (Error)` does. Concrete downstream example (the origin of this report): Elasticsearch exposes the expressions language as a scripting engine. A single small request containing a nested-parenthesis expression reaches `JavascriptCompiler.compile()`, the `StackOverflowError` escapes Elasticsearch's `ParseException`-only handler, and Elasticsearch's fatal-error handler halts the JVM. This is partly an Elasticsearch defense-in-depth gap (it catches this same error for its Painless engine but not for expressions), and that is being addressed separately with Elasticsearch. The stopped JVM is actually not a bug in Lucene (Elasticsearch kills itsself when the stack overflows or on any other error), but as this is partly caused by the ANTLR lexr/parser to work recursive, this should be handled. This PR adds a configurable limit on the nesting in Lucene expressions (defaults to 250) which is checked on parsing and building the `ParseTree` -- but also adds a "last safety": When a StackOverflowError happens during building the final class file, the parsing. If others think the additional listener is too much overhead, we can remove the configurable limit and only trigger on a stack overflow. The configurable limit has the pro that it is not JVM dependent. An expression will fail consistently with a given limit and not depending on the call stack and config of the JVM. This may also be used by software like Elasticsearch to mimit the complexity of expressions. We may also add a limit on how many method calls are allowed at a later stage. The PR also adds another catch for `LinkageError` that is rethrows as (documented) `IllegalStateException` if the parser created a class file which does not load at all. It also updates documentation to clarfiy which exceptions are thrown. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
