Skip to content

Commit 8e68bbb

Browse files
authored
Fix Windows backslash paths being mangled when adding an SSH target (#14554)
* Fix Windows backslash paths being mangled when adding an SSH target
1 parent fe18c4a commit 8e68bbb

5 files changed

Lines changed: 166 additions & 50 deletions

File tree

Extension/ThirdPartyNotices.txt

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3241,40 +3241,6 @@ IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
32413241
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
32423242

32433243

3244-
---------------------------------------------------------
3245-
3246-
---------------------------------------------------------
3247-
3248-
shell-quote 1.8.4 - MIT
3249-
https://github.com/ljharb/shell-quote
3250-
3251-
Copyright (c) 2013 James Halliday (mail@substack.net)
3252-
3253-
The MIT License
3254-
3255-
Copyright (c) 2013 James Halliday (mail@substack.net)
3256-
3257-
Permission is hereby granted, free of charge,
3258-
to any person obtaining a copy of this software and
3259-
associated documentation files (the "Software"), to
3260-
deal in the Software without restriction, including
3261-
without limitation the rights to use, copy, modify,
3262-
merge, publish, distribute, sublicense, and/or sell
3263-
copies of the Software, and to permit persons to whom
3264-
the Software is furnished to do so,
3265-
subject to the following conditions:
3266-
3267-
The above copyright notice and this permission notice
3268-
shall be included in all copies or substantial portions of the Software.
3269-
3270-
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
3271-
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
3272-
OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
3273-
IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR
3274-
ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
3275-
TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
3276-
SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
3277-
32783244
---------------------------------------------------------
32793245

32803246
---------------------------------------------------------

Extension/package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6870,7 +6870,6 @@
68706870
"@types/plist": "^3.0.5",
68716871
"@types/proxyquire": "^1.3.31",
68726872
"@types/semver": "^7.5.8",
6873-
"@types/shell-quote": "^1.7.5",
68746873
"@types/sinon": "^21.0.0",
68756874
"@types/tmp": "^0.2.6",
68766875
"@types/which": "^2.0.2",
@@ -6926,7 +6925,6 @@
69266925
"node-vswhere": "^1.0.2",
69276926
"plist": "^3.1.0",
69286927
"posix-getopt": "^1.2.1",
6929-
"shell-quote": "1.8.4",
69306928
"ssh-config": "^4.4.4",
69316929
"tmp": "^0.2.7",
69326930
"vscode-cpptools": "^7.1.1",

Extension/src/SSH/sshCommandToConfig.ts

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
* ------------------------------------------------------------------------------------------ */
55

66
import { BasicParser, IParsedOption } from 'posix-getopt';
7-
import { parse } from 'shell-quote';
87

98
/**
109
* Mapping of flags to functions that add the relevant flag to the map of
@@ -124,7 +123,11 @@ export class CommandParseError extends Error { }
124123
* Attempts to convert an SSH command to an SSH config entry.
125124
*/
126125
export function sshCommandToConfig(command: string, name?: string): { [key: string]: string } {
127-
const parts: string[] = parse(command) as string[];
126+
// Split the command line into arguments. We deliberately use shell-like tokenization that
127+
// strips single and double quotes and lets an unquoted backslash escape a following space,
128+
// while keeping backslashes before other characters literal, so both Unix paths with escaped
129+
// spaces (e.g. /home/me/my\ key) and Windows paths (e.g. C:\Users\me\key) are preserved.
130+
const parts: string[] = splitArgs(command);
128131

129132
// ignore 'ssh' if the user entered that as their first word
130133
if (parts[0] === 'ssh') {
@@ -167,6 +170,70 @@ export function sshCommandToConfig(command: string, name?: string): { [key: stri
167170
return { Host, HostName, ...options };
168171
}
169172

173+
/**
174+
* Splits a command line into arguments using shell-like tokenization that behaves
175+
* consistently across platforms. Both single and double quotes group their contents
176+
* and are removed, and unquoted whitespace separates arguments.
177+
*
178+
* Outside of quotes, a backslash escapes only a following whitespace character (so a
179+
* Unix path such as `/home/me/my\ key` keeps its space as a single argument). Before
180+
* any other character a backslash is kept literal, so Windows paths such as
181+
* `C:\Users\me\key` are preserved rather than being consumed as escape sequences.
182+
*
183+
* This tokenizer is intentionally lenient for a single-line input box: an unterminated
184+
* quote is not treated as an error but simply runs to the end of the string.
185+
*/
186+
export function splitArgs(command: string): string[] {
187+
const args: string[] = [];
188+
let current: string = '';
189+
let inToken: boolean = false;
190+
let quoteChar: string | undefined;
191+
for (let i: number = 0; i < command.length; i++) {
192+
const c: string = command[i];
193+
if (quoteChar !== undefined) {
194+
if (c === quoteChar) {
195+
quoteChar = undefined;
196+
} else {
197+
current += c;
198+
}
199+
continue;
200+
}
201+
if (c === '"' || c === '\'') {
202+
quoteChar = c;
203+
inToken = true;
204+
continue;
205+
}
206+
if (c === '\\') {
207+
const next: string | undefined = command[i + 1];
208+
// Only escape a following whitespace character; otherwise keep the backslash
209+
// literal so Windows path separators survive.
210+
if (next === ' ' || next === '\t' || next === '\r' || next === '\n') {
211+
current += next;
212+
inToken = true;
213+
i++;
214+
continue;
215+
}
216+
current += c;
217+
inToken = true;
218+
continue;
219+
}
220+
if (c === ' ' || c === '\t' || c === '\r' || c === '\n') {
221+
if (inToken) {
222+
args.push(current);
223+
current = '';
224+
inToken = false;
225+
}
226+
continue;
227+
}
228+
current += c;
229+
inToken = true;
230+
}
231+
if (inToken) {
232+
args.push(current);
233+
}
234+
return args;
235+
}
236+
170237
/**
171238
* Parses flags from the given array of arguments, returning the index of the
172239
* next non-flag in the input (or the total length of the input if none are found).
@@ -218,8 +285,8 @@ function parseFlags(input: string[], entries: { [key: string]: string }): number
218285
* are not mentioned on the ssh(1) man page and don't seem to have use in the
219286
* wild. In the OpenSSH source, they appear to be ignored[3].
220287
*
221-
* The `shell-quote` library, like libc does for OpenSSH, takes care of dealing
222-
* with quotations for for us.
288+
* The `splitArgs` tokenizer has already stripped any surrounding quotes before this
289+
* function sees a token, so it only has to deal with the unquoted connection string.
223290
*
224291
* 1. https://github.com/openssh/openssh-portable/blob/e3b6c966b79c3ea5d51b923c3bbdc41e13b96ea0/ssh.c#L999
225292
* 2. https://tools.ietf.org/html/draft-ietf-secsh-scp-sftp-ssh-uri-04#section-3.3
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/* --------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All Rights Reserved.
3+
* See 'LICENSE' in the project root for license information.
4+
* ------------------------------------------------------------------------------------------ */
5+
6+
import { deepStrictEqual, strictEqual } from 'assert';
7+
import { describe, it } from 'mocha';
8+
import { splitArgs, sshCommandToConfig } from '../../src/SSH/sshCommandToConfig';
9+
10+
// eslint-disable-next-line import/no-unassigned-import
11+
require('source-map-support/register');
12+
13+
describe('splitArgs', () => {
14+
// [description, input, expected tokens]
15+
const cases: [string, string, string[]][] = [
16+
['empty string', '', []],
17+
['whitespace only', ' \t ', []],
18+
['simple words', 'ssh user@host', ['ssh', 'user@host']],
19+
['collapses runs of whitespace', 'ssh \t user@host', ['ssh', 'user@host']],
20+
['trims leading/trailing whitespace', ' ssh user@host ', ['ssh', 'user@host']],
21+
22+
// Windows paths: backslashes must stay literal (the original bug).
23+
['bare Windows path', 'ssh -i C:\\Users\\me\\key user@host', ['ssh', '-i', 'C:\\Users\\me\\key', 'user@host']],
24+
['double-quoted Windows path with spaces', 'ssh -i "C:\\Program Files\\me\\key" user@host', ['ssh', '-i', 'C:\\Program Files\\me\\key', 'user@host']],
25+
['single-quoted Windows path with spaces', "ssh -i 'C:\\Program Files\\me\\key' user@host", ['ssh', '-i', 'C:\\Program Files\\me\\key', 'user@host']],
26+
['single-quoted Windows path without spaces', "ssh -i 'C:\\Users\\me\\key' user@host", ['ssh', '-i', 'C:\\Users\\me\\key', 'user@host']],
27+
28+
// Quote handling.
29+
['strips double quotes', '"a b" c', ['a b', 'c']],
30+
['strips single quotes', "'a b' c", ['a b', 'c']],
31+
['quotes joined to adjacent text', 'a"b c"d', ['ab cd']],
32+
['single quotes inside double quotes are literal', '"it\'s here"', ["it's here"]],
33+
['double quotes inside single quotes are literal', "'say \"hi\"'", ['say "hi"']],
34+
['empty double-quoted token is preserved', 'a "" b', ['a', '', 'b']],
35+
['empty single-quoted token is preserved', "a '' b", ['a', '', 'b']],
36+
37+
// Forward-slash (POSIX-style) paths are unaffected.
38+
['forward-slash path', 'ssh -i /home/me/.ssh/id_rsa user@host', ['ssh', '-i', '/home/me/.ssh/id_rsa', 'user@host']],
39+
40+
// An unquoted backslash escapes a following whitespace character (POSIX behavior), so a
41+
// Unix path with an escaped space stays a single argument.
42+
['backslash escapes a space', 'ssh -i /home/me/my\\ key user@host', ['ssh', '-i', '/home/me/my key', 'user@host']],
43+
['backslash escapes multiple spaces', 'ssh -i /home/me/key\\ with\\ spaces user@host', ['ssh', '-i', '/home/me/key with spaces', 'user@host']],
44+
['backslash escapes a tab', 'a\\\tb', ['a\tb']],
45+
['trailing backslash is literal', 'foo\\', ['foo\\']],
46+
['backslash before a letter stays literal (Windows path)', 'C:\\Users\\me', ['C:\\Users\\me']],
47+
['UNC path keeps doubled backslashes', '\\\\server\\share', ['\\\\server\\share']],
48+
// A backslash that ends a quoted segment is literal and must not escape the following
49+
// separator (only an unquoted backslash directly before whitespace escapes).
50+
['quoted path ending in backslash is not joined to the next arg', '"C:\\Program Files\\" next', ['C:\\Program Files\\', 'next']],
51+
52+
// Lenient handling of an unterminated quote: runs to end of string.
53+
['unterminated double quote runs to end', 'ssh -i "C:\\Users\\me', ['ssh', '-i', 'C:\\Users\\me']],
54+
['unterminated single quote runs to end', "ssh -i 'C:\\Users\\me", ['ssh', '-i', 'C:\\Users\\me']]
55+
];
56+
57+
for (const [description, input, expected] of cases) {
58+
it(`${description}: ${JSON.stringify(input)}`, () => {
59+
deepStrictEqual(splitArgs(input), expected);
60+
});
61+
}
62+
});
63+
64+
describe('sshCommandToConfig', () => {
65+
it('preserves a bare Windows identity-file path', () => {
66+
const config = sshCommandToConfig('ssh -i C:\\Users\\me\\.ssh\\id_rsa user@host');
67+
strictEqual(config.IdentityFile, 'C:\\Users\\me\\.ssh\\id_rsa');
68+
strictEqual(config.HostName, 'host');
69+
strictEqual(config.User, 'user');
70+
});
71+
72+
it('preserves a single-quoted Windows identity-file path with spaces', () => {
73+
const config = sshCommandToConfig("ssh -i 'C:\\Program Files\\me\\key' user@host");
74+
strictEqual(config.IdentityFile, 'C:\\Program Files\\me\\key');
75+
});
76+
77+
it('preserves a double-quoted Windows identity-file path with spaces', () => {
78+
const config = sshCommandToConfig('ssh -i "C:\\Program Files\\me\\key" user@host');
79+
strictEqual(config.IdentityFile, 'C:\\Program Files\\me\\key');
80+
});
81+
82+
it('preserves a Unix identity-file path with a backslash-escaped space', () => {
83+
const config = sshCommandToConfig('ssh -i /home/me/my\\ key user@host');
84+
strictEqual(config.IdentityFile, '/home/me/my key');
85+
strictEqual(config.HostName, 'host');
86+
strictEqual(config.User, 'user');
87+
});
88+
89+
it('parses host, user, and port from the connection string', () => {
90+
const config = sshCommandToConfig('ssh -p 2222 user@host');
91+
strictEqual(config.HostName, 'host');
92+
strictEqual(config.User, 'user');
93+
strictEqual(config.Port, '2222');
94+
});
95+
});

Extension/yarn.lock

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -896,11 +896,6 @@
896896
resolved "https://pkgs.dev.azure.com/azure-public/VisualCpp/_packaging/cpp_PublicPackages/npm/registry/@types/semver/-/semver-7.7.1.tgz#3ce3af1a5524ef327d2da9e4fd8b6d95c8d70528"
897897
integrity sha1-POOvGlUk7zJ9Lank/YttlcjXBSg=
898898

899-
"@types/shell-quote@^1.7.5":
900-
version "1.7.5"
901-
resolved "https://pkgs.dev.azure.com/azure-public/VisualCpp/_packaging/cpp_PublicPackages/npm/registry/@types/shell-quote/-/shell-quote-1.7.5.tgz#6db4704742d307cd6d604e124e3ad6cd5ed943f3"
902-
integrity sha1-bbRwR0LTB81tYE4STjrWzV7ZQ/M=
903-
904899
"@types/sinon@^21.0.0":
905900
version "21.0.0"
906901
resolved "https://pkgs.dev.azure.com/azure-public/VisualCpp/_packaging/cpp_PublicPackages/npm/registry/@types/sinon/-/sinon-21.0.0.tgz#3a598a29b3aec0512a21e57ae0fd4c09aa013ca9"
@@ -5508,11 +5503,6 @@ shebang-regex@^3.0.0:
55085503
resolved "https://pkgs.dev.azure.com/azure-public/VisualCpp/_packaging/cpp_PublicPackages/npm/registry/shebang-regex/-/shebang-regex-3.0.0.tgz#ae16f1644d873ecad843b0307b143362d4c42172"
55095504
integrity sha1-rhbxZE2HPsrYQ7AwexQzYtTEIXI=
55105505

5511-
shell-quote@1.8.4:
5512-
version "1.8.4"
5513-
resolved "https://pkgs.dev.azure.com/azure-public/VisualCpp/_packaging/cpp_PublicPackages/npm/registry/shell-quote/-/shell-quote-1.8.4.tgz#2edd9a4dcefc96649e2e2cb12f637b1f1d92a190"
5514-
integrity sha1-Lt2aTc78lmSeLiyxL2N7Hx2SoZA=
5515-
55165506
side-channel-list@^1.0.0:
55175507
version "1.0.0"
55185508
resolved "https://pkgs.dev.azure.com/azure-public/VisualCpp/_packaging/cpp_PublicPackages/npm/registry/side-channel-list/-/side-channel-list-1.0.0.tgz#10cb5984263115d3b7a0e336591e290a830af8ad"

0 commit comments

Comments
 (0)