Эх сурвалжийг харах

[misc] skip duplicate JSON serialization for size check

Jakob Ackermann 6 жил өмнө
parent
commit
af53d3b603

+ 8 - 0
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.

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

@@ -204,22 +204,6 @@ module.exports = WebsocketController =
 			return callback(error) if error?
 			return callback(new Error("no project_id found on client")) if !project_id?
 
-			updateSize = JSON.stringify(update).length
-			if updateSize > settings.maxUpdateSize
-				metrics.inc "update_too_large"
-				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
-
 			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"
@@ -236,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()

+ 4 - 2
services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee

@@ -71,8 +71,10 @@ describe "applyOtUpdate", ->
 	describe "when authorized with a huge edit update", ->
 		before (done) ->
 			@update = {
-				op: {p: 12, t: "foo"},
-				junk: 'this update is too large'.repeat(1024 * 300) # >7MB
+				op: {
+					p: 12,
+					t: "update is too large".repeat(1024 * 400) # >7MB
+				}
 			}
 			async.series [
 				(cb) =>

+ 15 - 0
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 = {

+ 3 - 5
services/real-time/test/unit/coffee/WebsocketControllerTests.coffee

@@ -32,7 +32,6 @@ describe 'WebsocketController', ->
 			"./DocumentUpdaterManager": @DocumentUpdaterManager = {}
 			"./ConnectedUsersManager": @ConnectedUsersManager = {}
 			"./WebsocketLoadBalancer": @WebsocketLoadBalancer = {}
-			"settings-sharelatex": {maxUpdateSize: 7 * 1024 * 1024}
 			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() }
 			"metrics-sharelatex": @metrics =
 				inc: sinon.stub()
@@ -673,12 +672,11 @@ describe 'WebsocketController', ->
 			beforeEach (done) ->
 				@client.disconnect = sinon.stub()
 				@client.emit = sinon.stub()
-				@update = {
-					op: {p: 12, t: "foo"},
-					junk: 'this update is too large'.repeat(1024 * 300) # >7MB
-				}
 				@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()