From c5db1f1dc62c88bac27fafd3432a9d3a8ac6f45a Mon Sep 17 00:00:00 2001 From: Richard Marshall Date: Tue, 2 Apr 2024 18:18:51 -0700 Subject: [PATCH 1/4] Store function state in a stack As functions are called store the current execution state for the function in a stack. This includes the local variables and re.group.{N} variables. This stack is used to detect recursion and prevent an interpreter panic. In the future this information could be used for including a VCL stack trace in error output or for accessing calling function state in the debugger. --- interpreter/expression.go | 2 +- interpreter/interpreter.go | 23 +++++++++-- interpreter/statement.go | 8 ++-- interpreter/statement_test.go | 6 +-- interpreter/subroutine.go | 70 ++++++++++++++++++++++------------ interpreter/subroutine_test.go | 25 +++++++++++- 6 files changed, 97 insertions(+), 37 deletions(-) diff --git a/interpreter/expression.go b/interpreter/expression.go index a8b4507c..cac7e3e0 100644 --- a/interpreter/expression.go +++ b/interpreter/expression.go @@ -36,7 +36,7 @@ func (i *Interpreter) IdentValue(val string, withCondition bool) (value.Value, e } else if _, ok := i.ctx.Ratecounters[val]; ok { return &value.Ident{Value: val, Literal: true}, nil } else if strings.HasPrefix(val, "var.") { - if v, err := i.localVars.Get(val); err != nil { + if v, err := i.StackPointer.Locals.Get(val); err != nil { return value.Null, errors.WithStack(err) } else { return v, nil diff --git a/interpreter/interpreter.go b/interpreter/interpreter.go index cd481e11..a8d8d8be 100644 --- a/interpreter/interpreter.go +++ b/interpreter/interpreter.go @@ -22,10 +22,15 @@ import ( "github.com/ysugimoto/falco/parser" ) +type StackFrame struct { + Locals variable.LocalVariables + Regex map[string]*value.String + Subroutine *ast.SubroutineDeclaration +} + type Interpreter struct { - vars variable.Variable - localVars variable.LocalVariables - lock sync.Mutex + vars variable.Variable + lock sync.Mutex options []context.Option @@ -36,15 +41,25 @@ type Interpreter struct { IdentResolver func(v string) value.Value TestingState State + + Stack []*StackFrame + StackPointer *StackFrame } func New(options ...context.Option) *Interpreter { + stack := []*StackFrame{ + { + Locals: variable.LocalVariables{}, + Regex: map[string]*value.String{}, + }, + } return &Interpreter{ options: options, cache: cache.New(), - localVars: variable.LocalVariables{}, Debugger: DefaultDebugger{}, TestingState: NONE, + Stack: stack, + StackPointer: stack[0], } } diff --git a/interpreter/statement.go b/interpreter/statement.go index 919aee57..4ef558d6 100644 --- a/interpreter/statement.go +++ b/interpreter/statement.go @@ -196,7 +196,7 @@ func (i *Interpreter) ProcessBlockStatement( } func (i *Interpreter) ProcessDeclareStatement(stmt *ast.DeclareStatement) error { - return i.localVars.Declare(stmt.Name.Value, stmt.ValueType.Value) + return i.StackPointer.Locals.Declare(stmt.Name.Value, stmt.ValueType.Value) } func (i *Interpreter) ProcessReturnStatement(stmt *ast.ReturnStatement) State { @@ -213,7 +213,7 @@ func (i *Interpreter) ProcessSetStatement(stmt *ast.SetStatement) error { } if strings.HasPrefix(stmt.Ident.Value, "var.") { - err = i.localVars.Set(stmt.Ident.Value, stmt.Operator.Operator, right) + err = i.StackPointer.Locals.Set(stmt.Ident.Value, stmt.Operator.Operator, right) } else { err = i.vars.Set(i.ctx.Scope, stmt.Ident.Value, stmt.Operator.Operator, right) } @@ -252,7 +252,7 @@ func (i *Interpreter) ProcessAddStatement(stmt *ast.AddStatement) error { func (i *Interpreter) ProcessUnsetStatement(stmt *ast.UnsetStatement) error { var err error if strings.HasPrefix(stmt.Ident.Value, "var.") { - err = i.localVars.Unset(stmt.Ident.Value) + err = i.StackPointer.Locals.Unset(stmt.Ident.Value) } else { err = i.vars.Unset(i.ctx.Scope, stmt.Ident.Value) } @@ -267,7 +267,7 @@ func (i *Interpreter) ProcessRemoveStatement(stmt *ast.RemoveStatement) error { // Alias of unset var err error if strings.HasPrefix(stmt.Ident.Value, "var.") { - err = i.localVars.Unset(stmt.Ident.Value) + err = i.StackPointer.Locals.Unset(stmt.Ident.Value) } else { err = i.vars.Unset(i.ctx.Scope, stmt.Ident.Value) } diff --git a/interpreter/statement_test.go b/interpreter/statement_test.go index 0b615d29..a7aaa433 100644 --- a/interpreter/statement_test.go +++ b/interpreter/statement_test.go @@ -95,7 +95,7 @@ func TestDeclareStatement(t *testing.T) { for _, tt := range tests { ip := New(nil) - err := ip.localVars.Declare(tt.decl.Name.Value, tt.decl.ValueType.Value) + err := ip.StackPointer.Locals.Declare(tt.decl.Name.Value, tt.decl.ValueType.Value) if err != nil { if !tt.isError { t.Errorf("%s: unexpected error returned: %s", tt.name, err) @@ -103,7 +103,7 @@ func TestDeclareStatement(t *testing.T) { continue } - v, err := ip.localVars.Get(tt.decl.Name.Value) + v, err := ip.StackPointer.Locals.Get(tt.decl.Name.Value) if err != nil { t.Errorf("%s: %s varible must be declared: %s", tt.name, tt.decl.Name.Value, err) continue @@ -191,7 +191,7 @@ func TestSetStatement(t *testing.T) { for _, tt := range tests { ip := New(nil) - if err := ip.localVars.Declare("var.foo", "INTEGER"); err != nil { + if err := ip.StackPointer.Locals.Declare("var.foo", "INTEGER"); err != nil { t.Errorf("%s: unexpected error returned: %s", tt.name, err) } diff --git a/interpreter/subroutine.go b/interpreter/subroutine.go index 9a57924c..d9d0c8e3 100644 --- a/interpreter/subroutine.go +++ b/interpreter/subroutine.go @@ -20,20 +20,47 @@ func (i *Interpreter) ProcessTestSubroutine(scope context.Scope, sub *ast.Subrou return nil } -func (i *Interpreter) ProcessSubroutine(sub *ast.SubroutineDeclaration, ds DebugState) (State, error) { - i.process.Flows = append(i.process.Flows, process.NewFlow(i.ctx, sub)) +func (i *Interpreter) subroutineInStack(sub *ast.SubroutineDeclaration) bool { + for _, s := range i.Stack { + if s.Subroutine == sub { + return true + } + } + return false +} + +func (i *Interpreter) pushStackFrame(sub *ast.SubroutineDeclaration) { + sf := &StackFrame{ + Locals: variable.LocalVariables{}, + Regex: make(map[string]*value.String), + Subroutine: sub, + } + i.Stack = append(i.Stack, sf) + i.StackPointer = sf + i.ctx.RegexMatchedValues = sf.Regex +} - // Store the current values and restore after subroutine has ended - regex := i.ctx.RegexMatchedValues - local := i.localVars - i.ctx.RegexMatchedValues = make(map[string]*value.String) - i.localVars = variable.LocalVariables{} +func (i *Interpreter) popStackFrame() { + var sf *StackFrame + sf, i.Stack = i.Stack[len(i.Stack)-1], i.Stack[:len(i.Stack)-1] + if len(i.Stack) > 0 { + i.StackPointer = i.Stack[len(i.Stack)-1] + } else { + i.StackPointer = nil + } + i.ctx.SubroutineCalls[sf.Subroutine.Name.Value]++ + i.ctx.RegexMatchedValues = sf.Regex +} - defer func() { - i.ctx.RegexMatchedValues = regex - i.localVars = local - i.ctx.SubroutineCalls[sub.Name.Value]++ - }() +func (i *Interpreter) ProcessSubroutine(sub *ast.SubroutineDeclaration, ds DebugState) (State, error) { + if i.subroutineInStack(sub) { + return NONE, errors.WithStack( + errors.Errorf("Recursion detected, subroutine %s already in stack", sub.Name.Value), + ) + } + i.process.Flows = append(i.process.Flows, process.NewFlow(i.ctx, sub)) + i.pushStackFrame(sub) + defer i.popStackFrame() // Try to extract fastly reserved subroutine macro if err := i.extractBoilerplateMacro(sub); err != nil { @@ -52,19 +79,14 @@ func (i *Interpreter) ProcessSubroutine(sub *ast.SubroutineDeclaration, ds Debug // nolint: gocognit func (i *Interpreter) ProcessFunctionSubroutine(sub *ast.SubroutineDeclaration, ds DebugState) (value.Value, State, error) { + if i.subroutineInStack(sub) { + return value.Null, NONE, errors.WithStack( + errors.Errorf("Recursion detected, subroutine %s already in stack", sub.Name.Value), + ) + } i.process.Flows = append(i.process.Flows, process.NewFlow(i.ctx, sub)) - - // Store the current values and restore after subroutine has ended - regex := i.ctx.RegexMatchedValues - local := i.localVars - i.ctx.RegexMatchedValues = make(map[string]*value.String) - i.localVars = variable.LocalVariables{} - - defer func() { - i.ctx.RegexMatchedValues = regex - i.localVars = local - i.ctx.SubroutineCalls[sub.Name.Value]++ - }() + i.pushStackFrame(sub) + defer i.popStackFrame() var err error var debugState DebugState = ds diff --git a/interpreter/subroutine_test.go b/interpreter/subroutine_test.go index b1350bf9..2561fc56 100644 --- a/interpreter/subroutine_test.go +++ b/interpreter/subroutine_test.go @@ -44,6 +44,17 @@ func TestSubroutine(t *testing.T) { }`, isError: true, }, + { + name: "Recursion produces an error not a panic", + vcl: `sub func { + call func(); + } + + sub vcl_recv { + call func(); + }`, + isError: true, + }, } for _, tt := range tests { @@ -58,6 +69,7 @@ func TestFunctionSubroutine(t *testing.T) { name string vcl string assertions map[string]value.Value + isError bool }{ { name: "Functional subroutine returns a value", @@ -134,11 +146,22 @@ func TestFunctionSubroutine(t *testing.T) { "req.http.X-Int-Value": &value.String{Value: "2"}, }, }, + { + name: "Recursion produces an error not a panic", + vcl: `sub func STRING { + return func(); + } + + sub vcl_recv { + set req.http.foo = func(); + }`, + isError: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assertInterpreter(t, tt.vcl, context.RecvScope, tt.assertions, false) + assertInterpreter(t, tt.vcl, context.RecvScope, tt.assertions, tt.isError) }) } } From 5a62f44b1e05ae3efce3db75d3653c80c654eb82 Mon Sep 17 00:00:00 2001 From: Richard Marshall Date: Thu, 4 Apr 2024 21:19:05 -0700 Subject: [PATCH 2/4] Set max stackdepth to 100 --- interpreter/interpreter.go | 2 ++ interpreter/subroutine.go | 14 +++++++++++--- interpreter/subroutine_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/interpreter/interpreter.go b/interpreter/interpreter.go index a8d8d8be..613859d9 100644 --- a/interpreter/interpreter.go +++ b/interpreter/interpreter.go @@ -22,6 +22,8 @@ import ( "github.com/ysugimoto/falco/parser" ) +const MaxStackDepth = 100 + type StackFrame struct { Locals variable.LocalVariables Regex map[string]*value.String diff --git a/interpreter/subroutine.go b/interpreter/subroutine.go index d9d0c8e3..00b14ced 100644 --- a/interpreter/subroutine.go +++ b/interpreter/subroutine.go @@ -29,15 +29,19 @@ func (i *Interpreter) subroutineInStack(sub *ast.SubroutineDeclaration) bool { return false } -func (i *Interpreter) pushStackFrame(sub *ast.SubroutineDeclaration) { +func (i *Interpreter) pushStackFrame(sub *ast.SubroutineDeclaration) error { sf := &StackFrame{ Locals: variable.LocalVariables{}, Regex: make(map[string]*value.String), Subroutine: sub, } i.Stack = append(i.Stack, sf) + if len(i.Stack) > MaxStackDepth { + return errors.WithStack(exception.Runtime(&sub.Token, "max stack depth exceeded")) + } i.StackPointer = sf i.ctx.RegexMatchedValues = sf.Regex + return nil } func (i *Interpreter) popStackFrame() { @@ -59,7 +63,9 @@ func (i *Interpreter) ProcessSubroutine(sub *ast.SubroutineDeclaration, ds Debug ) } i.process.Flows = append(i.process.Flows, process.NewFlow(i.ctx, sub)) - i.pushStackFrame(sub) + if err := i.pushStackFrame(sub); err != nil { + return NONE, errors.WithStack(err) + } defer i.popStackFrame() // Try to extract fastly reserved subroutine macro @@ -85,7 +91,9 @@ func (i *Interpreter) ProcessFunctionSubroutine(sub *ast.SubroutineDeclaration, ) } i.process.Flows = append(i.process.Flows, process.NewFlow(i.ctx, sub)) - i.pushStackFrame(sub) + if err := i.pushStackFrame(sub); err != nil { + return value.Null, NONE, errors.WithStack(err) + } defer i.popStackFrame() var err error diff --git a/interpreter/subroutine_test.go b/interpreter/subroutine_test.go index 2561fc56..867bad0c 100644 --- a/interpreter/subroutine_test.go +++ b/interpreter/subroutine_test.go @@ -3,8 +3,12 @@ package interpreter import ( "testing" + "github.com/ysugimoto/falco/ast" "github.com/ysugimoto/falco/interpreter/context" + "github.com/ysugimoto/falco/interpreter/process" "github.com/ysugimoto/falco/interpreter/value" + "github.com/ysugimoto/falco/lexer" + "github.com/ysugimoto/falco/parser" ) func TestSubroutine(t *testing.T) { @@ -165,3 +169,28 @@ func TestFunctionSubroutine(t *testing.T) { }) } } + +func TestMaxStackDepth(t *testing.T) { + vcl, err := parser.New(lexer.NewFromString(`sub test STRING { return ""; }`)).ParseVCL() + if err != nil { + t.Errorf("VCL parser error: %s", err) + return + } + ip := New() + ip.ctx = context.New() + ip.process = process.New() + if err = ip.ProcessDeclarations(vcl.Statements); err != nil { + t.Errorf("Failed to process statement: %s", err) + return + } + for i := 0; i < MaxStackDepth; i++ { + ip.Stack = append(ip.Stack, &StackFrame{Subroutine: &ast.SubroutineDeclaration{}}) + } + n := ast.FunctionCallExpression{ + Function: &ast.Ident{Value: "test"}, + } + _, err = ip.ProcessFunctionCallExpression(&n, false) + if err == nil { + t.Error("Expected error got nil") + } +} From 765ebef3b1ccfef49cb24494ce7ebb4cbea28810 Mon Sep 17 00:00:00 2001 From: Richard Marshall Date: Thu, 4 Apr 2024 21:14:44 -0700 Subject: [PATCH 3/4] re.group return NotSet when unset --- interpreter/variable/all.go | 1 + 1 file changed, 1 insertion(+) diff --git a/interpreter/variable/all.go b/interpreter/variable/all.go index 2961e172..f1964ff4 100644 --- a/interpreter/variable/all.go +++ b/interpreter/variable/all.go @@ -636,6 +636,7 @@ func (v *AllScopeVariables) getFromRegex(name string) value.Value { if val, ok := v.ctx.RegexMatchedValues[match[1]]; ok { return val } + return &value.String{IsNotSet: true} } // HTTP request header matching From a42813b8b7cfcb3893df3789acbfe41cc37c4cc7 Mon Sep 17 00:00:00 2001 From: Richard Marshall Date: Thu, 4 Apr 2024 21:19:43 -0700 Subject: [PATCH 4/4] Remove regex matches from stack frame --- interpreter/interpreter.go | 8 +------- interpreter/subroutine.go | 3 --- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/interpreter/interpreter.go b/interpreter/interpreter.go index 613859d9..3138061e 100644 --- a/interpreter/interpreter.go +++ b/interpreter/interpreter.go @@ -26,7 +26,6 @@ const MaxStackDepth = 100 type StackFrame struct { Locals variable.LocalVariables - Regex map[string]*value.String Subroutine *ast.SubroutineDeclaration } @@ -49,12 +48,7 @@ type Interpreter struct { } func New(options ...context.Option) *Interpreter { - stack := []*StackFrame{ - { - Locals: variable.LocalVariables{}, - Regex: map[string]*value.String{}, - }, - } + stack := []*StackFrame{{Locals: variable.LocalVariables{}}} return &Interpreter{ options: options, cache: cache.New(), diff --git a/interpreter/subroutine.go b/interpreter/subroutine.go index 00b14ced..6f585adf 100644 --- a/interpreter/subroutine.go +++ b/interpreter/subroutine.go @@ -32,7 +32,6 @@ func (i *Interpreter) subroutineInStack(sub *ast.SubroutineDeclaration) bool { func (i *Interpreter) pushStackFrame(sub *ast.SubroutineDeclaration) error { sf := &StackFrame{ Locals: variable.LocalVariables{}, - Regex: make(map[string]*value.String), Subroutine: sub, } i.Stack = append(i.Stack, sf) @@ -40,7 +39,6 @@ func (i *Interpreter) pushStackFrame(sub *ast.SubroutineDeclaration) error { return errors.WithStack(exception.Runtime(&sub.Token, "max stack depth exceeded")) } i.StackPointer = sf - i.ctx.RegexMatchedValues = sf.Regex return nil } @@ -53,7 +51,6 @@ func (i *Interpreter) popStackFrame() { i.StackPointer = nil } i.ctx.SubroutineCalls[sf.Subroutine.Name.Value]++ - i.ctx.RegexMatchedValues = sf.Regex } func (i *Interpreter) ProcessSubroutine(sub *ast.SubroutineDeclaration, ds DebugState) (State, error) {