From d8460f69bcc092fa9f657a3a91ffd67d0e90625c Mon Sep 17 00:00:00 2001 From: Leon Mika Date: Sun, 25 May 2025 09:50:25 +1000 Subject: [PATCH 1/3] Added separate Eval mode with option to isolate the environment This allows for keeping variables isolated --- repl/evaldisplay.go | 2 +- ucl/ast.go | 2 +- ucl/builtins/csv_test.go | 2 +- ucl/builtins/fs_test.go | 2 +- ucl/builtins/itrs_test.go | 2 +- ucl/builtins/lists_test.go | 2 +- ucl/builtins/log_test.go | 2 +- ucl/builtins/os_test.go | 2 +- ucl/builtins/strs_test.go | 12 ++--- ucl/builtins/time_test.go | 4 +- ucl/env.go | 3 ++ ucl/inst.go | 42 +++++++++++++++--- ucl/inst_test.go | 89 +++++++++++++++++++++++++++++++++++++- ucl/testbuiltins_test.go | 28 ++++++------ ucl/userbuiltin_test.go | 40 ++++++++--------- 15 files changed, 176 insertions(+), 58 deletions(-) diff --git a/repl/evaldisplay.go b/repl/evaldisplay.go index a37e74f..083e4e3 100644 --- a/repl/evaldisplay.go +++ b/repl/evaldisplay.go @@ -14,7 +14,7 @@ import ( type NoResults struct{} func (r *REPL) EvalAndDisplay(ctx context.Context, expr string) error { - res, err := r.inst.Eval(ctx, expr) + res, err := r.inst.EvalString(ctx, expr) if err != nil { if errors.Is(err, ucl.ErrNotConvertable) { return nil diff --git a/ucl/ast.go b/ucl/ast.go index ab2e2bc..3b567d9 100644 --- a/ucl/ast.go +++ b/ucl/ast.go @@ -172,6 +172,6 @@ var scanner = lexer.MustStateful(lexer.Rules{ var parser = participle.MustBuild[astScript](participle.Lexer(scanner), participle.Elide("Whitespace", "Comment")) -func parse(r io.Reader) (*astScript, error) { +func parse(fname string, r io.Reader) (*astScript, error) { return parser.Parse("test", r) } diff --git a/ucl/builtins/csv_test.go b/ucl/builtins/csv_test.go index 5b60834..56953af 100644 --- a/ucl/builtins/csv_test.go +++ b/ucl/builtins/csv_test.go @@ -45,7 +45,7 @@ func TestCSV_ReadRecord(t *testing.T) { ucl.WithOut(&bfr), ) - _, err := inst.Eval(context.Background(), tt.eval) + _, err := inst.EvalString(context.Background(), tt.eval) assert.NoError(t, err) assert.Equal(t, tt.wantOut, bfr.String()) }) diff --git a/ucl/builtins/fs_test.go b/ucl/builtins/fs_test.go index f7f0e41..818cf11 100644 --- a/ucl/builtins/fs_test.go +++ b/ucl/builtins/fs_test.go @@ -29,7 +29,7 @@ func TestFS_Cat(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.FS(testFS)), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) diff --git a/ucl/builtins/itrs_test.go b/ucl/builtins/itrs_test.go index e852209..f27ba68 100644 --- a/ucl/builtins/itrs_test.go +++ b/ucl/builtins/itrs_test.go @@ -26,7 +26,7 @@ func TestItrs_ToList(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.Itrs()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { diff --git a/ucl/builtins/lists_test.go b/ucl/builtins/lists_test.go index ca86731..526176e 100644 --- a/ucl/builtins/lists_test.go +++ b/ucl/builtins/lists_test.go @@ -34,7 +34,7 @@ func TestLists_First(t *testing.T) { ucl.WithModule(builtins.Itrs()), ucl.WithModule(builtins.Lists()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { diff --git a/ucl/builtins/log_test.go b/ucl/builtins/log_test.go index 9071c02..f4b8e95 100644 --- a/ucl/builtins/log_test.go +++ b/ucl/builtins/log_test.go @@ -34,7 +34,7 @@ func TestLog_Puts(t *testing.T) { })), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { diff --git a/ucl/builtins/os_test.go b/ucl/builtins/os_test.go index a246892..cbefcbf 100644 --- a/ucl/builtins/os_test.go +++ b/ucl/builtins/os_test.go @@ -28,7 +28,7 @@ func TestOS_Env(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.OS()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) diff --git a/ucl/builtins/strs_test.go b/ucl/builtins/strs_test.go index 3fc4cf6..c38065b 100644 --- a/ucl/builtins/strs_test.go +++ b/ucl/builtins/strs_test.go @@ -28,7 +28,7 @@ func TestStrs_ToUpper(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.Strs()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { @@ -59,7 +59,7 @@ func TestStrs_ToLower(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.Strs()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { @@ -90,7 +90,7 @@ func TestStrs_Trim(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.Strs()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { @@ -122,7 +122,7 @@ func TestStrs_HasPrefix(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.Strs()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { @@ -160,7 +160,7 @@ func TestStrs_Split(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.Strs()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { @@ -191,7 +191,7 @@ func TestStrs_Join(t *testing.T) { ucl.WithModule(builtins.Itrs()), ucl.WithModule(builtins.Strs()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { diff --git a/ucl/builtins/time_test.go b/ucl/builtins/time_test.go index 4c95b5a..a0087fa 100644 --- a/ucl/builtins/time_test.go +++ b/ucl/builtins/time_test.go @@ -25,7 +25,7 @@ func TestTime_FromUnix(t *testing.T) { inst := ucl.New( ucl.WithModule(builtins.Time()), ) - res, err := inst.Eval(context.Background(), tt.eval) + res, err := inst.EvalString(context.Background(), tt.eval) if tt.wantErr { assert.Error(t, err) } else { @@ -47,7 +47,7 @@ func TestTime_Sleep(t *testing.T) { ucl.WithModule(builtins.Time()), ) - _, err := inst.Eval(ctx, `time:sleep 1`) + _, err := inst.EvalString(ctx, `time:sleep 1`) assert.Error(t, err) assert.Equal(t, "context canceled", err.Error()) assert.True(t, time.Now().Sub(st) < time.Second) diff --git a/ucl/env.go b/ucl/env.go index 3d217ca..6d92ffb 100644 --- a/ucl/env.go +++ b/ucl/env.go @@ -61,6 +61,9 @@ func (ec *evalCtx) setOrDefineVar(name string, val Object) { func (ec *evalCtx) getVar(name string) (Object, bool) { if ec.vars == nil { + if ec.parent == nil { + return nil, false + } return ec.parent.getVar(name) } diff --git a/ucl/inst.go b/ucl/inst.go index 585bea3..791b202 100644 --- a/ucl/inst.go +++ b/ucl/inst.go @@ -141,8 +141,25 @@ func (inst *Inst) Out() io.Writer { return inst.out } -func (inst *Inst) Eval(ctx context.Context, expr string) (any, error) { - res, err := inst.eval(ctx, expr) +type EvalOption func(*evalOptions) + +func WithSubEnv() EvalOption { + return func(opts *evalOptions) { + opts.forkEnv = true + } +} + +func (inst *Inst) Eval(ctx context.Context, r io.Reader, options ...EvalOption) (any, error) { + opts := evalOptions{ + filename: "unnamed", + forkEnv: false, + } + + for _, opt := range options { + opt(&opts) + } + + res, err := inst.eval(ctx, r, opts) if err != nil { if errors.Is(err, ErrHalt) { return nil, nil @@ -158,16 +175,29 @@ func (inst *Inst) Eval(ctx context.Context, expr string) (any, error) { return goRes, nil } -func (inst *Inst) eval(ctx context.Context, expr string) (Object, error) { - ast, err := parse(strings.NewReader(expr)) +func (inst *Inst) EvalString(ctx context.Context, expr string) (any, error) { + return inst.Eval(ctx, strings.NewReader(expr)) +} + +type evalOptions struct { + filename string + forkEnv bool +} + +func (inst *Inst) eval(ctx context.Context, r io.Reader, opts evalOptions) (Object, error) { + ast, err := parse(opts.filename, r) if err != nil { return nil, err } eval := evaluator{inst: inst} - // TODO: this should be a separate forkAndIsolate() session - return eval.evalScript(ctx, inst.rootEC, ast) + env := inst.rootEC + if opts.forkEnv { + env = env.fork() + } + + return eval.evalScript(ctx, env, ast) } type PseudoVarHandler interface { diff --git a/ucl/inst_test.go b/ucl/inst_test.go index 479c845..184a3b0 100644 --- a/ucl/inst_test.go +++ b/ucl/inst_test.go @@ -3,6 +3,7 @@ package ucl_test import ( "bytes" "context" + "strings" "ucl.lmika.dev/ucl" "github.com/stretchr/testify/assert" @@ -113,7 +114,7 @@ func TestInst_Eval(t *testing.T) { outW := bytes.NewBuffer(nil) inst := ucl.New(ucl.WithOut(outW), ucl.WithTestBuiltin()) - res, err := inst.Eval(ctx, tt.expr) + res, err := inst.EvalString(ctx, tt.expr) if tt.wantErr != nil { assert.ErrorIs(t, err, tt.wantErr) @@ -129,6 +130,90 @@ func TestInst_Eval(t *testing.T) { } } +func TestInst_Eval_WithSubEnv(t *testing.T) { + t.Run("global symbols should not leak across environments", func(t *testing.T) { + ctx := t.Context() + + inst := ucl.New() + + res, err := inst.Eval(ctx, strings.NewReader(`$a = "hello" ; $a`), ucl.WithSubEnv()) + assert.NoError(t, err) + assert.Equal(t, "hello", res) + + res, err = inst.Eval(ctx, strings.NewReader(`$a`), ucl.WithSubEnv()) + assert.NoError(t, err) + assert.Nil(t, res) + }) + + t.Run("when using hooks, reading vars are not permitted to leak across environment", func(t *testing.T) { + ctx := t.Context() + + hooks := make([]ucl.Invokable, 0) + + inst := ucl.New() + inst.SetBuiltin("hook", func(ctx context.Context, args ucl.CallArgs) (any, error) { + var hookProc ucl.Invokable + + if err := args.Bind(&hookProc); err != nil { + return nil, err + } + hooks = append(hooks, hookProc) + + return nil, nil + }) + + res, err := inst.Eval(ctx, strings.NewReader(`$a = "hello" ; hook { $a }`), ucl.WithSubEnv()) + assert.NoError(t, err) + assert.Nil(t, res) + + res, err = inst.Eval(ctx, strings.NewReader(`$a = "world" ; hook { $a }`), ucl.WithSubEnv()) + assert.NoError(t, err) + assert.Nil(t, res) + + h1, err := hooks[0].Invoke(ctx, ucl.CallArgs{}) + assert.NoError(t, err) + assert.Equal(t, "hello", h1) + + h2, err := hooks[1].Invoke(ctx, ucl.CallArgs{}) + assert.NoError(t, err) + assert.Equal(t, "world", h2) + }) + + t.Run("when using hooks, writing vars are not permitted to leak across environment", func(t *testing.T) { + ctx := t.Context() + + hooks := make([]ucl.Invokable, 0) + + inst := ucl.New() + inst.SetBuiltin("hook", func(ctx context.Context, args ucl.CallArgs) (any, error) { + var hookProc ucl.Invokable + + if err := args.Bind(&hookProc); err != nil { + return nil, err + } + hooks = append(hooks, hookProc) + + return nil, nil + }) + + res, err := inst.Eval(ctx, strings.NewReader(`$a = "hello" ; hook { $a = "new value" ; $a }`), ucl.WithSubEnv()) + assert.NoError(t, err) + assert.Nil(t, res) + + res, err = inst.Eval(ctx, strings.NewReader(`$a = "world" ; hook { $a }`), ucl.WithSubEnv()) + assert.NoError(t, err) + assert.Nil(t, res) + + h1, err := hooks[0].Invoke(ctx, ucl.CallArgs{}) + assert.NoError(t, err) + assert.Equal(t, "new value", h1) + + h2, err := hooks[1].Invoke(ctx, ucl.CallArgs{}) + assert.NoError(t, err) + assert.Equal(t, "world", h2) + }) +} + func TestInst_SetPseudoVar(t *testing.T) { tests := []struct { desc string @@ -155,7 +240,7 @@ func TestInst_SetPseudoVar(t *testing.T) { inst.SetPseudoVar("bar", bar) inst.SetMissingPseudoVarHandler(missingPseudoVarType{}) - res, err := inst.Eval(t.Context(), tt.expr) + res, err := inst.EvalString(t.Context(), tt.expr) if tt.wantErr { assert.Error(t, err) diff --git a/ucl/testbuiltins_test.go b/ucl/testbuiltins_test.go index 096e446..9ac1d61 100644 --- a/ucl/testbuiltins_test.go +++ b/ucl/testbuiltins_test.go @@ -151,7 +151,7 @@ func TestBuiltins_Echo(t *testing.T) { outW := bytes.NewBuffer(nil) inst := New(WithOut(outW), WithTestBuiltin()) - res, err := inst.Eval(ctx, tt.expr) + res, err := inst.EvalString(ctx, tt.expr) assert.NoError(t, err) assert.Nil(t, res) @@ -1309,7 +1309,7 @@ func TestBuiltins_Keys(t *testing.T) { }, nil }) - res, err := inst.Eval(ctx, tt.expr) + res, err := inst.EvalString(ctx, tt.expr) assert.NoError(t, err) assert.Len(t, res, len(tt.wantItems)) for _, i := range tt.wantItems { @@ -1349,7 +1349,7 @@ func TestBuiltins_Filter(t *testing.T) { inst := New(WithOut(outW), WithTestBuiltin()) - res, err := inst.Eval(ctx, tt.expr) + res, err := inst.EvalString(ctx, tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) @@ -1375,7 +1375,7 @@ func TestBuiltins_Reduce(t *testing.T) { inst := New(WithOut(outW), WithTestBuiltin()) - res, err := inst.Eval(ctx, tt.expr) + res, err := inst.EvalString(ctx, tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) @@ -1404,7 +1404,7 @@ func TestBuiltins_Head(t *testing.T) { inst := New(WithOut(outW), WithTestBuiltin()) - res, err := inst.Eval(ctx, tt.expr) + res, err := inst.EvalString(ctx, tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) @@ -1459,7 +1459,7 @@ func TestBuiltins_LtLeGtLe(t *testing.T) { inst.SetVar("true", true) inst.SetVar("false", false) - eqRes, err := inst.Eval(ctx, tt.expr) + eqRes, err := inst.EvalString(ctx, tt.expr) if tt.wantErr { assert.Error(t, err) @@ -1527,11 +1527,11 @@ func TestBuiltins_EqNe(t *testing.T) { inst.SetVar("true", true) inst.SetVar("false", false) - eqRes, err := inst.Eval(ctx, tt.expr) + eqRes, err := inst.EvalString(ctx, tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, eqRes) - neRes, err := inst.Eval(ctx, strings.ReplaceAll(tt.expr, "eq", "ne")) + neRes, err := inst.EvalString(ctx, strings.ReplaceAll(tt.expr, "eq", "ne")) assert.NoError(t, err) assert.Equal(t, !tt.want, neRes) }) @@ -1562,7 +1562,7 @@ func TestBuiltins_Str(t *testing.T) { inst := New(WithOut(outW), WithTestBuiltin()) - eqRes, err := inst.Eval(ctx, tt.expr) + eqRes, err := inst.EvalString(ctx, tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, eqRes) }) @@ -1598,7 +1598,7 @@ func TestBuiltins_Int(t *testing.T) { inst := New(WithOut(outW), WithTestBuiltin()) - eqRes, err := inst.Eval(ctx, tt.expr) + eqRes, err := inst.EvalString(ctx, tt.expr) if tt.wantErr { assert.Error(t, err) } else { @@ -1654,7 +1654,7 @@ func TestBuiltins_AddSubMupDivMod(t *testing.T) { inst := New(WithOut(outW), WithTestBuiltin()) - eqRes, err := inst.Eval(ctx, tt.expr) + eqRes, err := inst.EvalString(ctx, tt.expr) if tt.wantErr { assert.Error(t, err) } else { @@ -1705,7 +1705,7 @@ func TestBuiltins_AndOrNot(t *testing.T) { inst.SetVar("true", true) inst.SetVar("false", false) - eqRes, err := inst.Eval(ctx, tt.expr) + eqRes, err := inst.EvalString(ctx, tt.expr) if tt.wantErr { assert.Error(t, err) } else { @@ -1742,7 +1742,7 @@ func TestBuiltins_Cat(t *testing.T) { inst.SetVar("true", true) inst.SetVar("false", false) - eqRes, err := inst.Eval(ctx, tt.expr) + eqRes, err := inst.EvalString(ctx, tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, eqRes) }) @@ -1750,7 +1750,7 @@ func TestBuiltins_Cat(t *testing.T) { } func evalAndDisplay(ctx context.Context, inst *Inst, expr string) error { - res, err := inst.eval(ctx, expr) + res, err := inst.eval(ctx, strings.NewReader(expr), evalOptions{}) if err != nil { return err } diff --git a/ucl/userbuiltin_test.go b/ucl/userbuiltin_test.go index 8b1951f..58bf5ab 100644 --- a/ucl/userbuiltin_test.go +++ b/ucl/userbuiltin_test.go @@ -24,7 +24,7 @@ func TestInst_SetBuiltin(t *testing.T) { return x + y, nil }) - res, err := inst.Eval(context.Background(), `add2 "Hello, " "World"`) + res, err := inst.EvalString(context.Background(), `add2 "Hello, " "World"`) assert.NoError(t, err) assert.Equal(t, "Hello, World", res) }) @@ -44,7 +44,7 @@ func TestInst_SetBuiltin(t *testing.T) { return x + y, nil }) - res, err := inst.Eval(context.Background(), `add2 "Hello, " "World"`) + res, err := inst.EvalString(context.Background(), `add2 "Hello, " "World"`) assert.NoError(t, err) assert.Equal(t, "Hello, World", res) }) @@ -85,7 +85,7 @@ func TestInst_SetBuiltin(t *testing.T) { for _, tt := range tests { t.Run(tt.descr, func(t *testing.T) { - res, err := inst.Eval(context.Background(), tt.expr) + res, err := inst.EvalString(context.Background(), tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) @@ -108,7 +108,7 @@ func TestInst_SetBuiltin(t *testing.T) { return pair{x, y}, nil }) - res, err := inst.Eval(context.Background(), `add2 "Hello" "World"`) + res, err := inst.EvalString(context.Background(), `add2 "Hello" "World"`) assert.NoError(t, err) assert.Equal(t, pair{"Hello", "World"}, res) }) @@ -149,7 +149,7 @@ func TestInst_SetBuiltin(t *testing.T) { return x.x + ":" + x.y, nil }) - res, err := inst.Eval(context.Background(), tt.expr) + res, err := inst.EvalString(context.Background(), tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) @@ -176,7 +176,7 @@ func TestInst_SetBuiltin(t *testing.T) { return []string{"1", "2", "3"}, nil }) - res, err := inst.Eval(context.Background(), tt.expr) + res, err := inst.EvalString(context.Background(), tt.expr) assert.NoError(t, err) assert.Equal(t, tt.want, res) assert.Equal(t, tt.wantOut, outW.String()) @@ -206,11 +206,11 @@ func TestCallArgs_Bind(t *testing.T) { return ds.DoString(), nil }) - va, err := inst.Eval(ctx, `dostr (sa)`) + va, err := inst.EvalString(ctx, `dostr (sa)`) assert.NoError(t, err) assert.Equal(t, "do string A: a val", va) - vb, err := inst.Eval(ctx, `dostr (sb)`) + vb, err := inst.EvalString(ctx, `dostr (sb)`) assert.NoError(t, err) assert.Equal(t, "do string B: foo bar", vb) }) @@ -240,7 +240,7 @@ func TestCallArgs_Bind(t *testing.T) { return fmt.Sprintf("[%v]", v), nil }) - res, err := inst.Eval(ctx, tt.eval) + res, err := inst.EvalString(ctx, tt.eval) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) @@ -295,7 +295,7 @@ func TestCallArgs_CanBind(t *testing.T) { return nil, nil }) - _, err := inst.Eval(ctx, tt.eval) + _, err := inst.EvalString(ctx, tt.eval) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) @@ -332,7 +332,7 @@ func TestCallArgs_CanBind(t *testing.T) { return h.Value(k), nil }) - res, err := inst.Eval(ctx, tt.eval) + res, err := inst.EvalString(ctx, tt.eval) if tt.wantErr { assert.Error(t, err) assert.Nil(t, res) @@ -370,7 +370,7 @@ func TestCallArgs_CanBind(t *testing.T) { ctx := context.Background() - res, err := inst.Eval(ctx, `wrap { |x| toUpper $x }`) + res, err := inst.EvalString(ctx, `wrap { |x| toUpper $x }`) assert.NoError(t, err) assert.Equal(t, "[[HELLO]]", res) }) @@ -401,7 +401,7 @@ func TestCallArgs_CanBind(t *testing.T) { assert.NoError(t, err) assert.Nil(t, before) - res, err := inst.Eval(ctx, `wrap { |x| toUpper $x }`) + res, err := inst.EvalString(ctx, `wrap { |x| toUpper $x }`) assert.NoError(t, err) assert.Nil(t, res) @@ -437,7 +437,7 @@ func TestCallArgs_MissingCommandHandler(t *testing.T) { return fmt.Sprintf("was %v", name), nil })) - res, err := inst.Eval(ctx, tt.eval) + res, err := inst.EvalString(ctx, tt.eval) assert.NoError(t, err) assert.Equal(t, tt.want, res) }) @@ -460,27 +460,27 @@ func TestCallArgs_IsTopLevel(t *testing.T) { return nil, nil }) - _, err := inst.Eval(ctx, `lvl "one"`) + _, err := inst.EvalString(ctx, `lvl "one"`) assert.NoError(t, err) assert.True(t, res["one"]) - _, err = inst.Eval(ctx, `echo (lvl "two")`) + _, err = inst.EvalString(ctx, `echo (lvl "two")`) assert.NoError(t, err) assert.True(t, res["two"]) - _, err = inst.Eval(ctx, `proc doLvl { |n| lvl $n } ; doLvl "three"`) + _, err = inst.EvalString(ctx, `proc doLvl { |n| lvl $n } ; doLvl "three"`) assert.NoError(t, err) assert.False(t, res["three"]) - _, err = inst.Eval(ctx, `doLvl "four"`) + _, err = inst.EvalString(ctx, `doLvl "four"`) assert.NoError(t, err) assert.False(t, res["four"]) - _, err = inst.Eval(ctx, `["a"] | map { |x| doLvl "five" ; $x }`) + _, err = inst.EvalString(ctx, `["a"] | map { |x| doLvl "five" ; $x }`) assert.NoError(t, err) assert.False(t, res["five"]) - _, err = inst.Eval(ctx, `if 1 { lvl "six" }`) + _, err = inst.EvalString(ctx, `if 1 { lvl "six" }`) assert.NoError(t, err) assert.True(t, res["six"]) }) From ba6d42acbb7fd7af5d9adf250d4599600079067a Mon Sep 17 00:00:00 2001 From: Leon Mika Date: Sun, 25 May 2025 10:08:56 +1000 Subject: [PATCH 2/3] Fixed bug which was not isolating procs --- ucl/builtins.go | 2 +- ucl/env.go | 36 +++++++++++---- ucl/inst.go | 6 ++- ucl/inst_test.go | 114 +++++++++++++++++++++++------------------------ 4 files changed, 89 insertions(+), 69 deletions(-) diff --git a/ucl/builtins.go b/ucl/builtins.go index da7ddc4..71bad9b 100644 --- a/ucl/builtins.go +++ b/ucl/builtins.go @@ -1161,7 +1161,7 @@ func procBuiltin(ctx context.Context, args macroArgs) (Object, error) { obj := procObject{args.eval, args.ec, blockObj.block} if procName != "" { - args.ec.addCmd(procName, obj) + args.ec.addUserCmd(procName, obj) } return obj, nil } diff --git a/ucl/env.go b/ucl/env.go index 6d92ffb..7a58cc1 100644 --- a/ucl/env.go +++ b/ucl/env.go @@ -1,18 +1,17 @@ package ucl type evalCtx struct { - root *evalCtx - parent *evalCtx - commands map[string]invokable - macros map[string]macroable - vars map[string]Object - pseudoVars map[string]pseudoVar + root *evalCtx + parent *evalCtx + commands map[string]invokable + macros map[string]macroable + vars map[string]Object + pseudoVars map[string]pseudoVar + userCommandFrame bool // Frame to use for user-defined commands } func (ec *evalCtx) forkAndIsolate() *evalCtx { - newEc := &evalCtx{parent: ec} - newEc.root = newEc - return newEc + return &evalCtx{parent: ec, root: ec.root, userCommandFrame: true} } func (ec *evalCtx) fork() *evalCtx { @@ -27,6 +26,25 @@ func (ec *evalCtx) addCmd(name string, inv invokable) { ec.root.commands[name] = inv } +func (ec *evalCtx) addUserCmd(name string, inv invokable) { + frame := ec + for frame != nil { + if frame.userCommandFrame { + break + } + frame = frame.parent + } + if frame == nil { + panic("no user command frame found") + } + + if frame.commands == nil { + frame.commands = make(map[string]invokable) + } + + frame.commands[name] = inv +} + func (ec *evalCtx) addMacro(name string, inv macroable) { if ec.root.macros == nil { ec.root.macros = make(map[string]macroable) diff --git a/ucl/inst.go b/ucl/inst.go index 791b202..074433e 100644 --- a/ucl/inst.go +++ b/ucl/inst.go @@ -53,7 +53,9 @@ type Module struct { } func New(opts ...InstOption) *Inst { - rootEC := &evalCtx{} + rootEC := &evalCtx{ + userCommandFrame: true, + } rootEC.root = rootEC rootEC.addCmd("echo", invokableFunc(echoBuiltin)) @@ -194,7 +196,7 @@ func (inst *Inst) eval(ctx context.Context, r io.Reader, opts evalOptions) (Obje env := inst.rootEC if opts.forkEnv { - env = env.fork() + env = env.forkAndIsolate() } return eval.evalScript(ctx, env, ast) diff --git a/ucl/inst_test.go b/ucl/inst_test.go index 184a3b0..06c60a0 100644 --- a/ucl/inst_test.go +++ b/ucl/inst_test.go @@ -145,72 +145,72 @@ func TestInst_Eval_WithSubEnv(t *testing.T) { assert.Nil(t, res) }) - t.Run("when using hooks, reading vars are not permitted to leak across environment", func(t *testing.T) { - ctx := t.Context() + t.Run("environments should not leak when using hooks", func(t *testing.T) { + tests := []struct { + descr string + eval1 string + eval2 string + want1 any + want2 any + }{ + { + descr: "reading vars", + eval1: `$a = "hello" ; hook { $a }`, + eval2: `$a = "world" ; hook { $a }`, + want1: "hello", + want2: "world", + }, + { + descr: "modifying vars", + eval1: `$a = "hello" ; hook { $a = "new value" ; $a }`, + eval2: `$a = "world" ; hook { $a }`, + want1: "new value", + want2: "world", + }, + { + descr: "defining procs", + eval1: `proc say_hello { "hello" } ; hook { say_hello }`, + eval2: `proc say_hello { "world" } ; hook { say_hello }`, + want1: "hello", + want2: "world", + }, + } - hooks := make([]ucl.Invokable, 0) + for _, tt := range tests { + t.Run(tt.descr, func(t *testing.T) { + ctx := t.Context() - inst := ucl.New() - inst.SetBuiltin("hook", func(ctx context.Context, args ucl.CallArgs) (any, error) { - var hookProc ucl.Invokable + hooks := make([]ucl.Invokable, 0) - if err := args.Bind(&hookProc); err != nil { - return nil, err - } - hooks = append(hooks, hookProc) + inst := ucl.New() + inst.SetBuiltin("hook", func(ctx context.Context, args ucl.CallArgs) (any, error) { + var hookProc ucl.Invokable - return nil, nil - }) + if err := args.Bind(&hookProc); err != nil { + return nil, err + } + hooks = append(hooks, hookProc) - res, err := inst.Eval(ctx, strings.NewReader(`$a = "hello" ; hook { $a }`), ucl.WithSubEnv()) - assert.NoError(t, err) - assert.Nil(t, res) + return nil, nil + }) - res, err = inst.Eval(ctx, strings.NewReader(`$a = "world" ; hook { $a }`), ucl.WithSubEnv()) - assert.NoError(t, err) - assert.Nil(t, res) + res, err := inst.Eval(ctx, strings.NewReader(tt.eval1), ucl.WithSubEnv()) + assert.NoError(t, err) + assert.Nil(t, res) - h1, err := hooks[0].Invoke(ctx, ucl.CallArgs{}) - assert.NoError(t, err) - assert.Equal(t, "hello", h1) + res, err = inst.Eval(ctx, strings.NewReader(tt.eval2), ucl.WithSubEnv()) + assert.NoError(t, err) + assert.Nil(t, res) - h2, err := hooks[1].Invoke(ctx, ucl.CallArgs{}) - assert.NoError(t, err) - assert.Equal(t, "world", h2) - }) + h1, err := hooks[0].Invoke(ctx, ucl.CallArgs{}) + assert.NoError(t, err) + assert.Equal(t, tt.want1, h1) - t.Run("when using hooks, writing vars are not permitted to leak across environment", func(t *testing.T) { - ctx := t.Context() - - hooks := make([]ucl.Invokable, 0) - - inst := ucl.New() - inst.SetBuiltin("hook", func(ctx context.Context, args ucl.CallArgs) (any, error) { - var hookProc ucl.Invokable - - if err := args.Bind(&hookProc); err != nil { - return nil, err - } - hooks = append(hooks, hookProc) - - return nil, nil - }) - - res, err := inst.Eval(ctx, strings.NewReader(`$a = "hello" ; hook { $a = "new value" ; $a }`), ucl.WithSubEnv()) - assert.NoError(t, err) - assert.Nil(t, res) - - res, err = inst.Eval(ctx, strings.NewReader(`$a = "world" ; hook { $a }`), ucl.WithSubEnv()) - assert.NoError(t, err) - assert.Nil(t, res) - - h1, err := hooks[0].Invoke(ctx, ucl.CallArgs{}) - assert.NoError(t, err) - assert.Equal(t, "new value", h1) - - h2, err := hooks[1].Invoke(ctx, ucl.CallArgs{}) - assert.NoError(t, err) - assert.Equal(t, "world", h2) + h2, err := hooks[1].Invoke(ctx, ucl.CallArgs{}) + assert.NoError(t, err) + assert.Equal(t, tt.want2, h2) + }) + } }) } From f1cdf6dd938fb956469c548c8286b9b71e06c63b Mon Sep 17 00:00:00 2001 From: Leon Mika Date: Sun, 25 May 2025 12:36:27 +1000 Subject: [PATCH 3/3] Added facility for defining top-level commands from invokables --- ucl/inst_test.go | 27 +++++++++++++++++++++++++++ ucl/userbuiltin.go | 4 ++++ 2 files changed, 31 insertions(+) diff --git a/ucl/inst_test.go b/ucl/inst_test.go index 06c60a0..7a9149a 100644 --- a/ucl/inst_test.go +++ b/ucl/inst_test.go @@ -174,6 +174,20 @@ func TestInst_Eval_WithSubEnv(t *testing.T) { want1: "hello", want2: "world", }, + { + descr: "exporting procs 1", + eval1: `export say_hello { "hello" } ; hook { say_hello }`, + eval2: `hook { say_hello }`, + want1: "hello", + want2: "hello", + }, + { + descr: "exporting procs 2", + eval1: `$a = "hello" ; export say_hello { $a = "world"; $a } ; hook { say_hello }`, + eval2: `$a = "other" ; hook { say_hello }`, + want1: "world", + want2: "world", + }, } for _, tt := range tests { @@ -193,6 +207,19 @@ func TestInst_Eval_WithSubEnv(t *testing.T) { return nil, nil }) + inst.SetBuiltin("export", func(ctx context.Context, args ucl.CallArgs) (any, error) { + var ( + name string + hookProc ucl.Invokable + ) + + if err := args.Bind(&name, &hookProc); err != nil { + return nil, err + } + inst.SetBuiltinInvokable(name, hookProc) + + return nil, nil + }) res, err := inst.Eval(ctx, strings.NewReader(tt.eval1), ucl.WithSubEnv()) assert.NoError(t, err) diff --git a/ucl/userbuiltin.go b/ucl/userbuiltin.go index 7058b0e..83d72d6 100644 --- a/ucl/userbuiltin.go +++ b/ucl/userbuiltin.go @@ -81,6 +81,10 @@ func (inst *Inst) SetBuiltin(name string, fn BuiltinHandler) { inst.rootEC.addCmd(name, userBuiltin{fn: fn}) } +func (inst *Inst) SetBuiltinInvokable(name string, fn Invokable) { + inst.rootEC.addCmd(name, fn.inv) +} + type userBuiltin struct { fn func(ctx context.Context, args CallArgs) (any, error) }