Merge pull request #15 from sharelatex/bg-verify-writes

store sha1 hash of docLines in redis

services/document-updater/app/coffee/RedisKeyBuilder.coffee

@@ -28,6 +28,8 @@ module.exports = RedisKeyBuilder =
 		return (key_schema) -> key_schema.docOps({doc_id})
 	docVersion: ({doc_id}) ->
 		return (key_schema) -> key_schema.docVersion({doc_id})
+	docHash: ({doc_id}) ->
+		return (key_schema) -> key_schema.docHash({doc_id})
 	projectKey: ({doc_id}) ->
 		return (key_schema) -> key_schema.projectKey({doc_id})
 	uncompressedHistoryOp: ({doc_id}) ->

services/document-updater/app/coffee/RedisManager.coffee

@@ -6,6 +6,7 @@ keys = require('./RedisKeyBuilder')
 logger = require('logger-sharelatex')
 metrics = require('./Metrics')
 Errors = require "./Errors"
+crypto = require "crypto"
 
 # Make times easy to read
 minutes = 60 # seconds for Redis expire
@@ -18,12 +19,15 @@ module.exports = RedisManager =
 		callback = (error) ->
 			timer.done()
 			_callback(error)
-		logger.log project_id:project_id, doc_id:doc_id, version: version, "putting doc in redis"
+		docLines = JSON.stringify(docLines)
+		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)
 		multi = rclient.multi()
-		multi.set keys.docLines(doc_id:doc_id), JSON.stringify(docLines)
+		multi.set keys.docLines(doc_id:doc_id), docLines
 		multi.set keys.projectKey({doc_id:doc_id}), project_id
 		multi.set keys.docVersion(doc_id:doc_id), version
+		multi.set keys.docHash(doc_id:doc_id), docHash
 		if ranges?
 			multi.set keys.ranges(doc_id:doc_id), ranges
 		else
@@ -46,6 +50,7 @@ module.exports = RedisManager =
 		multi.del keys.docLines(doc_id:doc_id)
 		multi.del keys.projectKey(doc_id:doc_id)
 		multi.del keys.docVersion(doc_id:doc_id)
+		multi.del keys.docHash(doc_id:doc_id)
 		multi.del keys.ranges(doc_id:doc_id)
 		multi.exec (error) ->
 			return callback(error) if error?
@@ -56,11 +61,19 @@ module.exports = RedisManager =
 		multi = rclient.multi()
 		multi.get keys.docLines(doc_id:doc_id)
 		multi.get keys.docVersion(doc_id:doc_id)
+		multi.get keys.docHash(doc_id:doc_id)
 		multi.get keys.projectKey(doc_id:doc_id)
 		multi.get keys.ranges(doc_id:doc_id)
-		multi.exec (error, [docLines, version, doc_project_id, ranges])->
+		multi.exec (error, [docLines, version, storedHash, doc_project_id, ranges])->
 			timer.done()
 			return callback(error) if error?
+
+			# check sha1 hash value if present
+			if docLines? and storedHash?
+				computedHash = RedisManager._computeHash(docLines)
+				if computedHash isnt storedHash
+					logger.error project_id: project_id, doc_id: doc_id, doc_project_id: doc_project_id, computedHash: computedHash, storedHash: storedHash, "hash mismatch on retrieved document"
+
 			try
 				docLines = JSON.parse docLines
 				ranges = RedisManager._deserializeRanges(ranges)
@@ -121,8 +134,11 @@ module.exports = RedisManager =
 				return callback(error)
 			jsonOps = appliedOps.map (op) -> JSON.stringify op
 			multi = rclient.multi()
-			multi.set    keys.docLines(doc_id:doc_id), JSON.stringify(docLines)
+			newDocLines = JSON.stringify(docLines)
+			newHash = RedisManager._computeHash(newDocLines)
+			multi.set    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
 			if jsonOps.length > 0
 				multi.rpush  keys.docOps(doc_id: doc_id), jsonOps...
 			multi.expire keys.docOps(doc_id: doc_id), RedisManager.DOC_OPS_TTL
@@ -150,4 +166,8 @@ module.exports = RedisManager =
 		if !ranges? or ranges == ""
 			return {}
 		else
-			return JSON.parse(ranges)
+			return JSON.parse(ranges)
+
+	_computeHash: (docLines) ->
+		# use sha1 checksum of doclines to detect data corruption
+		return crypto.createHash('sha1').update(docLines).digest('hex')

services/document-updater/config/settings.defaults.coffee

@@ -30,6 +30,7 @@ module.exports =
 				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}"
 				docsInProject: ({project_id}) -> "DocsIn:#{project_id}"
 				ranges: ({doc_id}) -> "Ranges:#{doc_id}"

services/document-updater/test/unit/coffee/RedisBackend/RedisBackendTests.coffee

@@ -19,6 +19,7 @@ describe "RedisBackend", ->
 						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}"
@@ -33,6 +34,7 @@ describe "RedisBackend", ->
 						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}}"

services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee

@@ -4,6 +4,7 @@ should = chai.should()
 modulePath = "../../../../app/js/RedisManager.js"
 SandboxedModule = require('sandboxed-module')
 Errors = require "../../../../app/js/Errors"
+crypto = require "crypto"
 
 describe "RedisManager", ->
 	beforeEach ->
@@ -19,6 +20,7 @@ describe "RedisManager", ->
 				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}"
@@ -38,10 +40,11 @@ describe "RedisManager", ->
 			@lines = ["one", "two", "three"]
 			@jsonlines = JSON.stringify @lines
 			@version = 42
+			@hash = crypto.createHash('sha1').update(@jsonlines).digest('hex')
 			@ranges = { comments: "mock", entries: "mock" }
 			@json_ranges = JSON.stringify @ranges
 			@rclient.get = sinon.stub()
-			@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @project_id, @json_ranges])
+			@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @hash, @project_id, @json_ranges])
 
 		describe "successfully", ->
 			beforeEach ->
@@ -56,7 +59,12 @@ describe "RedisManager", ->
 				@rclient.get
 					.calledWith("DocVersion:#{@doc_id}")
 					.should.equal true
-			
+
+			it 'should get the hash', ->
+				@rclient.get
+					.calledWith("DocHash:#{@doc_id}")
+					.should.equal true
+
 			it "should get the ranges", ->
 				@rclient.get
 					.calledWith("Ranges:#{@doc_id}")
@@ -67,6 +75,26 @@ describe "RedisManager", ->
 					.calledWith(null, @lines, @version, @ranges)
 					.should.equal true
 
+			it 'should not log any errors', ->
+				@logger.error.calledWith()
+					.should.equal false
+
+		describe "with a corrupted document", ->
+			beforeEach ->
+				@badHash = "INVALID-HASH-VALUE"
+				@rclient.exec = sinon.stub().callsArgWith(0, null, [@jsonlines, @version, @badHash, @project_id, @json_ranges])
+				@RedisManager.getDoc @project_id, @doc_id, @callback
+
+			it 'should log a hash error', ->
+				@logger.error.calledWith()
+					.should.equal true
+
+			it 'should return the document', ->
+				@callback
+					.calledWith(null, @lines, @version, @ranges)
+					.should.equal true
+
+
 		describe "getDoc with an invalid project id", ->
 			beforeEach ->
 				@another_project_id = "project-id-456"
@@ -174,6 +202,7 @@ describe "RedisManager", ->
 			@lines = ["one", "two", "three"]
 			@ops = [{ op: [{ i: "foo", p: 4 }] },{ op: [{ i: "bar", p: 8 }] }]
 			@version = 42
+			@hash = crypto.createHash('sha1').update(JSON.stringify(@lines)).digest('hex')
 			@ranges = { comments: "mock", entries: "mock" }
 
 			@rclient.exec = sinon.stub().callsArg(0)
@@ -197,6 +226,11 @@ describe "RedisManager", ->
 				@rclient.set
 					.calledWith("DocVersion:#{@doc_id}", @version)
 					.should.equal true
+
+			it "should set the hash", ->
+				@rclient.set
+					.calledWith("DocHash:#{@doc_id}", @hash)
+					.should.equal true
 				
 			it "should set the ranges", ->
 				@rclient.set
@@ -272,6 +306,7 @@ describe "RedisManager", ->
 			@rclient.exec.yields()
 			@lines = ["one", "two", "three"]
 			@version = 42
+			@hash = crypto.createHash('sha1').update(JSON.stringify(@lines)).digest('hex')
 			@ranges = { comments: "mock", entries: "mock" }
 		
 		describe "with non-empty ranges", ->
@@ -287,6 +322,11 @@ describe "RedisManager", ->
 				@rclient.set
 					.calledWith("DocVersion:#{@doc_id}", @version)
 					.should.equal true
+
+			it "should set the hash", ->
+				@rclient.set
+					.calledWith("DocHash:#{@doc_id}", @hash)
+					.should.equal true
 				
 			it "should set the ranges", ->
 				@rclient.set
@@ -333,6 +373,11 @@ describe "RedisManager", ->
 			@rclient.del
 				.calledWith("DocVersion:#{@doc_id}")
 				.should.equal true
+
+		it "should delete the hash", ->
+			@rclient.del
+				.calledWith("DocHash:#{@doc_id}")
+				.should.equal true
 		
 		it "should delete the project_id for the doc", ->
 			@rclient.del