Skip to content

Commit a276ee6

Browse files
authored
Merge pull request microsoft#201157 from microsoft/tyriar/180257
Use safer pasted input chunking algorithm
2 parents 595e7d4 + 0e6e556 commit a276ee6

File tree

3 files changed

+84
-16
lines changed

3 files changed

+84
-16
lines changed

src/vs/platform/terminal/common/terminalProcess.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,41 @@ export interface ReplayEntry {
6767
rows: number;
6868
data: string;
6969
}
70+
71+
const enum Constants {
72+
/**
73+
* Writing large amounts of data can be corrupted for some reason, after looking into this is
74+
* appears to be a race condition around writing to the FD which may be based on how powerful
75+
* the hardware is. The workaround for this is to space out when large amounts of data is being
76+
* written to the terminal. See https://github.com/microsoft/vscode/issues/38137
77+
*/
78+
WriteMaxChunkSize = 50,
79+
}
80+
81+
/**
82+
* Splits incoming pty data into chunks to try prevent data corruption that could occur when pasting
83+
* large amounts of data.
84+
*/
85+
export function chunkInput(data: string): string[] {
86+
const chunks: string[] = [];
87+
let nextChunkStartIndex = 0;
88+
for (let i = 0; i < data.length - 1; i++) {
89+
if (
90+
// If the max chunk size is reached
91+
i - nextChunkStartIndex + 1 >= Constants.WriteMaxChunkSize ||
92+
// If the next character is ESC, send the pending data to avoid splitting the escape
93+
// sequence.
94+
data[i + 1] === '\x1b'
95+
) {
96+
chunks.push(data.substring(nextChunkStartIndex, i + 1));
97+
nextChunkStartIndex = i + 1;
98+
// Skip the next character as the chunk would be a single character
99+
i++;
100+
}
101+
}
102+
// Push final chunk
103+
if (nextChunkStartIndex !== data.length) {
104+
chunks.push(data.substring(nextChunkStartIndex));
105+
}
106+
return chunks;
107+
}

src/vs/platform/terminal/node/terminalProcess.ts

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { ChildProcessMonitor } from 'vs/platform/terminal/node/childProcessMonit
1919
import { findExecutable, getShellIntegrationInjection, getWindowsBuildNumber, IShellIntegrationConfigInjection } from 'vs/platform/terminal/node/terminalEnvironment';
2020
import { WindowsShellHelper } from 'vs/platform/terminal/node/windowsShellHelper';
2121
import { IPty, IPtyForkOptions, IWindowsPtyForkOptions, spawn } from 'node-pty';
22+
import { chunkInput } from 'vs/platform/terminal/common/terminalProcess';
2223

2324
const enum ShutdownConstants {
2425
/**
@@ -54,14 +55,6 @@ const enum Constants {
5455
* interval.
5556
*/
5657
KillSpawnSpacingDuration = 50,
57-
58-
/**
59-
* Writing large amounts of data can be corrupted for some reason, after looking into this is
60-
* appears to be a race condition around writing to the FD which may be based on how powerful
61-
* the hardware is. The workaround for this is to space out when large amounts of data is being
62-
* written to the terminal. See https://github.com/microsoft/vscode/issues/38137
63-
*/
64-
WriteMaxChunkSize = 50,
6558
/**
6659
* How long to wait between chunk writes.
6760
*/
@@ -437,17 +430,13 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess
437430
}
438431
}
439432

440-
input(data: string, isBinary?: boolean): void {
433+
input(data: string, isBinary: boolean = false): void {
441434
if (this._store.isDisposed || !this._ptyProcess) {
442435
return;
443436
}
444-
for (let i = 0; i <= Math.floor(data.length / Constants.WriteMaxChunkSize); i++) {
445-
const obj = {
446-
isBinary: isBinary || false,
447-
data: data.substr(i * Constants.WriteMaxChunkSize, Constants.WriteMaxChunkSize)
448-
};
449-
this._writeQueue.push(obj);
450-
}
437+
this._writeQueue.push(...chunkInput(data).map(e => {
438+
return { isBinary, data: e };
439+
}));
451440
this._startWrite();
452441
}
453442

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { deepStrictEqual } from 'assert';
7+
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
8+
import { chunkInput } from 'vs/platform/terminal/common/terminalProcess';
9+
10+
suite('platform - terminalProcess', () => {
11+
ensureNoDisposablesAreLeakedInTestSuite();
12+
suite('chunkInput', () => {
13+
test('single chunk', () => {
14+
deepStrictEqual(chunkInput('foo bar'), ['foo bar']);
15+
});
16+
test('multi chunk', () => {
17+
deepStrictEqual(chunkInput('foo'.repeat(50)), [
18+
'foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofo',
19+
'ofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoof',
20+
'oofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo'
21+
]);
22+
});
23+
test('small data with escapes', () => {
24+
deepStrictEqual(chunkInput('foo \x1b[30mbar'), [
25+
'foo ',
26+
'\x1b[30mbar'
27+
]);
28+
});
29+
test('large data with escapes', () => {
30+
deepStrictEqual(chunkInput('foofoofoofoo\x1b[30mbarbarbarbarbar\x1b[0m'.repeat(3)), [
31+
'foofoofoofoo',
32+
'\x1B[30mbarbarbarbarbar',
33+
'\x1B[0mfoofoofoofoo',
34+
'\x1B[30mbarbarbarbarbar',
35+
'\x1B[0mfoofoofoofoo',
36+
'\x1B[30mbarbarbarbarbar',
37+
'\x1B[0m'
38+
]);
39+
});
40+
});
41+
});

0 commit comments

Comments
 (0)