From e106cd02cfcfe1c63144b2c3ab3affd9d3cbb5c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= <jerch@rockborn.de>
Date: Fri, 26 Mar 2021 12:46:39 +0100
Subject: [PATCH] fix callstack overflow of writeSync

---
 src/common/CoreTerminal.ts           |  4 +-
 src/common/input/WriteBuffer.test.ts | 21 ++++++++++
 src/common/input/WriteBuffer.ts      | 60 +++++++++++++++++++---------
 3 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/src/common/CoreTerminal.ts b/src/common/CoreTerminal.ts
index 0236d39f..c11554f1 100644
--- a/src/common/CoreTerminal.ts
+++ b/src/common/CoreTerminal.ts
@@ -137,12 +137,12 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
    *
    * @deprecated Unreliable, will be removed soon.
    */
-  public writeSync(data: string | Uint8Array): void {
+  public writeSync(data: string | Uint8Array, maxSubsequentCalls?: number): void {
     if (this._logService.logLevel <= LogLevelEnum.WARN && !hasWriteSyncWarnHappened) {
       this._logService.warn('writeSync is unreliable and will be removed soon.');
       hasWriteSyncWarnHappened = true;
     }
-    this._writeBuffer.writeSync(data);
+    this._writeBuffer.writeSync(data, maxSubsequentCalls);
   }
 
   public resize(x: number, y: number): void {
diff --git a/src/common/input/WriteBuffer.test.ts b/src/common/input/WriteBuffer.test.ts
index d3dfebd0..89106423 100644
--- a/src/common/input/WriteBuffer.test.ts
+++ b/src/common/input/WriteBuffer.test.ts
@@ -85,5 +85,26 @@ describe('WriteBuffer', () => {
         done();
       });
     });
+    it('writeSync called from action does not overflow callstack - issue #3265', () => {
+      wb = new WriteBuffer(data => {
+        const num = parseInt(data as string);
+        if (num < 1000000) {
+          wb.writeSync('' + (num + 1));
+        }
+      });
+      wb.writeSync('1');
+    });
+    it('writeSync maxSubsequentCalls argument', () => {
+      let last: string = '';
+      wb = new WriteBuffer(data => {
+        last = data as string;
+        const num = parseInt(data as string);
+        if (num < 1000000) {
+          wb.writeSync('' + (num + 1), 10);
+        }
+      });
+      wb.writeSync('1', 10);
+      assert.equal(last, '11'); // 1 + 10 sub calls = 11
+    });
   });
 });
diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts
index d22de912..cc84c9ab 100644
--- a/src/common/input/WriteBuffer.ts
+++ b/src/common/input/WriteBuffer.ts
@@ -42,31 +42,55 @@ export class WriteBuffer {
   private _callbacks: ((() => void) | undefined)[] = [];
   private _pendingData = 0;
   private _bufferOffset = 0;
+  private _isSyncWriting = false;
+  private _syncCalls = 0;
 
   constructor(private _action: (data: string | Uint8Array, promiseResult?: boolean) => void | Promise<boolean>) { }
 
   /**
    * @deprecated Unreliable, to be removed soon.
    */
-  public writeSync(data: string | Uint8Array): void {
+  public writeSync(data: string | Uint8Array, maxSubsequentCalls?: number): void {
+    // stop writeSync recursions with maxSubsequentCalls argument
+    // This is dangerous to use as it will lose the current data chunk
+    // and return immediately.
+    if (maxSubsequentCalls !== undefined && this._syncCalls > maxSubsequentCalls) {
+      // comment next line if a whole loop block should only contain x `writeSync` calls
+      // (total flat vs. deep nested limit)
+      this._syncCalls = 0;
+      return;
+    }
+    // append chunk to buffer
+    this._pendingData += data.length;
+    this._writeBuffer.push(data);
+    this._callbacks.push(undefined);
+
+    // increase recursion counter
+    this._syncCalls++;
+    // exit early if another writeSync loop is active
+    if (this._isSyncWriting) {
+      return;
+    }
+    this._isSyncWriting = true;
+
     // force sync processing on pending data chunks to avoid in-band data scrambling
     // does the same as innerWrite but without event loop
-    if (this._writeBuffer.length) {
-      for (let i = this._bufferOffset; i < this._writeBuffer.length; ++i) {
-        const data = this._writeBuffer[i];
-        const cb = this._callbacks[i];
-        this._action(data);
-        if (cb) cb();
-      }
-      // reset all to avoid reprocessing of chunks with scheduled innerWrite call
-      this._writeBuffer = [];
-      this._callbacks = [];
-      this._pendingData = 0;
-      // stop scheduled innerWrite by offset > length condition
-      this._bufferOffset = 0x7FFFFFFF;
+    // we have to do it here as single loop steps to not corrupt loop subject
+    // by another writeSync call triggered from _action
+    let chunk: string | Uint8Array | undefined;
+    while (chunk = this._writeBuffer.shift()) {
+      this._action(chunk);
+      const cb = this._callbacks.shift();
+      if (cb) cb();
     }
-    // handle current data chunk
-    this._action(data);
+    // reset to avoid reprocessing of chunks with scheduled innerWrite call
+    // stopping scheduled innerWrite by offset > length condition
+    this._pendingData = 0;
+    this._bufferOffset = 0x7FFFFFFF;
+
+    // allow another writeSync to loop
+    this._isSyncWriting = false;
+    this._syncCalls = 0;
   }
 
   public write(data: string | Uint8Array, callback?: () => void): void {
@@ -191,8 +215,8 @@ export class WriteBuffer {
       }
       setTimeout(() => this._innerWrite());
     } else {
-      this._writeBuffer = [];
-      this._callbacks = [];
+      this._writeBuffer.length = 0;
+      this._callbacks.length = 0;
       this._pendingData = 0;
       this._bufferOffset = 0;
     }