diff --git a/parser/src/main/java/dev/cel/parser/PrattParser.java b/parser/src/main/java/dev/cel/parser/PrattParser.java index 829ac1dcc..15b59b269 100644 --- a/parser/src/main/java/dev/cel/parser/PrattParser.java +++ b/parser/src/main/java/dev/cel/parser/PrattParser.java @@ -253,7 +253,7 @@ private boolean expect(Lexer.TokenType type, String msg) { nextToken(); return true; } - if (isRecoveryLimitExceeded()) { + if (recursionLimitExceeded || isRecoveryLimitExceeded()) { return false; } if (peekToken.type != Lexer.TokenType.ERROR) { @@ -274,7 +274,7 @@ private boolean expect(Lexer.TokenType type, String msg) { // Find the next delimiter to prevent a cascade of spurious secondary errors. private void synchronizeOnDelimiter() { - if (isRecoveryLimitExceeded()) { + if (recursionLimitExceeded || isRecoveryLimitExceeded()) { peekToken = END_TOKEN; return; } @@ -412,6 +412,7 @@ private void reportRecursionLimit(int position) { LOCALE, "Expression recursion limit exceeded. limit: %d", options.maxParseRecursionDepth())); + peekToken = END_TOKEN; } } @@ -429,11 +430,14 @@ private CelExpr parseExpr() { return expr; } - @SuppressWarnings("EnumOrdinal") // Using ordinal for O(1) binary operator lookup table private CelExpr parseBinaryAndTernary(int minPrec) { - CelExpr lhs = parseSelectorChain(); + return parseBinaryAndTernaryFromLhs(parseSelectorChain(), minPrec); + } + + @SuppressWarnings("EnumOrdinal") // Using ordinal for O(1) binary operator lookup table + private CelExpr parseBinaryAndTernaryFromLhs(CelExpr lhs, int minPrec) { int chainDepth = currentLhsDepth; - while (true) { + while (!recursionLimitExceeded && !isRecoveryLimitExceeded()) { Lexer.TokenType tok = peekToken.type; if (tok == Lexer.TokenType.QUESTION && minPrec <= 0) { lhs = parseTernary(lhs); @@ -771,16 +775,39 @@ private CelExpr parsePrimary() { switch (peekToken.type) { case LEFT_PAREN: { - int groupingParenCount = countGroupingParentheses(); - if (checkRecursion(groupingParenCount, peekToken)) { + if (recursionLimitExceeded || isRecoveryLimitExceeded()) { return ERROR; } - for (int i = 0; i < groupingParenCount; ++i) { + // To avoid deep call-stack recursion on heavily nested parentheses (e.g. "((((a))))" or + // "((((a + 1) + 1) + 1))"), consume all consecutive leading '(' tokens upfront, parse the + // innermost expression once, and then iteratively unwind each enclosing '(' from + // innermost to outermost. After consuming each matching ')', if more enclosing '(' remain + // open and the next token is not another ')', continue parsing any trailing selectors or + // binary/ternary operators belonging to that enclosing parenthesized level using the + // already-parsed inner expression as the LHS. + int openParens = 0; + while (peekToken.type == Lexer.TokenType.LEFT_PAREN) { + openParens++; + if (checkRecursion(openParens, peekToken)) { + return ERROR; + } nextToken(); } - CelExpr expr = parseExpr(); - for (int i = 0; i < groupingParenCount; ++i) { + recursionDepth += openParens; + CelExpr expr = parseBinaryAndTernary(0); + for (int i = 0; i < openParens; ++i) { expect(Lexer.TokenType.RIGHT_PAREN, ""); + recursionDepth--; + if (i < openParens - 1 && peekToken.type != Lexer.TokenType.RIGHT_PAREN) { + currentLhsDepth = 0; + Lexer.TokenType tok = peekToken.type; + if (tok == Lexer.TokenType.DOT + || tok == Lexer.TokenType.LEFT_BRACKET + || tok == Lexer.TokenType.LEFT_BRACE) { + expr = parseSelectorChainTail(expr); + } + expr = parseBinaryAndTernaryFromLhs(expr, 0); + } } return expr; } @@ -1098,8 +1125,7 @@ private Optional tryExpandMacro( return Optional.empty(); } if (nodeLimitExceeded) { - reportError( - getPosition(exprId), "could not expand macro: expression node limit exceeded"); + reportError(getPosition(exprId), "could not expand macro: expression node limit exceeded"); return Optional.empty(); } @@ -1178,76 +1204,6 @@ private CelExpr buildMacroCallArgs(CelExpr expr) { return expr; } - private int countGroupingParentheses() { - if (peekToken.type != Lexer.TokenType.LEFT_PAREN) { - return 0; - } - - // Fast path: if the next non-whitespace character is not '(', leading open parens is 1. - int pos = peekToken.end; - int size = content.size(); - while (pos < size) { - int c = content.get(pos); - if (c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\f' && c != 11) { - if (c == '/') { - // A comment might precede another '('. - break; - } - if (c == '(') { - break; - } - // Next significant token is definitely not '('. - return 1; - } - pos++; - } - - int savedPos = lexer.savePosition(); - try { - int leadingOpenParens = 1; - Lexer.Token tok = nextSignificantToken(/* reportError= */ false); - while (tok.type == Lexer.TokenType.LEFT_PAREN) { - leadingOpenParens++; - tok = nextSignificantToken(/* reportError= */ false); - } - if (leadingOpenParens == 1) { - return 1; - } - - int openParens = leadingOpenParens; - int consecutiveLeadingClosed = 0; - - while (openParens > 0) { - if (tok.type == Lexer.TokenType.END || tok.type == Lexer.TokenType.ERROR) { - return 1; - } - - if (tok.type == Lexer.TokenType.LEFT_PAREN) { - openParens++; - consecutiveLeadingClosed = 0; - } else if (tok.type == Lexer.TokenType.RIGHT_PAREN) { - if (leadingOpenParens == openParens) { - leadingOpenParens--; - consecutiveLeadingClosed++; - } else { - consecutiveLeadingClosed = 0; - } - openParens--; - } else { - consecutiveLeadingClosed = 0; - } - - if (openParens > 0) { - tok = nextSignificantToken(/* reportError= */ false); - } - } - - return Math.max(1, consecutiveLeadingClosed); - } finally { - lexer.restorePosition(savedPos); - } - } - private final class PrattMacroExprFactory extends CelMacroExprFactory { private final ArrayDeque macroPositions = new ArrayDeque<>(1); diff --git a/parser/src/test/java/dev/cel/parser/CelParserImplTest.java b/parser/src/test/java/dev/cel/parser/CelParserImplTest.java index 09d578a36..6d1556466 100644 --- a/parser/src/test/java/dev/cel/parser/CelParserImplTest.java +++ b/parser/src/test/java/dev/cel/parser/CelParserImplTest.java @@ -229,7 +229,18 @@ private enum MaxParseRecursionDepthTestCase { + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !=" + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !=" + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !=" - + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20]"); + + " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20]"), + TERNARY( + "a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :" + + " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :" + + " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :" + + " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : c"), + TERNARY_TRUE_BRANCH_PARENS( + "a ? ((((((((((((((((((((((((((((((((b)))))))))))))))))))))))))))))))) : c"), + NESTED_LEFT_PARENS_WITH_CALC( + "((((((((((((((((((((((((((((((((7) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1)" + + " + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) +" + + " 1) + 1) + 1) + 1)"); static final int MAX_RECURSION_LIMIT = 32; final String source; diff --git a/parser/src/test/resources/parser_errors.baseline b/parser/src/test/resources/parser_errors.baseline index cbd9f087b..4f23e4a9c 100644 --- a/parser/src/test/resources/parser_errors.baseline +++ b/parser/src/test/resources/parser_errors.baseline @@ -1098,9 +1098,6 @@ E/A: Expression recursion limit exceeded. limit: 250 E/P: ERROR: :1:251: Expression recursion limit exceeded. limit: 250 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] | ..........................................................................................................................................................................................................................................................^ -ERROR: :1:251: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] - | ..........................................................................................................................................................................................................................................................^ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ »»»[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]] @@ -1110,9 +1107,6 @@ E/A: Expression recursion limit exceeded. limit: 32 E/P: ERROR: :1:33: Expression recursion limit exceeded. limit: 32 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ | ................................^ -ERROR: :1:33: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ - | ................................^ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] =====> @@ -1120,9 +1114,6 @@ E/A: Expression recursion limit exceeded. limit: 32 E/P: ERROR: :1:33: Expression recursion limit exceeded. limit: 32 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] | ................................^ -ERROR: :1:33: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] - | ................................^ I: a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.A.B.C.D.E.F.G.H =====> diff --git a/parser/src/test/resources/pratt_parser_errors.baseline b/parser/src/test/resources/pratt_parser_errors.baseline index d21e65ab3..541f9e14f 100644 --- a/parser/src/test/resources/pratt_parser_errors.baseline +++ b/parser/src/test/resources/pratt_parser_errors.baseline @@ -398,18 +398,12 @@ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ E: ERROR: :1:33: Expression recursion limit exceeded. limit: 32 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ | ................................^ -ERROR: :1:33: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[ - | ................................^ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] =====> E: ERROR: :1:33: Expression recursion limit exceeded. limit: 32 | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] | ................................^ -ERROR: :1:33: Syntax error: expected ']' - | [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] - | ................................^ I: a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.A.B.C.D.E.F.G.H =====>