Merge pull request #196 from evanphx/b-null-mistype-panic

Handle null correctly when introduced by replace. Fixes #171

v5/merge.go

@@ -130,6 +130,10 @@ func doMergePatch(docData, patchData []byte, mergeMerge bool) ([]byte, error) {
 	}
 
 	if isSyntaxError(patchErr) {
+		if json.Valid(patchData) {
+			return patchData, nil
+		}
+
 		return nil, errBadJSONPatch
 	}
 
@@ -154,6 +158,10 @@ func doMergePatch(docData, patchData []byte, mergeMerge bool) ([]byte, error) {
 			patchErr = unmarshal(patchData, patchAry)
 
 			if patchErr != nil {
+				// Not an array either, a literal is the result directly.
+				if json.Valid(patchData) {
+					return patchData, nil
+				}
 				return nil, errBadJSONPatch
 			}
 

v5/merge_test.go

@@ -2,15 +2,14 @@ package jsonpatch
 
 import (
 	"fmt"
-	"strings"
 	"testing"
 )
 
 func mergePatch(doc, patch string) string {
 	out, err := MergePatch([]byte(doc), []byte(patch))
 
 	if err != nil {
-		panic(err)
+		panic(fmt.Sprintf("%s: %s", err, patch))
 	}
 
 	return string(out)
@@ -166,8 +165,8 @@ var rfcTests = []struct {
 	{target: `{"a":[{"b":"c"}]}`, patch: `{"a":[1]}`, expected: `{"a":[1]}`},
 	{target: `["a","b"]`, patch: `["c","d"]`, expected: `["c","d"]`},
 	{target: `{"a":"b"}`, patch: `["c"]`, expected: `["c"]`},
-	// {target: `{"a":"foo"}`, patch: `null`, expected: `null`},
-	// {target: `{"a":"foo"}`, patch: `"bar"`, expected: `"bar"`},
+	{target: `{"a":"foo"}`, patch: `null`, expected: `null`},
+	{target: `{"a":"foo"}`, patch: `"bar"`, expected: `"bar"`},
 	{target: `{"e":null}`, patch: `{"a":1}`, expected: `{"a":1,"e":null}`},
 	{target: `[1,2]`, patch: `{"a":"b","c":null}`, expected: `{"a":"b"}`},
 	{target: `{}`, patch: `{"a":{"bb":{"ccc":null}}}`, expected: `{"a":{"bb":{}}}`},
@@ -183,33 +182,6 @@ func TestMergePatchRFCCases(t *testing.T) {
 	}
 }
 
-var rfcFailTests = `
-     {"a":"foo"}  |   null
-     {"a":"foo"}  |   "bar"
-`
-
-func TestMergePatchFailRFCCases(t *testing.T) {
-	tests := strings.Split(rfcFailTests, "\n")
-
-	for _, c := range tests {
-		if strings.TrimSpace(c) == "" {
-			continue
-		}
-
-		parts := strings.SplitN(c, "|", 2)
-
-		doc := strings.TrimSpace(parts[0])
-		pat := strings.TrimSpace(parts[1])
-
-		out, err := MergePatch([]byte(doc), []byte(pat))
-
-		if err != errBadJSONPatch {
-			t.Errorf("error not returned properly: %s, %s", err, string(out))
-		}
-	}
-
-}
-
 func TestResembleJSONArray(t *testing.T) {
 	testCases := []struct {
 		input    []byte

v5/patch.go

@@ -254,6 +254,10 @@ func (n *lazyNode) intoDoc() (*partialDoc, error) {
 
 	err := unmarshal(*n.raw, &n.doc)
 
+	if n.doc == nil {
+		return nil, ErrInvalid
+	}
+
 	if err != nil {
 		return nil, err
 	}
@@ -308,6 +312,10 @@ func (n *lazyNode) tryDoc() bool {
 		return false
 	}
 
+	if n.doc == nil {
+		return false
+	}
+
 	n.which = eDoc
 	return true
 }
@@ -327,6 +335,18 @@ func (n *lazyNode) tryAry() bool {
 	return true
 }
 
+func (n *lazyNode) isNull() bool {
+	if n == nil {
+		return true
+	}
+
+	if n.raw == nil {
+		return true
+	}
+
+	return bytes.Equal(n.compact(), rawJSONNull)
+}
+
 func (n *lazyNode) equal(o *lazyNode) bool {
 	if n.which == eRaw {
 		if !n.tryDoc() && !n.tryAry() {
@@ -466,6 +486,10 @@ func (o Operation) From() (string, error) {
 
 func (o Operation) value() *lazyNode {
 	if obj, ok := o["value"]; ok {
+		// A `null` gets decoded as a nil RawMessage, so let's fix it up here.
+		if obj == nil {
+			return newLazyNode(newRawMessage(rawJSONNull))
+		}
 		return newLazyNode(obj)
 	}
 
@@ -818,7 +842,10 @@ func ensurePathExists(pd *container, path string, options *ApplyOptions) error {
 				newNode := newLazyNode(newRawMessage(rawJSONObject))
 
 				doc.add(part, newNode, options)
-				doc, _ = newNode.intoDoc()
+				doc, err = newNode.intoDoc()
+				if err != nil {
+					return err
+				}
 			}
 		} else {
 			if isArray(*target.raw) {
@@ -1025,12 +1052,14 @@ func (p Patch) test(doc *container, op Operation, options *ApplyOptions) error {
 		return errors.Wrapf(err, "error in test for path: '%s'", path)
 	}
 
+	ov := op.value()
+
 	if val == nil {
-		if op.value() == nil || op.value().raw == nil {
+		if ov.isNull() {
 			return nil
 		}
 		return errors.Wrapf(ErrTestFailed, "testing value %s failed", path)
-	} else if op.value() == nil {
+	} else if ov.isNull() {
 		return errors.Wrapf(ErrTestFailed, "testing value %s failed", path)
 	}
 

v5/patch_test.go

@@ -578,6 +578,13 @@ var Cases = []Case{
 		false,
 		false,
 	},
+	{
+		`{}`,
+		`[{"op": "replace", "path": "", "value": null}]`,
+		`null`,
+		false,
+		false,
+	},
 }
 
 type BadCase struct {
@@ -727,6 +734,16 @@ var BadCases = []BadCase{
 		`[{"op": "copy", "path": "/qux", "from": "/baz"}]`,
 		false,
 	},
+	{
+		`{ "foo": {"bar": []}}`,
+		`[{"op": "replace", "path": "/foo/bar", "value": null}, {"op": "add", "path": "/foo/bar/0", "value": "blah"}]`,
+		false,
+	},
+	{
+		`{}`,
+		`[{"op": "replace", "path": ""}]`,
+		true,
+	},
 }
 
 // This is not thread safe, so we cannot run patch tests in parallel.