Issue #1625: Fix to client-side-induced changeset spamming.

THE BUG - HIGH LEVEL:
- When client A sends out an attribute change, client B applies that change to itself but
  also thinks that it made the change itself, which is incorrect. This means that when client B
  next makes a change, he will send out that he made the attrib change that A actually made.
- Ex: Have 2 clients on the same pad. Have A apply bold on some text. Next, have B type a character.
  B will broadcast that it both added a character AND applied bold, when in reality it did NOT
  apply bold at all, that change was done by the other client and this client incorrectly adopted it as its own.
- This root bug behavior results in clients continuing to think that they each made the other client's change,
  thus resulting in an infinite loop of changeset spamming that bogs down clients and harms server stability.

THE BUG - IN DEPTH:
- The root issue is in the way that Changesets are combined in Changeset.follow(). Specifically, in the case of a
  changeset with ONLY new attrib changes (no text changes) being merged with an identity changeset (has no ops).
- In this case, Changeset.follow() copies the ops of the new CS and fully overrides the other CS.
- applyChangesToBase invokes Changeset.follow to determine the final client document CS state after applying the new CS.
  If the final client document CS state is NOT the identity CS, then the client broadcasts that it made a change.
- When client A changes just attribs, client B's applyChangesToBase calls Changeset.follow() and passes client A's
  changeset (attrib change) and Client B's current changeset state (identity).
- As per the noted bug, Changeset.follow() returns client A's changeset as a result, causing client B to adopt
  client A's changeset as its own document state. Thus, client A ends up thinking it has made client B's changes.

THE FIX:
- Changeset.follow() should NOT copy the ops of the new CS passed in if those changes are only attrib changes.
  This allows applyChangesToBase to properly set the client's CS back to the identity after applying an
  external attrib change, instead of incorrectly adopting the external client's changes.
This commit is contained in:
Brian Emerick 2013-04-24 15:02:58 -07:00
parent f0f98b41fe
commit e7d8f124ad
1 changed files with 3 additions and 1 deletions

View File

@ -2105,7 +2105,9 @@ exports.follow = function (cs1, cs2, reverseInsertOrder, pool) {
exports.copyOp(op2, opOut);
op2.opcode = '';
} else if (!op2.opcode) {
exports.copyOp(op1, opOut);
// @NOTE: Critical bugfix for EPL issue #1625. We do not copy op1 here
// in order to prevent attributes from leaking into result changesets.
// exports.copyOp(op1, opOut);
op1.opcode = '';
} else {
// both keeps