ReplaceOp clones value so that it's not accidently mutated later

Author: dmitriy kalinin

patch/replace_op.go

@@ -2,18 +2,26 @@ package patch
 
 import (
 	"fmt"
+
+	"gopkg.in/yaml.v2"
 )
 
 type ReplaceOp struct {
 	Path  Pointer
-	Value interface{}
+	Value interface{} // will be cloned using yaml library
 }
 
 func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
+	// Ensure that value is not modified by future operations
+	clonedValue, err := op.cloneValue(op.Value)
+	if err != nil {
+		return nil, fmt.Errorf("ReplaceOp cloning value: %s", err)
+	}
+
 	tokens := op.Path.Tokens()
 
 	if len(tokens) == 1 {
-		return op.Value, nil
+		return clonedValue, nil
 	}
 
 	obj := doc
@@ -36,7 +44,7 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
 			}
 
 			if isLast {
-				typedObj[idx] = op.Value
+				typedObj[idx] = clonedValue
 			} else {
 				obj = typedObj[idx]
 				prevUpdate = func(newObj interface{}) { typedObj[idx] = newObj }
@@ -49,7 +57,7 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
 			}
 
 			if isLast {
-				prevUpdate(append(typedObj, op.Value))
+				prevUpdate(append(typedObj, clonedValue))
 			} else {
 				return nil, fmt.Errorf("Expected after last index token to be last in path '%s'", op.Path)
 			}
@@ -73,7 +81,7 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
 
 			if typedToken.Optional && len(idxs) == 0 {
 				if isLast {
-					prevUpdate(append(typedObj, op.Value))
+					prevUpdate(append(typedObj, clonedValue))
 				} else {
 					obj = map[interface{}]interface{}{typedToken.Key: typedToken.Value}
 					prevUpdate(append(typedObj, obj))
@@ -87,7 +95,7 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
 				idx := idxs[0]
 
 				if isLast {
-					typedObj[idx] = op.Value
+					typedObj[idx] = clonedValue
 				} else {
 					obj = typedObj[idx]
 					// no need to change prevUpdate since matching item can only be a map
@@ -108,7 +116,7 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
 			}
 
 			if isLast {
-				typedObj[typedToken.Key] = op.Value
+				typedObj[typedToken.Key] = clonedValue
 			} else {
 				prevUpdate = func(newObj interface{}) { typedObj[typedToken.Key] = newObj }
 
@@ -137,3 +145,23 @@ func (op ReplaceOp) Apply(doc interface{}) (interface{}, error) {
 
 	return doc, nil
 }
+
+func (ReplaceOp) cloneValue(in interface{}) (out interface{}, err error) {
+	defer func() {
+		if recoverVal := recover(); recoverVal != nil {
+			err = fmt.Errorf("Recovered: %s", recoverVal)
+		}
+	}()
+
+	bytes, err := yaml.Marshal(in)
+	if err != nil {
+		return nil, err
+	}
+
+	err = yaml.Unmarshal(bytes, &out)
+	if err != nil {
+		return nil, err
+	}
+
+	return out, nil
+}

patch/replace_op_test.go

@@ -8,6 +8,23 @@ import (
 )
 
 var _ = Describe("ReplaceOp.Apply", func() {
+	It("returns error if replacement value cloning fails", func() {
+		_, err := ReplaceOp{Path: MustNewPointerFromString(""), Value: func() {}}.Apply("a")
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("ReplaceOp cloning value"))
+	})
+
+	It("uses cloned value for replacement", func() {
+		repVal := map[interface{}]interface{}{"a": "b"}
+
+		res, err := ReplaceOp{Path: MustNewPointerFromString(""), Value: repVal}.Apply("a")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(res).To(Equal(repVal))
+
+		res.(map[interface{}]interface{})["c"] = "d"
+		Expect(res).ToNot(Equal(repVal))
+	})
+
 	It("replaces document if path is for the entire document", func() {
 		res, err := ReplaceOp{Path: MustNewPointerFromString(""), Value: "b"}.Apply("a")
 		Expect(err).ToNot(HaveOccurred())
@@ -233,7 +250,7 @@ var _ = Describe("ReplaceOp.Apply", func() {
 				map[interface{}]interface{}{"xyz": "xyz"},
 				map[interface{}]interface{}{
 					"name": "val",
-					"efg": []interface{}{1},
+					"efg":  []interface{}{1},
 				},
 			}))
 		})