This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit a74178c9a400018a9432a68135fd0508cbaeb021 Author: opencode <[email protected]> AuthorDate: Thu Oct 8 13:53:48 2026 +0200 Report a parse error for SSI expressions with missing operands Malformed SSI conditional expressions such as "a =", "= a" or "!" made OppNode.popValues() call removeFirst() on an empty node stack, throwing a NoSuchElementException out of the ExpressionParseTree constructor. Since SSIConditional only handles ParseException, the unchecked exception bypassed the SSI error handling and surfaced as a 500 error for the whole request. Check before each pop that the node stack holds at least the number of operands the operator needs (new OppNode.getOperandCount(), one for NotNode, two otherwise) and throw a ParseException with a new missingOperand message otherwise. Also let resolveGroup() tolerate an empty operator stack, which occurs when the expression contains more closing parentheses than opening ones and the artificial group marker has been consumed; such expressions now parse leniently or produce a ParseException, as already the case for balanced groups. --- .../apache/catalina/ssi/ExpressionParseTree.java | 81 +++++++++++++++++----- .../apache/catalina/ssi/LocalStrings.properties | 1 + .../catalina/ssi/TestExpressionParseTree.java | 34 +++++++++ 3 files changed, 98 insertions(+), 18 deletions(-) diff --git a/java/org/apache/catalina/ssi/ExpressionParseTree.java b/java/org/apache/catalina/ssi/ExpressionParseTree.java index 736b60cb6a..2b2d74aca5 100644 --- a/java/org/apache/catalina/ssi/ExpressionParseTree.java +++ b/java/org/apache/catalina/ssi/ExpressionParseTree.java @@ -86,9 +86,12 @@ public class ExpressionParseTree { /** * Pushes a new operator onto the opp stack, resolving existing opps as needed. * - * @param node The operator node + * @param node The operator node + * @param index The current tokenizer index, for error reporting + * + * @throws ParseException An operator was missing one of its operands */ - private void pushOpp(OppNode node) { + private void pushOpp(OppNode node, int index) throws ParseException { // If node is null then it's just a group marker if (node == null) { oppStack.addFirst(null); @@ -109,6 +112,7 @@ public class ExpressionParseTree { if (top.getPrecedence() < node.getPrecedence()) { break; } + checkOperands(top, index); // Remove the top node oppStack.removeFirst(); // Let it fill its branches @@ -121,12 +125,39 @@ public class ExpressionParseTree { } + /** + * Checks that the node stack holds at least the number of operands the given + * operator needs. + * + * @param node The operator to check the operands for + * @param index The current tokenizer index, for error reporting + * + * @throws ParseException The operator is missing one of its operands + */ + private void checkOperands(OppNode node, int index) throws ParseException { + if (nodeStack.size() < node.getOperandCount()) { + throw new ParseException(sm.getString("expressionParseTree.missingOperand"), index); + } + } + + /** * Resolves all pending opp nodes on the stack until the next group marker is reached. + * + * @param index The current tokenizer index, for error reporting + * + * @throws ParseException An operator was missing one of its operands */ - private void resolveGroup() { - OppNode top; - while ((top = oppStack.removeFirst()) != null) { + 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; + } + checkOperands(top, index); // Let it fill its branches top.popValues(nodeStack); // Stick it on the resolved node stack @@ -146,7 +177,7 @@ public class ExpressionParseTree { StringNode currStringNode = null; // We cheat a little and start an artificial // group right away. It makes finishing easier. - pushOpp(null); + pushOpp(null, 0); ExpressionTokenizer et = new ExpressionTokenizer(expr); while (et.hasMoreTokens()) { int token = et.nextToken(); @@ -165,55 +196,55 @@ public class ExpressionParseTree { } break; case ExpressionTokenizer.TOKEN_AND: - pushOpp(new AndNode()); + pushOpp(new AndNode(), et.getIndex()); break; case ExpressionTokenizer.TOKEN_OR: - pushOpp(new OrNode()); + pushOpp(new OrNode(), et.getIndex()); break; case ExpressionTokenizer.TOKEN_NOT: - pushOpp(new NotNode()); + pushOpp(new NotNode(), et.getIndex()); break; case ExpressionTokenizer.TOKEN_EQ: - pushOpp(new EqualNode()); + pushOpp(new EqualNode(), et.getIndex()); break; case ExpressionTokenizer.TOKEN_NOT_EQ: - pushOpp(new NotNode()); + pushOpp(new NotNode(), et.getIndex()); // Sneak the regular node in. They will NOT // be resolved when the next opp comes along. oppStack.addFirst(new EqualNode()); break; case ExpressionTokenizer.TOKEN_RBRACE: // Closeout the current group - resolveGroup(); + resolveGroup(et.getIndex()); break; case ExpressionTokenizer.TOKEN_LBRACE: // Push a group marker - pushOpp(null); + pushOpp(null, et.getIndex()); break; case ExpressionTokenizer.TOKEN_GE: - pushOpp(new NotNode()); + pushOpp(new NotNode(), et.getIndex()); // Similar strategy to NOT_EQ above, except this // is NOT less than oppStack.addFirst(new LessThanNode()); break; case ExpressionTokenizer.TOKEN_LE: - pushOpp(new NotNode()); + pushOpp(new NotNode(), et.getIndex()); // Similar strategy to NOT_EQ above, except this // is NOT greater than oppStack.addFirst(new GreaterThanNode()); break; case ExpressionTokenizer.TOKEN_GT: - pushOpp(new GreaterThanNode()); + pushOpp(new GreaterThanNode(), et.getIndex()); break; case ExpressionTokenizer.TOKEN_LT: - pushOpp(new LessThanNode()); + pushOpp(new LessThanNode(), et.getIndex()); break; case ExpressionTokenizer.TOKEN_END: break; } } // Finish off the rest of the opps - resolveGroup(); + resolveGroup(et.getIndex()); if (nodeStack.isEmpty()) { throw new ParseException(sm.getString("expressionParseTree.noNodes"), et.getIndex()); } @@ -301,6 +332,14 @@ public class ExpressionParseTree { public abstract int getPrecedence(); + /** + * @return the number of operands this operator pops from the node stack, two by default. + */ + public int getOperandCount() { + return 2; + } + + /** * Lets the node pop its own branch nodes off the front of the specified list. The default pulls two. * @@ -325,6 +364,12 @@ public class ExpressionParseTree { } + @Override + public int getOperandCount() { + return 1; + } + + /** * Overridden to pop only one value. */ diff --git a/java/org/apache/catalina/ssi/LocalStrings.properties b/java/org/apache/catalina/ssi/LocalStrings.properties index b5ea18ab25..5cebaec4d0 100644 --- a/java/org/apache/catalina/ssi/LocalStrings.properties +++ b/java/org/apache/catalina/ssi/LocalStrings.properties @@ -18,6 +18,7 @@ expressionParseTree.extraNodes=Extra nodes created expressionParseTree.invalidExpression=Invalid expression [{0}] +expressionParseTree.missingOperand=Missing operand expressionParseTree.noNodes=No nodes created expressionParseTree.unusedOpCodes=Unused nodes exist diff --git a/test/org/apache/catalina/ssi/TestExpressionParseTree.java b/test/org/apache/catalina/ssi/TestExpressionParseTree.java index 14b63233da..a635c8258d 100644 --- a/test/org/apache/catalina/ssi/TestExpressionParseTree.java +++ b/test/org/apache/catalina/ssi/TestExpressionParseTree.java @@ -17,6 +17,7 @@ package org.apache.catalina.ssi; import java.io.IOException; +import java.text.ParseException; import java.util.Collection; import java.util.Date; import java.util.HashMap; @@ -136,6 +137,39 @@ public class TestExpressionParseTree { } + @Test + 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" }; + for (String expression : expressions) { + SSIMediator mediator = new SSIMediator(new TesterSSIExternalResolver(), LAST_MODIFIED); + try { + new ExpressionParseTree(expression, mediator); + Assert.fail("Expected a parse error for [" + expression + "]"); + } catch (ParseException pe) { + // Expected + } + } + } + + + @Test + public void testExtraClosingParen() throws Exception { + // Unbalanced closing parentheses consumed the artificial group marker + // and used to result in a NoSuchElementException from the parser + String[] expressions = { ")", "a)", "a))", "(a))", "a ) b" }; + for (String expression : expressions) { + SSIMediator mediator = new SSIMediator(new TesterSSIExternalResolver(), LAST_MODIFIED); + try { + new ExpressionParseTree(expression, mediator); + } catch (ParseException pe) { + // Also a valid outcome + } + } + } + + @Test public void testSubstituteVariablesPlainVar() throws Exception { SSIMediator mediator = new SSIMediator(new TesterSSIExternalResolver(), LAST_MODIFIED); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
