diff --git a/release-notes.md b/release-notes.md index 588d5218..bb10cdaa 100644 --- a/release-notes.md +++ b/release-notes.md @@ -2,12 +2,13 @@ ## 8.0.0 (pending release) -- [#580](https://github.com/kpdecker/jsdiff/pull/580) Multiple tweaks to `diffSentences`: +- [#580](https://github.com/kpdecker/jsdiff/pull/580) **Multiple tweaks to `diffSentences`**: * tokenization no longer takes quadratic time on pathological inputs (reported as a ReDOS vulnerability by Snyk); is now linear instead * the final sentence in the string is now handled the same by the tokenizer regardless of whether it has a trailing punctuation mark or not. (Previously, "foo. bar." tokenized to `["foo.", " ", "bar."]` but "foo. bar" tokenized to `["foo.", " bar"]` - i.e. whether the space between sentences was treated as a separate token depended upon whether the final sentence had trailing punctuation or not. This was arbitrary and surprising; it is no longer the case.) * in a string that starts with a sentence end, like "! hello.", the "!" is now treated as a separate sentence * the README now correctly documents the tokenization behaviour (it was wrong before) -- [#581](https://github.com/kpdecker/jsdiff/pull/581) - fix some regex operations used for tokenization in `diffWords` taking O(n^2) time in pathological cases +- [#581](https://github.com/kpdecker/jsdiff/pull/581) - **fixed some regex operations used for tokenization in `diffWords` taking O(n^2) time** in pathological cases +- [#595](https://github.com/kpdecker/jsdiff/pull/595) - **fixed a crash in patch creation functions when handling a single hunk consisting of a very large number (e.g. >130k) of lines**. (This was caused by spreading indefinitely-large arrays to `.push()` using `.apply` or the spread operator and hitting the JS-implementation-specific limit on the maximum number of arguments to a function, as shown at https://stackoverflow.com/a/56809779/1709587; thus the exact threshold to hit the error will depend on the environment in which you were running JsDiff.) ## 7.0.0 diff --git a/src/patch/create.js b/src/patch/create.js index cd6af7e3..f73f6379 100644 --- a/src/patch/create.js +++ b/src/patch/create.js @@ -68,9 +68,9 @@ export function structuredPatch(oldFileName, newFileName, oldStr, newStr, oldHea } // Output our changes - curRange.push(... lines.map(function(entry) { - return (current.added ? '+' : '-') + entry; - })); + for (const line of lines) { + curRange.push((current.added ? '+' : '-') + line); + } // Track the updated file position if (current.added) { @@ -84,11 +84,15 @@ export function structuredPatch(oldFileName, newFileName, oldStr, newStr, oldHea // Close out any changes that have been output (or join overlapping) if (lines.length <= options.context * 2 && i < diff.length - 2) { // Overlapping - curRange.push(... contextLines(lines)); + for (const line of contextLines(lines)) { + curRange.push(line); + } } else { // end the range and output let contextSize = Math.min(lines.length, options.context); - curRange.push(... contextLines(lines.slice(0, contextSize))); + for (const line of contextLines(lines.slice(0, contextSize))) { + curRange.push(line); + } let hunk = { oldStart: oldRangeStart, @@ -159,7 +163,9 @@ export function formatPatch(diff) { + ' +' + hunk.newStart + ',' + hunk.newLines + ' @@' ); - ret.push.apply(ret, hunk.lines); + for (const line of hunk.lines) { + ret.push(line); + } } return ret.join('\n') + '\n'; diff --git a/src/patch/merge.js b/src/patch/merge.js index 4509fb38..ea35d3c1 100644 --- a/src/patch/merge.js +++ b/src/patch/merge.js @@ -136,6 +136,8 @@ function cloneHunk(hunk, offset) { }; } +// TODO: Fix use of push(... ) pattern here which probably trigger an error for really big changes +// due to the maximum argument limit function mergeLines(hunk, mineOffset, mineLines, theirOffset, theirLines) { // This will generally result in a conflicted hunk, but there are cases where the context // is the only overlap where we can successfully merge the content here. diff --git a/test/patch/create.js b/test/patch/create.js index 1d1772ab..d4cdaf76 100644 --- a/test/patch/create.js +++ b/test/patch/create.js @@ -671,6 +671,22 @@ describe('patch/create', function() { expect(createPatch('test', 'line1\nline2\nline3\n', 'lineX\nlineY\nlineZ\nline42\n', 'header1', 'header2', {maxEditLength: 1})).to.be.undefined; }); + it('should not crash when encountering an enormous hunk', function() { + // Regression test; we used to crash here due to lines like + // ret.push.apply(ret, hunk.lines); + // and + // curRange.push(... contextLines(lines.slice(0, contextSize))); + // in the patch creation code that fail in practice due to JS implementations having a + // maximum argument count. + expect(createTwoFilesPatch('foo', 'bar', '', 'foo\n'.repeat(1000000))).to.equal( + '===================================================================\n' + + '--- foo\n' + + '+++ bar\n' + + '@@ -0,0 +1,1000000 @@\n' + + '+foo\n'.repeat(1000000) + ); + }); + describe('ignoreWhitespace', function() { it('ignoreWhitespace: false', function() { const expectedResult =