Skip to content

Add limit for expression nodes in parsed ASTs. #2166

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

Merged
copybara-service merged 1 commit into from
Jul 27, 2026
Merged

Add limit for expression nodes in parsed ASTs. #2166

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
1 change: 1 addition & 0 deletions parser/internal/options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ inline constexpr int kDefaultErrorRecoveryLimit = 12;
inline constexpr int kDefaultMaxRecursionDepth = 32;
inline constexpr int kExpressionSizeCodepointLimit = 100'000;
inline constexpr int kDefaultErrorRecoveryTokenLookaheadLimit = 512;
inline constexpr int kDefaultExpressionNodeLimit = 100'000;
inline constexpr bool kDefaultAddMacroCalls = false;

} // namespace cel::parser_internal
Expand Down
10 changes: 10 additions & 0 deletions parser/internal/pratt_parser_worker.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,24 @@ void ParserWorker::SynchronizeOnDelimiter() {
NextToken();
}
}

int64_t ParserWorker::NextId(int32_t position) {
int64_t id = next_id_++;
if (id > options_.expression_node_limit) {
ReportError(position, "expression node limit exceeded");
}
if (position >= 0) {
positions_.insert({id, position});
}
return id;
}

int64_t ParserWorker::NextId() { return NextId(-1); }

bool ParserWorker::NodeLimitExceeded() {
return next_id_ > options_.expression_node_limit;
}

int64_t ParserWorker::CopyId(int64_t id) {
if (id == 0) {
return 0;
Expand Down
8 changes: 7 additions & 1 deletion parser/internal/pratt_parser_worker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ class ParserWorker {
// ID and Position tracking
int64_t NextId(int32_t position);
int64_t NextId(const Token& token) { return NextId(token.start); }
int64_t NextId() { return next_id_++; }
int64_t NextId();
bool NodeLimitExceeded();
int64_t CopyId(int64_t id);
void EraseId(int64_t id);

Expand Down Expand Up @@ -1050,6 +1051,11 @@ std::optional<ExprNode> PrattParserWorker<ExprNode>::TryExpandMacro(
if (!expander) {
return std::nullopt;
}
if (NodeLimitExceeded()) {
ReportError(expr_id,
"could not expand macro: expression node limit exceeded");
return std::nullopt;
}

std::vector<ExprNode> macro_args;
ExprNode macro_target;
Expand Down
8 changes: 8 additions & 0 deletions parser/options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ struct ParserOptions final {
int error_recovery_token_lookahead_limit =
::cel::parser_internal::kDefaultErrorRecoveryTokenLookaheadLimit;

// Limit on the number of expression nodes in the abstract syntax tree for the
// expression. This prevents cases where macro expansion results in an AST
// that is larger than expected from the source expression. Once exceeded,
// the parser will record an error and stop expanding macros but continue
// parsing to report other errors.
int expression_node_limit =
::cel::parser_internal::kDefaultExpressionNodeLimit;

// Add macro calls to macro_calls list in source_info.
bool add_macro_calls = ::cel::parser_internal::kDefaultAddMacroCalls;

Expand Down
122 changes: 76 additions & 46 deletions parser/parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,9 @@ SourceRange SourceRangeFromParserRuleContext(

class ParserMacroExprFactory final : public MacroExprFactory {
public:
explicit ParserMacroExprFactory(const cel::Source& source)
: source_(source) {}
explicit ParserMacroExprFactory(const cel::Source& source,
int expression_node_limit)
: source_(source), expression_node_limit_(expression_node_limit) {}

void BeginMacro(SourceRange macro_position) {
macro_position_ = macro_position;
Expand Down Expand Up @@ -203,12 +204,18 @@ class ParserMacroExprFactory final : public MacroExprFactory {

int64_t NextId(const SourceRange& range) {
auto id = expr_id_++;
if (id > expression_node_limit_ && !node_limit_exceeded_) {
node_limit_exceeded_ = true;
ReportError(range, "expression node limit exceeded");
}
if (range.begin != -1 || range.end != -1) {
positions_.insert(std::pair{id, range});
}
return id;
}

bool is_node_limit_exceeded() const { return node_limit_exceeded_; }

bool HasErrors() const { return error_count_ != 0; }

std::vector<cel::ParseIssue> CollectIssues() {
Expand Down Expand Up @@ -409,6 +416,8 @@ class ParserMacroExprFactory final : public MacroExprFactory {
std::vector<ParserError> errors_;
size_t error_count_ = 0;
const Source& source_;
int expression_node_limit_;
bool node_limit_exceeded_ = false;
SourceRange macro_position_;
};

Expand Down Expand Up @@ -623,13 +632,14 @@ class ParserVisitor final : public CelBaseVisitor,
public antlr4::BaseErrorListener {
public:
ParserVisitor(const cel::Source& source, int max_recursion_depth,
int max_expression_node_count,
const cel::MacroRegistry& macro_registry,
bool add_macro_calls = false,
bool enable_optional_syntax = false,
bool enable_quoted_identifiers = false,
bool enable_variadic_logical_operators = false)
: source_(source),
factory_(source_),
factory_(source_, max_expression_node_count),
macro_registry_(macro_registry),
recursion_depth_(0),
max_recursion_depth_(max_recursion_depth),
Expand Down Expand Up @@ -1227,6 +1237,8 @@ std::vector<ListExprElement> ParserVisitor::visitList(
if (!enable_optional_syntax_ && expr_ctx->opt != nullptr) {
factory_.ReportError(SourceRangeFromParserRuleContext(ctx),
"unsupported syntax '?'");
// Still generate an ID to detect node limit exceeded.
factory_.NextId(SourceRangeFromParserRuleContext(ctx));
rv.push_back(factory_.NewListElement(factory_.NewUnspecified(0), false));
continue;
}
Expand Down Expand Up @@ -1298,6 +1310,9 @@ std::vector<MapExprEntry> ParserVisitor::visitEntries(
if (!enable_optional_syntax_ && ctx->keys[i]->opt) {
factory_.ReportError(SourceRangeFromParserRuleContext(ctx),
"unsupported syntax '?'");
// Still generate an ID to detect node limit exceeded.
factory_.NextId(SourceRangeFromParserRuleContext(ctx));
factory_.NextId(SourceRangeFromParserRuleContext(ctx));
res.push_back(factory_.NewMapEntry(0, factory_.NewUnspecified(0),
factory_.NewUnspecified(0), false));
continue;
Expand Down Expand Up @@ -1461,60 +1476,74 @@ std::vector<cel::ParseIssue> ParserVisitor::CollectIssues() {
Expr ParserVisitor::GlobalCallOrMacroImpl(int64_t expr_id,
absl::string_view function,
std::vector<Expr> args) {
if (auto macro = macro_registry_.FindMacro(function, args.size(), false);
macro) {
std::vector<Expr> macro_args;
if (add_macro_calls_) {
macro_args.reserve(args.size());
for (const auto& arg : args) {
macro_args.push_back(factory_.BuildMacroCallArg(arg));
}
auto macro = macro_registry_.FindMacro(function, args.size(), false);
if (!macro) {
return factory_.NewCall(expr_id, function, std::move(args));
}
if (factory_.is_node_limit_exceeded()) {
return factory_.ReportError(
factory_.GetSourceRange(expr_id),
"could not expand macro: expression node limit exceeded");
}
std::vector<Expr> macro_args;
if (add_macro_calls_) {
macro_args.reserve(args.size());
for (const auto& arg : args) {
macro_args.push_back(factory_.BuildMacroCallArg(arg));
}
factory_.BeginMacro(factory_.GetSourceRange(expr_id));
auto expr = macro->Expand(factory_, std::nullopt, absl::MakeSpan(args));
factory_.EndMacro();
if (expr) {
if (add_macro_calls_) {
factory_.AddMacroCall(expr->id(), function, std::nullopt,
std::move(macro_args));
}
// We did not end up using `expr_id`. Delete metadata.
factory_.EraseId(expr_id);
return std::move(*expr);
}
factory_.BeginMacro(factory_.GetSourceRange(expr_id));
auto expr = macro->Expand(factory_, std::nullopt, absl::MakeSpan(args));
factory_.EndMacro();
if (expr) {
if (add_macro_calls_) {
factory_.AddMacroCall(expr->id(), function, std::nullopt,
std::move(macro_args));
}
// We did not end up using `expr_id`. Delete metadata.
factory_.EraseId(expr_id);
return std::move(*expr);
}

return factory_.NewCall(expr_id, function, std::move(args));
}

Expr ParserVisitor::ReceiverCallOrMacroImpl(int64_t expr_id,
absl::string_view function,
Expr target,
std::vector<Expr> args) {
if (auto macro = macro_registry_.FindMacro(function, args.size(), true);
macro) {
Expr macro_target;
std::vector<Expr> macro_args;
if (add_macro_calls_) {
macro_args.reserve(args.size());
macro_target = factory_.BuildMacroCallArg(target);
for (const auto& arg : args) {
macro_args.push_back(factory_.BuildMacroCallArg(arg));
}
auto macro = macro_registry_.FindMacro(function, args.size(), true);
if (!macro) {
return factory_.NewMemberCall(expr_id, function, std::move(target),
std::move(args));
}
if (factory_.is_node_limit_exceeded()) {
return factory_.ReportError(
factory_.GetSourceRange(expr_id),
"could not expand macro: expression node limit exceeded");
}

Expr macro_target;
std::vector<Expr> macro_args;
if (add_macro_calls_) {
macro_args.reserve(args.size());
macro_target = factory_.BuildMacroCallArg(target);
for (const auto& arg : args) {
macro_args.push_back(factory_.BuildMacroCallArg(arg));
}
factory_.BeginMacro(factory_.GetSourceRange(expr_id));
auto expr = macro->Expand(factory_, std::ref(target), absl::MakeSpan(args));
factory_.EndMacro();
if (expr) {
if (add_macro_calls_) {
factory_.AddMacroCall(expr->id(), function, std::move(macro_target),
std::move(macro_args));
}
// We did not end up using `expr_id`. Delete metadata.
factory_.EraseId(expr_id);
return std::move(*expr);
}
factory_.BeginMacro(factory_.GetSourceRange(expr_id));
auto expr = macro->Expand(factory_, std::ref(target), absl::MakeSpan(args));
factory_.EndMacro();
if (expr) {
if (add_macro_calls_) {
factory_.AddMacroCall(expr->id(), function, std::move(macro_target),
std::move(macro_args));
}
// We did not end up using `expr_id`. Delete metadata.
factory_.EraseId(expr_id);
return std::move(*expr);
}

return factory_.NewMemberCall(expr_id, function, std::move(target),
std::move(args));
}
Expand Down Expand Up @@ -1677,8 +1706,9 @@ absl::StatusOr<ParseResult> ParseImpl(
CelParser parser(&tokens);
ExprRecursionListener listener(options.max_recursion_depth);
ParserVisitor visitor(
source, options.max_recursion_depth, registry, options.add_macro_calls,
options.enable_optional_syntax, options.enable_quoted_identifiers,
source, options.max_recursion_depth, options.expression_node_limit,
registry, options.add_macro_calls, options.enable_optional_syntax,
options.enable_quoted_identifiers,
options.enable_variadic_logical_operators);

lexer.removeErrorListeners();
Expand Down
49 changes: 49 additions & 0 deletions parser/parser_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2140,6 +2140,55 @@ TEST_P(ParserTest, ParseFailurePopulatesIssues) {
EXPECT_EQ(issues[0].location().column, 3);
}

TEST_P(ParserTest, ExpressionNodeLimitExceeded) {
auto builder = cel::NewParserBuilder(options_);
builder->GetOptions().expression_node_limit = 2;
ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build());

ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("a + b + c", "test.cel"));
std::vector<cel::ParseIssue> issues;
auto ast_result = parser->Parse(*source, &issues);
EXPECT_THAT(ast_result, Not(IsOk()));
ASSERT_THAT(issues, testing::Not(testing::IsEmpty()));
EXPECT_THAT(ast_result.status().message(),
HasSubstr("expression node limit exceeded"));
EXPECT_THAT(issues[0].message(), HasSubstr("expression node limit exceeded"));
}

TEST_P(ParserTest, MacroExpansionNodeLimitExceeded) {
auto builder = cel::NewParserBuilder(options_);
builder->GetOptions().expression_node_limit = 5;
ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build());

ASSERT_OK_AND_ASSIGN(
auto source, cel::NewSource("[1, 2, 3, 4, 5].map(x, x * 2)", "test.cel"));
std::vector<cel::ParseIssue> issues;
auto ast_result = parser->Parse(*source, &issues);
EXPECT_THAT(ast_result, Not(IsOk()));
ASSERT_THAT(issues, testing::Not(testing::IsEmpty()));
EXPECT_THAT(ast_result.status().message(),
HasSubstr("expression node limit exceeded"));
EXPECT_THAT(
issues,
testing::Contains(testing::Property(
&cel::ParseIssue::message,
HasSubstr(
"could not expand macro: expression node limit exceeded"))));
}

TEST_P(ParserTest, MacroExpansionNodeLimitNotExceeded) {
auto builder = cel::NewParserBuilder(options_);
builder->GetOptions().expression_node_limit = 100;
ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build());

ASSERT_OK_AND_ASSIGN(
auto source, cel::NewSource("[1, 2, 3, 4, 5].map(x, x * 2)", "test.cel"));
std::vector<cel::ParseIssue> issues;
auto ast_result = parser->Parse(*source, &issues);
ASSERT_THAT(ast_result, IsOk());
EXPECT_THAT(issues, testing::IsEmpty());
}

std::string ExpressionTestName(
const testing::TestParamInfo<std::tuple<TestInfo, bool>>& test_info) {
const TestInfo& info = std::get<0>(test_info.param);
Expand Down
Loading