From 23dfe68cb891bc37b6f1ad2d3a71c3001c39c31d Mon Sep 17 00:00:00 2001
From: James Allen <james@sharelatex.com>
Date: Fri, 25 Sep 2015 13:44:44 +0100
Subject: [PATCH] Don't error when rewinding and insert op which is beyond the
 length of the document.

ShareJS will accept an op where p > content.length when applied,
and it applies as though p == content.length. However, the op is
passed to us with the original p > content.length. Detect if that
is the case with this op, and shift p back appropriately to match
ShareJS if so.
---
 .../app/coffee/DiffGenerator.coffee             | 17 +++++++++++++++--
 .../DiffGenerator/DiffGeneratorTests.coffee     |  9 ++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/services/track-changes/app/coffee/DiffGenerator.coffee b/services/track-changes/app/coffee/DiffGenerator.coffee
index 9593f42f36..e0fcf744cd 100644
--- a/services/track-changes/app/coffee/DiffGenerator.coffee
+++ b/services/track-changes/app/coffee/DiffGenerator.coffee
@@ -5,6 +5,8 @@ ConsistencyError = (message) ->
 	return error
 ConsistencyError.prototype.__proto__ = Error.prototype
 
+logger = require "logger-sharelatex"
+
 module.exports = DiffGenerator =
 	ConsistencyError: ConsistencyError
 
@@ -15,13 +17,24 @@ module.exports = DiffGenerator =
 
 	rewindOp: (content, op) ->
 		if op.i?
-			textToBeRemoved = content.slice(op.p, op.p + op.i.length)
+			# ShareJS will accept an op where p > content.length when applied,
+			# and it applies as though p == content.length. However, the op is
+			# passed to us with the original p > content.length. Detect if that
+			# is the case with this op, and shift p back appropriately to match
+			# ShareJS if so.
+			p = op.p
+			max_p = content.length - op.i.length
+			if p > max_p
+				logger.warn {max_p, p}, "truncating position to content length"
+				p = max_p
+
+			textToBeRemoved = content.slice(p, p + op.i.length)
 			if op.i != textToBeRemoved
 				throw new ConsistencyError(
 					"Inserted content, '#{op.i}', does not match text to be removed, '#{textToBeRemoved}'"
 				)
 
-			return content.slice(0, op.p) + content.slice(op.p + op.i.length)
+			return content.slice(0, p) + content.slice(p + op.i.length)
 
 		else if op.d?
 			return content.slice(0, op.p) + op.d + content.slice(op.p)
diff --git a/services/track-changes/test/unit/coffee/DiffGenerator/DiffGeneratorTests.coffee b/services/track-changes/test/unit/coffee/DiffGenerator/DiffGeneratorTests.coffee
index 3263d84dda..b956606a7d 100644
--- a/services/track-changes/test/unit/coffee/DiffGenerator/DiffGeneratorTests.coffee
+++ b/services/track-changes/test/unit/coffee/DiffGenerator/DiffGeneratorTests.coffee
@@ -7,7 +7,8 @@ SandboxedModule = require('sandboxed-module')
 
 describe "DiffGenerator", ->
 	beforeEach ->
-		@DiffGenerator = SandboxedModule.require modulePath
+		@DiffGenerator = SandboxedModule.require modulePath, requires:
+			"logger-sharelatex": { warn: sinon.stub() }
 		@ts = Date.now()
 		@user_id = "mock-user-id"
 		@user_id_2 = "mock-user-id-2"
@@ -34,6 +35,12 @@ describe "DiffGenerator", ->
 				expect( () =>
 					@DiffGenerator.rewindOp content, { p: 6, i: "foo" }
 				).to.throw(@DiffGenerator.ConsistencyError)
+		
+		describe "with an update which is beyond the length of the content", ->
+			it "should undo the insert as if it were at the end of the content", ->
+				content = "foobar"
+				rewoundContent = @DiffGenerator.rewindOp content, { p: 4, i: "bar" }
+				rewoundContent.should.equal "foo"
 
 	describe "rewindUpdate", ->
 		it "should rewind ops in reverse", ->