You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/09/15 13:45:14 UTC
(maven) branch master updated: Evaluate profile activation conditions lazily
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 2a555d00a5 Evaluate profile activation conditions lazily
2a555d00a5 is described below
commit 2a555d00a562b68f23af2ab88fedb87eace66afe
Author: Leo Camus <le...@gmail.com>
AuthorDate: Tue Sep 15 15:45:07 2026 +0200
Evaluate profile activation conditions lazily
* Evaluate profile activation conditions lazily
The condition parser evaluated every operand as it parsed it, so both
branches of if(..) and the right-hand side of && and || were always
computed. A branch that is not valid for the current input then fails
the whole condition. The if(..) example from the condition
documentation, substring(${java.version}, 0, indexOf(${java.version},
'-')), throws StringIndexOutOfBoundsException on any java.version
without a '-', and a guard such as length(x) >= 3 cannot protect a
substring(x, 0, 3) that follows it with &&.
Skip the unselected if(..) branch and the right operand of && / || when
the left one already decides the result. Skipped operands are still
checked for balanced parentheses, so malformed expressions keep failing.
* Name the if function checked by the condition parser
Replace the "if" literal in parseArgumentList with a named constant, so
the one place where the parser depends on a function name is visible,
and cover the two-argument if(false, 'a') call next to if(true, 'a').
---
.../maven/impl/model/profile/ConditionParser.java | 72 +++++++++++++++++++---
.../impl/model/profile/ConditionParserTest.java | 35 +++++++++++
2 files changed, 99 insertions(+), 8 deletions(-)
diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionParser.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionParser.java
index 7322959fd3..03f8c166c2 100644
--- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionParser.java
+++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionParser.java
@@ -22,6 +22,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Set;
import java.util.function.UnaryOperator;
/**
@@ -46,6 +47,11 @@ public interface ExpressionFunction {
Object apply(List<Object> args);
}
+ /**
+ * Name of the {@code if} function, whose unselected branch is skipped instead of evaluated.
+ */
+ private static final String IF_FUNCTION = "if";
+
private final Map<String, ExpressionFunction> functions; // Map to store functions by their names
private final UnaryOperator<String> propertyResolver; // Property resolver
private List<String> tokens; // List of tokens derived from the expression
@@ -196,6 +202,7 @@ private Object parseExpression() {
/**
* Parses logical OR operations.
+ * The right operand is skipped without being evaluated when the left one is already {@code true}.
*
* @return the result of parsing logical OR operations
*/
@@ -203,14 +210,18 @@ private Object parseLogicalOr() {
Object left = parseLogicalAnd();
while (current < tokens.size() && tokens.get(current).equals("||")) {
current++;
- Object right = parseLogicalAnd();
- left = (boolean) left || (boolean) right;
+ if ((boolean) left) {
+ skipOperand(Set.of("||", ",", ")"));
+ } else {
+ left = (boolean) parseLogicalAnd();
+ }
}
return left;
}
/**
* Parses logical AND operations.
+ * The right operand is skipped without being evaluated when the left one is already {@code false}.
*
* @return the result of parsing logical AND operations
*/
@@ -218,12 +229,46 @@ private Object parseLogicalAnd() {
Object left = parseComparison();
while (current < tokens.size() && tokens.get(current).equals("&&")) {
current++;
- Object right = parseComparison();
- left = (boolean) left && (boolean) right;
+ if ((boolean) left) {
+ left = (boolean) parseComparison();
+ } else {
+ skipOperand(Set.of("&&", "||", ",", ")"));
+ }
}
return left;
}
+ /**
+ * Skips an operand without evaluating it, so that functions are not invoked on a branch whose
+ * value is not needed. Skipping stops before the first of the given terminators found outside
+ * of parentheses.
+ *
+ * @param terminators the tokens that end the operand
+ * @throws RuntimeException if the operand is missing or its parentheses are not balanced
+ */
+ private void skipOperand(Set<String> terminators) {
+ int start = current;
+ int depth = 0;
+ while (current < tokens.size()) {
+ String token = tokens.get(current);
+ if (depth == 0 && terminators.contains(token)) {
+ break;
+ }
+ if (token.equals("(")) {
+ depth++;
+ } else if (token.equals(")")) {
+ depth--;
+ }
+ current++;
+ }
+ if (depth > 0) {
+ throw new RuntimeException("Mismatched parentheses: missing closing parenthesis");
+ }
+ if (current == start) {
+ throw new RuntimeException("Unexpected end of expression");
+ }
+ }
+
/**
* Parses comparison operations.
*
@@ -363,7 +408,7 @@ private Object parseVariableOrUnknownFunction() {
// Check if it's followed by an opening parenthesis, indicating a function call
if (current < tokens.size() && tokens.get(current).equals("(")) {
// It's a function call, parse it as such
- List<Object> args = parseArgumentList();
+ List<Object> args = parseArgumentList(name);
if (functions.containsKey(name)) {
return functions.get(name).apply(args);
} else {
@@ -379,15 +424,26 @@ private Object parseVariableOrUnknownFunction() {
/**
* Parses a list of arguments for a function call.
+ * For the {@code if} function, only the branch selected by the condition is evaluated,
+ * the other one is skipped and passed as {@code null}.
*
+ * @param functionName the name of the called function
* @return a list of parsed arguments
* @throws RuntimeException if there's a mismatch in parentheses
*/
- private List<Object> parseArgumentList() {
+ private List<Object> parseArgumentList(String functionName) {
List<Object> args = new ArrayList<>();
current++; // Skip the opening parenthesis
while (current < tokens.size() && !tokens.get(current).equals(")")) {
- args.add(parseLogicalOr());
+ int index = args.size();
+ if (IF_FUNCTION.equals(functionName)
+ && (index == 1 || index == 2)
+ && toBoolean(args.get(0)) != (index == 1)) {
+ skipOperand(Set.of(",", ")"));
+ args.add(null);
+ } else {
+ args.add(parseLogicalOr());
+ }
if (current < tokens.size() && tokens.get(current).equals(",")) {
current++;
}
@@ -407,7 +463,7 @@ private List<Object> parseArgumentList() {
private Object parseFunction() {
String functionName = tokens.get(current);
current++;
- List<Object> args = parseArgumentList();
+ List<Object> args = parseArgumentList(functionName);
return functions.get(functionName).apply(args);
}
diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionParserTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionParserTest.java
index 84130fd4f2..fee4cbdfe2 100644
--- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionParserTest.java
+++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionParserTest.java
@@ -182,6 +182,41 @@ void testIfFunction() {
assertEquals("short", parser.parse("if(length('hi') > 3, 'long', 'short')"));
}
+ /**
+ * Only the selected branch of {@code if(..)} is evaluated. The other one may not be valid for
+ * the current input, as in the example from the {@code condition} documentation: with a
+ * {@code java.version} that has no {@code -}, the unselected {@code substring(..)} would get an
+ * end index of -1.
+ */
+ @Test
+ void testIfFunctionOnlyEvaluatesSelectedBranch() {
+ assertEquals(
+ "1.8.0_292",
+ parser.parse("if(contains(${java.version}, '-'), "
+ + "substring(${java.version}, 0, indexOf(${java.version}, '-')), ${java.version})"));
+ assertEquals(
+ "21",
+ parser.parse("if(contains('21-ea', '-'), substring('21-ea', 0, indexOf('21-ea', '-')), '21-ea')"));
+ assertThrows(RuntimeException.class, () -> parser.parse("if(true, 'a')"));
+ assertThrows(RuntimeException.class, () -> parser.parse("if(false, 'a')"));
+ assertThrows(RuntimeException.class, () -> parser.parse("if(false, substring('a', 0, 5), 'b'"));
+ }
+
+ /**
+ * {@code &&} and {@code ||} do not evaluate their right operand when the left one already decides
+ * the result, so a guard such as {@code length(..) >= 3} can protect the call that follows it.
+ */
+ @Test
+ void testLogicalOperatorsShortCircuit() {
+ assertFalse((Boolean) parser.parse("length('ab') >= 3 && substring('ab', 0, 3) == 'abc'"));
+ assertTrue((Boolean) parser.parse("length('ab') < 3 || substring('ab', 0, 3) == 'abc'"));
+ assertTrue((Boolean) parser.parse("false && substring('ab', 0, 3) == 'abc' || true"));
+ assertFalse((Boolean) parser.parse("false && (substring('ab', 0, 3) == 'abc' || true) && true"));
+ assertTrue((Boolean) parser.parse("true && (false || substring('abc', 0, 3) == 'abc')"));
+ assertThrows(RuntimeException.class, () -> parser.parse("false &&"));
+ assertThrows(RuntimeException.class, () -> parser.parse("true || (false"));
+ }
+
@Test
void testContainsFunction() {
assertTrue((Boolean) parser.parse("contains('Hello, World!', 'World')"));