Skip to content

Commit

Permalink
mapstr: fix M.Clone behaviour for arrays of M (#262)
Browse files Browse the repository at this point in the history
The current implementation does not clone in branches of slices in a
mapstr.M, resulting in cases where the clone will share values with the
original and allowing unexpected mutation of the original. This change
fixes that by adding slices type handling. Note that this will not fix
cases of unexpected sharing where the node value type is not a mapstr.M
or map[string]any, so structs that are included with pointer fields will
still show this effect.
  • Loading branch information
efd6 authored Jan 27, 2025
1 parent 05eff47 commit e11ce6b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 8 deletions.
41 changes: 33 additions & 8 deletions mapstr/mapstr.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,19 +154,44 @@ func (m M) CopyFieldsTo(to M, key string) error {
}

// Clone returns a copy of the M. It recursively makes copies of inner
// maps.
// maps. Nested arrays and non-map types are not cloned.
func (m M) Clone() M {
result := make(M, len(m))
cloneMap(result, m)
return result
}

for k := range m {
if innerMap, ok := tryToMapStr(m[k]); ok {
result[k] = innerMap.Clone()
} else {
result[k] = m[k]
func cloneMap(dst, src M) {
for k, v := range src {
switch v := v.(type) {
case M:
d := make(M, len(v))
dst[k] = d
cloneMap(d, v)
case map[string]interface{}:
d := make(map[string]interface{}, len(v))
dst[k] = d
cloneMap(d, v)
case []M:
a := make([]M, 0, len(v))
for _, m := range v {
d := make(M, len(m))
cloneMap(d, m)
a = append(a, d)
}
dst[k] = a
case []map[string]interface{}:
a := make([]map[string]interface{}, 0, len(v))
for _, m := range v {
d := make(map[string]interface{}, len(m))
cloneMap(d, m)
a = append(a, d)
}
dst[k] = a
default:
dst[k] = v
}
}

return result
}

// HasKey returns true if the key exist. If an error occurs then false is
Expand Down
6 changes: 6 additions & 0 deletions mapstr/mapstr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ func TestClone(t *testing.T) {
"c31": 1,
"c32": 2,
},
"c4": []M{{"c41": 1}},
}

// Clone the original mapstr and then increment every value in it. Ensures the test will fail if
Expand All @@ -364,6 +365,7 @@ func TestClone(t *testing.T) {
"c31": 1,
"c32": 2,
},
"c4": []M{{"c41": 1}},
},
cloned,
)
Expand All @@ -377,6 +379,10 @@ func incrementMapstrValues(m M) {
m[k] = v + 1
case M:
incrementMapstrValues(m[k].(M))
case []M:
for _, c := range v {
incrementMapstrValues(c)
}
}

}
Expand Down

0 comments on commit e11ce6b

Please sign in to comment.