From 401009f8c5cfd3cee26252f8419cc7bf163452f9 Mon Sep 17 00:00:00 2001 From: Diego Augusto Molina Date: Tue, 24 Jun 2025 15:47:26 -0300 Subject: [PATCH 1/5] allow ignoring struct member using a - in expr struct tag --- builtin/lib.go | 5 +-- checker/nature/utils.go | 16 +++++++--- expr_test.go | 69 +++++++++++++++++++++++++++++++++++++++-- vm/runtime/runtime.go | 5 +-- 4 files changed, 85 insertions(+), 10 deletions(-) diff --git a/builtin/lib.go b/builtin/lib.go index 579393161..2af5b07b9 100644 --- a/builtin/lib.go +++ b/builtin/lib.go @@ -421,10 +421,11 @@ func get(params ...any) (out any, err error) { fieldName := i.(string) value := v.FieldByNameFunc(func(name string) bool { field, _ := v.Type().FieldByName(name) - if field.Tag.Get("expr") == fieldName { + tagName := field.Tag.Get("expr") + if tagName == fieldName { return true } - return name == fieldName + return tagName != "-" && name == fieldName }) if value.IsValid() { return value.Interface(), nil diff --git a/checker/nature/utils.go b/checker/nature/utils.go index 179459874..c654ceac5 100644 --- a/checker/nature/utils.go +++ b/checker/nature/utils.go @@ -7,10 +7,14 @@ import ( ) func fieldName(field reflect.StructField) string { - if taggedName := field.Tag.Get("expr"); taggedName != "" { + switch taggedName := field.Tag.Get("expr"); taggedName { + case "-": + return "" + case "": + return field.Name + default: return taggedName } - return field.Name } func fetchField(t reflect.Type, name string) (reflect.StructField, bool) { @@ -23,7 +27,7 @@ func fetchField(t reflect.Type, name string) (reflect.StructField, bool) { for i := 0; i < t.NumField(); i++ { field := t.Field(i) // Search all fields, even embedded structs. - if fieldName(field) == name { + if n := fieldName(field); n != "" && n == name { return field, true } } @@ -69,7 +73,11 @@ func StructFields(t reflect.Type) map[string]Nature { } } - table[fieldName(f)] = Nature{ + name := fieldName(f) + if name == "" { + continue + } + table[name] = Nature{ Type: f.Type, FieldIndex: f.Index, } diff --git a/expr_test.go b/expr_test.go index 57d2cfbbb..1494f329c 100644 --- a/expr_test.go +++ b/expr_test.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "reflect" + "strings" "sync" "testing" "time" @@ -139,7 +140,71 @@ func ExampleEnv_tagged_field_names() { fmt.Printf("%v", output) - // Output : Hello World + // Output: Hello World +} + +func ExampleEnv_hidden_tagged_field_names() { + type Internal struct { + Visible string + Hidden string `expr:"-"` + } + type environment struct { + Visible string + Hidden string `expr:"-"` + HiddenInternal Internal `expr:"-"` + VisibleInternal Internal + } + + env := environment{ + Hidden: "First level secret", + HiddenInternal: Internal{ + Visible: "Second level secret", + Hidden: "Also hidden", + }, + VisibleInternal: Internal{ + Visible: "Not a secret", + Hidden: "Hidden too", + }, + } + + hiddenValues := []string{ + `Hidden`, + `HiddenInternal`, + `HiddenInternal.Visible`, + `HiddenInternal.Hidden`, + `VisibleInternal.Hidden`, + } + for _, expression := range hiddenValues { + output, err := expr.Eval(expression, env) + if err == nil || !strings.Contains(err.Error(), "cannot fetch") { + fmt.Printf("unexpected output: %v; err: %v\n", output, err) + return + } + fmt.Printf("%q is hidden as expected\n", expression) + } + + visibleValues := []string{ + `Visible`, + `VisibleInternal`, + `VisibleInternal.Visible`, + } + for _, expression := range visibleValues { + _, err := expr.Eval(expression, env) + if err != nil { + fmt.Printf("unexpected error: %v\n", err) + return + } + fmt.Printf("%q is visible as expected\n", expression) + } + + // Output: "Hidden" is hidden as expected + // "HiddenInternal" is hidden as expected + // "HiddenInternal.Visible" is hidden as expected + // "HiddenInternal.Hidden" is hidden as expected + // "VisibleInternal.Hidden" is hidden as expected + // "Visible" is visible as expected + // "VisibleInternal" is visible as expected + // "VisibleInternal.Visible" is visible as expected } func ExampleAsKind() { @@ -529,7 +594,7 @@ func ExamplePatch() { } fmt.Printf("%v", output) - // Output : Hello, you, world! + // Output: Hello, you, world! } func ExampleWithContext() { diff --git a/vm/runtime/runtime.go b/vm/runtime/runtime.go index cd48a280d..9844993b3 100644 --- a/vm/runtime/runtime.go +++ b/vm/runtime/runtime.go @@ -65,10 +65,11 @@ func Fetch(from, i any) any { fieldName := i.(string) value := v.FieldByNameFunc(func(name string) bool { field, _ := v.Type().FieldByName(name) - if field.Tag.Get("expr") == fieldName { + tagName := field.Tag.Get("expr") + if tagName == fieldName { return true } - return name == fieldName + return tagName != "-" && name == fieldName }) if value.IsValid() { return value.Interface() From 6bd3eb59de31021fd88e9e9dd4e917ce715ffb99 Mon Sep 17 00:00:00 2001 From: Diego Augusto Molina Date: Tue, 24 Jun 2025 16:35:04 -0300 Subject: [PATCH 2/5] fix in operator --- expr_test.go | 23 +++++++++++++++++++---- vm/runtime/runtime.go | 3 ++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/expr_test.go b/expr_test.go index 1494f329c..e1b151cd4 100644 --- a/expr_test.go +++ b/expr_test.go @@ -172,7 +172,7 @@ func ExampleEnv_hidden_tagged_field_names() { `HiddenInternal`, `HiddenInternal.Visible`, `HiddenInternal.Hidden`, - `VisibleInternal.Hidden`, + `VisibleInternal["Hidden"]`, } for _, expression := range hiddenValues { output, err := expr.Eval(expression, env) @@ -186,7 +186,7 @@ func ExampleEnv_hidden_tagged_field_names() { visibleValues := []string{ `Visible`, `VisibleInternal`, - `VisibleInternal.Visible`, + `VisibleInternal["Visible"]`, } for _, expression := range visibleValues { _, err := expr.Eval(expression, env) @@ -197,14 +197,29 @@ func ExampleEnv_hidden_tagged_field_names() { fmt.Printf("%q is visible as expected\n", expression) } + testWithIn := []string{ + `not ("Hidden" in $env)`, + `"Visible" in $env`, + `not ("Hidden" in VisibleInternal)`, + `"Visible" in VisibleInternal`, + } + for _, expression := range testWithIn { + val, err := expr.Eval(expression, env) + shouldBeTrue, ok := val.(bool) + if err != nil || !ok || !shouldBeTrue { + fmt.Printf("unexpected result; value: %v; error: %v\n", val, err) + return + } + } + // Output: "Hidden" is hidden as expected // "HiddenInternal" is hidden as expected // "HiddenInternal.Visible" is hidden as expected // "HiddenInternal.Hidden" is hidden as expected - // "VisibleInternal.Hidden" is hidden as expected + // "VisibleInternal[\"Hidden\"]" is hidden as expected // "Visible" is visible as expected // "VisibleInternal" is visible as expected - // "VisibleInternal.Visible" is visible as expected + // "VisibleInternal[\"Visible\"]" is visible as expected } func ExampleAsKind() { diff --git a/vm/runtime/runtime.go b/vm/runtime/runtime.go index 9844993b3..c0fcf7e08 100644 --- a/vm/runtime/runtime.go +++ b/vm/runtime/runtime.go @@ -215,7 +215,8 @@ func In(needle any, array any) bool { panic(fmt.Sprintf("cannot use %T as field name of %T", needle, array)) } value := v.FieldByName(n.String()) - if value.IsValid() { + structValue, ok := v.Type().FieldByName(n.String()) + if ok && structValue.Tag.Get("expr") != "-" && value.IsValid() { return true } return false From 56d05286a971affbbf38d545b0ca5a0e25fd117f Mon Sep 17 00:00:00 2001 From: Diego Augusto Molina Date: Fri, 27 Jun 2025 11:18:19 -0300 Subject: [PATCH 3/5] simplify logic --- builtin/lib.go | 9 ++++++--- checker/nature/utils.go | 14 +++++++------- vm/runtime/runtime.go | 9 ++++++--- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/builtin/lib.go b/builtin/lib.go index 2af5b07b9..6f6a3b6cd 100644 --- a/builtin/lib.go +++ b/builtin/lib.go @@ -421,11 +421,14 @@ func get(params ...any) (out any, err error) { fieldName := i.(string) value := v.FieldByNameFunc(func(name string) bool { field, _ := v.Type().FieldByName(name) - tagName := field.Tag.Get("expr") - if tagName == fieldName { + switch field.Tag.Get("expr") { + case "-": + return false + case fieldName: return true + default: + return name == fieldName } - return tagName != "-" && name == fieldName }) if value.IsValid() { return value.Interface(), nil diff --git a/checker/nature/utils.go b/checker/nature/utils.go index c654ceac5..c1551546c 100644 --- a/checker/nature/utils.go +++ b/checker/nature/utils.go @@ -6,14 +6,14 @@ import ( "github.com/expr-lang/expr/internal/deref" ) -func fieldName(field reflect.StructField) string { +func fieldName(field reflect.StructField) (string, bool) { switch taggedName := field.Tag.Get("expr"); taggedName { case "-": - return "" + return "", false case "": - return field.Name + return field.Name, true default: - return taggedName + return taggedName, true } } @@ -27,7 +27,7 @@ func fetchField(t reflect.Type, name string) (reflect.StructField, bool) { for i := 0; i < t.NumField(); i++ { field := t.Field(i) // Search all fields, even embedded structs. - if n := fieldName(field); n != "" && n == name { + if n, ok := fieldName(field); ok && n == name { return field, true } } @@ -73,8 +73,8 @@ func StructFields(t reflect.Type) map[string]Nature { } } - name := fieldName(f) - if name == "" { + name, ok := fieldName(f) + if !ok { continue } table[name] = Nature{ diff --git a/vm/runtime/runtime.go b/vm/runtime/runtime.go index c0fcf7e08..00d39a9ea 100644 --- a/vm/runtime/runtime.go +++ b/vm/runtime/runtime.go @@ -65,11 +65,14 @@ func Fetch(from, i any) any { fieldName := i.(string) value := v.FieldByNameFunc(func(name string) bool { field, _ := v.Type().FieldByName(name) - tagName := field.Tag.Get("expr") - if tagName == fieldName { + switch field.Tag.Get("expr") { + case "-": + return false + case fieldName: return true + default: + return name == fieldName } - return tagName != "-" && name == fieldName }) if value.IsValid() { return value.Interface() From eecf37195749b4f6a337af87bca14119678ff157 Mon Sep 17 00:00:00 2001 From: Diego Augusto Molina Date: Fri, 27 Jun 2025 17:05:05 -0300 Subject: [PATCH 4/5] add regression test for #807 --- expr_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/expr_test.go b/expr_test.go index e1b151cd4..32ec6dfb8 100644 --- a/expr_test.go +++ b/expr_test.go @@ -2845,3 +2845,20 @@ func TestMemoryBudget(t *testing.T) { }) } } + +func TestIssue807(t *testing.T) { + type MyStruct struct { + nonExported string + } + out, err := expr.Eval(` "nonExported" in $env `, MyStruct{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + b, ok := out.(bool) + if !ok { + t.Fatalf("expected boolean type, got %T: %v", b, b) + } + if b { + t.Fatalf("expected 'in' operator to return false for unexported field") + } +} From 1089aa98fad5f3f2abc57f9e1201925853cc58aa Mon Sep 17 00:00:00 2001 From: Diego Augusto Molina Date: Fri, 27 Jun 2025 17:20:07 -0300 Subject: [PATCH 5/5] fixes #807: make in operator return false for unexported struct fields --- vm/runtime/runtime.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/vm/runtime/runtime.go b/vm/runtime/runtime.go index 00d39a9ea..fa14c4d0a 100644 --- a/vm/runtime/runtime.go +++ b/vm/runtime/runtime.go @@ -217,9 +217,12 @@ func In(needle any, array any) bool { if !n.IsValid() || n.Kind() != reflect.String { panic(fmt.Sprintf("cannot use %T as field name of %T", needle, array)) } - value := v.FieldByName(n.String()) - structValue, ok := v.Type().FieldByName(n.String()) - if ok && structValue.Tag.Get("expr") != "-" && value.IsValid() { + field, ok := v.Type().FieldByName(n.String()) + if !ok || !field.IsExported() || field.Tag.Get("expr") == "-" { + return false + } + value := v.FieldByIndex(field.Index) + if value.IsValid() { return true } return false