Skip to content

Commit 1d536b3

Browse files
committed
Bug#35983164 NdbProcess treats space, backslash, double quote different on Windows and posix [#4]
Introduce quoting functions suitable for POSIX shell (sh) and running C/C++ programs on Windows via CMD.EXE. Use them when running a program via ssh. A simple heuristic to guess the kind of quoting needed on remote host is. If a \ appears in any argument use the quoting function for Windows. If / appears in any argument use the quoting function for POSIX. Change-Id: I851eb3da22d716d181319e825e888631cd16aeb7
1 parent c56eb1d commit 1d536b3

File tree

1 file changed

+156
-6
lines changed

1 file changed

+156
-6
lines changed

storage/ndb/include/portlib/NdbProcess.hpp

Lines changed: 156 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,14 @@ class NdbProcess {
168168
static std::optional<BaseString> quote_for_windows_crt(
169169
const char *str) noexcept;
170170

171+
static std::optional<BaseString> quote_for_windows_cmd_crt(
172+
const char *str) noexcept;
173+
174+
static std::optional<BaseString> quote_for_posix_sh(const char *str) noexcept;
175+
176+
static std::optional<BaseString> quote_for_unknown_shell(
177+
const char *str) noexcept;
178+
171179
static bool start_process(process_handle_t &pid, const char *path,
172180
const char *cwd, const Args &args, Pipes *pipes);
173181
};
@@ -258,16 +266,47 @@ std::unique_ptr<NdbProcess> NdbProcess::create_via_ssh(
258266
* And for Windows also what quoting the command itself requires on the
259267
* command line.
260268
*
261-
* Currently caller of this function need to provide proper quoting.
262-
*
263-
* This function do quote the arguments such that ssh sees them as is, as
264-
* done by start_process called via create.
269+
* As a rough heuristic the type of quoting needed on remote host we look at
270+
* command path and arguments. If backslash (\) is in any of them it is
271+
* assumed that ssh executes command via cmd.exe and that command is a C/C++
272+
* program. And if slash (/) is found it is assumed that ssh execute command
273+
* via Bourne shell, sh, or compatible on remote host. This is not perfect but
274+
* is a simple rule to document.
265275
*
266276
* On Windows it is assumed that ssh follows the quoting rules for Microsoft
267277
* C/C++ runtime.
268278
*/
269-
ssh_args.add(path.c_str());
270-
ssh_args.add(args);
279+
bool has_backslash = (strchr(path.c_str(), '\\'));
280+
bool has_slash = (strchr(path.c_str(), '/'));
281+
for (size_t i = 0; i < args.args().size(); i++) {
282+
if (strchr(args.args()[i].c_str(), '\\')) has_backslash = true;
283+
if (strchr(args.args()[i].c_str(), '/')) has_slash = true;
284+
}
285+
std::optional<BaseString> (*quote_func)(const char *str);
286+
if (has_backslash && !has_slash)
287+
quote_func = quote_for_windows_cmd_crt;
288+
else if (!has_backslash && has_slash)
289+
quote_func = quote_for_posix_sh;
290+
else
291+
quote_func = quote_for_unknown_shell;
292+
293+
auto qpath = quote_func(path.c_str());
294+
if (!qpath) {
295+
fprintf(stderr, "Function failed, could not quote command name: %s\n",
296+
path.c_str());
297+
return {};
298+
}
299+
ssh_args.add(qpath.value().c_str());
300+
for (size_t i = 0; i < args.args().size(); i++) {
301+
auto &arg = args.args()[i];
302+
auto qarg = quote_func(arg.c_str());
303+
if (!qarg) {
304+
fprintf(stderr, "Function failed, could not quote command argument: %s\n",
305+
arg.c_str());
306+
return {};
307+
}
308+
ssh_args.add(qarg.value().c_str());
309+
}
271310
return create(name, ssh_name, cwd, ssh_args, fds);
272311
}
273312

@@ -310,6 +349,117 @@ std::optional<BaseString> NdbProcess::quote_for_windows_crt(
310349
return ret;
311350
}
312351

352+
std::optional<BaseString> NdbProcess::quote_for_windows_cmd_crt(
353+
const char *str) noexcept {
354+
/*
355+
* Quoting of % is not handled and likely not possible when using double
356+
* quotes. If for %xxx% there is an environment variable named xxx defined,
357+
* that will be replaced in string by cmd.exe. If there is no such environment
358+
* variable the %xxx% will be intact as is.
359+
*
360+
* Since cmd.exe do not allow any quoting of a double quote within double
361+
* quotes we should make sure each argument have an even number of double
362+
* quotes, else cmd.exe may treat the last double quote from one argument as
363+
* beginning of a quotation ending at the first quote of next argument. That
364+
* can be accomplished using "" when quoting a single " within an argument.
365+
* But to make it more likely that a argument containing an even number of "
366+
* be quoted in the same way for windows as for posix shell the alternate
367+
* quoting method \" will be used in those cases. But only if none of ^ < > &
368+
* | appear between even and odd double quotes since cmd.exe will interpret
369+
* them specially.
370+
*/
371+
const char *p = str;
372+
bool dquote = true; // Assume quoted will started with "
373+
bool need_dquote = false;
374+
bool need_quote = (*p == '\0');
375+
while (!need_dquote && *p) {
376+
switch (*p) {
377+
case '^':
378+
case '<':
379+
case '>':
380+
case '|':
381+
case '&':
382+
if (!dquote)
383+
need_dquote = true;
384+
else
385+
need_quote = true;
386+
break;
387+
case '"':
388+
dquote = !dquote;
389+
need_quote = true;
390+
break;
391+
case ' ':
392+
case '*':
393+
case '?':
394+
need_dquote = true;
395+
break;
396+
default:
397+
if (!isprint(*p)) need_dquote = true;
398+
}
399+
p++;
400+
}
401+
if (!need_quote && !need_dquote) return str;
402+
// If argument had even number of double quotes, dquote should be true.
403+
if (!dquote) need_dquote = true;
404+
BaseString ret;
405+
ret.append('"');
406+
int backslashes = 0;
407+
while (*str) {
408+
switch (*str) {
409+
case '"':
410+
if (backslashes) ret.append(backslashes, '\\');
411+
backslashes = 0;
412+
if (need_dquote)
413+
ret.append('"');
414+
else
415+
ret.append('\\');
416+
break;
417+
case '\\':
418+
backslashes++;
419+
break;
420+
default:
421+
backslashes = 0;
422+
}
423+
ret.append(*str);
424+
str++;
425+
}
426+
if (backslashes) ret.append(backslashes, '\\');
427+
ret.append('"');
428+
return ret;
429+
}
430+
431+
std::optional<BaseString> NdbProcess::quote_for_posix_sh(
432+
const char *str) noexcept {
433+
const char *p = str;
434+
while (*p && !strchr("\t\n \"#$&'()*;<>?\\`|~", *p)) p++;
435+
if (*p == '\0' && p != str) return str;
436+
BaseString ret;
437+
ret.append('"');
438+
while (*str) {
439+
char ch = *str;
440+
switch (ch) {
441+
case '"':
442+
case '$':
443+
case '\\':
444+
case '`':
445+
ret.append('\\');
446+
break;
447+
}
448+
ret.append(ch);
449+
str++;
450+
}
451+
ret.append('"');
452+
return ret;
453+
}
454+
455+
std::optional<BaseString> NdbProcess::quote_for_unknown_shell(
456+
const char *str) noexcept {
457+
auto windows = quote_for_windows_cmd_crt(str);
458+
auto posix = quote_for_posix_sh(str);
459+
if (windows != posix) return {};
460+
return windows;
461+
}
462+
313463
#ifdef _WIN32
314464

315465
/******

0 commit comments

Comments
 (0)