diff --git a/lib/client/doc.js b/lib/client/doc.js index d3713492..8205915c 100644 --- a/lib/client/doc.js +++ b/lib/client/doc.js @@ -1,5 +1,6 @@ var emitter = require('../emitter'); var logger = require('../logger'); +var ot = require('../ot'); var ShareDBError = require('../error'); var types = require('../types'); var util = require('../util'); @@ -599,6 +600,9 @@ Doc.prototype._otApply = function(op, source) { ); } + var pathError = ot.checkOpPathsForType(this.type.name, op); + if (pathError) throw pathError; + // NB: If we need to add another argument to this event, we should consider // the fact that the 'op' event has op.src as its 3rd argument this.emit('before op batch', op.op, source); @@ -765,6 +769,14 @@ Doc.prototype._submit = function(op, source, callback) { } // Try to normalize the op. This removes trailing skip:0's and things like that. if (this.type.normalize) op.op = this.type.normalize(op.op); + + // This has to happen before _pushOp(), because _tryCompose() applies the op + // to a pending create, well before _otApply() gets a chance to check it + var pathError = ot.checkOpPathsForType(this.type.name, op); + if (pathError) { + if (callback) return callback(pathError); + return this.emit('error', pathError); + } } try { diff --git a/lib/ot.js b/lib/ot.js index 4618598b..184117d2 100644 --- a/lib/ot.js +++ b/lib/ot.js @@ -165,6 +165,15 @@ exports.checkOpForType = function(typeName, op) { return checkJson0Op(op.op); }; +// Ops that are only being applied, rather than submitted, get their paths +// checked but not their shape +exports.checkOpPathsForType = function(typeName, op) { + if (!('op' in op)) return; + var type = types.map[typeName]; + if (!type || type.name !== 'json0') return; + return checkJson0OpPaths(op.op); +}; + exports.transform = function(type, op, appliedOp) { // There are 16 cases this function needs to deal with - which are all the // combinations of create/delete/op/noop from both op and appliedOp diff --git a/test/client/doc.js b/test/client/doc.js index 55833ca2..c4ab7b86 100644 --- a/test/client/doc.js +++ b/test/client/doc.js @@ -829,6 +829,123 @@ describe('Doc', function() { }); }); }); + + function expectInvalidPathSegment(error) { + expect(error.code).to.equal(ShareDBError.CODES.ERR_OT_OP_NOT_APPLIED); + expect(error.message).to.equal('Invalid path segment'); + expect({}.polluted).to.equal(undefined); + } + + describe('ops from the server', function() { + var multiComponentOp = [{p: ['baz'], oi: true}, {p: ['__proto__', 'polluted'], oi: 'oops'}]; + var connection; + var doc; + + beforeEach(function(done) { + connection = this.connection; + doc = connection.get('test-collection', 'test-doc'); + doc.create({foo: 'bar'}, done); + }); + + function sendOp(op) { + connection.handleMessage({ + a: 'op', + c: doc.collection, + d: doc.id, + v: 1, + src: 'hostile', + seq: 1, + op: op + }); + } + + [ + { + name: 'ops with a dangerous first path segment', + op: [{p: ['__proto__', 'polluted'], oi: 'oops'}] + }, + { + name: 'ops with a dangerous later path segment', + op: [{p: ['foo', 'constructor'], oi: 'oops'}] + }, + { + name: 'an array-like op', + op: {0: {p: ['__proto__', 'polluted'], oi: 'oops'}, length: 1} + }, + { + name: 'ops with a path segment that is not a string', + op: [{p: [['__proto__'], 'polluted'], oi: 'oops'}] + }, + { + name: 'multi-component ops', + op: multiComponentOp + } + ].forEach(function(test) { + it('Rejects ' + test.name, function(done) { + doc.on('error', function(error) { + expectInvalidPathSegment(error); + done(); + }); + sendOp(test.op); + }); + }); + + it('does not apply the valid components of a rejected multi-component op', function(done) { + doc.on('error', function() { + expect(doc.data).to.not.have.property('baz'); + done(); + }); + sendOp(multiComponentOp); + }); + }); + + describe('locally submitted ops', function() { + var badOp = [{p: ['__proto__', 'polluted'], oi: 'oops'}]; + + it('rejects an op composed into a pending create', function(done) { + var doc = this.connection.get('test-collection', 'test-doc'); + doc.create({foo: 'bar'}); + doc.submitOp(badOp, function(error) { + expectInvalidPathSegment(error); + done(); + }); + }); + + it('rejects an op without sending it to the server', function(done) { + var doc = this.connection.get('test-collection', 'test-doc'); + doc.create({foo: 'bar'}, function(error) { + if (error) return done(error); + var calledBack = false; + doc.submitOp(badOp, function(error) { + calledBack = true; + expectInvalidPathSegment(error); + }); + // The server would only reject asynchronously, so calling back + // synchronously is how we know the op never left the client + expect(calledBack).to.equal(true); + done(); + }); + }); + + it('leaves the doc usable after rejecting an op', function(done) { + var doc = this.connection.get('test-collection', 'test-doc'); + async.series([ + doc.create.bind(doc, {foo: 'bar'}), + function(next) { + doc.submitOp(badOp, function(error) { + expect(error).to.be.instanceOf(Error); + next(); + }); + }, + doc.submitOp.bind(doc, [{p: ['baz'], oi: true}]), + doc.whenNothingPending.bind(doc), + function(next) { + expect(doc.data).to.eql({foo: 'bar', baz: true}); + next(); + } + ], done); + }); + }); }); describe('toSnapshot', function() {