From a5048dbdb1963f5a28ef6971d9335a1f8a0eba77 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 5 Oct 2026 11:43:48 +0200 Subject: [PATCH 1/3] structaccess: say parent is nil when setting through a nil parent Set returned 'cannot set key "k" on invalid' for nil map/pointer parents omitted via omitempty. Report the parent path as nil instead. Also avoids a panic when writing into a nil map. Co-authored-by: Isaac --- libs/structs/structaccess/set.go | 13 ++++++++- libs/structs/structaccess/set_test.go | 40 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/libs/structs/structaccess/set.go b/libs/structs/structaccess/set.go index 02752e456ea..20752e63e0d 100644 --- a/libs/structs/structaccess/set.go +++ b/libs/structs/structaccess/set.go @@ -70,11 +70,17 @@ func setValueAtNode(parentVal reflect.Value, node *structpath.PathNode, value an // Dereference parent if it's a pointer for parentVal.Kind() == reflect.Pointer { if parentVal.IsNil() { - return errors.New("parent is nil pointer") + return nilParentError(node) } parentVal = parentVal.Elem() } + // getValue returns an invalid value for a nil parent omitted via json omitempty. + // Writing into a nil map would panic. + if !parentVal.IsValid() || ((parentVal.Kind() == reflect.Map || parentVal.Kind() == reflect.Slice) && parentVal.IsNil()) { + return nilParentError(node) + } + valueVal := reflect.ValueOf(value) if idx, isIndex := node.Index(); isIndex { @@ -101,6 +107,11 @@ func setValueAtNode(parentVal reflect.Value, node *structpath.PathNode, value an return errors.New("unsupported path node type") } +// nilParentError reports that the parent of node is nil. Missing parents are not allocated. +func nilParentError(node *structpath.PathNode) error { + return fmt.Errorf("cannot set %s: parent %s is nil", node.String(), node.Parent().String()) +} + // setArrayElement sets an element in an array or slice func setArrayElement(parentVal reflect.Value, index int, valueVal reflect.Value) error { kind := parentVal.Kind() diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 0d55b9881f8..7921e226a52 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -910,3 +910,43 @@ func TestGetSet_DashIsAFieldNameWhenTheTagHasOptions(t *testing.T) { assert.Equal(t, "set", target.Named, "Set must write to Named") assert.Equal(t, "s", target.Skipped, "json:\"-\" field must stay out of reach") } + +func TestSet_NilParent(t *testing.T) { + type Leaf struct { + N int `json:"n,omitempty"` + } + type Mid struct { + C *Leaf `json:"c,omitempty"` + } + type T struct { + M map[string]string `json:"m,omitempty"` + P *Leaf `json:"p,omitempty"` + A *Mid `json:"a,omitempty"` + S []string `json:"s,omitempty"` + NM map[string]string `json:"nm"` + NP *Leaf `json:"np"` + NS []string `json:"ns"` + } + + tests := []struct { + path string + errorMsg string + }{ + {"m.k", "cannot set m.k: parent m is nil"}, + {"p.n", "cannot set p.n: parent p is nil"}, + {"a.c.n", "cannot set a.c.n: parent a.c is nil"}, + {"s[0]", "cannot set s[0]: parent s is nil"}, + {"nm.k", "cannot set nm.k: parent nm is nil"}, + {"np.n", "cannot set np.n: parent np is nil"}, + {"ns[0]", "cannot set ns[0]: parent ns is nil"}, + } + + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + target := &T{A: &Mid{}} + err := structaccess.SetByString(target, tt.path, "5") + assert.EqualError(t, err, tt.errorMsg) + assert.Equal(t, &T{A: &Mid{}}, target) + }) + } +} From 3e0d7151e4c5637b82f56833affe6a1a9e883580 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 5 Oct 2026 13:17:14 +0200 Subject: [PATCH 2/3] structaccess: allocate nil maps, pointers and slices in Set Co-authored-by: Isaac --- libs/structs/structaccess/set.go | 163 ++++++++++++++++++++------ libs/structs/structaccess/set_test.go | 72 ++++++++---- 2 files changed, 181 insertions(+), 54 deletions(-) diff --git a/libs/structs/structaccess/set.go b/libs/structs/structaccess/set.go index 20752e63e0d..99596d27fba 100644 --- a/libs/structs/structaccess/set.go +++ b/libs/structs/structaccess/set.go @@ -29,25 +29,139 @@ func Set(target any, path *structpath.PathNode, value any) error { return errors.New("target must be a pointer") } - // For single-level paths, get the target directly - if pathLen == 1 { - return setValueAtNode(targetVal.Elem(), path, value) - } + return setAt(targetVal.Elem(), path.AsSlice(), path.Parent(), value) +} - // For multi-level paths, get the parent container - parent := path.Parent() - if parent == nil { - return errors.New("failed to get parent path") +// setAt sets value at nodes relative to cur, allocating nil maps, pointers and slices on the way. +// parent is the parent of the final node and is only used in error messages. +func setAt(cur reflect.Value, nodes []*structpath.PathNode, parent *structpath.PathNode, value any) error { + node := nodes[0] + cur, err := allocate(cur, node) + if err != nil { + return err + } + if len(nodes) == 1 { + return setValueAtNode(cur, node, value) } - // Get the parent container using getValue, passing the original target - parentVal, err := getValue(target, parent) + next, writeBack, err := child(cur, node) if err != nil { return fmt.Errorf("failed to navigate to parent %s: %w", parent.String(), err) } + if err := setAt(next, nodes[1:], parent, value); err != nil { + return err + } + if writeBack != nil { + writeBack() + } + return nil +} + +// allocate dereferences cur and makes it ready to hold node: nil pointers are allocated, +// a nil map is made and a nil slice is grown to fit the index. +func allocate(cur reflect.Value, node *structpath.PathNode) (reflect.Value, error) { + for cur.Kind() == reflect.Pointer || cur.Kind() == reflect.Interface { + if cur.IsNil() { + if cur.Kind() == reflect.Interface || !cur.CanSet() { + return cur, nilParentError(node) + } + cur.Set(reflect.New(cur.Type().Elem())) + } + cur = cur.Elem() + } + + idx, isIndex := node.Index() + if isIndex && cur.Kind() == reflect.Struct { + if embed := findEmbedField(cur); embed.IsValid() { + cur = embed + } + } - // Set the value at the final node - return setValueAtNode(parentVal, path, value) + var alloc reflect.Value + switch { + case cur.Kind() == reflect.Map && cur.IsNil(): + alloc = reflect.MakeMap(cur.Type()) + case isIndex && idx >= 0 && cur.Kind() == reflect.Slice && cur.IsNil(): + alloc = reflect.MakeSlice(cur.Type(), idx+1, idx+1) + default: + return cur, nil + } + if !cur.CanSet() { + return cur, nilParentError(node) + } + cur.Set(alloc) + return cur, nil +} + +// child returns the addressable value selected by node in cur, which allocate has prepared. +// Map values are not addressable, so for maps it returns a copy and a function that stores it back. +func child(cur reflect.Value, node *structpath.PathNode) (reflect.Value, func(), error) { + if idx, isIndex := node.Index(); isIndex { + switch cur.Kind() { + case reflect.Slice, reflect.Array: + if idx < 0 || idx >= cur.Len() { + return reflect.Value{}, nil, &NotFoundError{fmt.Sprintf("%s: index out of range, length is %d", node.String(), cur.Len())} + } + return cur.Index(idx), nil, nil + case reflect.Struct: + // Terraform represents single-block fields as lists and uses [0] to access them. + if idx == 0 { + return cur, nil, nil + } + default: + } + return reflect.Value{}, nil, fmt.Errorf("%s: cannot index %s", node.String(), cur.Kind()) + } + + if key, value, ok := node.KeyValue(); ok { + if cur.Kind() == reflect.Struct { + if embed := findEmbedField(cur); embed.IsValid() { + cur = embed + } + } + nv, err := accessKeyValue(cur, key, value, node) + return nv, nil, err + } + + key, ok := node.StringKey() + if !ok { + return reflect.Value{}, nil, errors.New("unsupported path node type") + } + + switch cur.Kind() { + case reflect.Struct: + fv, _, _, ok := findStructFieldByKey(cur, key) + if !ok { + return reflect.Value{}, nil, fmt.Errorf("%s: field %q not found in %s", node.String(), key, cur.Type()) + } + return fv, nil, nil + case reflect.Map: + mk, err := mapKey(cur, key) + if err != nil { + return reflect.Value{}, nil, fmt.Errorf("%s: %w", node.String(), err) + } + elem := reflect.New(cur.Type().Elem()).Elem() + if existing := cur.MapIndex(mk); existing.IsValid() { + elem.Set(existing) + } + return elem, func() { cur.SetMapIndex(mk, elem) }, nil + default: + return reflect.Value{}, nil, fmt.Errorf("%s: cannot access key %q on %s", node.String(), key, cur.Kind()) + } +} + +// mapKey converts key to the key type of the map m, which must be a string type. +func mapKey(m reflect.Value, key string) (reflect.Value, error) { + kt := m.Type().Key() + if kt.Kind() != reflect.String { + return reflect.Value{}, fmt.Errorf("map key must be string, got %s", kt) + } + return reflect.ValueOf(key).Convert(kt), nil +} + +// nilParentError reports that the parent of node is nil and cannot be allocated. +func nilParentError(node *structpath.PathNode) error { + return fmt.Errorf("cannot set %s: parent %s is nil", node.String(), node.Parent().String()) } // SetByString sets the value at the given path string inside the target object. @@ -70,17 +184,10 @@ func setValueAtNode(parentVal reflect.Value, node *structpath.PathNode, value an // Dereference parent if it's a pointer for parentVal.Kind() == reflect.Pointer { if parentVal.IsNil() { - return nilParentError(node) + return errors.New("parent is nil pointer") } parentVal = parentVal.Elem() } - - // getValue returns an invalid value for a nil parent omitted via json omitempty. - // Writing into a nil map would panic. - if !parentVal.IsValid() || ((parentVal.Kind() == reflect.Map || parentVal.Kind() == reflect.Slice) && parentVal.IsNil()) { - return nilParentError(node) - } - valueVal := reflect.ValueOf(value) if idx, isIndex := node.Index(); isIndex { @@ -107,11 +214,6 @@ func setValueAtNode(parentVal reflect.Value, node *structpath.PathNode, value an return errors.New("unsupported path node type") } -// nilParentError reports that the parent of node is nil. Missing parents are not allocated. -func nilParentError(node *structpath.PathNode) error { - return fmt.Errorf("cannot set %s: parent %s is nil", node.String(), node.Parent().String()) -} - // setArrayElement sets an element in an array or slice func setArrayElement(parentVal reflect.Value, index int, valueVal reflect.Value) error { kind := parentVal.Kind() @@ -173,14 +275,9 @@ func setStructField(parentVal reflect.Value, fieldName string, valueVal reflect. // setMapValue sets a value in a map func setMapValue(parentVal reflect.Value, key string, valueVal reflect.Value) error { - kt := parentVal.Type().Key() - if kt.Kind() != reflect.String { - return fmt.Errorf("map key must be string, got %s", kt) - } - - mk := reflect.ValueOf(key) - if kt != mk.Type() { - mk = mk.Convert(kt) + mk, err := mapKey(parentVal, key) + if err != nil { + return err } // For maps, we need to handle the value type diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 7921e226a52..d631a3be209 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -911,42 +911,72 @@ func TestGetSet_DashIsAFieldNameWhenTheTagHasOptions(t *testing.T) { assert.Equal(t, "s", target.Skipped, "json:\"-\" field must stay out of reach") } -func TestSet_NilParent(t *testing.T) { +func TestSet_AllocatesNilParents(t *testing.T) { type Leaf struct { N int `json:"n,omitempty"` } type Mid struct { - C *Leaf `json:"c,omitempty"` + C *Leaf `json:"c,omitempty"` + LM map[string]Leaf `json:"lm,omitempty"` } type T struct { - M map[string]string `json:"m,omitempty"` - P *Leaf `json:"p,omitempty"` - A *Mid `json:"a,omitempty"` - S []string `json:"s,omitempty"` - NM map[string]string `json:"nm"` - NP *Leaf `json:"np"` - NS []string `json:"ns"` + M map[string]string `json:"m,omitempty"` + P *Leaf `json:"p,omitempty"` + A *Mid `json:"a,omitempty"` + S []string `json:"s,omitempty"` + SL []Leaf `json:"sl,omitempty"` + NM map[string]string `json:"nm"` + NP *Leaf `json:"np"` + MM map[string]map[string]string `json:"mm,omitempty"` } tests := []struct { path string - errorMsg string + value any + expected T }{ - {"m.k", "cannot set m.k: parent m is nil"}, - {"p.n", "cannot set p.n: parent p is nil"}, - {"a.c.n", "cannot set a.c.n: parent a.c is nil"}, - {"s[0]", "cannot set s[0]: parent s is nil"}, - {"nm.k", "cannot set nm.k: parent nm is nil"}, - {"np.n", "cannot set np.n: parent np is nil"}, - {"ns[0]", "cannot set ns[0]: parent ns is nil"}, + {"m.k", "x", T{M: map[string]string{"k": "x"}}}, + {"nm.k", "x", T{NM: map[string]string{"k": "x"}}}, + {"p.n", 5, T{P: &Leaf{N: 5}}}, + {"np.n", 5, T{NP: &Leaf{N: 5}}}, + {"a.c.n", 5, T{A: &Mid{C: &Leaf{N: 5}}}}, + {"a.lm.k.n", 5, T{A: &Mid{LM: map[string]Leaf{"k": {N: 5}}}}}, + {"mm.a.b", "x", T{MM: map[string]map[string]string{"a": {"b": "x"}}}}, + {"s[0]", "x", T{S: []string{"x"}}}, + {"s[2]", "x", T{S: []string{"", "", "x"}}}, + {"sl[0].n", 5, T{SL: []Leaf{{N: 5}}}}, } for _, tt := range tests { t.Run(tt.path, func(t *testing.T) { - target := &T{A: &Mid{}} - err := structaccess.SetByString(target, tt.path, "5") - assert.EqualError(t, err, tt.errorMsg) - assert.Equal(t, &T{A: &Mid{}}, target) + target := &T{} + require.NoError(t, structaccess.SetByString(target, tt.path, tt.value)) + assert.Equal(t, tt.expected, *target) }) } } + +func TestSet_NilParentNotAllocatable(t *testing.T) { + type T struct { + I any `json:"i,omitempty"` + } + err := structaccess.SetByString(&T{}, "i.k", "x") + assert.EqualError(t, err, "cannot set i.k: parent i is nil") +} + +func TestSet_AllocationWithExistingParents(t *testing.T) { + type T struct { + M map[string]map[string]string `json:"m,omitempty"` + SL []map[string]string `json:"sl,omitempty"` + } + target := &T{M: map[string]map[string]string{"a": {"b": "1"}}, SL: []map[string]string{nil}} + require.NoError(t, structaccess.SetByString(target, "m.a.c", "2")) + require.NoError(t, structaccess.SetByString(target, "sl[0].k", "3")) + assert.Equal(t, T{ + M: map[string]map[string]string{"a": {"b": "1", "c": "2"}}, + SL: []map[string]string{{"k": "3"}}, + }, *target) + + err := structaccess.SetByString(target, "sl[1].k", "3") + assert.EqualError(t, err, "failed to navigate to parent sl[1]: sl[1]: index out of range, length is 1") +} From c19cb165ea7a01a00189b21f9bd99dd81f4dbdc3 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 5 Oct 2026 13:26:57 +0200 Subject: [PATCH 3/3] Silence dupword lint in test Co-authored-by: Isaac --- libs/structs/structaccess/set_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index d631a3be209..a8138004dbc 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -978,5 +978,5 @@ func TestSet_AllocationWithExistingParents(t *testing.T) { }, *target) err := structaccess.SetByString(target, "sl[1].k", "3") - assert.EqualError(t, err, "failed to navigate to parent sl[1]: sl[1]: index out of range, length is 1") + assert.EqualError(t, err, "failed to navigate to parent sl[1]: sl[1]: index out of range, length is 1") //nolint:dupword }