Skip to content

Commit 3e9fef2

Browse files
author
Alec Gibson
committed
Throw when array and object deletion values do not match current version
This change adds validation to array and object deletion. If the values provided in either `ld` or `od` do not match the current value, then `apply` will `throw`. It will also throw if `oi` overwrites an existing value without providing `od`. The primary motivation of this change is to ensure that all submitted ops remain reversible. At the moment, the following series of ops is possible: - start with `{ foo: 'bar' }` - `apply` this op: `{ p: ['foo'], oi: 'baz' }` - ...resulting in `{ foo: 'baz' }` - `invert` the previous op: `{ p: ['foo'], od: 'baz' }` - and `apply` the inverted op: `{}` When I apply, invert and apply, I should always wind up where I started, but in this case I clearly do not. By ensuring that the `od` matches the current value, we make sure that all ops remain reversible. Deep equality adds a dependency on [`fast-deep-equal`][1]. [1]: https://github.com/epoberezkin/fast-deep-equal
1 parent 9df44f0 commit 3e9fef2

File tree

3 files changed

+30
-9
lines changed

3 files changed

+30
-9
lines changed

lib/json0.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
var deepEqual = require('fast-deep-equal');
2+
13
/*
24
This is the implementation of the JSON OT type.
35
@@ -181,7 +183,8 @@ json.apply = function(snapshot, op) {
181183
// List replace
182184
else if (c.li !== void 0 && c.ld !== void 0) {
183185
json.checkList(elem);
184-
// Should check the list element matches c.ld
186+
if (!deepEqual(elem[key], c.ld))
187+
throw new Error('List deletion value does not match current value')
185188
elem[key] = c.li;
186189
}
187190

@@ -194,7 +197,8 @@ json.apply = function(snapshot, op) {
194197
// List delete
195198
else if (c.ld !== void 0) {
196199
json.checkList(elem);
197-
// Should check the list element matches c.ld here too.
200+
if (!deepEqual(elem[key], c.ld))
201+
throw new Error('List deletion value does not match current value')
198202
elem.splice(key,1);
199203
}
200204

@@ -214,15 +218,17 @@ json.apply = function(snapshot, op) {
214218
else if (c.oi !== void 0) {
215219
json.checkObj(elem);
216220

217-
// Should check that elem[key] == c.od
221+
if (!deepEqual(elem[key], c.od))
222+
throw new Error('Object deletion value does not match current value')
218223
elem[key] = c.oi;
219224
}
220225

221226
// Object delete
222227
else if (c.od !== void 0) {
223228
json.checkObj(elem);
224229

225-
// Should check that elem[key] == c.od
230+
if (!deepEqual(elem[key], c.od))
231+
throw new Error('Object deletion value does not match current value')
226232
delete elem[key];
227233
}
228234

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
"directories": {
77
"test": "test"
88
},
9-
"dependencies": {},
9+
"dependencies": {
10+
"fast-deep-equal": "^2.0.1"
11+
},
1012
"devDependencies": {
1113
"ot-fuzzer": "^1.0.0",
1214
"mocha": "^1.20.1",

test/json0.coffee

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,14 @@ genTests = (type) ->
6767

6868
# Strings should be handled internally by the text type. We'll just do some basic sanity checks here.
6969
describe 'string', ->
70-
describe '#apply()', -> it 'works', ->
71-
assert.deepEqual 'abc', type.apply 'a', [{p:[1], si:'bc'}]
72-
assert.deepEqual 'bc', type.apply 'abc', [{p:[0], sd:'a'}]
73-
assert.deepEqual {x:'abc'}, type.apply {x:'a'}, [{p:['x', 1], si:'bc'}]
70+
describe '#apply()', ->
71+
it 'works', ->
72+
assert.deepEqual 'abc', type.apply 'a', [{p:[1], si:'bc'}]
73+
assert.deepEqual 'bc', type.apply 'abc', [{p:[0], sd:'a'}]
74+
assert.deepEqual {x:'abc'}, type.apply {x:'a'}, [{p:['x', 1], si:'bc'}]
75+
76+
it 'throws when the deletion target does not match', ->
77+
assert.throws -> type.apply 'abc', [{p:[0], sd:'x'}]
7478

7579
describe '#transform()', ->
7680
it 'splits deletes', ->
@@ -127,6 +131,10 @@ genTests = (type) ->
127131
assert.deepEqual ['a', 'b', 'c'], type.apply ['b', 'a', 'c'], [{p:[1], lm:0}]
128132
assert.deepEqual ['a', 'b', 'c'], type.apply ['b', 'a', 'c'], [{p:[0], lm:1}]
129133

134+
it 'throws when the deletion target does not match', ->
135+
assert.throws -> type.apply ['a', 'b', 'c'], [{p:[0], ld: 'x'}]
136+
assert.throws -> type.apply ['a', 'b', 'c'], [{p:[0], li: 'd', ld: 'x'}]
137+
130138
###
131139
'null moves compose to nops', ->
132140
assert.deepEqual [], type.compose [], [{p:[3],lm:3}]
@@ -389,6 +397,11 @@ genTests = (type) ->
389397
assert.deepEqual [], type.transform [{p:['k'], od:'x'}], [{p:['k'], od:'x'}], 'left'
390398
assert.deepEqual [], type.transform [{p:['k'], od:'x'}], [{p:['k'], od:'x'}], 'right'
391399

400+
it 'throws when the deletion target does not match', ->
401+
assert.throws -> type.apply {x:'a'}, [{p:['x'], od: 'b'}]
402+
assert.throws -> type.apply {x:'a'}, [{p:['x'], oi: 'c', od: 'b'}]
403+
assert.throws -> type.apply {x:'a'}, [{p:['x'], oi: 'b'}]
404+
392405
describe 'randomizer', ->
393406
@timeout 20000
394407
@slow 6000

0 commit comments

Comments
 (0)