mirror of
https://github.com/overleaf/overleaf.git
synced 2024-11-21 20:47:08 -05:00
[misc] WebsocketController: limit the update size to 7mb
bail out early on -- especially do not push the update into redis for doc-updater to discard it. Confirm the update silently, otherwise the frontend will send it again. Broadcast a 'otUpdateError' message and disconnect the client, like doc-updater would do.
This commit is contained in:
parent
7321e80b1e
commit
15244a54be
4 changed files with 109 additions and 0 deletions
|
@ -1,5 +1,6 @@
|
||||||
logger = require "logger-sharelatex"
|
logger = require "logger-sharelatex"
|
||||||
metrics = require "metrics-sharelatex"
|
metrics = require "metrics-sharelatex"
|
||||||
|
settings = require "settings-sharelatex"
|
||||||
WebApiManager = require "./WebApiManager"
|
WebApiManager = require "./WebApiManager"
|
||||||
AuthorizationManager = require "./AuthorizationManager"
|
AuthorizationManager = require "./AuthorizationManager"
|
||||||
DocumentUpdaterManager = require "./DocumentUpdaterManager"
|
DocumentUpdaterManager = require "./DocumentUpdaterManager"
|
||||||
|
@ -202,6 +203,23 @@ module.exports = WebsocketController =
|
||||||
Utils.getClientAttributes client, ["user_id", "project_id"], (error, {user_id, project_id}) ->
|
Utils.getClientAttributes client, ["user_id", "project_id"], (error, {user_id, project_id}) ->
|
||||||
return callback(error) if error?
|
return callback(error) if error?
|
||||||
return callback(new Error("no project_id found on client")) if !project_id?
|
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) ->
|
WebsocketController._assertClientCanApplyUpdate client, doc_id, update, (error) ->
|
||||||
if error?
|
if error?
|
||||||
logger.warn {err: error, doc_id, client_id: client.id, version: update.v}, "client is not authorized to make update"
|
logger.warn {err: error, doc_id, client_id: client.id, version: update.v}, "client is not authorized to make update"
|
||||||
|
|
|
@ -52,6 +52,11 @@ settings =
|
||||||
|
|
||||||
max_doc_length: 2 * 1024 * 1024 # 2mb
|
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 doc-updater)
|
||||||
|
maxUpdateSize: parseInt(process.env['MAX_UPDATE_SIZE']) or 7 * 1024 * 1024
|
||||||
|
|
||||||
shutdownDrainTimeWindow: process.env['SHUTDOWN_DRAIN_TIME_WINDOW'] or 9
|
shutdownDrainTimeWindow: process.env['SHUTDOWN_DRAIN_TIME_WINDOW'] or 9
|
||||||
|
|
||||||
continualPubsubTraffic: process.env['CONTINUAL_PUBSUB_TRAFFIC'] or false
|
continualPubsubTraffic: process.env['CONTINUAL_PUBSUB_TRAFFIC'] or false
|
||||||
|
|
|
@ -68,6 +68,60 @@ describe "applyOtUpdate", ->
|
||||||
(cb) => rclient.del redisSettings.documentupdater.key_schema.pendingUpdates(@doc_id), cb
|
(cb) => rclient.del redisSettings.documentupdater.key_schema.pendingUpdates(@doc_id), cb
|
||||||
], done
|
], done
|
||||||
|
|
||||||
|
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
|
||||||
|
}
|
||||||
|
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", ->
|
describe "when authorized to read-only with an edit update", ->
|
||||||
before (done) ->
|
before (done) ->
|
||||||
async.series [
|
async.series [
|
||||||
|
|
|
@ -32,6 +32,7 @@ describe 'WebsocketController', ->
|
||||||
"./DocumentUpdaterManager": @DocumentUpdaterManager = {}
|
"./DocumentUpdaterManager": @DocumentUpdaterManager = {}
|
||||||
"./ConnectedUsersManager": @ConnectedUsersManager = {}
|
"./ConnectedUsersManager": @ConnectedUsersManager = {}
|
||||||
"./WebsocketLoadBalancer": @WebsocketLoadBalancer = {}
|
"./WebsocketLoadBalancer": @WebsocketLoadBalancer = {}
|
||||||
|
"settings-sharelatex": {maxUpdateSize: 7 * 1024 * 1024}
|
||||||
"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() }
|
"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), warn: sinon.stub() }
|
||||||
"metrics-sharelatex": @metrics =
|
"metrics-sharelatex": @metrics =
|
||||||
inc: sinon.stub()
|
inc: sinon.stub()
|
||||||
|
@ -668,6 +669,37 @@ describe 'WebsocketController', ->
|
||||||
it "should call the callback with the error", ->
|
it "should call the callback with the error", ->
|
||||||
@callback.calledWith(@error).should.equal true
|
@callback.calledWith(@error).should.equal true
|
||||||
|
|
||||||
|
describe "update_too_large", ->
|
||||||
|
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
|
||||||
|
@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", ->
|
describe "_assertClientCanApplyUpdate", ->
|
||||||
beforeEach ->
|
beforeEach ->
|
||||||
@edit_update = { op: [{i: "foo", p: 42}, {c: "bar", p: 132}] } # comments may still be in an edit op
|
@edit_update = { op: [{i: "foo", p: 42}, {c: "bar", p: 132}] } # comments may still be in an edit op
|
||||||
|
|
Loading…
Reference in a new issue