diff --git a/libraries/overleaf-editor-core/lib/blob.js b/libraries/overleaf-editor-core/lib/blob.js index 96eb7f7412..8acd0b6dcd 100644 --- a/libraries/overleaf-editor-core/lib/blob.js +++ b/libraries/overleaf-editor-core/lib/blob.js @@ -3,6 +3,8 @@ 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 }) @@ -24,14 +26,17 @@ 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. * - * 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. + * 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} */ - static MAX_EDITABLE_BYTE_LENGTH_BOUND = 16 * 1024 * 1024 + static MAX_EDITABLE_BYTE_LENGTH_BOUND = 3 * TextOperation.MAX_STRING_LENGTH 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 77896ea487..fd4f016ed9 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,17 +41,6 @@ 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 c5f8282f53..29690ce74e 100644 --- a/libraries/overleaf-editor-core/lib/operation/text_operation.js +++ b/libraries/overleaf-editor-core/lib/operation/text_operation.js @@ -46,18 +46,9 @@ class TextOperation extends EditOperation { /** * Length of the longest file that we'll attempt to edit, in characters. * - * 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. + * @type {number} */ - 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 MAX_STRING_LENGTH = 2 * Math.pow(1024, 2) static UnprocessableError = UnprocessableError static ApplyError = ApplyError static InvalidInsertionError = InvalidInsertionError @@ -334,30 +325,9 @@ 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 ad5cf9e0cc..e40fec228f 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,15 +7,16 @@ const TextOperation = ot.TextOperation describe('HollowStringFileData', function () { it('validates string length when edited', function () { - const length = 200 - const fileData = new HollowStringFileData(length) + const maxLength = TextOperation.MAX_STRING_LENGTH + const fileData = new HollowStringFileData(maxLength) + expect(fileData.getStringLength()).to.equal(maxLength) expect(() => { - 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).insert('x')) + }).to.throw(TextOperation.TooLongError) + expect(fileData.getStringLength()).to.equal(maxLength) - fileData.edit(new TextOperation().retain(length).insert('x')) - expect(fileData.getStringLength()).to.equal(length + 1) + fileData.edit(new TextOperation().retain(maxLength - 1).remove(1)) + expect(fileData.getStringLength()).to.equal(maxLength - 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 770f5af0db..4c9f4aa497 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', 1000) + const longString = _.repeat('a', TextOperation.MAX_STRING_LENGTH) fileData.edit(new TextOperation().insert(longString)) expect(fileData.getHash()).not.to.exist expect(fileData.getByteLength()).to.equal(longString.length) // approximate @@ -186,10 +186,8 @@ describe('LazyStringFileData', function () { expect(fileData.getOperations()).to.have.length(1) expect(() => { - fileData.edit( - new TextOperation().retain(longString.length - 123).insert('x') - ) - }).to.throw(TextOperation.ApplyError) + fileData.edit(new TextOperation().retain(longString.length).insert('x')) + }).to.throw(TextOperation.TooLongError) 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 47ce408a95..45a0337da1 100644 --- a/libraries/overleaf-editor-core/test/string_file_data.test.js +++ b/libraries/overleaf-editor-core/test/string_file_data.test.js @@ -5,10 +5,7 @@ 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 () { @@ -23,10 +20,7 @@ describe('StringFileData', function () { }) it('validates string length when edited', function () { - const longString = _.repeat( - 'a', - TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES - ) + const longString = _.repeat('a', TextOperation.MAX_STRING_LENGTH) const fileData = new StringFileData(longString) expect(fileData.getByteLength()).to.equal(longString.length) expect(fileData.getStringLength()).to.equal(longString.length) @@ -42,38 +36,6 @@ 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([])