Issue #3007: Fix search clone array corruption

Method-chaining in gorm is predicated on `search`'s `clone` method
returning a copy of the `search` instance such that no change that
is made to the copy affects the original.

However, the original implementation copied the slices of the various
slice properties (such as `whereConditions`), with the incorrect
understanding that `append` would create a new slice each time that
it was called.  In some cases, this is true.  Practically, go
doubles the size of the slice once it gets full, so the following
slice `append` calls would result in a new slice:

* 0 -> 1
* 1 -> 2
* 2 -> 4
* 4 -> 8
* and so on.

So, when the number of "where" conditions (or any other slice
property) was 0, 1, 2, or 4, method-chaining would work as expected.
However, when it was 3, 5, 6, or 7, modifying the copy would modify
the original.

The solution is simply to create new slices in `clone`.
This commit is contained in:
Douglas Danger Manley 2020-05-07 16:54:33 -04:00
parent 7ea143b548
commit 2879c7e762
2 changed files with 73 additions and 1 deletions

View File

@ -32,7 +32,57 @@ type searchPreload struct {
} }
func (s *search) clone() *search { func (s *search) clone() *search {
clone := *s clone := search{
db: s.db,
whereConditions: make([]map[string]interface{}, len(s.whereConditions)),
orConditions: make([]map[string]interface{}, len(s.orConditions)),
notConditions: make([]map[string]interface{}, len(s.notConditions)),
havingConditions: make([]map[string]interface{}, len(s.havingConditions)),
joinConditions: make([]map[string]interface{}, len(s.joinConditions)),
initAttrs: make([]interface{}, len(s.initAttrs)),
assignAttrs: make([]interface{}, len(s.assignAttrs)),
selects: s.selects,
omits: make([]string, len(s.omits)),
orders: make([]interface{}, len(s.orders)),
preload: make([]searchPreload, len(s.preload)),
offset: s.offset,
limit: s.limit,
group: s.group,
tableName: s.tableName,
raw: s.raw,
Unscoped: s.Unscoped,
ignoreOrderQuery: s.ignoreOrderQuery,
}
for i, value := range s.whereConditions {
clone.whereConditions[i] = value
}
for i, value := range s.orConditions {
clone.orConditions[i] = value
}
for i, value := range s.notConditions {
clone.notConditions[i] = value
}
for i, value := range s.havingConditions {
clone.havingConditions[i] = value
}
for i, value := range s.joinConditions {
clone.joinConditions[i] = value
}
for i, value := range s.initAttrs {
clone.initAttrs[i] = value
}
for i, value := range s.assignAttrs {
clone.assignAttrs[i] = value
}
for i, value := range s.omits {
clone.omits[i] = value
}
for i, value := range s.orders {
clone.orders[i] = value
}
for i, value := range s.preload {
clone.preload[i] = value
}
return &clone return &clone
} }

View File

@ -1,6 +1,7 @@
package gorm package gorm
import ( import (
"fmt"
"reflect" "reflect"
"testing" "testing"
) )
@ -28,3 +29,24 @@ func TestCloneSearch(t *testing.T) {
t.Errorf("selectStr should be copied") t.Errorf("selectStr should be copied")
} }
} }
func TestWhereCloneCorruption(t *testing.T) {
for whereCount := 1; whereCount <= 8; whereCount++ {
t.Run(fmt.Sprintf("w=%d", whereCount), func(t *testing.T) {
s := new(search)
for w := 0; w < whereCount; w++ {
s = s.clone().Where(fmt.Sprintf("w%d = ?", w), fmt.Sprintf("value%d", w))
}
if len(s.whereConditions) != whereCount {
t.Errorf("s: where count should be %d", whereCount)
}
q1 := s.clone().Where("finalThing = ?", "THING1")
q2 := s.clone().Where("finalThing = ?", "THING2")
if reflect.DeepEqual(q1.whereConditions, q2.whereConditions) {
t.Errorf("Where conditions should be different")
}
})
}
}