Merge pull request #20466 from overleaf/em-revert-history-limit

Revert #20421

GitOrigin-RevId: c2ab656214354e3927f0e56ee7c09a8d0a53fd73
This commit is contained in:
Eric Mc Sween 2024-09-17 16:50:14 -04:00 committed by Copybot
parent 7cd16a84e0
commit 1061152c31
6 changed files with 26 additions and 101 deletions

View file

@ -3,6 +3,8 @@
const assert = require('check-types').assert const assert = require('check-types').assert
const OError = require('@overleaf/o-error') const OError = require('@overleaf/o-error')
const TextOperation = require('./operation/text_operation')
class NotFoundError extends OError { class NotFoundError extends OError {
constructor(hash) { constructor(hash) {
super(`blob ${hash} not found`, { 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 * in to determine that; so it is useful to have an upper bound on the byte
* length of a file that might be editable. * length of a file that might be editable.
* *
* This used to be 3 times the max editable file length to account for 3-byte * The reason for the factor of 3 is as follows. We cannot currently edit files
* UTF-8 codepoints. However, editable file blobs now include tracked deletes * that contain characters outside of the basic multilingual plane, so we're
* and the system used to allow unlimited tracked deletes on a single file. * limited to characters that can be represented in a single, two-byte UCS-2
* A practical limit is the 16 MB Mongo size limit. It wouldn't have been * code unit. Encoding the largest such value, 0xFFFF (which is not actually
* possible to store more than 16 MB of tracked deletes. We therefore fall * a valid character), takes three bytes in UTF-8: 0xEF 0xBF 0xBF. A file
* back to this limit. * 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 static NotFoundError = NotFoundError

View file

@ -41,17 +41,6 @@ class TrackedChangeList {
return this._trackedChanges.length return this._trackedChanges.length
} }
/**
* Iterate over tracked changes
*
* @returns {Iterator<TrackedChange>}
*/
*[Symbol.iterator]() {
for (const change of this._trackedChanges) {
yield change
}
}
/** /**
* @returns {readonly TrackedChange[]} * @returns {readonly TrackedChange[]}
*/ */

View file

@ -46,18 +46,9 @@ class TextOperation extends EditOperation {
/** /**
* Length of the longest file that we'll attempt to edit, in characters. * 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 * @type {number}
* 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 = 16 * Math.pow(1024, 2) static MAX_STRING_LENGTH = 2 * 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 UnprocessableError = UnprocessableError
static ApplyError = ApplyError static ApplyError = ApplyError
static InvalidInsertionError = InvalidInsertionError static InvalidInsertionError = InvalidInsertionError
@ -334,30 +325,9 @@ class TextOperation extends EditOperation {
} }
if (result.length > TextOperation.MAX_STRING_LENGTH) { 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) 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 file.content = result
} }

View file

@ -7,15 +7,16 @@ const TextOperation = ot.TextOperation
describe('HollowStringFileData', function () { describe('HollowStringFileData', function () {
it('validates string length when edited', function () { it('validates string length when edited', function () {
const length = 200 const maxLength = TextOperation.MAX_STRING_LENGTH
const fileData = new HollowStringFileData(length) const fileData = new HollowStringFileData(maxLength)
expect(fileData.getStringLength()).to.equal(maxLength)
expect(() => { expect(() => {
fileData.edit(new TextOperation().retain(length + 10).insert('x')) fileData.edit(new TextOperation().retain(maxLength).insert('x'))
}).to.throw(TextOperation.ApplyError) }).to.throw(TextOperation.TooLongError)
expect(fileData.getStringLength()).to.equal(length) expect(fileData.getStringLength()).to.equal(maxLength)
fileData.edit(new TextOperation().retain(length).insert('x')) fileData.edit(new TextOperation().retain(maxLength - 1).remove(1))
expect(fileData.getStringLength()).to.equal(length + 1) expect(fileData.getStringLength()).to.equal(maxLength - 1)
}) })
}) })

View file

@ -178,7 +178,7 @@ describe('LazyStringFileData', function () {
expect(fileData.getStringLength()).to.equal(0) expect(fileData.getStringLength()).to.equal(0)
expect(fileData.getOperations()).to.have.length(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)) fileData.edit(new TextOperation().insert(longString))
expect(fileData.getHash()).not.to.exist expect(fileData.getHash()).not.to.exist
expect(fileData.getByteLength()).to.equal(longString.length) // approximate expect(fileData.getByteLength()).to.equal(longString.length) // approximate
@ -186,10 +186,8 @@ describe('LazyStringFileData', function () {
expect(fileData.getOperations()).to.have.length(1) expect(fileData.getOperations()).to.have.length(1)
expect(() => { expect(() => {
fileData.edit( fileData.edit(new TextOperation().retain(longString.length).insert('x'))
new TextOperation().retain(longString.length - 123).insert('x') }).to.throw(TextOperation.TooLongError)
)
}).to.throw(TextOperation.ApplyError)
expect(fileData.getHash()).not.to.exist expect(fileData.getHash()).not.to.exist
expect(fileData.getByteLength()).to.equal(longString.length) // approximate expect(fileData.getByteLength()).to.equal(longString.length) // approximate
expect(fileData.getStringLength()).to.equal(longString.length) expect(fileData.getStringLength()).to.equal(longString.length)

View file

@ -5,10 +5,7 @@ const { expect } = require('chai')
const _ = require('lodash') const _ = require('lodash')
const ot = require('..') const ot = require('..')
const Range = require('../lib/range')
const StringFileData = require('../lib/file_data/string_file_data') 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 const TextOperation = ot.TextOperation
describe('StringFileData', function () { describe('StringFileData', function () {
@ -23,10 +20,7 @@ describe('StringFileData', function () {
}) })
it('validates string length when edited', function () { it('validates string length when edited', function () {
const longString = _.repeat( const longString = _.repeat('a', TextOperation.MAX_STRING_LENGTH)
'a',
TextOperation.MAX_STRING_LENGTH_EXCLUDING_TRACKED_DELETES
)
const fileData = new StringFileData(longString) const fileData = new StringFileData(longString)
expect(fileData.getByteLength()).to.equal(longString.length) expect(fileData.getByteLength()).to.equal(longString.length)
expect(fileData.getStringLength()).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) 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 () { it('getComments() should return an empty array', function () {
const fileData = new StringFileData('test') const fileData = new StringFileData('test')
expect(fileData.getComments().toRaw()).to.eql([]) expect(fileData.getComments().toRaw()).to.eql([])