From 966478cac48fed23e3a26e40f3298a1585ffac72 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 11 Jun 2019 14:11:46 +0100 Subject: [PATCH 1/2] fix hash check to use 'v' field instead of version --- .../document-updater/app/coffee/ShareJsUpdateManager.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/document-updater/app/coffee/ShareJsUpdateManager.coffee b/services/document-updater/app/coffee/ShareJsUpdateManager.coffee index 66b8367395..c81cccdc77 100644 --- a/services/document-updater/app/coffee/ShareJsUpdateManager.coffee +++ b/services/document-updater/app/coffee/ShareJsUpdateManager.coffee @@ -26,7 +26,7 @@ module.exports = ShareJsUpdateManager = logger.log project_id: project_id, doc_id: doc_id, update: update, "applying sharejs updates" jobs = [] # record the update version before it is modified - incomingUpdateVersion = update.version + incomingUpdateVersion = update.v # We could use a global model for all docs, but we're hitting issues with the # internal state of ShareJS not being accessible for clearing caches, and # getting stuck due to queued callbacks (line 260 of sharejs/server/model.coffee) From 984b2d38e6b0867fe4dc7dc2f17f4e41277b34cd Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 11 Jun 2019 16:33:14 +0100 Subject: [PATCH 2/2] add unit tests --- .../ShareJsUpdateManagerTests.coffee | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee index 236112684a..363705845f 100644 --- a/services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/ShareJsUpdateManager/ShareJsUpdateManagerTests.coffee @@ -3,6 +3,7 @@ chai = require('chai') should = chai.should() modulePath = "../../../../app/js/ShareJsUpdateManager.js" SandboxedModule = require('sandboxed-module') +crypto = require('crypto') describe "ShareJsUpdateManager", -> beforeEach -> @@ -26,8 +27,10 @@ describe "ShareJsUpdateManager", -> beforeEach -> @lines = ["one", "two"] @version = 34 - @update = {p: 4, t: "foo"} @updatedDocLines = ["onefoo", "two"] + content = @updatedDocLines.join("\n") + @hash = crypto.createHash('sha1').update("blob " + content.length + "\x00").update(content, 'utf8').digest('hex') + @update = {p: 4, t: "foo", v:@version, hash:@hash} @model = applyOp: sinon.stub().callsArg(2) getSnapshot: sinon.stub() @@ -90,6 +93,18 @@ describe "ShareJsUpdateManager", -> it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true + describe "with an invalid hash", -> + beforeEach (done) -> + @error = new Error("invalid hash") + @model.getSnapshot.callsArgWith(1, null, {snapshot: "unexpected content", v: @version}) + @model.db.appliedOps["#{@project_id}:#{@doc_id}"] = @appliedOps = ["mock-ops"] + @ShareJsUpdateManager.applyUpdate @project_id, @doc_id, @update, @lines, @version, (err, docLines, version, appliedOps) => + @callback(err, docLines, version, appliedOps) + done() + + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true + describe "_listenForOps", -> beforeEach -> @model = on: (event, callback) =>