diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 1eef4971e7..d822f4ef74 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -31,6 +31,10 @@ module.exports = RedisManager = timer.done() _callback(error) docLines = JSON.stringify(docLines) + if docLines.indexOf("\u0000") != -1 + error = new Error("null bytes found in doc lines") + logger.error err: error, doc_id: doc_id, docLines: docLines, error.message + return callback(error) docHash = RedisManager._computeHash(docLines) logger.log project_id:project_id, doc_id:doc_id, version: version, hash:docHash, "putting doc in redis" ranges = RedisManager._serializeRanges(ranges) @@ -148,10 +152,18 @@ module.exports = RedisManager = error = new Error("Version mismatch. '#{doc_id}' is corrupted.") logger.error {err: error, doc_id, currentVersion, newVersion, opsLength: appliedOps.length}, "version mismatch" return callback(error) + jsonOps = appliedOps.map (op) -> JSON.stringify op - multi = rclient.multi() newDocLines = JSON.stringify(docLines) + if newDocLines.indexOf("\u0000") != -1 + error = new Error("null bytes found in doc lines") + logger.error err: error, doc_id: doc_id, newDocLines: newDocLines, error.message + return callback(error) newHash = RedisManager._computeHash(newDocLines) + + logger.log doc_id: doc_id, version: newVersion, hash: newHash, "updating doc in redis" + + multi = rclient.multi() multi.eval setScript, 1, keys.docLines(doc_id:doc_id), newDocLines multi.set keys.docVersion(doc_id:doc_id), newVersion multi.set keys.docHash(doc_id:doc_id), newHash diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index 89f58bfd1f..2ad3281bfe 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -61,7 +61,6 @@ module.exports = UpdateManager = return callback(error) if error? RangesManager.applyUpdate project_id, doc_id, ranges, appliedOps, (error, new_ranges) -> return callback(error) if error? - logger.log doc_id: doc_id, version: version, "updating doc in redis" RedisManager.updateDocument doc_id, updatedDocLines, version, appliedOps, new_ranges, (error) -> return callback(error) if error? HistoryManager.pushUncompressedHistoryOps project_id, doc_id, appliedOps, callback diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index 3e3128324e..abc7307c15 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -12,26 +12,30 @@ describe "RedisManager", -> auth: () -> exec: sinon.stub() @rclient.multi = () => @rclient - @RedisManager = SandboxedModule.require modulePath, requires: - "./RedisBackend": - createClient: () => @rclient - "./RedisKeyBuilder": - blockingKey: ({doc_id}) -> "Blocking:#{doc_id}" - docLines: ({doc_id}) -> "doclines:#{doc_id}" - docOps: ({doc_id}) -> "DocOps:#{doc_id}" - docVersion: ({doc_id}) -> "DocVersion:#{doc_id}" - docHash: ({doc_id}) -> "DocHash:#{doc_id}" - projectKey: ({doc_id}) -> "ProjectId:#{doc_id}" - pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}" - docsInProject: ({project_id}) -> "DocsIn:#{project_id}" - ranges: ({doc_id}) -> "Ranges:#{doc_id}" - "logger-sharelatex": @logger = { error: sinon.stub(), log: sinon.stub(), warn: sinon.stub() } - "settings-sharelatex": {documentupdater: {logHashErrors: {write:true, read:true}}} - "./Metrics": @metrics = - inc: sinon.stub() - Timer: class Timer - done: () -> - "./Errors": Errors + @RedisManager = SandboxedModule.require modulePath, + requires: + "./RedisBackend": + createClient: () => @rclient + "./RedisKeyBuilder": + blockingKey: ({doc_id}) -> "Blocking:#{doc_id}" + docLines: ({doc_id}) -> "doclines:#{doc_id}" + docOps: ({doc_id}) -> "DocOps:#{doc_id}" + docVersion: ({doc_id}) -> "DocVersion:#{doc_id}" + docHash: ({doc_id}) -> "DocHash:#{doc_id}" + projectKey: ({doc_id}) -> "ProjectId:#{doc_id}" + pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}" + docsInProject: ({project_id}) -> "DocsIn:#{project_id}" + ranges: ({doc_id}) -> "Ranges:#{doc_id}" + "logger-sharelatex": @logger = { error: sinon.stub(), log: sinon.stub(), warn: sinon.stub() } + "settings-sharelatex": {documentupdater: {logHashErrors: {write:true, read:true}}} + "./Metrics": @metrics = + inc: sinon.stub() + Timer: class Timer + done: () -> + "./Errors": Errors + globals: + JSON: @JSON = JSON + @doc_id = "doc-id-123" @project_id = "project-id-123" @callback = sinon.stub() @@ -318,6 +322,22 @@ describe "RedisManager", -> it "should call the callback", -> @callback.called.should.equal true + describe "with null bytes in the serialized doc lines", -> + beforeEach -> + @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) + @_stringify = JSON.stringify + @JSON.stringify = () -> return '["bad bytes! \u0000 <- here"]' + @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback + + afterEach -> + @JSON.stringify = @_stringify + + it "should log an error", -> + @logger.error.called.should.equal true + + it "should call the callback with an error", -> + @callback.calledWith(new Error("null bytes found in doc lines")).should.equal true + describe "putDocInMemory", -> beforeEach -> @rclient.set = sinon.stub() @@ -391,6 +411,21 @@ describe "RedisManager", -> @logger.error.calledWith() .should.equal true + describe "with null bytes in the serialized doc lines", -> + beforeEach -> + @_stringify = JSON.stringify + @JSON.stringify = () -> return '["bad bytes! \u0000 <- here"]' + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @callback + + afterEach -> + @JSON.stringify = @_stringify + + it "should log an error", -> + @logger.error.called.should.equal true + + it "should call the callback with an error", -> + @callback.calledWith(new Error("null bytes found in doc lines")).should.equal true + describe "removeDocFromMemory", -> beforeEach (done) -> @rclient.del = sinon.stub()