henrib commented on code in PR #414:
URL: https://github.com/apache/commons-jexl/pull/414#discussion_r3896699979
##########
src/main/java/org/apache/commons/jexl3/internal/Interpreter.java:
##########
@@ -1377,12 +1378,47 @@ protected Object visit(final ASTEQSNode node, final
Object data) {
@Override
protected Object visit(final ASTERNode node, final Object data) {
final Object left = node.jjtGetChild(0).jjtAccept(this, data);
- final Object right = node.jjtGetChild(1).jjtAccept(this, data);
+ final JexlNode rightNode = node.jjtGetChild(1);
+ final Object right = resolvePattern(rightNode,
rightNode.jjtAccept(this, data));
// note the arguments inversion between 'in'/'matches' and 'contains'
// if x in y then y contains x
return operators.contains(node, JexlOperator.CONTAINS, right, left);
}
+ /**
+ * If the right operand of {@code =~} / {@code !~} is a string literal,
compile it to a Pattern once and
+ * cache the result in the node's value slot (same mechanism as negated
numeric literals).
+ * Dynamic string values (from variables) are returned unchanged.
+ * The regex string length is validated before compilation (JEXL-security
f012).
+ * Uses double-check locking: first check without lock, then synchronized
recheck-and-set to avoid
+ * holding the lock during expensive Pattern.compile() when the same
compiled script runs concurrently.
+ */
+ private static Object resolvePattern(final JexlNode rightNode, final
Object right) {
+ if (right instanceof CharSequence && rightNode instanceof
JexlNode.Constant) {
+ // First check (volatile read, no lock)
+ Object cached = rightNode.jjtGetValue();
+ if (cached instanceof Pattern) {
+ return cached;
+ }
+ // Compile without holding lock
+ final String regex = right.toString();
+ if (regex.length() > JexlArithmetic.REGEX_PATTERN_MAX_LENGTH) {
+ throw new ArithmeticException("regular expression too long: "
+ regex.length()
+ + " > " + JexlArithmetic.REGEX_PATTERN_MAX_LENGTH);
+ }
+ final Pattern compiled = Pattern.compile(regex);
+ // Double-check and set under lock
+ synchronized (rightNode) {
+ cached = rightNode.jjtGetValue();
+ if (!(cached instanceof Pattern)) {
+ rightNode.jjtSetValue(compiled);
+ }
+ return cached instanceof Pattern ? cached : compiled;
+ }
+ }
+ return right;
+ }
Review Comment:
The probability of high concurrency on this method is close to nil; the
sprinkled synchronization should be enough to guarantee volatile semantics and
reduce contention to the minimum.
--
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]
