Fix Append modifying a possibly shared array (#8)
It's possible to have a multiError that is appended to multiple times
causing an underlying array to be shared and end up getting modified.
For now, remove the special case for `append(multiError, err)` which can
cause this bug.
diff --git a/error.go b/error.go
index 1e1fe28..a247250 100644
--- a/error.go
+++ b/error.go
@@ -264,14 +264,10 @@
}
if _, ok := right.(multiError); !ok {
- if l, ok := left.(multiError); ok {
- // Common case where the error on the left is constantly being
- // appended to.
- return append(l, right)
+ if _, ok := left.(multiError); !ok {
+ // Both errors are single errors.
+ return multiError{left, right}
}
-
- // Both errors are single errors.
- return multiError{left, right}
}
// Either right or both, left and right, are multiErrors. Rely on usual
diff --git a/error_test.go b/error_test.go
index 67b04bc..e3a6804 100644
--- a/error_test.go
+++ b/error_test.go
@@ -26,6 +26,14 @@
}
}
+func appendN(initial, err error, n int) error {
+ errs := initial
+ for i := 0; i < n; i++ {
+ errs = Append(errs, err)
+ }
+ return errs
+}
+
func TestCombine(t *testing.T) {
tests := []struct {
giveErrors []error
@@ -296,3 +304,21 @@
assert.Equal(t, tt.want, Append(tt.left, tt.right))
}
}
+
+func TestAppendDoesNotModify(t *testing.T) {
+ createInitial := func() error {
+ // Create a multiError that has capacity for more errors so Append will
+ // modify the underlying array that may be shared.
+ return appendN(nil, errors.New("initial"), 50)
+ }
+
+ initial := createInitial()
+ err1 := Append(initial, errors.New("err1"))
+ err2 := Append(initial, errors.New("err2"))
+
+ // initial should not have been modified.
+ assert.Equal(t, createInitial(), initial, "Initial should not be modified")
+
+ assert.Equal(t, Append(createInitial(), errors.New("err1")), err1)
+ assert.Equal(t, Append(createInitial(), errors.New("err2")), err2)
+}
