Copilot commented on code in PR #412:
URL: https://github.com/apache/commons-jexl/pull/412#discussion_r3853602850
##########
src/main/java/org/apache/commons/jexl3/internal/Interpreter.java:
##########
@@ -1313,11 +1313,42 @@ protected Object visit(final ASTDoWhileStatement node,
final Object data) {
return result;
}
+ /**
+ * Evaluates the argument of {@code empty()}/{@code size()} while
preserving cancellation and
+ * strict-mode error propagation (JEXL-security f028).
+ * <p>
+ * {@link JexlArithmetic#evaluate(Log, Supplier)} logs and swallows
<em>every</em> {@link JexlException}
+ * (including {@link JexlException.Cancel}), masking the failure as the
empty/0 result even in a strict
+ * engine. Here cancellation is always rethrown and, in a strict engine,
so is any other error; only a
+ * lenient engine maps a failed argument evaluation to the {@link
JexlEngine#TRY_FAILED} sentinel.
+ * </p>
Review Comment:
Interpreter now bypasses JexlArithmetic#evaluate(...) for empty()/size()
argument evaluation. Since JexlArithmetic#evaluate is public and explicitly
meant to be overridable, this change can silently break custom JexlArithmetic
implementations that relied on overriding evaluate() to control empty()/size()
fallback/logging behavior. Consider preserving the override hook (e.g., keep
delegating to arithmetic.evaluate in the lenient path after adjusting it to
rethrow JexlException.Cancel, or introduce a protected/overridable hook in
Interpreter) so custom arithmetic policies still apply.
##########
src/main/java/org/apache/commons/jexl3/parser/ASTRegexLiteral.java:
##########
@@ -54,7 +57,12 @@ public Object jjtAccept(final ParserVisitor visitor, final
Object data) {
}
void setLiteral(final String literal) {
- this.literal = Pattern.compile(literal);
+ try {
+ // report a malformed pattern as a parse error rather than leaking
PatternSyntaxException (JEXL-security f019)
+ this.literal = Pattern.compile(literal);
+ } catch (final PatternSyntaxException xpattern) {
+ throw new JexlException.Parsing(jexlInfo(), "invalid regular
expression: " + literal).clean();
+ }
Review Comment:
The parse error thrown for invalid regex literals drops the
PatternSyntaxException details (description/index) and only reports the whole
literal. This makes the error harder to act on, especially for long patterns.
Consider incorporating xpattern.getDescription() and xpattern.getIndex()
(and/or a short excerpt) into the JexlException.Parsing message so users can
locate the syntax problem quickly.
--
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]
