Skip to content

Iteratively parse nested parentheses in PrattParser #1245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
+50 −98
Open

Iteratively parse nested parentheses in PrattParser #1245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 38 additions & 82 deletions parser/src/main/java/dev/cel/parser/PrattParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -412,6 +412,7 @@ private void reportRecursionLimit(int position) {
LOCALE,
"Expression recursion limit exceeded. limit: %d",
options.maxParseRecursionDepth()));
peekToken = END_TOKEN;
}
}

Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -1098,8 +1125,7 @@ private Optional<CelExpr> 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();
}

Expand Down Expand Up @@ -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<Integer> macroPositions = new ArrayDeque<>(1);

Expand Down
13 changes: 12 additions & 1 deletion parser/src/test/java/dev/cel/parser/CelParserImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 0 additions & 9 deletions parser/src/test/resources/parser_errors.baseline
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1098,9 +1098,6 @@ E/A: Expression recursion limit exceeded. limit: 250
E/P: ERROR: <input>:1:251: Expression recursion limit exceeded. limit: 250
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
| ..........................................................................................................................................................................................................................................................^
ERROR: <input>:1:251: Syntax error: expected ']'
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
| ..........................................................................................................................................................................................................................................................^

I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
»»»[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]
Expand All @@ -1110,19 +1107,13 @@ E/A: Expression recursion limit exceeded. limit: 32
E/P: ERROR: <input>:1:33: Expression recursion limit exceeded. limit: 32
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
| ................................^
ERROR: <input>:1:33: Syntax error: expected ']'
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
| ................................^

I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
=====>
E/A: Expression recursion limit exceeded. limit: 32
E/P: ERROR: <input>:1:33: Expression recursion limit exceeded. limit: 32
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
| ................................^
ERROR: <input>: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
=====>
Expand Down
6 changes: 0 additions & 6 deletions parser/src/test/resources/pratt_parser_errors.baseline
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,18 +398,12 @@ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
E: ERROR: <input>:1:33: Expression recursion limit exceeded. limit: 32
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
| ................................^
ERROR: <input>:1:33: Syntax error: expected ']'
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
| ................................^

I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
=====>
E: ERROR: <input>:1:33: Expression recursion limit exceeded. limit: 32
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
| ................................^
ERROR: <input>: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
=====>
Expand Down
Loading