Merge pull request #60 from launchdarkly/prevent-bad-rm

Support array remove with negative index

Author: evanphx

patch.go

@@ -389,17 +389,13 @@ func (d *partialArray) add(key string, val *lazyNode) error {
 
 	cur := *d
 
-	if idx < 0 {
-		idx *= -1
-
-		if idx > len(ary) {
-			return fmt.Errorf("Unable to access invalid index: %d", idx)
-		}
-		idx = len(ary) - idx
-	}
-	if idx < 0 || idx >= len(ary) || idx > len(cur) {
+	if idx < -len(ary) || idx >= len(ary) {
 		return fmt.Errorf("Unable to access invalid index: %d", idx)
 	}
+
+	if idx < 0 {
+		idx += len(ary)
+	}
 	copy(ary[0:idx], cur[0:idx])
 	ary[idx] = val
 	copy(ary[idx+1:], cur[idx:])
@@ -430,9 +426,12 @@ func (d *partialArray) remove(key string) error {
 
 	cur := *d
 
-	if idx >= len(cur) {
+	if idx < -len(cur) || idx >= len(cur) {
 		return fmt.Errorf("Unable to remove invalid index: %d", idx)
 	}
+	if idx < 0 {
+		idx += len(cur)
+	}
 
 	ary := make([]*lazyNode, len(cur)-1)
 

patch_test.go

@@ -168,6 +168,16 @@ var Cases = []Case{
 		`[ { "op": "copy", "from": "/0/foo/bar", "path": "/0/baz/bar"}]`,
 		`[ { "baz": {"bar": ["qux","baz"], "qux":"bum"}, "foo": {"bar": ["qux","baz"]}}]`,
 	},
+	{
+		`{ "foo": ["bar","qux","baz"]}`,
+		`[ { "op": "remove", "path": "/foo/-2"}]`,
+		`{ "foo": ["bar", "baz"]}`,
+	},
+	{
+		`{ "foo": []}`,
+		`[ { "op": "add", "path": "/foo/-1", "value": "qux"}]`,
+		`{ "foo": ["qux"]}`,
+	},
 	{
 		`{ "bar": [{"baz": null}]}`,
 		`[ { "op": "replace", "path": "/bar/0/baz", "value": 1 } ]`,
@@ -242,7 +252,6 @@ var BadCases = []BadCase{
 		`{ "foo": ["bar","baz"]}`,
 		`[ { "op": "add", "path": "/foo/-4", "value": "bum"}]`,
 	},
-
 	{
 		`{ "name":{ "foo": "bat", "qux": "bum"}}`,
 		`[ { "op": "replace", "path": "/foo/bar", "value":"baz"}]`,
@@ -252,6 +261,18 @@ var BadCases = []BadCase{
 		`[ {"op": "add", "path": "/foo/2", "value": "bum"}]`,
 	},
 	{
+		`{ "foo": []}`,
+		`[ {"op": "remove", "path": "/foo/-"}]`,
+	},
+	{
+		`{ "foo": []}`,
+		`[ {"op": "remove", "path": "/foo/-1"}]`,
+	},
+	{
+		`{ "foo": ["bar"]}`,
+		`[ {"op": "remove", "path": "/foo/-2"}]`,
+  },
+  {
 		`{}`,
 		`[ {"op":null,"path":""} ]`,
 	},