Skip to content

Commit 02e8389

Browse files
fcharlieDHowett
authored andcommitted
Fix the conhost command line not being properly escaped (#1815)
This commit re-escapes the path to conhost's subprocess before it launches it.
1 parent 6d3001f commit 02e8389

File tree

2 files changed

+96
-3
lines changed

2 files changed

+96
-3
lines changed

src/host/ConsoleArguments.cpp

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,79 @@ const std::wstring_view ConsoleArguments::INHERIT_CURSOR_ARG = L"--inheritcursor
2121
const std::wstring_view ConsoleArguments::FEATURE_ARG = L"--feature";
2222
const std::wstring_view ConsoleArguments::FEATURE_PTY_ARG = L"pty";
2323

24+
std::wstring EscapeArgument(std::wstring_view ac)
25+
{
26+
if (ac.empty())
27+
{
28+
return L"\"\"";
29+
}
30+
bool hasSpace = false;
31+
auto n = ac.size();
32+
for (auto c : ac)
33+
{
34+
switch (c)
35+
{
36+
case L'"':
37+
case L'\\':
38+
n++;
39+
break;
40+
case ' ':
41+
case '\t':
42+
hasSpace = true;
43+
break;
44+
default:
45+
break;
46+
}
47+
}
48+
if (hasSpace)
49+
{
50+
n += 2;
51+
}
52+
if (n == ac.size())
53+
{
54+
return std::wstring{ ac };
55+
}
56+
std::wstring buf;
57+
if (hasSpace)
58+
{
59+
buf.push_back(L'"');
60+
}
61+
size_t slashes = 0;
62+
for (auto c : ac)
63+
{
64+
switch (c)
65+
{
66+
case L'\\':
67+
slashes++;
68+
buf.push_back(L'\\');
69+
break;
70+
case L'"':
71+
{
72+
for (; slashes > 0; slashes--)
73+
{
74+
buf.push_back(L'\\');
75+
}
76+
buf.push_back(L'\\');
77+
buf.push_back(c);
78+
}
79+
break;
80+
default:
81+
slashes = 0;
82+
buf.push_back(c);
83+
break;
84+
}
85+
}
86+
if (hasSpace)
87+
{
88+
for (; slashes > 0; slashes--)
89+
{
90+
buf.push_back(L'\\');
91+
}
92+
buf.push_back(L'"');
93+
}
94+
return buf;
95+
}
96+
2497
ConsoleArguments::ConsoleArguments(const std::wstring& commandline,
2598
const HANDLE hStdIn,
2699
const HANDLE hStdOut) :
@@ -272,7 +345,7 @@ void ConsoleArguments::s_ConsumeArg(_Inout_ std::vector<std::wstring>& args, _In
272345
size_t j = 0;
273346
for (j = index; j < args.size(); j++)
274347
{
275-
_clientCommandline += args[j];
348+
_clientCommandline += EscapeArgument(args[j]); // escape commandline
276349
if (j + 1 < args.size())
277350
{
278351
_clientCommandline += L" ";

src/host/ut_host/ConsoleArgumentsTests.cpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ void ConsoleArgumentsTests::ArgSplittingTests()
9090
INVALID_HANDLE_VALUE,
9191
INVALID_HANDLE_VALUE,
9292
ConsoleArguments(commandline,
93-
L"this is the commandline", // clientCommandLine
93+
L"\"this is the commandline\"", // clientCommandLine
9494
INVALID_HANDLE_VALUE,
9595
INVALID_HANDLE_VALUE,
9696
L"", // vtMode
@@ -110,7 +110,7 @@ void ConsoleArgumentsTests::ArgSplittingTests()
110110
INVALID_HANDLE_VALUE,
111111
INVALID_HANDLE_VALUE,
112112
ConsoleArguments(commandline,
113-
L"--vtmode bar this is the commandline", // clientCommandLine
113+
L"\"--vtmode bar this is the commandline\"", // clientCommandLine
114114
INVALID_HANDLE_VALUE,
115115
INVALID_HANDLE_VALUE,
116116
L"", // vtMode
@@ -223,6 +223,26 @@ void ConsoleArgumentsTests::ArgSplittingTests()
223223
0, // signalHandle
224224
false), // inheritCursor
225225
true); // successful parse?
226+
227+
commandline = L"conhost.exe this is the commandline";
228+
ArgTestsRunner(L"#9 commandline no quotes",
229+
commandline,
230+
INVALID_HANDLE_VALUE,
231+
INVALID_HANDLE_VALUE,
232+
ConsoleArguments(commandline,
233+
L"this is the commandline", // clientCommandLine
234+
INVALID_HANDLE_VALUE,
235+
INVALID_HANDLE_VALUE,
236+
L"", // vtMode
237+
0, // width
238+
0, // height
239+
false, // forceV1
240+
false, // headless
241+
true, // createServerHandle
242+
0, // serverHandle
243+
0, // signalHandle
244+
false), // inheritCursor
245+
true); // successful parse?
226246
}
227247

228248
void ConsoleArgumentsTests::ClientCommandlineTests()

0 commit comments

Comments
 (0)