diff --git a/services/web/app/src/util/bib2json.js b/services/web/app/src/util/bib2json.js index b105ff3f7e..9037bba990 100644 --- a/services/web/app/src/util/bib2json.js +++ b/services/web/app/src/util/bib2json.js @@ -38,6 +38,8 @@ * @param {array} allowedKeys optimization: do not output key/value pairs that are not on this allowlist * @constructor */ +const MAX_FIELD_VALUE_LENGTH = 1000 * 20 + function BibtexParser(arg0, allowedKeys) { // Determine how this function is to be used if (typeof arg0 === 'string') { @@ -473,6 +475,11 @@ BibtexParser.prototype.processCharacter_ = function (c) { this.SKIPCOMMENT_ = true this.STATE_ = this.STATES_.KV_VALUE this.PARSETMP_.Value = [] + // Tail tracks the last 2 raw characters independently of Value, so + // escape detection below stays correct even once Value stops + // growing at MAX_FIELD_VALUE_LENGTH (see the push site further + // down for why storage and escape state can't share one array). + this.PARSETMP_.Tail = [] this.VALBRACES_ = { '"': [], '{': [] } } break @@ -481,9 +488,10 @@ BibtexParser.prototype.processCharacter_ = function (c) { // -- Populate this.PARSETMP_.Value case this.STATES_.KV_VALUE: const delim = this.VALBRACES_ - // valueCharsArray is the list of characters that make up the - // current value - const valueCharsArray = this.PARSETMP_.Value + // tail is the last 2 raw characters consumed for the current value - + // see the push site below for why this is tracked separately from + // Value once Value is capped. + const tail = this.PARSETMP_.Tail let doneParsingValue = false // Test for special characters @@ -520,8 +528,7 @@ BibtexParser.prototype.processCharacter_ = function (c) { if ( delim['"'].length == 1 && delim['{'].length == 0 && - (valueCharsArray.length == 0 || - valueCharsArray[valueCharsArray.length - 1] != '\\') + (tail.length == 0 || tail[tail.length - 1] != '\\') ) { // closing delimiter doneParsingValue = true @@ -533,10 +540,7 @@ BibtexParser.prototype.processCharacter_ = function (c) { // This brace can mean: // (1) opening delimiter // (2) stacked verbatim delimiter - if ( - valueCharsArray.length == 0 || - valueCharsArray[valueCharsArray.length - 1] != '\\' - ) { + if (tail.length == 0 || tail[tail.length - 1] != '\\') { delim['{'].push(this.CHAR_) this.SKIPWS_ = false this.SKIPCOMMENT_ = false @@ -566,17 +570,17 @@ BibtexParser.prototype.processCharacter_ = function (c) { // which would otherwise break the parsing. we watch for these occurences of // 1+ backslashes in an empty bracket pair to gracefully handle the malformed bib file const doubleSlash = - valueCharsArray.length >= 2 && - valueCharsArray[valueCharsArray.length - 1] === '\\' && // for \\} - valueCharsArray[valueCharsArray.length - 2] === '\\' + tail.length >= 2 && + tail[tail.length - 1] === '\\' && // for \\} + tail[tail.length - 2] === '\\' const singleSlash = - valueCharsArray.length >= 2 && - valueCharsArray[valueCharsArray.length - 1] === '\\' && // for {\} - valueCharsArray[valueCharsArray.length - 2] === '{' + tail.length >= 2 && + tail[tail.length - 1] === '\\' && // for {\} + tail[tail.length - 2] === '{' if ( - valueCharsArray.length == 0 || - valueCharsArray[valueCharsArray.length - 1] != '\\' || // for } + tail.length == 0 || + tail[tail.length - 1] != '\\' || // for } doubleSlash || singleSlash ) { @@ -612,14 +616,17 @@ BibtexParser.prototype.processCharacter_ = function (c) { this.PARSETMP_ = { Key: '' } this.VALBRACES_ = null } else { - this.PARSETMP_.Value.push(c) - if (this.PARSETMP_.Value.length >= 1000 * 20) { - this.PARSETMP_.Value = [] - this.STATE_ = this.STATES_.ENTRY_OR_JUNK - this.DATA_ = { ObjectType: '' } - this.BRACETYPE_ = null - this.SKIPWS_ = true - this.SKIPCOMMENT_ = true + // Track the last 2 raw characters regardless of the storage cap + // below, so the escape checks above stay correct past the cap + // instead of freezing on whatever was last actually stored. + this.PARSETMP_.Tail.push(c) + if (this.PARSETMP_.Tail.length > 2) { + this.PARSETMP_.Tail.shift() + } + // Cap storage so a single huge field (e.g. thousands of authors) + // can't drive unbounded memory use while parsing. + if (this.PARSETMP_.Value.length < MAX_FIELD_VALUE_LENGTH) { + this.PARSETMP_.Value.push(c) } } break diff --git a/services/web/frontend/js/features/ide-react/references/bib2json.js b/services/web/frontend/js/features/ide-react/references/bib2json.js index eaa6bcff15..4657bec4fd 100644 --- a/services/web/frontend/js/features/ide-react/references/bib2json.js +++ b/services/web/frontend/js/features/ide-react/references/bib2json.js @@ -38,6 +38,8 @@ * @param {array} allowedKeys optimization: do not output key/value pairs that are not on this allowlist * @constructor */ +const MAX_FIELD_VALUE_LENGTH = 1000 * 20 + function BibtexParser(arg0, allowedKeys) { // Determine how this function is to be used if (typeof arg0 === 'string') { @@ -479,6 +481,11 @@ BibtexParser.prototype.processCharacter_ = function (c) { this.SKIPCOMMENT_ = true this.STATE_ = this.STATES_.KV_VALUE this.PARSETMP_.Value = [] + // Tail tracks the last 2 raw characters independently of Value, so + // escape detection below stays correct even once Value stops + // growing at MAX_FIELD_VALUE_LENGTH (see the push site further + // down for why storage and escape state can't share one array). + this.PARSETMP_.Tail = [] this.VALBRACES_ = { '"': [], '{': [] } } break @@ -487,9 +494,10 @@ BibtexParser.prototype.processCharacter_ = function (c) { // -- Populate this.PARSETMP_.Value case this.STATES_.KV_VALUE: const delim = this.VALBRACES_ - // valueCharsArray is the list of characters that make up the - // current value - const valueCharsArray = this.PARSETMP_.Value + // tail is the last 2 raw characters consumed for the current value - + // see the push site below for why this is tracked separately from + // Value once Value is capped. + const tail = this.PARSETMP_.Tail let doneParsingValue = false // Test for special characters @@ -526,8 +534,7 @@ BibtexParser.prototype.processCharacter_ = function (c) { if ( delim['"'].length == 1 && delim['{'].length == 0 && - (valueCharsArray.length == 0 || - valueCharsArray[valueCharsArray.length - 1] != '\\') + (tail.length == 0 || tail[tail.length - 1] != '\\') ) { // closing delimiter doneParsingValue = true @@ -539,10 +546,7 @@ BibtexParser.prototype.processCharacter_ = function (c) { // This brace can mean: // (1) opening delimiter // (2) stacked verbatim delimiter - if ( - valueCharsArray.length == 0 || - valueCharsArray[valueCharsArray.length - 1] != '\\' - ) { + if (tail.length == 0 || tail[tail.length - 1] != '\\') { delim['{'].push(this.CHAR_) this.SKIPWS_ = false this.SKIPCOMMENT_ = false @@ -572,17 +576,17 @@ BibtexParser.prototype.processCharacter_ = function (c) { // which would otherwise break the parsing. we watch for these occurences of // 1+ backslashes in an empty bracket pair to gracefully handle the malformed bib file const doubleSlash = - valueCharsArray.length >= 2 && - valueCharsArray[valueCharsArray.length - 1] === '\\' && // for \\} - valueCharsArray[valueCharsArray.length - 2] === '\\' + tail.length >= 2 && + tail[tail.length - 1] === '\\' && // for \\} + tail[tail.length - 2] === '\\' const singleSlash = - valueCharsArray.length >= 2 && - valueCharsArray[valueCharsArray.length - 1] === '\\' && // for {\} - valueCharsArray[valueCharsArray.length - 2] === '{' + tail.length >= 2 && + tail[tail.length - 1] === '\\' && // for {\} + tail[tail.length - 2] === '{' if ( - valueCharsArray.length == 0 || - valueCharsArray[valueCharsArray.length - 1] != '\\' || // for } + tail.length == 0 || + tail[tail.length - 1] != '\\' || // for } doubleSlash || singleSlash ) { @@ -618,14 +622,17 @@ BibtexParser.prototype.processCharacter_ = function (c) { this.PARSETMP_ = { Key: '' } this.VALBRACES_ = null } else { - this.PARSETMP_.Value.push(c) - if (this.PARSETMP_.Value.length >= 1000 * 20) { - this.PARSETMP_.Value = [] - this.STATE_ = this.STATES_.ENTRY_OR_JUNK - this.DATA_ = { ObjectType: '' } - this.BRACETYPE_ = null - this.SKIPWS_ = true - this.SKIPCOMMENT_ = true + // Track the last 2 raw characters regardless of the storage cap + // below, so the escape checks above stay correct past the cap + // instead of freezing on whatever was last actually stored. + this.PARSETMP_.Tail.push(c) + if (this.PARSETMP_.Tail.length > 2) { + this.PARSETMP_.Tail.shift() + } + // Cap storage so a single huge field (e.g. thousands of authors) + // can't drive unbounded memory use while parsing. + if (this.PARSETMP_.Value.length < MAX_FIELD_VALUE_LENGTH) { + this.PARSETMP_.Value.push(c) } } break diff --git a/services/web/test/frontend/features/ide-react/unit/references/bib2json.spec.ts b/services/web/test/frontend/features/ide-react/unit/references/bib2json.spec.ts index 411061748b..2391594dd2 100644 --- a/services/web/test/frontend/features/ide-react/unit/references/bib2json.spec.ts +++ b/services/web/test/frontend/features/ide-react/unit/references/bib2json.spec.ts @@ -172,6 +172,173 @@ describe('Bib2JsonTests', function () { }) }) + describe('oversized field values', function () { + // overleaf/internal#24246: an oversized field is now truncated to + // ~MAX_FIELD_VALUE_LENGTH instead of dropping the whole entry. Braced + // values include the opening brace in the raw length, so the final + // length can land a character under the limit rather than exactly on it. + const MAX_FIELD_VALUE_LENGTH = 1000 * 20 + + it('truncates an oversized field but keeps the entry and its other fields', function () { + const hugeAuthorList = 'A'.repeat(MAX_FIELD_VALUE_LENGTH + 5000) + const text = `@article{ key, title={Some title}, author={${hugeAuthorList}}, year={2024} }` + const result = BibtexParser(text) + expect(result.entries.length).to.equal(1) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title).to.equal('Some title') + expect(result.entries[0].Fields.year).to.equal('2024') + + const author = result.entries[0].Fields.author + expect(author.length).to.be.at.most(MAX_FIELD_VALUE_LENGTH) + expect(author.length).to.be.lessThan(hugeAuthorList.length) + expect(hugeAuthorList.startsWith(author)).to.equal(true) + }) + + it('keeps parsing later entries after an oversized field', function () { + const hugeAuthorList = 'A'.repeat(MAX_FIELD_VALUE_LENGTH + 5000) + const text = + `@article{ first, title={First}, author={${hugeAuthorList}} }` + + '@article{ second, title={Second}, author={Doe, Jane} }' + const result = BibtexParser(text) + expect(result.entries.length).to.equal(2) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title).to.equal('First') + expect(result.entries[0].Fields.author.length).to.be.at.most( + MAX_FIELD_VALUE_LENGTH + ) + expect(result.entries[1].Fields.title).to.equal('Second') + expect(result.entries[1].Fields.author).to.equal('Doe, Jane') + }) + + it('does not truncate a field comfortably under the limit', function () { + const authorList = 'A'.repeat(MAX_FIELD_VALUE_LENGTH - 100) + const text = `@article{ key, author={${authorList}} }` + const result = BibtexParser(text) + expect(result.entries.length).to.equal(1) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.author).to.equal(authorList) + }) + + it('truncates an oversized quoted field but keeps the entry and its other fields', function () { + const hugeAuthorList = 'A'.repeat(MAX_FIELD_VALUE_LENGTH + 5000) + const text = `@article{ key, title="Some title", author="${hugeAuthorList}", year="2024" }` + const result = BibtexParser(text) + expect(result.entries.length).to.equal(1) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title).to.equal('Some title') + expect(result.entries[0].Fields.year).to.equal('2024') + + const author = result.entries[0].Fields.author + expect(author.length).to.be.at.most(MAX_FIELD_VALUE_LENGTH) + expect(author.length).to.be.lessThan(hugeAuthorList.length) + expect(hugeAuthorList.startsWith(author)).to.equal(true) + }) + + it('truncates cleanly through nested braces spanning the length boundary', function () { + // cap lands inside a nested {Middle} group + const before = 'A'.repeat(MAX_FIELD_VALUE_LENGTH - 3) + const text = `@article{ key, title={${before}{Middle} end}, year={2024} }` + const result = BibtexParser(text) + expect(result.entries.length).to.equal(1) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.year).to.equal('2024') + + const title = result.entries[0].Fields.title + expect(title.length).to.be.at.most(MAX_FIELD_VALUE_LENGTH) + expect(title.startsWith('A'.repeat(100))).to.equal(true) + expect(title.endsWith('end')).to.equal(false) + }) + + it('keeps parsing correctly when the length cap lands right after a backslash', function () { + // Regression test: capping the value array mid-parse (instead of + // truncating once complete) froze the escape check on a trailing + // backslash, permanently misreading later quotes/braces as escaped and + // swallowing the rest of the document with no error. + const before = 'A'.repeat(MAX_FIELD_VALUE_LENGTH - 1) + const text = + `@article{ key, title="${before}\\"still inside", year="2024" }` + + '@article{ second, title={Second} }' + const result = BibtexParser(text) + expect(result.entries.length).to.equal(2) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title.length).to.equal( + MAX_FIELD_VALUE_LENGTH + ) + // "year" follows the truncated "title" field in the same entry - the + // bug above would have swallowed this too. + expect(result.entries[0].Fields.year).to.equal('2024') + expect(result.entries[1].Fields.title).to.equal('Second') + }) + + it('does not record parse errors as a side effect of truncation', function () { + const hugeAuthorList = 'A'.repeat(MAX_FIELD_VALUE_LENGTH + 5000) + const text = + `@article{ first, title={First}, author={${hugeAuthorList}} }` + + `@article{ second, title={Second}, author="${hugeAuthorList}" }` + const result = BibtexParser(text) + expect(result.entries.length).to.equal(2) + expect(result.errors.length).to.equal(0) + }) + }) + + describe('boundary sweep near the truncation cap', function () { + // Regression safety net for a planned refactor that will cap + // PARSETMP_.Value while parsing (to bound memory on huge fields) + // instead of only truncating once the value is complete. That refactor + // touches the escape/verbatim-brace detection this suite exercises, so + // these tests pin the exact current output at many cap offsets before + // the change, using a helper that mirrors processEntry_'s brace/ + // backslash stripping (bib2json.js) rather than trusting the parser to + // grade its own homework. + const MAX_FIELD_VALUE_LENGTH = 1000 * 20 + + function stripBracesAndBackslashes(raw: string): string { + let result = '' + for (let i = 0; i < raw.length; i++) { + let c = raw[i] + if (c === '\\' && i < raw.length - 1) { + c = raw[++i] + } else if (c === '{' || c === '}') { + continue + } + result += c + } + return result + } + + // decoded from the "handles one backslash {\}" fixture above + const oneBackslash = '{\\}{A} \\\\very \\{{Big} \\"Book\\".' + // decoded from the "handles two backslashes {\\}" fixture above - + // exercises the doubleSlash branch that the single-backslash case doesn't + const twoBackslashes = '{\\\\}{A} \\\\very \\{{Big} \\"Book\\".' + + function checkTruncationAtOffset(rawSpecial: string, k: number) { + it(`truncates correctly when the cap lands ${k} chars into the sequence`, function () { + const paddingLen = MAX_FIELD_VALUE_LENGTH - k + const value = 'A'.repeat(paddingLen) + rawSpecial + const expectedTitle = stripBracesAndBackslashes( + value.slice(0, MAX_FIELD_VALUE_LENGTH) + ) + const text = + `@article{ first, title="${value}", year="2024" }` + + '@article{ second, title={Second} }' + const result = BibtexParser(text) + expect(result.entries.length).to.equal(2) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title).to.equal(expectedTitle) + expect(result.entries[0].Fields.year).to.equal('2024') + expect(result.entries[1].Fields.title).to.equal('Second') + }) + } + + ;[0, 1, 2, 3, 9, 16, 23, 31].forEach(k => + checkTruncationAtOffset(oneBackslash, k) + ) + ;[0, 1, 2, 3, 4, 10, 17].forEach(k => + checkTruncationAtOffset(twoBackslashes, k) + ) + }) + return describe('with allowedKeys set', function () { it('should skip the unknown key Random', function () { const text = '@book{ id, Random = "ABC", title="VALUE" }' diff --git a/services/web/test/unit/src/util/bib2json.test.mjs b/services/web/test/unit/src/util/bib2json.test.mjs new file mode 100644 index 0000000000..83985a9fe0 --- /dev/null +++ b/services/web/test/unit/src/util/bib2json.test.mjs @@ -0,0 +1,121 @@ +import { describe, expect, it } from 'vitest' +import BibtexParser from '../../../../app/src/util/bib2json.js' + +describe('bib2json', function () { + // Server-side parser used by Write & Cite / Zotero / Papers sync + // (services/web/modules/tpr-webmodule). overleaf/internal#24246: an + // oversized field is now truncated instead of dropping the whole entry. + const MAX_FIELD_VALUE_LENGTH = 1000 * 20 + + it('parses a simple entry', function () { + const result = BibtexParser('@book{ pollock, title={A Book} }') + expect(result.entries.length).to.equal(1) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title).to.equal('A Book') + }) + + it('truncates an oversized field but keeps the entry and its other fields', function () { + const hugeAuthorList = 'A'.repeat(MAX_FIELD_VALUE_LENGTH + 5000) + const text = `@article{ key, title={Some title}, author={${hugeAuthorList}}, year={2024} }` + const result = BibtexParser(text) + expect(result.entries.length).to.equal(1) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title).to.equal('Some title') + expect(result.entries[0].Fields.year).to.equal('2024') + + const author = result.entries[0].Fields.author + expect(author.length).to.be.at.most(MAX_FIELD_VALUE_LENGTH) + expect(author.length).to.be.lessThan(hugeAuthorList.length) + expect(hugeAuthorList.startsWith(author)).to.equal(true) + }) + + it('keeps parsing later entries after an oversized field', function () { + const hugeAuthorList = 'A'.repeat(MAX_FIELD_VALUE_LENGTH + 5000) + const text = + `@article{ first, title={First}, author={${hugeAuthorList}} }` + + '@article{ second, title={Second}, author={Doe, Jane} }' + const result = BibtexParser(text) + expect(result.entries.length).to.equal(2) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title).to.equal('First') + expect(result.entries[1].Fields.title).to.equal('Second') + expect(result.entries[1].Fields.author).to.equal('Doe, Jane') + }) + + it('keeps parsing correctly when the length cap lands right after a backslash', function () { + // Regression test: capping the value array mid-parse (instead of + // truncating once complete) froze the escape check on a trailing + // backslash, permanently misreading later quotes/braces as escaped and + // swallowing the rest of the document with no error. + const before = 'A'.repeat(MAX_FIELD_VALUE_LENGTH - 1) + const text = + `@article{ key, title="${before}\\"still inside", year="2024" }` + + '@article{ second, title={Second} }' + const result = BibtexParser(text) + expect(result.entries.length).to.equal(2) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title.length).to.equal( + MAX_FIELD_VALUE_LENGTH + ) + // "year" follows the truncated "title" field in the same entry - the + // bug above would have swallowed this too. + expect(result.entries[0].Fields.year).to.equal('2024') + expect(result.entries[1].Fields.title).to.equal('Second') + }) + + describe('boundary sweep near the truncation cap', function () { + // Regression safety net for a planned refactor that will cap + // PARSETMP_.Value while parsing (to bound memory on huge fields) + // instead of only truncating once the value is complete. That refactor + // touches the escape/verbatim-brace detection this suite exercises, so + // these tests pin the exact current output at many cap offsets before + // the change, using a helper that mirrors processEntry_'s brace/ + // backslash stripping (bib2json.js) rather than trusting the parser to + // grade its own homework. + function stripBracesAndBackslashes(raw) { + let result = '' + for (let i = 0; i < raw.length; i++) { + let c = raw[i] + if (c === '\\' && i < raw.length - 1) { + c = raw[++i] + } else if (c === '{' || c === '}') { + continue + } + result += c + } + return result + } + + // decoded from the "one backslash {\}" style fixture in the frontend spec + const oneBackslash = '{\\}{A} \\\\very \\{{Big} \\"Book\\".' + // decoded from the "two backslashes {\\}" style fixture - exercises the + // doubleSlash branch that the single-backslash case doesn't + const twoBackslashes = '{\\\\}{A} \\\\very \\{{Big} \\"Book\\".' + + function checkTruncationAtOffset(rawSpecial, k) { + it(`truncates correctly when the cap lands ${k} chars into the sequence`, function () { + const paddingLen = MAX_FIELD_VALUE_LENGTH - k + const value = 'A'.repeat(paddingLen) + rawSpecial + const expectedTitle = stripBracesAndBackslashes( + value.slice(0, MAX_FIELD_VALUE_LENGTH) + ) + const text = + `@article{ first, title="${value}", year="2024" }` + + '@article{ second, title={Second} }' + const result = BibtexParser(text) + expect(result.entries.length).to.equal(2) + expect(result.errors.length).to.equal(0) + expect(result.entries[0].Fields.title).to.equal(expectedTitle) + expect(result.entries[0].Fields.year).to.equal('2024') + expect(result.entries[1].Fields.title).to.equal('Second') + }) + } + + ;[0, 1, 2, 3, 9, 16, 23, 31].forEach(k => + checkTruncationAtOffset(oneBackslash, k) + ) + ;[0, 1, 2, 3, 4, 10, 17].forEach(k => + checkTruncationAtOffset(twoBackslashes, k) + ) + }) +})