From 8e1262fc25138d3c5f90d95e7f163a9116ce2e5b Mon Sep 17 00:00:00 2001 From: jaideeppyne Date: Fri, 21 Aug 2026 14:40:08 +0530 Subject: [PATCH 1/3] fix(conf): deref pointer-to-map environment instead of panicking EnvWithCache selected the map branch using the dereferenced value's kind, but then read the map length and keys from the original (pointer) value. Passing a *map as the env therefore panicked with: reflect: call of reflect.Value.Len on ptr to non-array Value Iterate the dereferenced value in the map branch so a pointer-to-map env is treated exactly like the map it points to (same strict mode and element types), matching the existing behaviour for pointer-to-struct environments. Fixes #825 Co-Authored-By: Claude Opus 4.8 Signed-off-by: jaideeppyne --- conf/env.go | 12 +++++---- test/issues/825/issue_test.go | 48 +++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 test/issues/825/issue_test.go diff --git a/conf/env.go b/conf/env.go index 0acd44570..da4ac0829 100644 --- a/conf/env.go +++ b/conf/env.go @@ -32,22 +32,24 @@ func EnvWithCache(c *Cache, env any) Nature { v := reflect.ValueOf(env) t := v.Type() - switch deref.Value(v).Kind() { + d := deref.Value(v) + + switch d.Kind() { case reflect.Struct: n := c.FromType(t) n.Strict = true return n case reflect.Map: - n := c.FromType(v.Type()) + n := c.FromType(d.Type()) if n.TypeData == nil { n.TypeData = new(TypeData) } n.Strict = true - n.Fields = make(map[string]Nature, v.Len()) + n.Fields = make(map[string]Nature, d.Len()) - for _, key := range v.MapKeys() { - elem := v.MapIndex(key) + for _, key := range d.MapKeys() { + elem := d.MapIndex(key) if !elem.IsValid() || !elem.CanInterface() { panic(fmt.Sprintf("invalid map value: %s", key)) } diff --git a/test/issues/825/issue_test.go b/test/issues/825/issue_test.go new file mode 100644 index 000000000..3cb871760 --- /dev/null +++ b/test/issues/825/issue_test.go @@ -0,0 +1,48 @@ +package issue_test + +import ( + "testing" + + "github.com/expr-lang/expr" + "github.com/expr-lang/expr/internal/testify/assert" + "github.com/expr-lang/expr/internal/testify/require" +) + +// TestIssue825 verifies that passing a pointer to a map as the environment is +// dereferenced instead of panicking. +// +// conf.EnvWithCache selected the map branch using the dereferenced value's +// kind, but then read the map keys/length from the original (pointer) value, +// panicking with: +// +// reflect: call of reflect.Value.Len on ptr to non-array Value +func TestIssue825(t *testing.T) { + m := map[string]any{"foo": 42} + + program, err := expr.Compile("foo + 1", expr.Env(&m)) + require.NoError(t, err) + + out, err := expr.Run(program, m) + require.NoError(t, err) + assert.Equal(t, 43, out) +} + +// TestIssue825_Strict verifies that a pointer-to-map env keeps the strict-mode +// and element-type information of the dereferenced map, i.e. it behaves exactly +// like compiling with the map value itself. +func TestIssue825_Strict(t *testing.T) { + m := map[string]int{"a": 1} + + // Unknown names are rejected (strict), just like a plain map env. + _, err := expr.Compile("unknown + 1", expr.Env(&m)) + require.Error(t, err) + require.Contains(t, err.Error(), "unknown name unknown") + + // The element type (int) is inferred from the dereferenced map. + program, err := expr.Compile("a + 1", expr.Env(&m)) + require.NoError(t, err) + + out, err := expr.Run(program, m) + require.NoError(t, err) + assert.Equal(t, 2, out) +} From 0e542063afe0dce35e2d7a5e315c08de49ab0ed7 Mon Sep 17 00:00:00 2001 From: jaideeppyne Date: Tue, 25 Aug 2026 05:11:22 +0530 Subject: [PATCH 2/3] fix(conf): reject pointer-to-map environment with a clear error Per #825 (wontfix for deref-support): rather than dereferencing a *map env, reject it with a descriptive message instead of the opaque "reflect: call of reflect.Value.Len on ptr to non-array Value" panic. Passing a map by value is unaffected. --- conf/env.go | 19 ++++++++++------ test/issues/825/issue_test.go | 41 +++++++++++++++-------------------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/conf/env.go b/conf/env.go index da4ac0829..9994090df 100644 --- a/conf/env.go +++ b/conf/env.go @@ -32,24 +32,29 @@ func EnvWithCache(c *Cache, env any) Nature { v := reflect.ValueOf(env) t := v.Type() - d := deref.Value(v) - - switch d.Kind() { + switch deref.Value(v).Kind() { case reflect.Struct: n := c.FromType(t) n.Strict = true return n case reflect.Map: - n := c.FromType(d.Type()) + // A pointer to a map is not supported as an environment (see #825). + // Reject it with a clear message instead of dereferencing it or + // panicking deep inside reflect. + if v.Kind() == reflect.Ptr { + panic(fmt.Sprintf("environment must be a map, not a pointer to a map: %s", t)) + } + + n := c.FromType(v.Type()) if n.TypeData == nil { n.TypeData = new(TypeData) } n.Strict = true - n.Fields = make(map[string]Nature, d.Len()) + n.Fields = make(map[string]Nature, v.Len()) - for _, key := range d.MapKeys() { - elem := d.MapIndex(key) + for _, key := range v.MapKeys() { + elem := v.MapIndex(key) if !elem.IsValid() || !elem.CanInterface() { panic(fmt.Sprintf("invalid map value: %s", key)) } diff --git a/test/issues/825/issue_test.go b/test/issues/825/issue_test.go index 3cb871760..2528ba167 100644 --- a/test/issues/825/issue_test.go +++ b/test/issues/825/issue_test.go @@ -9,40 +9,35 @@ import ( ) // TestIssue825 verifies that passing a pointer to a map as the environment is -// dereferenced instead of panicking. +// rejected with a clear message instead of panicking deep inside reflect. // -// conf.EnvWithCache selected the map branch using the dereferenced value's -// kind, but then read the map keys/length from the original (pointer) value, -// panicking with: +// Supporting *map by dereferencing it was declined by the maintainer (#825 is +// labeled wontfix); the agreed direction was to reject *map with an error +// message. Previously conf.EnvWithCache selected the map branch on the +// dereferenced kind but then read the map keys/length from the original +// (pointer) value, panicking with the opaque: // // reflect: call of reflect.Value.Len on ptr to non-array Value func TestIssue825(t *testing.T) { m := map[string]any{"foo": 42} - program, err := expr.Compile("foo + 1", expr.Env(&m)) - require.NoError(t, err) - - out, err := expr.Run(program, m) - require.NoError(t, err) - assert.Equal(t, 43, out) + assert.PanicsWithValue(t, + "environment must be a map, not a pointer to a map: *map[string]interface {}", + func() { + _, _ = expr.Compile("foo > 0", expr.Env(&m)) + }, + ) } -// TestIssue825_Strict verifies that a pointer-to-map env keeps the strict-mode -// and element-type information of the dereferenced map, i.e. it behaves exactly -// like compiling with the map value itself. -func TestIssue825_Strict(t *testing.T) { - m := map[string]int{"a": 1} - - // Unknown names are rejected (strict), just like a plain map env. - _, err := expr.Compile("unknown + 1", expr.Env(&m)) - require.Error(t, err) - require.Contains(t, err.Error(), "unknown name unknown") +// TestIssue825_MapStillWorks guards the common case: a map passed by value is +// unaffected and continues to work exactly as before. +func TestIssue825_MapStillWorks(t *testing.T) { + m := map[string]any{"foo": 42} - // The element type (int) is inferred from the dereferenced map. - program, err := expr.Compile("a + 1", expr.Env(&m)) + program, err := expr.Compile("foo + 1", expr.Env(m)) require.NoError(t, err) out, err := expr.Run(program, m) require.NoError(t, err) - assert.Equal(t, 2, out) + assert.Equal(t, 43, out) } From d8a376c484ee47aae05ca90bfdba5eb51e7e1275 Mon Sep 17 00:00:00 2001 From: jaideeppyne Date: Tue, 25 Aug 2026 20:55:46 +0530 Subject: [PATCH 3/3] fix(conf): reject nil pointer-to-map env with the same clear error Detect pointer-to-map by type so a nil *map is rejected with the same message as a non-nil one, instead of the generic "unknown type" panic. Pointer-to-struct envs are unaffected. --- conf/env.go | 15 ++++++++------- test/issues/825/issue_test.go | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/conf/env.go b/conf/env.go index 9994090df..867797f9b 100644 --- a/conf/env.go +++ b/conf/env.go @@ -32,6 +32,14 @@ func EnvWithCache(c *Cache, env any) Nature { v := reflect.ValueOf(env) t := v.Type() + // A pointer to a map is not supported as an environment (see #825). + // Detect it by type so that a nil *map is rejected with the same clear + // message instead of falling through to the generic "unknown type" panic, + // and a non-nil *map is rejected before it reaches reflect map access. + if t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Map { + panic(fmt.Sprintf("environment must be a map, not a pointer to a map: %s", t)) + } + switch deref.Value(v).Kind() { case reflect.Struct: n := c.FromType(t) @@ -39,13 +47,6 @@ func EnvWithCache(c *Cache, env any) Nature { return n case reflect.Map: - // A pointer to a map is not supported as an environment (see #825). - // Reject it with a clear message instead of dereferencing it or - // panicking deep inside reflect. - if v.Kind() == reflect.Ptr { - panic(fmt.Sprintf("environment must be a map, not a pointer to a map: %s", t)) - } - n := c.FromType(v.Type()) if n.TypeData == nil { n.TypeData = new(TypeData) diff --git a/test/issues/825/issue_test.go b/test/issues/825/issue_test.go index 2528ba167..2f9a77e56 100644 --- a/test/issues/825/issue_test.go +++ b/test/issues/825/issue_test.go @@ -29,6 +29,20 @@ func TestIssue825(t *testing.T) { ) } +// TestIssue825_NilPointer verifies that a nil pointer to a map is rejected with +// the same clear message as a non-nil one, rather than falling through to the +// generic "unknown type" panic (the pointer-to-map is detected by type). +func TestIssue825_NilPointer(t *testing.T) { + var m *map[string]any + + assert.PanicsWithValue(t, + "environment must be a map, not a pointer to a map: *map[string]interface {}", + func() { + _, _ = expr.Compile("foo > 0", expr.Env(m)) + }, + ) +} + // TestIssue825_MapStillWorks guards the common case: a map passed by value is // unaffected and continues to work exactly as before. func TestIssue825_MapStillWorks(t *testing.T) {