From ccf4683558928925ec2f637be05916e8f1a2e91e Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 10 Oct 2013 16:38:16 +0200 Subject: [PATCH 1/3] Easysync: Throw an error if an unknown attrib is referneced --- src/static/js/Changeset.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index f92703851..5841daddc 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -1504,6 +1504,7 @@ exports.moveOpsToNewPool = function (cs, oldPool, newPool) { return upToDollar.replace(/\*([0-9a-z]+)/g, function (_, a) { var oldNum = exports.parseNum(a); var pair = oldPool.getAttrib(oldNum); + if(!pair) exports.error('Can\'t copy unknown attrib (reference attrib string to non-existant pool entry). Inconsistent attrib state!'); var newNum = newPool.putAttrib(pair); return '*' + exports.numToString(newNum); }) + fromDollar; From 6689a3c02e8c25ef30832f27b6647627f3f97d12 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 10 Oct 2013 16:38:41 +0200 Subject: [PATCH 2/3] Catch errors during preparation of client vars ... and disconnect the user --- src/node/handler/PadMessageHandler.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 4adf60022..0bdc402f4 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -1006,11 +1006,17 @@ function handleClientReady(client, message) //This is a normal first connect else { - //prepare all values for the wire - var atext = Changeset.cloneAText(pad.atext); - var attribsForWire = Changeset.prepareForWire(atext.attribs, pad.pool); - var apool = attribsForWire.pool.toJsonable(); - atext.attribs = attribsForWire.translated; + //prepare all values for the wire, there'S a chance that this throws, if the pad is corrupted + try { + var atext = Changeset.cloneAText(pad.atext); + var attribsForWire = Changeset.prepareForWire(atext.attribs, pad.pool); + var apool = attribsForWire.pool.toJsonable(); + atext.attribs = attribsForWire.translated; + }catch(e) { + console.error(e.stack || e) + client.json.send({disconnect:"padCorrupted"});// pull the breaks + return callback(); + } // Warning: never ever send padIds.padId to the client. If the // client is read only you would open a security hole 1 swedish From d4c99d40b832a0742de0f466ef30f7c680888dcf Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Thu, 10 Oct 2013 18:19:25 +0200 Subject: [PATCH 3/3] Never keep processing a changeset if it's corrupted --- src/node/handler/PadMessageHandler.js | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 0bdc402f4..e96f64eb3 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -594,7 +594,7 @@ function handleUserChanges(data, cb) // defined in the accompanying attribute pool. Changeset.eachAttribNumber(changeset, function(n) { if (! wireApool.getAttrib(n)) { - throw "Attribute pool is missing attribute "+n+" for changeset "+changeset; + throw new Error("Attribute pool is missing attribute "+n+" for changeset "+changeset); } }); @@ -608,23 +608,22 @@ function handleUserChanges(data, cb) if(!attr) return attr = wireApool.getAttrib(attr) if(!attr) return - if('author' == attr[0] && attr[1] != thisSession.author) throw "Trying to submit changes as another author" + if('author' == attr[0] && attr[1] != thisSession.author) throw new Error("Trying to submit changes as another author in changeset "+changeset); }) } + + //ex. adoptChangesetAttribs + + //Afaik, it copies the new attributes from the changeset, to the global Attribute Pool + changeset = Changeset.moveOpsToNewPool(changeset, wireApool, pad.pool); } catch(e) { // There is an error in this changeset, so just refuse it - console.warn("Can't apply USER_CHANGES "+changeset+", because: "+e); client.json.send({disconnect:"badChangeset"}); - return callback(); + return callback(new Error("Can't apply USER_CHANGES, because "+e.message)); } - //ex. adoptChangesetAttribs - - //Afaik, it copies the new attributes from the changeset, to the global Attribute Pool - changeset = Changeset.moveOpsToNewPool(changeset, wireApool, pad.pool); - //ex. applyUserChanges apool = pad.pool; r = baseRev; @@ -651,9 +650,8 @@ function handleUserChanges(data, cb) { changeset = Changeset.follow(c, changeset, false, apool); }catch(e){ - console.warn("Can't apply USER_CHANGES "+changeset+", possibly because of mismatched follow error"); client.json.send({disconnect:"badChangeset"}); - return callback(); + return callback(new Error("Can't apply USER_CHANGES, because "+e.message)); } if ((r - baseRev) % 200 == 0) { // don't let the stack get too deep @@ -674,9 +672,8 @@ function handleUserChanges(data, cb) if (Changeset.oldLen(changeset) != prevText.length) { - console.warn("Can't apply USER_CHANGES "+changeset+" with oldLen " + Changeset.oldLen(changeset) + " to document of length " + prevText.length); client.json.send({disconnect:"badChangeset"}); - return callback(); + return callback(new Error("Can't apply USER_CHANGES "+changeset+" with oldLen " + Changeset.oldLen(changeset) + " to document of length " + prevText.length)); } pad.appendRevision(changeset, thisSession.author); @@ -700,7 +697,7 @@ function handleUserChanges(data, cb) ], function(err) { cb(); - ERR(err); + if(err) console.warn(err.stack || err) }); }