This is an automated email from the ASF dual-hosted git repository.

markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 008cede5bd Follow-on to "Report a parse error for SSI expressions 
with..."
008cede5bd is described below

commit 008cede5bdc05b05ff30606175b4408ac8242088
Author: Mark Thomas <[email protected]>
AuthorDate: Mon Oct 19 18:45:32 2026 +0100

    Follow-on to "Report a parse error for SSI expressions with..."
    
    Reject expressions with mis-matched parenthesis
    Clean-up tests
---
 .../apache/catalina/ssi/ExpressionParseTree.java   | 23 +++++++++++++---------
 .../apache/catalina/ssi/LocalStrings.properties    |  1 +
 .../catalina/ssi/TestExpressionParseTree.java      | 18 +++++++++--------
 3 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/java/org/apache/catalina/ssi/ExpressionParseTree.java 
b/java/org/apache/catalina/ssi/ExpressionParseTree.java
index 7ba512c1b5..f4fd7fddb6 100644
--- a/java/org/apache/catalina/ssi/ExpressionParseTree.java
+++ b/java/org/apache/catalina/ssi/ExpressionParseTree.java
@@ -42,6 +42,10 @@ public class ExpressionParseTree {
      * since it can contain {@code null}s.
      */
     private final LinkedList<OppNode> oppStack = new LinkedList<>();
+    /**
+     * Contains the node stack size at the start of each group.
+     */
+    private final LinkedList<Integer> groupNodeCounts = new LinkedList<>();
     /**
      * The root node after the expression has been parsed.
      */
@@ -95,6 +99,7 @@ public class ExpressionParseTree {
         // If node is null then it's just a group marker
         if (node == null) {
             oppStack.addFirst(null);
+            groupNodeCounts.addFirst(Integer.valueOf(nodeStack.size()));
             return;
         }
         while (true) {
@@ -135,7 +140,8 @@ public class ExpressionParseTree {
      * @throws ParseException The operator is missing one of its operands
      */
     private void checkOperands(OppNode node, int index) throws ParseException {
-        if (nodeStack.size() < node.getOperandCount()) {
+        int groupNodeCount = nodeStack.size() - 
groupNodeCounts.getFirst().intValue();
+        if (groupNodeCount < node.getOperandCount()) {
             throw new 
ParseException(sm.getString("expressionParseTree.missingOperand"), index);
         }
     }
@@ -149,20 +155,15 @@ public class ExpressionParseTree {
      * @throws ParseException An operator was missing one of its operands
      */
     private void resolveGroup(int index) throws ParseException {
-        // The stack may be empty if the expression contained more closing
-        // parentheses than opening ones, which consumed the artificial group
-        // marker
-        while (!oppStack.isEmpty()) {
-            OppNode top = oppStack.removeFirst();
-            if (top == null) {
-                break;
-            }
+        OppNode top;
+        while ((top = oppStack.removeFirst()) != null) {
             checkOperands(top, index);
             // Let it fill its branches
             top.popValues(nodeStack);
             // Stick it on the resolved node stack
             nodeStack.addFirst(top);
         }
+        groupNodeCounts.removeFirst();
     }
 
 
@@ -214,6 +215,10 @@ public class ExpressionParseTree {
                     oppStack.addFirst(new EqualNode());
                     break;
                 case ExpressionTokenizer.TOKEN_RBRACE:
+                    if (groupNodeCounts.size() == 1) {
+                        throw new 
ParseException(sm.getString("expressionParseTree.unmatchedClosingParenthesis"),
+                                et.getIndex());
+                    }
                     // Closeout the current group
                     resolveGroup(et.getIndex());
                     break;
diff --git a/java/org/apache/catalina/ssi/LocalStrings.properties 
b/java/org/apache/catalina/ssi/LocalStrings.properties
index 5969272c58..8519dbd829 100644
--- a/java/org/apache/catalina/ssi/LocalStrings.properties
+++ b/java/org/apache/catalina/ssi/LocalStrings.properties
@@ -20,6 +20,7 @@ expressionParseTree.extraNodes=Extra nodes created
 expressionParseTree.invalidExpression=Invalid expression [{0}]
 expressionParseTree.missingOperand=Missing operand
 expressionParseTree.noNodes=No nodes created
+expressionParseTree.unmatchedClosingParenthesis=Unmatched closing parenthesis
 expressionParseTree.unterminatedLiteral=Unterminated string or regular 
expression
 expressionParseTree.unusedOpCodes=Unused nodes exist
 
diff --git a/test/org/apache/catalina/ssi/TestExpressionParseTree.java 
b/test/org/apache/catalina/ssi/TestExpressionParseTree.java
index 0a3cbd5bf5..522da074d8 100644
--- a/test/org/apache/catalina/ssi/TestExpressionParseTree.java
+++ b/test/org/apache/catalina/ssi/TestExpressionParseTree.java
@@ -164,7 +164,8 @@ public class TestExpressionParseTree {
         for (String expression : expressions) {
             SSIMediator mediator = new SSIMediator(new 
TesterSSIExternalResolver(), LAST_MODIFIED);
             try {
-                new ExpressionParseTree(expression, mediator);
+                @SuppressWarnings("unused")
+                ExpressionParseTree unused = new 
ExpressionParseTree(expression, mediator);
                 Assert.fail("Expected a parse error for [" + expression + "]");
             } catch (ParseException pe) {
                 // Expected
@@ -177,11 +178,12 @@ public class TestExpressionParseTree {
     public void testMissingOperand() throws Exception {
         // Operators missing an operand must produce a parse error rather than
         // an unchecked exception
-        String[] expressions = { "= a", "a =", "a ! =", "!", "a = = b", "!= a" 
};
+        String[] expressions = { "= a", "a =", "a ! =", "!", "a = = b", "!= 
a", "a (!)", "a (= b)" };
         for (String expression : expressions) {
             SSIMediator mediator = new SSIMediator(new 
TesterSSIExternalResolver(), LAST_MODIFIED);
             try {
-                new ExpressionParseTree(expression, mediator);
+                @SuppressWarnings("unused")
+                ExpressionParseTree unused = new 
ExpressionParseTree(expression, mediator);
                 Assert.fail("Expected a parse error for [" + expression + "]");
             } catch (ParseException pe) {
                 // Expected
@@ -191,16 +193,16 @@ public class TestExpressionParseTree {
 
 
     @Test
-    public void testExtraClosingParen() throws Exception {
-        // Unbalanced closing parentheses consumed the artificial group marker
-        // and used to result in a NoSuchElementException from the parser
+    public void testUnmatchedClosingParenthesis() throws Exception {
         String[] expressions = { ")", "a)", "a))", "(a))", "a ) b" };
         for (String expression : expressions) {
             SSIMediator mediator = new SSIMediator(new 
TesterSSIExternalResolver(), LAST_MODIFIED);
             try {
-                new ExpressionParseTree(expression, mediator);
+                @SuppressWarnings("unused")
+                ExpressionParseTree unused = new 
ExpressionParseTree(expression, mediator);
+                Assert.fail("Expected a parse error for [" + expression + "]");
             } catch (ParseException pe) {
-                // Also a valid outcome
+                // Expected
             }
         }
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to