Merge pull request #120 from overleaf/jpa-limit-update-size

[misc] limit update size

services/real-time/app/coffee/DocumentUpdaterManager.coffee

@@ -61,9 +61,17 @@ module.exports = DocumentUpdaterManager =
 		change = _.pick change, allowedKeys
 		jsonChange = JSON.stringify change
 		if jsonChange.indexOf("\u0000") != -1
+			# memory corruption check
 			error = new Error("null bytes found in op")
 			logger.error err: error, project_id: project_id, doc_id: doc_id, jsonChange: jsonChange, error.message
 			return callback(error)
+
+		updateSize = jsonChange.length
+		if updateSize > settings.maxUpdateSize
+			error = new Error("update is too large")
+			error.updateSize = updateSize
+			return callback(error)
+
 		doc_key = "#{project_id}:#{doc_id}"
 		# Push onto pendingUpdates for doc_id first, because once the doc updater
 		# gets an entry on pending-updates-list, it starts processing.

services/real-time/app/coffee/SafeJsonParse.coffee

@@ -3,8 +3,8 @@ logger = require "logger-sharelatex"
 
 module.exports =
 	parse: (data, callback = (error, parsed) ->) ->
-		if data.length > (Settings.max_doc_length or 2 * 1024 * 1024)
-			logger.error {head: data.slice(0,1024)}, "data too large to parse"
+		if data.length > Settings.maxUpdateSize
+			logger.error {head: data.slice(0,1024), length: data.length}, "data too large to parse"
 			return callback new Error("data too large to parse")
 		try
 			parsed = JSON.parse(data)

services/real-time/app/coffee/WebsocketController.coffee

@@ -1,5 +1,6 @@
 logger = require "logger-sharelatex"
 metrics = require "metrics-sharelatex"
+settings = require "settings-sharelatex"
 WebApiManager = require "./WebApiManager"
 AuthorizationManager = require "./AuthorizationManager"
 DocumentUpdaterManager = require "./DocumentUpdaterManager"
@@ -202,6 +203,7 @@ module.exports = WebsocketController =
 		Utils.getClientAttributes client, ["user_id", "project_id"], (error, {user_id, project_id}) ->
 			return callback(error) if error?
 			return callback(new Error("no project_id found on client")) if !project_id?
+
 			WebsocketController._assertClientCanApplyUpdate client, doc_id, update, (error) ->
 				if error?
 					logger.warn {err: error, doc_id, client_id: client.id, version: update.v}, "client is not authorized to make update"
@@ -218,6 +220,22 @@ module.exports = WebsocketController =
 				logger.log {user_id, doc_id, project_id, client_id: client.id, version: update.v}, "sending update to doc updater"
 
 				DocumentUpdaterManager.queueChange project_id, doc_id, update, (error) ->
+					if error?.message == "update is too large"
+						metrics.inc "update_too_large"
+						updateSize = error.updateSize
+						logger.warn({user_id, project_id, doc_id, updateSize}, "update is too large")
+
+						# mark the update as received -- the client should not send it again!
+						callback()
+
+						# trigger an out-of-sync error
+						message = {project_id, doc_id, error: "update is too large"}
+						setTimeout () ->
+							client.emit "otUpdateError", message.error, message
+							client.disconnect()
+						, 100
+						return
+
 					if error?
 						logger.error {err: error, project_id, doc_id, client_id: client.id, version: update.v}, "document was not available for update"
 						client.disconnect()

services/real-time/config/settings.defaults.coffee

@@ -52,6 +52,12 @@ settings =
 	
 	max_doc_length: 2 * 1024 * 1024 # 2mb
 
+	# combine
+	# max_doc_length (2mb see above) * 2 (delete + insert)
+	# max_ranges_size (3mb see MAX_RANGES_SIZE in document-updater)
+	# overhead for JSON serialization
+	maxUpdateSize: parseInt(process.env['MAX_UPDATE_SIZE']) or 7 * 1024 * 1024 + 64 * 1024
+
 	shutdownDrainTimeWindow: process.env['SHUTDOWN_DRAIN_TIME_WINDOW'] or 9
 
 	continualPubsubTraffic: process.env['CONTINUAL_PUBSUB_TRAFFIC'] or false

services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee

@@ -68,6 +68,62 @@ describe "applyOtUpdate", ->
 				(cb) => rclient.del redisSettings.documentupdater.key_schema.pendingUpdates(@doc_id), cb
 			], done
 		
+	describe "when authorized with a huge edit update", ->
+		before (done) ->
+			@update = {
+				op: {
+					p: 12,
+					t: "update is too large".repeat(1024 * 400) # >7MB
+				}
+			}
+			async.series [
+				(cb) =>
+					FixturesManager.setUpProject {
+						privilegeLevel: "readAndWrite"
+					}, (e, {@project_id, @user_id}) =>
+						cb(e)
+
+				(cb) =>
+					FixturesManager.setUpDoc @project_id, {@lines, @version, @ops}, (e, {@doc_id}) =>
+						cb(e)
+
+				(cb) =>
+					@client = RealTimeClient.connect()
+					@client.on "connectionAccepted", cb
+					@client.on "otUpdateError", (@otUpdateError) =>
+
+				(cb) =>
+					@client.emit "joinProject", project_id: @project_id, cb
+
+				(cb) =>
+					@client.emit "joinDoc", @doc_id, cb
+
+				(cb) =>
+					@client.emit "applyOtUpdate", @doc_id, @update, (@error) =>
+						cb()
+			], done
+
+		it "should not return an error", ->
+			expect(@error).to.not.exist
+
+		it "should send an otUpdateError to the client", (done) ->
+			setTimeout () =>
+				expect(@otUpdateError).to.exist
+				done()
+			, 300
+
+		it "should disconnect the client", (done) ->
+			setTimeout () =>
+				@client.socket.connected.should.equal false
+				done()
+			, 300
+
+		it "should not put the update in redis", (done) ->
+			rclient.llen redisSettings.documentupdater.key_schema.pendingUpdates({@doc_id}), (error, len) =>
+				len.should.equal 0
+				done()
+			return null
+
 	describe "when authorized to read-only with an edit update", ->
 		before (done) ->
 			async.series [

services/real-time/test/unit/coffee/DocumentUpdaterManagerTests.coffee

@@ -15,6 +15,7 @@ describe 'DocumentUpdaterManager', ->
 			redis: documentupdater:
 				key_schema:
 					pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}"
+			maxUpdateSize: 7 * 1024 * 1024
 		@rclient = {auth:->}
 
 		@DocumentUpdaterManager = SandboxedModule.require modulePath,
@@ -163,6 +164,20 @@ describe 'DocumentUpdaterManager', ->
 			it "should not push the change onto the pending-updates-list queue", ->
 				@rclient.rpush.called.should.equal false
 
+		describe "when the update is too large", ->
+			beforeEach ->
+				@change = {op: {p: 12,t: "update is too large".repeat(1024 * 400)}}
+				@DocumentUpdaterManager.queueChange(@project_id, @doc_id, @change, @callback)
+
+			it "should return an error", ->
+				@callback.calledWithExactly(sinon.match(Error)).should.equal true
+
+			it "should add the size to the error", ->
+				@callback.args[0][0].updateSize.should.equal 7782422
+
+			it "should not push the change onto the pending-updates-list queue", ->
+				@rclient.rpush.called.should.equal false
+
 		describe "with invalid keys", ->
 			beforeEach ->
 				@change = {

services/real-time/test/unit/coffee/SafeJsonParseTest.coffee

@@ -8,7 +8,7 @@ describe 'SafeJsonParse', ->
 	beforeEach ->
 		@SafeJsonParse = SandboxedModule.require modulePath, requires:
 			"settings-sharelatex": @Settings = {
-				max_doc_length: 16 * 1024
+				maxUpdateSize: 16 * 1024
 			}
 			"logger-sharelatex": @logger = {error: sinon.stub()}
 
@@ -27,7 +27,7 @@ describe 'SafeJsonParse', ->
 			# we have a 2k overhead on top of max size
 			big_blob = Array(16*1024).join("A")
 			data = "{\"foo\": \"#{big_blob}\"}"
-			@Settings.max_doc_length = 2 * 1024
+			@Settings.maxUpdateSize = 2 * 1024
 			@SafeJsonParse.parse data, (error, parsed) =>
 				@logger.error.called.should.equal true
 				expect(error).to.exist

services/real-time/test/unit/coffee/WebsocketControllerTests.coffee

@@ -668,6 +668,36 @@ describe 'WebsocketController', ->
 			it "should call the callback with the error", ->
 				@callback.calledWith(@error).should.equal true
 
+		describe "update_too_large", ->
+			beforeEach (done) ->
+				@client.disconnect = sinon.stub()
+				@client.emit = sinon.stub()
+				@client.params.user_id = @user_id
+				@client.params.project_id = @project_id
+				error = new Error("update is too large")
+				error.updateSize = 7372835
+				@DocumentUpdaterManager.queueChange = sinon.stub().callsArgWith(3, error)
+				@WebsocketController.applyOtUpdate @client, @doc_id, @update, @callback
+				setTimeout ->
+					done()
+				, 201
+
+			it "should call the callback with no error", ->
+				@callback.called.should.equal true
+				@callback.args[0].should.deep.equal []
+
+			it "should log a warning with the size and context", ->
+				@logger.warn.called.should.equal true
+				@logger.warn.args[0].should.deep.equal [{
+					@user_id, @project_id, @doc_id, updateSize: 7372835
+				}, 'update is too large']
+
+			it "should send an otUpdateError the client", ->
+				@client.emit.calledWith('otUpdateError').should.equal true
+
+			it "should disconnect the client", ->
+				@client.disconnect.called.should.equal true
+
 	describe "_assertClientCanApplyUpdate", ->
 		beforeEach ->
 			@edit_update = { op: [{i: "foo", p: 42}, {c: "bar", p: 132}] } # comments may still be in an edit op