Skip to content

Commit a081492

Browse files
author
Alec Gibson
committed
Add guards for large seq
Previously, `seq` would get reset every time a client reconnects. Whilst not entirely robust, it did help to mitigate the possibility that very long-lived connections might hit an integer overflow when incrementing their `seq`. This change adds a guard against this case, and throws a sensible error, both on the client, and on the server. Note that we cannot rely on the client's `Number.MAX_SAFE_INTEGER`, because it is not supported by Internet Explorer.
1 parent c231c8f commit a081492

File tree

6 files changed

+49
-1
lines changed

6 files changed

+49
-1
lines changed

lib/agent.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,12 @@ Agent.prototype._handleMessage = function(request, callback) {
351351
case 'op':
352352
// Normalize the properties submitted
353353
var op = createClientOp(request, this.clientId);
354+
if (op.seq >= util.MAX_SAFE_INTEGER) {
355+
return callback(new ShareDBError(
356+
ERROR_CODE.ERR_CONNECTION_SEQ_INTEGER_OVERFLOW,
357+
'Connection seq has exceeded the max safe integer, maybe from being open for too long'
358+
));
359+
}
354360
if (!op) return callback(new ShareDBError(ERROR_CODE.ERR_MESSAGE_BADLY_FORMED, 'Invalid op message'));
355361
return this._submit(request.c, request.d, op, callback);
356362
case 'nf':

lib/client/doc.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ var emitter = require('../emitter');
22
var logger = require('../logger');
33
var ShareDBError = require('../error');
44
var types = require('../types');
5+
var util = require('../util');
56

67
var ERROR_CODE = ShareDBError.CODES;
78

@@ -641,7 +642,16 @@ Doc.prototype._sendOp = function() {
641642
// reconnect, since an op may still be pending after the reconnection and
642643
// this.connection.id will change. In case an op is sent multiple times, we
643644
// also need to be careful not to override the original seq value.
644-
if (op.seq == null) op.seq = this.connection.seq++;
645+
if (op.seq == null) {
646+
if (this.connection.seq >= util.MAX_SAFE_INTEGER) {
647+
return this.emit('error', new ShareDBError(
648+
ERROR_CODE.ERR_CONNECTION_SEQ_INTEGER_OVERFLOW,
649+
'Connection seq has exceeded the max safe integer, maybe from being open for too long'
650+
));
651+
}
652+
653+
op.seq = this.connection.seq++;
654+
}
645655

646656
this.connection.sendOp(this, op);
647657

lib/error.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ ShareDBError.CODES = {
1616
ERR_APPLY_OP_VERSION_DOES_NOT_MATCH_SNAPSHOT: 'ERR_APPLY_OP_VERSION_DOES_NOT_MATCH_SNAPSHOT',
1717
ERR_APPLY_SNAPSHOT_NOT_PROVIDED: 'ERR_APPLY_SNAPSHOT_NOT_PROVIDED',
1818
ERR_CLIENT_ID_BADLY_FORMED: 'ERR_CLIENT_ID_BADLY_FORMED',
19+
ERR_CONNECTION_SEQ_INTEGER_OVERFLOW: 'ERR_CONNECTION_SEQ_INTEGER_OVERFLOW',
1920
ERR_CONNECTION_STATE_TRANSITION_INVALID: 'ERR_CONNECTION_STATE_TRANSITION_INVALID',
2021
ERR_DATABASE_ADAPTER_NOT_FOUND: 'ERR_DATABASE_ADAPTER_NOT_FOUND',
2122
ERR_DATABASE_DOES_NOT_SUPPORT_SUBSCRIBE: 'ERR_DATABASE_DOES_NOT_SUPPORT_SUBSCRIBE',

lib/util.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,5 @@ exports.isValidVersion = function(version) {
2222
exports.isValidTimestamp = function(timestamp) {
2323
return exports.isValidVersion(timestamp);
2424
};
25+
26+
exports.MAX_SAFE_INTEGER = 9007199254740991;

test/client/connection.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,4 +216,22 @@ describe('client connection', function() {
216216
});
217217
});
218218
});
219+
220+
it('errors when submitting an op with a very large seq', function(done) {
221+
this.backend.connect(function(connection) {
222+
var doc = connection.get('test', '123');
223+
doc.create({foo: 'bar'}, function(error) {
224+
if (error) return done(error);
225+
connection.sendOp(doc, {
226+
op: {p: ['foo'], od: 'bar'},
227+
src: connection.id,
228+
seq: Number.MAX_SAFE_INTEGER
229+
});
230+
doc.once('error', function(error) {
231+
expect(error.code).to.equal('ERR_CONNECTION_SEQ_INTEGER_OVERFLOW');
232+
done();
233+
});
234+
});
235+
});
236+
});
219237
});

test/client/doc.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ describe('Doc', function() {
4949
doc.destroy();
5050
});
5151

52+
it('errors when trying to set a very large seq', function(done) {
53+
var connection = this.connection;
54+
connection.seq = Number.MAX_SAFE_INTEGER;
55+
var doc = connection.get('dogs', 'fido');
56+
doc.create({name: 'fido'});
57+
doc.once('error', function(error) {
58+
expect(error.code).to.equal('ERR_CONNECTION_SEQ_INTEGER_OVERFLOW');
59+
done();
60+
});
61+
});
62+
5263
describe('when connection closed', function() {
5364
beforeEach(function(done) {
5465
this.op1 = [{p: ['snacks'], oi: true}];

0 commit comments

Comments
 (0)