You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "uschindler (via GitHub)" <gi...@apache.org> on 2026/07/27 12:22:35 UTC
[PR] Add (configurable) guards for complexity of expressions [lucene]
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #16423:
URL: https://github.com/apache/lucene/pull/16423#issuecomment-5094588363
Backport was possible with few modifications (mostly test annotation and some conflicts): 1dc74b98be6d4a1b8b09c89ecd23fb404f759011
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "msokolov (via GitHub)" <gi...@apache.org>.
msokolov commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657330799
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
hm, I did not realize a single expression with lots of + would be included. You know at one time we attempted to use Lucene's expression language to encode machine-learned decision trees that had thousands and thousands of nodes. Ultimately we concluded this was bad idea and implemented a custom handling function,, but the the reason was not StackOverflow, but some internal limit in the JDK on byte code. I'm forgetting exactly what was limited - maybe the total size of byte code, number of instructions, or number of identifiers? Anyway there can be cases with very large expressions, so I get your point that 250 may not be enough. Still, this feels something like BooleanQuery's 1024 limit or vector search's 1024-dimension limit - can we pick a large default (maybe 1024 is our magic number)? But I don't fundamentally object to exposing the config parameter, just wonder if anyone would use it
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #16423:
URL: https://github.com/apache/lucene/pull/16423#issuecomment-5091345577
This can be backported if needed (it does not depend on the ASM vs. Classfile library used for compiling. It may just create minor merge conflicts.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657810879
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
Yeah, this is now also handled in this PR. If the JVM refuses the class file due to internal limitations, an Exception is thrown, too.
There may also be other limitations in the JVM that could cause a stack overflow also at runtime. Basically those two "identical" expressions (semantically), will cause a different bytecode. The first and second one have same AST and are more optimal, but the third one consumes much more space during execution:
- `a + b + c + d`: bytecode: push a, push b, add, push c, add, push d, add
- `((a + b) + c) +d`: bytecode: push a, push b, add, push c, add, push d, add
- `a + (b + (c + d))`: bytecode push a, push b, push c, push d, add, add, add
The last one pushes all arguments onto stack and calls three times "add". This is consuming more stack than the first two variants (with same AST). But as first and second are identical in the AST, you see why the first one has implicit precedence included and therefor may cause a stack overflow during parsing.
You see it is complicated and therefor a hardcoded nesting limit, but possibly also a "number of tokens" limit should be enforced (configurable).
The number of tokens limit can be added, if we agree on it.
I hope this helps to understand what's going on!
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657070173
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
But I agree, if we can remove it and have a sane default - I am fine. But if we don't want to have it, catching "StackOverFlowException" might be enough, too.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #16423:
URL: https://github.com/apache/lucene/pull/16423#issuecomment-5093702847
Thanks for review, I added a small improvement to not traverse the tree two times. The depth check is now done inside the visitor!
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657810879
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
Yeah, this is now also handled in this PR. If the JVM refuses the class file due to internal limitations, an IllegalStateException is thrown, too (instead of an error).
There may also be other limitations in the JVM that could cause a stack overflow also at runtime. Basically those two "identical" expressions (semantically), will cause a different bytecode. The first and second one have same AST and are more optimal, but the third one consumes much more space during execution:
- `a + b + c + d`: bytecode: push a, push b, add, push c, add, push d, add
- `((a + b) + c) +d`: bytecode: push a, push b, add, push c, add, push d, add
- `a + (b + (c + d))`: bytecode push a, push b, push c, push d, add, add, add
The last one pushes all arguments onto stack and calls three times "add". This is consuming more stack than the first two variants (with same AST). But as first and second are identical in the AST, you see why the first one has implicit precedence included and therefor may cause a stack overflow during parsing.
You see it is complicated and therefor a hardcoded nesting limit, but possibly also a "number of tokens" limit should be enforced (configurable).
The number of tokens limit can be added, if we agree on it.
I hope this helps to understand what's going on!
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler merged PR #16423:
URL: https://github.com/apache/lucene/pull/16423
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657065668
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
Actually if you read the whole thing, you may notice that this not only affects "nesting" of expressions. Also a long "1 + 2 + 3 + 4 + ..... + 10000" will trigger the failure. I am not sure if 250 is enough for all cases, so I'd like to have it configurable (at least for a while, so people can tune it).
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "msokolov (via GitHub)" <gi...@apache.org>.
msokolov commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657043708
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
do we need to make the nestingDepth configurable? Given that this is a safety check, 250 really seems like plenty
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657070173
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
But I agree, if we can remove it I am fine. But if we don't want to have it, catching "StackOverFlowException" might be enough, too.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657906357
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
Anyways maybe 1024 is a better default limit! In my testing 2048 sometimes causes stack overflows already (especially in testing environment while the "picky" mode is enabled).
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add (configurable) guards for complexity of expressions [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657998331
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText) throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String, MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls, precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have correct signature
+ * @throws IllegalStateException if the resulting expression class fails to link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int maxNestingDepth)
Review Comment:
I updated javadocs to clarify what nesting depth means.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org