Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the JEXL runtime against several DoS/cancellation edge cases by making regex matching cancellable/interruptible and by bounding BigInteger literal parsing and arithmetic growth to avoid unbounded memory/CPU costs.
Changes:
- Add interrupt sampling via a
CharSequencewrapper so regex matching can be cancelled via thread interruption. - Enforce
MathContext-derived precision bounds on BigInteger arithmetic and add parse-time digit caps for BigInteger literals. - Add parser wrapping for
NumberFormatExceptionand corresponding regression tests and release notes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/org/apache/commons/jexl3/ArithmeticTest.java | Adds regression tests for regex length/interruptibility and BigInteger precision/literal-size guards. |
| src/main/java/org/apache/commons/jexl3/parser/Parser.jjt | Wraps numeric literal parsing failures as JexlException.Parsing. |
| src/main/java/org/apache/commons/jexl3/parser/NumberParser.java | Adds parse-time digit cap logic for BigInteger literals (optionally driven by thread-engine precision). |
| src/main/java/org/apache/commons/jexl3/JexlArithmetic.java | Adds regex interruptibility + length guard and BigInteger precision enforcement. |
| src/main/java/org/apache/commons/jexl3/internal/Operator.java | Propagates interruption as JexlException.Cancel from operator evaluation. |
| src/main/java/org/apache/commons/jexl3/internal/Interpreter.java | Caches compiled regex Pattern for string-literal RHS of =~/!~. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ure regex interruptibility 1. Make regex matching (=~ operator) interruptible * Wrap matched string in InterruptibleCharSequence * Samples Thread.isInterrupted() every 256 chars * Throws ArithmeticException -> JexlException.Cancel on interruption * Add length guard on regex patterns (max 2048 chars) 2. Enforce MathContext precision on BigInteger arithmetic * Move checkBigIntegerPrecision() outside try-catch in add()/subtract()/etc * Prevent ArithmeticException from being silently swallowed * Bounded results prevent memory exhaustion 3. Prevent O(n²) DoS from huge BigInteger literals at parse time * Cap literal digit count by MathContext.getPrecision() * Fallback to hardcoded 256-digit limit if no precision configured * NumberFormatException wraps as JexlException.Parsing Tests added: * testRegexMatchingInterruptible() * testRegexPatternTooLong() * testBigIntegerArithmeticPrecisionCap() * testBigIntegerLiteralTooLong() Co-Authored-By: Claude <noreply@anthropic.com>
henrib
force-pushed
the
JEXL-471
branch
2 times, most recently
from
August 31, 2026 10:52
bc10ac9 to
97427ba
Compare
Proper double-check locking: first volatile read without lock, compile Pattern outside lock, then synchronized recheck-and-set to minimize contention and avoid duplicate compilations when same script runs concurrently. Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+2471
to
+2507
| // Catastrophic backtracking pattern on non-matching input to force long character scanning | ||
| final String evilPattern = "(a+)+b"; | ||
| // Use a larger set of 'a's to extend matching time | ||
| final char[] chars = new char[50]; | ||
| java.util.Arrays.fill(chars, 'a'); | ||
| final String evilValue = new String(chars) + "c"; | ||
|
|
||
| final java.util.concurrent.atomic.AtomicReference<Exception> caught = | ||
| new java.util.concurrent.atomic.AtomicReference<>(); | ||
| final java.util.concurrent.CountDownLatch started = new java.util.concurrent.CountDownLatch(1); | ||
|
|
||
| final Thread t = new Thread(() -> { | ||
| try { | ||
| started.countDown(); | ||
| script.execute(null, evilValue, evilPattern); | ||
| } catch (final Exception e) { | ||
| caught.set(e); | ||
| } | ||
| }); | ||
|
|
||
| t.start(); | ||
| // Wait for thread to actually start executing | ||
| started.await(); | ||
| // Give regex matching time to engage (50 'a's with (a+)+b pattern causes backtracking) | ||
| Thread.sleep(300); | ||
| // Interrupt the matching thread | ||
| t.interrupt(); | ||
| // Wait for thread to complete (should exit promptly if InterruptibleCharSequence is working) | ||
| t.join(5000); | ||
|
|
||
| assertFalse(t.isAlive(), "Thread should have completed after interruption (regex should be interruptible)"); | ||
| // The thread may complete without exception if the regex finishes faster than interruption catches it, | ||
| // or it may throw Cancel if interrupted during charset access. Both are acceptable here. | ||
| if (caught.get() != null) { | ||
| assertTrue(caught.get() instanceof JexlException.Cancel, | ||
| "If interrupted during matching, expected JexlException.Cancel, got " + caught.get().getClass().getSimpleName()); | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Runtime hardening PR addressing three security concerns:
Regex matching interruptibility — Make
=~/!~operators responsive to thread interruption, preventing indefinite hangs on catastrophic backtracking patterns. Includes pattern caching in AST nodes to avoid recompilation.BigInteger arithmetic precision bounds — Enforce
MathContextprecision limits on BigInteger results to prevent unbounded growth and memory exhaustion.BigInteger literal parsing DoS prevention — Cap parse-time digit count to prevent O(n²) complexity attacks via huge literals.
Changes
resolvePattern()caches compiledPatternobjects in AST node value slots for string literalsInterruptibleCharSequencewrapper with 256-char interrupt sampling, regex length guard (2048 chars), hoist precision check outside try-catchJexlException.CancelMathContext.getPrecision()NumberFormatExceptionasJexlException.ParsingTests
All 1204 tests passing:
testRegexMatchingInterruptible()testRegexPatternTooLong()testBigIntegerArithmeticPrecisionCap()testBigIntegerLiteralTooLong()🤖 Generated with Claude Code