From c548e8f97afcfbce6ba24ff3693f3eaeacc49b79 Mon Sep 17 00:00:00 2001 From: Anton Medvedev Date: Tue, 3 Jun 2025 17:12:39 +0200 Subject: [PATCH 1/3] Add expr.MaxNodes() option --- conf/config.go | 44 +++++++++++++++--------------- expr.go | 8 ++++++ expr_test.go | 63 +++++++++++++++++++++++++++++-------------- parser/parser_test.go | 2 +- vm/vm.go | 1 - 5 files changed, 73 insertions(+), 45 deletions(-) diff --git a/conf/config.go b/conf/config.go index d629958e5..47876ca17 100644 --- a/conf/config.go +++ b/conf/config.go @@ -10,43 +10,41 @@ import ( "github.com/expr-lang/expr/vm/runtime" ) -const ( +var ( // DefaultMemoryBudget represents an upper limit of memory usage DefaultMemoryBudget uint = 1e6 // DefaultMaxNodes represents an upper limit of AST nodes - DefaultMaxNodes uint = 10000 + DefaultMaxNodes uint = 1e4 ) type FunctionsTable map[string]*builtin.Function type Config struct { - EnvObject any - Env nature.Nature - Expect reflect.Kind - ExpectAny bool - Optimize bool - Strict bool - Profile bool - MaxNodes uint - MemoryBudget uint - ConstFns map[string]reflect.Value - Visitors []ast.Visitor - Functions FunctionsTable - Builtins FunctionsTable - Disabled map[string]bool // disabled builtins + EnvObject any + Env nature.Nature + Expect reflect.Kind + ExpectAny bool + Optimize bool + Strict bool + Profile bool + MaxNodes uint + ConstFns map[string]reflect.Value + Visitors []ast.Visitor + Functions FunctionsTable + Builtins FunctionsTable + Disabled map[string]bool // disabled builtins } // CreateNew creates new config with default values. func CreateNew() *Config { c := &Config{ - Optimize: true, - MaxNodes: DefaultMaxNodes, - MemoryBudget: DefaultMemoryBudget, - ConstFns: make(map[string]reflect.Value), - Functions: make(map[string]*builtin.Function), - Builtins: make(map[string]*builtin.Function), - Disabled: make(map[string]bool), + Optimize: true, + MaxNodes: DefaultMaxNodes, + ConstFns: make(map[string]reflect.Value), + Functions: make(map[string]*builtin.Function), + Builtins: make(map[string]*builtin.Function), + Disabled: make(map[string]bool), } for _, f := range builtin.Builtins { c.Builtins[f.Name] = f diff --git a/expr.go b/expr.go index 33b7cf354..5456f4454 100644 --- a/expr.go +++ b/expr.go @@ -195,6 +195,14 @@ func Timezone(name string) Option { }) } +// MaxNodes sets the maximum number of nodes allowed in the expression. +// By default, the maximum number of nodes is conf.DefaultMaxNodes. +func MaxNodes(n uint) Option { + return func(c *conf.Config) { + c.MaxNodes = n + } +} + // Compile parses and compiles given input expression to bytecode program. func Compile(input string, ops ...Option) (*vm.Program, error) { config := conf.CreateNew() diff --git a/expr_test.go b/expr_test.go index 241767a09..73d414d09 100644 --- a/expr_test.go +++ b/expr_test.go @@ -13,6 +13,7 @@ import ( "github.com/expr-lang/expr/internal/testify/assert" "github.com/expr-lang/expr/internal/testify/require" "github.com/expr-lang/expr/types" + "github.com/expr-lang/expr/vm" "github.com/expr-lang/expr" "github.com/expr-lang/expr/ast" @@ -2225,26 +2226,6 @@ func TestEval_slices_out_of_bound(t *testing.T) { } } -func TestMemoryBudget(t *testing.T) { - tests := []struct { - code string - }{ - {`map(1..100, {map(1..100, {map(1..100, {0})})})`}, - {`len(1..10000000)`}, - } - - for _, tt := range tests { - t.Run(tt.code, func(t *testing.T) { - program, err := expr.Compile(tt.code) - require.NoError(t, err, "compile error") - - _, err = expr.Run(program, nil) - assert.Error(t, err, "run error") - assert.Contains(t, err.Error(), "memory budget exceeded") - }) - } -} - func TestExpr_custom_tests(t *testing.T) { f, err := os.Open("custom_tests.json") if os.IsNotExist(err) { @@ -2731,3 +2712,45 @@ func TestIssue785_get_nil(t *testing.T) { }) } } + +func TestMaxNodes(t *testing.T) { + maxNodes := uint(100) + + code := "" + for i := 0; i < int(maxNodes); i++ { + code += "1; " + } + + _, err := expr.Compile(code, expr.MaxNodes(maxNodes)) + require.Error(t, err) + assert.Contains(t, err.Error(), "exceeds maximum allowed nodes") + + _, err = expr.Compile(code, expr.MaxNodes(maxNodes+1)) + require.NoError(t, err) +} + +func TestMemoryBudget(t *testing.T) { + tests := []struct { + code string + max int + }{ + {`map(1..100, {map(1..100, {map(1..100, {0})})})`, -1}, + {`len(1..10000000)`, -1}, + {`1..100`, 100}, + } + + for _, tt := range tests { + t.Run(tt.code, func(t *testing.T) { + program, err := expr.Compile(tt.code) + require.NoError(t, err, "compile error") + + vm := vm.VM{} + if tt.max > 0 { + vm.MemoryBudget = uint(tt.max) + } + _, err = vm.Run(program, nil) + require.Error(t, err, "run error") + assert.Contains(t, err.Error(), "memory budget exceeded") + }) + } +} diff --git a/parser/parser_test.go b/parser/parser_test.go index 16dac6b79..5ea005a1b 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -1240,6 +1240,6 @@ func TestNodeBudgetDisabled(t *testing.T) { _, err := parser.ParseWithConfig(expr, config) if err != nil && strings.Contains(err.Error(), "exceeds maximum allowed nodes") { - t.Error("Node budget check should be disabled when MaxNodes is 0") + t.Error("Node budget check should be disabled when DefaultMaxNodes is 0") } } diff --git a/vm/vm.go b/vm/vm.go index de13cade1..3018619d9 100644 --- a/vm/vm.go +++ b/vm/vm.go @@ -75,7 +75,6 @@ func (vm *VM) Run(program *Program, env any) (_ any, err error) { if len(vm.Variables) < program.variables { vm.Variables = make([]any, program.variables) } - if vm.MemoryBudget == 0 { vm.MemoryBudget = conf.DefaultMemoryBudget } From 0d1e62050a05c8ed6743846993b7bb2de340ed64 Mon Sep 17 00:00:00 2001 From: Anton Medvedev Date: Tue, 3 Jun 2025 17:14:52 +0200 Subject: [PATCH 2/3] Fix error message --- parser/parser_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parser/parser_test.go b/parser/parser_test.go index 5ea005a1b..16dac6b79 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -1240,6 +1240,6 @@ func TestNodeBudgetDisabled(t *testing.T) { _, err := parser.ParseWithConfig(expr, config) if err != nil && strings.Contains(err.Error(), "exceeds maximum allowed nodes") { - t.Error("Node budget check should be disabled when DefaultMaxNodes is 0") + t.Error("Node budget check should be disabled when MaxNodes is 0") } } From 2ba671e5669e53711a215021faf5f85d8568001c Mon Sep 17 00:00:00 2001 From: Anton Medvedev Date: Tue, 3 Jun 2025 17:19:26 +0200 Subject: [PATCH 3/3] Add more tests and comments --- conf/config.go | 4 ++-- expr.go | 1 + expr_test.go | 11 +++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/conf/config.go b/conf/config.go index 47876ca17..2c14d9882 100644 --- a/conf/config.go +++ b/conf/config.go @@ -11,10 +11,10 @@ import ( ) var ( - // DefaultMemoryBudget represents an upper limit of memory usage + // DefaultMemoryBudget represents default maximum allowed memory usage by the vm.VM. DefaultMemoryBudget uint = 1e6 - // DefaultMaxNodes represents an upper limit of AST nodes + // DefaultMaxNodes represents default maximum allowed AST nodes by the compiler. DefaultMaxNodes uint = 1e4 ) diff --git a/expr.go b/expr.go index 5456f4454..48298fe7e 100644 --- a/expr.go +++ b/expr.go @@ -197,6 +197,7 @@ func Timezone(name string) Option { // MaxNodes sets the maximum number of nodes allowed in the expression. // By default, the maximum number of nodes is conf.DefaultMaxNodes. +// If MaxNodes is set to 0, the node budget check is disabled. func MaxNodes(n uint) Option { return func(c *conf.Config) { c.MaxNodes = n diff --git a/expr_test.go b/expr_test.go index 73d414d09..57d2cfbbb 100644 --- a/expr_test.go +++ b/expr_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/expr-lang/expr/conf" "github.com/expr-lang/expr/internal/testify/assert" "github.com/expr-lang/expr/internal/testify/require" "github.com/expr-lang/expr/types" @@ -2729,6 +2730,16 @@ func TestMaxNodes(t *testing.T) { require.NoError(t, err) } +func TestMaxNodesDisabled(t *testing.T) { + code := "" + for i := 0; i < 2*int(conf.DefaultMaxNodes); i++ { + code += "1; " + } + + _, err := expr.Compile(code, expr.MaxNodes(0)) + require.NoError(t, err) +} + func TestMemoryBudget(t *testing.T) { tests := []struct { code string