Merge pull request #21 from sharelatex/ja-check-for-null-bytes

Check for null bytes before writing to redis and log hashes when writing

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

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

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:
+		@RedisManager = SandboxedModule.require modulePath,
-			"./RedisBackend":
+			requires:
-				createClient: () => @rclient
+				"./RedisBackend":
-			"./RedisKeyBuilder":
+					createClient: () => @rclient
-				blockingKey: ({doc_id}) -> "Blocking:#{doc_id}"
+				"./RedisKeyBuilder":
-				docLines: ({doc_id}) -> "doclines:#{doc_id}"
+					blockingKey: ({doc_id}) -> "Blocking:#{doc_id}"
-				docOps: ({doc_id}) -> "DocOps:#{doc_id}"
+					docLines: ({doc_id}) -> "doclines:#{doc_id}"
-				docVersion: ({doc_id}) -> "DocVersion:#{doc_id}"
+					docOps: ({doc_id}) -> "DocOps:#{doc_id}"
-				docHash: ({doc_id}) -> "DocHash:#{doc_id}"
+					docVersion: ({doc_id}) -> "DocVersion:#{doc_id}"
-				projectKey: ({doc_id}) -> "ProjectId:#{doc_id}"
+					docHash: ({doc_id}) -> "DocHash:#{doc_id}"
-				pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}"
+					projectKey: ({doc_id}) -> "ProjectId:#{doc_id}"
-				docsInProject: ({project_id}) -> "DocsIn:#{project_id}"
+					pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}"
-				ranges: ({doc_id}) -> "Ranges:#{doc_id}"
+					docsInProject: ({project_id}) -> "DocsIn:#{project_id}"
-			"logger-sharelatex": @logger = { error: sinon.stub(), log: sinon.stub(), warn: sinon.stub() }
+					ranges: ({doc_id}) -> "Ranges:#{doc_id}"
-			"settings-sharelatex": {documentupdater: {logHashErrors: {write:true, read:true}}}
+				"logger-sharelatex": @logger = { error: sinon.stub(), log: sinon.stub(), warn: sinon.stub() }
-			"./Metrics": @metrics =
+				"settings-sharelatex": {documentupdater: {logHashErrors: {write:true, read:true}}}
-				inc: sinon.stub()
+				"./Metrics": @metrics =
-				Timer: class Timer
+					inc: sinon.stub()
-					done: () ->
+					Timer: class Timer
-			"./Errors": Errors
+						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()