Repository navigation
Conversation
When given a poorly formed regular expression depending on data the regular expression matching can take a long time to compute. This commit modifies the expression engine so that regular expression based functions time bound evaluation to avoid runaway expressions.
Ensures that the ARQ property function strSplit time bounds regular expression splitting time
REPLACE uses regular expressions to perform replacements so also impose regular expression limits on its evaluation
Shex regular expression constraints could run too long depending on the regular expression and data combinations. This commit uses the shared RegexEngine.timeBounded() mechanism to limit evaluation time. Also disables a test scenario where optimisation effects means that the test does not reliably fail so is not a useful stable test.
|
|
||
| @ParameterizedTest(name = "{1}") | ||
| @MethodSource("slowSplits") | ||
| public void givenSlowRegex_whenSplittingWithTimeLimit_thenFails(String data, String pattern, int expectedResultCount) { |
There was a problem hiding this comment.
A more human oriented name perhaps.
There was a problem hiding this comment.
Yeah sorry, defaulting to $dayjob style test naming
|
|
||
| @ParameterizedTest(name = "{1}") | ||
| @MethodSource("slowSplits") | ||
| public void givenSlowRegex_whenSplittingWithNoTimeLimit_thenEventuallySucceeds(String data, String pattern, int expectedResultCount) { |
There was a problem hiding this comment.
A more human oriented name perhaps.
|
|
||
| @ParameterizedTest(name = "{1}") | ||
| @MethodSource("backtrackingRegex") | ||
| public void givenBacktrackingRegex_whenEvaluatingRegexWithoutTimeLimit_thenEventuallyCompletes(String input, String pattern, String flags, boolean shouldMatch) { |
There was a problem hiding this comment.
A more human oriented name perhaps.
|
|
||
| @ParameterizedTest(name = "{1}") | ||
| @MethodSource("backtrackingRegex") | ||
| public void givenBacktrackingRegex_whenEvaluatingRegexWithTimeLimit_thenFails(String input, String pattern, String flags, boolean shouldMatch) { |
There was a problem hiding this comment.
A more human oriented name perhaps.
| * Default maximum permitted regular expression evaluation time | ||
| * @see #MAX_REGEX_EVALUATION_TIME | ||
| */ | ||
| public static final long DEFAULT_MAX_REGEX_EVALUATION_TIME = 250; |
There was a problem hiding this comment.
I think it would be useful to put all limits (settings and defaults) in one place, including MathLimits from #4276. It makes things findable.
One possible place is org.apache.jena.sparql.ARQConstants; another is a new class for this purpose org.apache.jena.sparql.SysLimits.
There was a problem hiding this comment.
Yeah I will go ahead and consolidate into one place, a new SysLimits seems a good place
2 participants
Various SPARQL functions and Jena provided extension functions provide access to regular expressions. Depending on the expression and data combination some regular expressions can take too long to run leading to slow queries. This PR bounds regular expression evaluation time via statically controlled limits so regular expressions that take too long to run are aborted with an
ExprEvalException.As ShEx also uses the same Jena subsystem is was also upgraded to apply limits.
By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.
See the Apache Jena "Contributing" guide.