diff --git a/libraries/overleaf-editor-core/lib/blob.js b/libraries/overleaf-editor-core/lib/blob.js index 8acd0b6dcd..96eb7f7412 100644 --- a/libraries/overleaf-editor-core/lib/blob.js +++ b/libraries/overleaf-editor-core/lib/blob.js @@ -3,8 +3,6 @@ const assert = require('check-types').assert const OError = require('@overleaf/o-error') -const TextOperation = require('./operation/text_operation') - class NotFoundError extends OError { constructor(hash) { super(`blob ${hash} not found`, { hash }) @@ -26,17 +24,14 @@ class Blob { * in to determine that; so it is useful to have an upper bound on the byte * length of a file that might be editable. * - * The reason for the factor of 3 is as follows. We cannot currently edit files - * that contain characters outside of the basic multilingual plane, so we're - * limited to characters that can be represented in a single, two-byte UCS-2 - * code unit. Encoding the largest such value, 0xFFFF (which is not actually - * a valid character), takes three bytes in UTF-8: 0xEF 0xBF 0xBF. A file - * composed entirely of three-byte UTF-8 codepoints is the worst case; in - * practice, this is a very conservative upper bound. - * - * @type {number} + * This used to be 3 times the max editable file length to account for 3-byte + * UTF-8 codepoints. However, editable file blobs now include tracked deletes + * and the system used to allow unlimited tracked deletes on a single file. + * A practical limit is the 16 MB Mongo size limit. It wouldn't have been + * possible to store more than 16 MB of tracked deletes. We therefore fall + * back to this limit. */ - static MAX_EDITABLE_BYTE_LENGTH_BOUND = 3 * TextOperation.MAX_STRING_LENGTH + static MAX_EDITABLE_BYTE_LENGTH_BOUND = 16 * 1024 * 1024 static NotFoundError = NotFoundError diff --git a/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js b/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js index fd4f016ed9..77896ea487 100644 --- a/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js +++ b/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js @@ -41,6 +41,17 @@ class TrackedChangeList { return this._trackedChanges.length } + /** + * Iterate over tracked changes + * + * @returns {Iterator} + */ + *[Symbol.iterator]() { + for (const change of this._trackedChanges) { + yield change + } + } + /** * @returns {readonly TrackedChange[]} */ diff --git a/libraries/overleaf-editor-core/lib/operation/text_operation.js b/libraries/overleaf-editor-core/lib/operation/text_operation.js index 29690ce74e..c5f8282f53 100644 --- a/libraries/overleaf-editor-core/lib/operation/text_operation.js +++ b/libraries/overleaf-editor-core/lib/operation/text_operation.js @@ -46,9 +46,18 @@ class TextOperation extends EditOperation { /** * Length of the longest file that we'll attempt to edit, in characters. * - * @type {number} + * This number used to be 2M characters, but it didn't include tracked + * deletes. Now that it includes tracked deletes, we raise the limit to the + * Mongo document size limit. It would have been impossible to store more + * tracked deletes in a single document. */ - static MAX_STRING_LENGTH = 2 * Math.pow(1024, 2) + static MAX_STRING_LENGTH = 16 * Math.pow(1024, 2) + + /** + * Max editable file length, excluding tracked deletes. + */ + static MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES = 2 * Math.pow(1024, 2) + static UnprocessableError = UnprocessableError static ApplyError = ApplyError static InvalidInsertionError = InvalidInsertionError @@ -325,9 +334,30 @@ class TextOperation extends EditOperation { } if (result.length > TextOperation.MAX_STRING_LENGTH) { + // We're over the hard limit, with or without tracked deletes throw new TextOperation.TooLongError(operation, result.length) } + if ( + result.length > TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES + ) { + // We might be over the limit, but we need to check how much of the + // length is tracked deletes. + let trackedDeletesLength = 0 + for (const change of file.trackedChanges) { + if (change.tracking.type === 'delete') { + trackedDeletesLength += change.range.length + } + } + + if ( + result.length - trackedDeletesLength > + TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES + ) { + throw new TextOperation.TooLongError(operation, result.length) + } + } + file.content = result } diff --git a/libraries/overleaf-editor-core/test/hollow_string_file_data.test.js b/libraries/overleaf-editor-core/test/hollow_string_file_data.test.js index e40fec228f..ad5cf9e0cc 100644 --- a/libraries/overleaf-editor-core/test/hollow_string_file_data.test.js +++ b/libraries/overleaf-editor-core/test/hollow_string_file_data.test.js @@ -7,16 +7,15 @@ const TextOperation = ot.TextOperation describe('HollowStringFileData', function () { it('validates string length when edited', function () { - const maxLength = TextOperation.MAX_STRING_LENGTH - const fileData = new HollowStringFileData(maxLength) - expect(fileData.getStringLength()).to.equal(maxLength) + const length = 200 + const fileData = new HollowStringFileData(length) expect(() => { - fileData.edit(new TextOperation().retain(maxLength).insert('x')) - }).to.throw(TextOperation.TooLongError) - expect(fileData.getStringLength()).to.equal(maxLength) + fileData.edit(new TextOperation().retain(length + 10).insert('x')) + }).to.throw(TextOperation.ApplyError) + expect(fileData.getStringLength()).to.equal(length) - fileData.edit(new TextOperation().retain(maxLength - 1).remove(1)) - expect(fileData.getStringLength()).to.equal(maxLength - 1) + fileData.edit(new TextOperation().retain(length).insert('x')) + expect(fileData.getStringLength()).to.equal(length + 1) }) }) diff --git a/libraries/overleaf-editor-core/test/lazy_string_file_data.test.js b/libraries/overleaf-editor-core/test/lazy_string_file_data.test.js index 4c9f4aa497..770f5af0db 100644 --- a/libraries/overleaf-editor-core/test/lazy_string_file_data.test.js +++ b/libraries/overleaf-editor-core/test/lazy_string_file_data.test.js @@ -178,7 +178,7 @@ describe('LazyStringFileData', function () { expect(fileData.getStringLength()).to.equal(0) expect(fileData.getOperations()).to.have.length(0) - const longString = _.repeat('a', TextOperation.MAX_STRING_LENGTH) + const longString = _.repeat('a', 1000) fileData.edit(new TextOperation().insert(longString)) expect(fileData.getHash()).not.to.exist expect(fileData.getByteLength()).to.equal(longString.length) // approximate @@ -186,8 +186,10 @@ describe('LazyStringFileData', function () { expect(fileData.getOperations()).to.have.length(1) expect(() => { - fileData.edit(new TextOperation().retain(longString.length).insert('x')) - }).to.throw(TextOperation.TooLongError) + fileData.edit( + new TextOperation().retain(longString.length - 123).insert('x') + ) + }).to.throw(TextOperation.ApplyError) expect(fileData.getHash()).not.to.exist expect(fileData.getByteLength()).to.equal(longString.length) // approximate expect(fileData.getStringLength()).to.equal(longString.length) diff --git a/libraries/overleaf-editor-core/test/string_file_data.test.js b/libraries/overleaf-editor-core/test/string_file_data.test.js index 45a0337da1..47ce408a95 100644 --- a/libraries/overleaf-editor-core/test/string_file_data.test.js +++ b/libraries/overleaf-editor-core/test/string_file_data.test.js @@ -5,7 +5,10 @@ const { expect } = require('chai') const _ = require('lodash') const ot = require('..') +const Range = require('../lib/range') const StringFileData = require('../lib/file_data/string_file_data') +const TrackedChange = require('../lib/file_data/tracked_change') +const TrackingProps = require('../lib/file_data/tracking_props') const TextOperation = ot.TextOperation describe('StringFileData', function () { @@ -20,7 +23,10 @@ describe('StringFileData', function () { }) it('validates string length when edited', function () { - const longString = _.repeat('a', TextOperation.MAX_STRING_LENGTH) + const longString = _.repeat( + 'a', + TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES + ) const fileData = new StringFileData(longString) expect(fileData.getByteLength()).to.equal(longString.length) expect(fileData.getStringLength()).to.equal(longString.length) @@ -36,6 +42,38 @@ describe('StringFileData', function () { expect(fileData.getStringLength()).to.equal(longString.length - 1) }) + it('ignores tracked deletes when checking the max string length', function () { + const longString = _.repeat( + 'a', + TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES + ) + const fileData = new StringFileData(longString) + fileData.trackedChanges.add( + new TrackedChange( + new Range(123, 100), + new TrackingProps('delete', 'some-user', new Date()) + ) + ) + fileData.trackedChanges.add( + new TrackedChange( + new Range(456, 50), + new TrackingProps('insert', 'some-user', new Date()) + ) + ) + + // Add text the same length as the tracked delete + fileData.edit( + new TextOperation().retain(longString.length).insert('x'.repeat(100)) + ) + + // Add more text + expect(() => { + fileData.edit( + new TextOperation().retain(longString.length + 100).insert('x') + ) + }).to.throw(TextOperation.TooLongError) + }) + it('getComments() should return an empty array', function () { const fileData = new StringFileData('test') expect(fileData.getComments().toRaw()).to.eql([])