Don't crash during patch creation if there's a massive single hunk (#595)

* Add failing test

* Fix crash, add a TODO about similar brokenness in merge.js

* Flesh out test

* Add release notes

* Use bold for key points in release notes again

Author: ExplodingCabbage

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
 

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';

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.

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 =