From f6c1e81394d9bd97a4b91cc041d3a08518907bce Mon Sep 17 00:00:00 2001
From: Takase <[EMAIL REDACTED]>
Date: Tue, 13 May 2025 00:17:21 +0800
Subject: [PATCH] [Process API] Quoting enhancements (#12946)
---
include/SDL3/SDL_process.h | 7 +
src/process/SDL_process.c | 8 ++
src/process/windows/SDL_windowsprocess.c | 30 ++--
test/childprocess.c | 10 ++
test/testprocess.c | 171 ++++++++++++++++++++++-
5 files changed, 217 insertions(+), 9 deletions(-)
diff --git a/include/SDL3/SDL_process.h b/include/SDL3/SDL_process.h
index 0e19bfff1dc9a..b6aac6a931708 100644
--- a/include/SDL3/SDL_process.h
+++ b/include/SDL3/SDL_process.h
@@ -195,6 +195,12 @@ typedef enum SDL_ProcessIO
* run in the background. In this case the default input and output is
* `SDL_PROCESS_STDIO_NULL` and the exitcode of the process is not
* available, and will always be 0.
+ * - `SDL_PROP_PROCESS_CREATE_CMDLINE_STRING`: a string containing the program
+ * to run and any parameters. This string is passed directly to
+ * `CreateProcess` on Windows, and does nothing on other platforms.
+ * This property is only important if you want to start programs that does
+ * non-standard command-line processing, and in most cases using
+ * `SDL_PROP_PROCESS_CREATE_ARGS_POINTER` is sufficient.
*
* On POSIX platforms, wait() and waitpid(-1, ...) should not be called, and
* SIGCHLD should not be ignored or handled because those would prevent SDL
@@ -231,6 +237,7 @@ extern SDL_DECLSPEC SDL_Process * SDLCALL SDL_CreateProcessWithProperties(SDL_Pr
#define SDL_PROP_PROCESS_CREATE_STDERR_POINTER "SDL.process.create.stderr_source"
#define SDL_PROP_PROCESS_CREATE_STDERR_TO_STDOUT_BOOLEAN "SDL.process.create.stderr_to_stdout"
#define SDL_PROP_PROCESS_CREATE_BACKGROUND_BOOLEAN "SDL.process.create.background"
+#define SDL_PROP_PROCESS_CREATE_CMDLINE_STRING "SDL.process.create.cmdline"
/**
* Get the properties associated with a process.
diff --git a/src/process/SDL_process.c b/src/process/SDL_process.c
index 3ccf5d582b185..5db6db49b44ef 100644
--- a/src/process/SDL_process.c
+++ b/src/process/SDL_process.c
@@ -45,10 +45,18 @@ SDL_Process *SDL_CreateProcess(const char * const *args, bool pipe_stdio)
SDL_Process *SDL_CreateProcessWithProperties(SDL_PropertiesID props)
{
const char * const *args = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, NULL);
+#if defined(SDL_PLATFORM_WINDOWS)
+ const char *cmdline = SDL_GetStringProperty(props, SDL_PROP_PROCESS_CREATE_CMDLINE_STRING, NULL);
+ if ((!args || !args[0] || !args[0][0]) && (!cmdline || !cmdline[0])) {
+ SDL_SetError("Either SDL_PROP_PROCESS_CREATE_ARGS_POINTER or SDL_PROP_PROCESS_CREATE_CMDLINE_STRING must be valid");
+ return NULL;
+ }
+#else
if (!args || !args[0] || !args[0][0]) {
SDL_InvalidParamError("SDL_PROP_PROCESS_CREATE_ARGS_POINTER");
return NULL;
}
+#endif
SDL_Process *process = (SDL_Process *)SDL_calloc(1, sizeof(*process));
if (!process) {
diff --git a/src/process/windows/SDL_windowsprocess.c b/src/process/windows/SDL_windowsprocess.c
index 3e0249ebd8140..65d8a394a0fab 100644
--- a/src/process/windows/SDL_windowsprocess.c
+++ b/src/process/windows/SDL_windowsprocess.c
@@ -106,9 +106,12 @@ static bool join_arguments(const char * const *args, LPWSTR *args_out)
len = 0;
for (i = 0; args[i]; i++) {
const char *a = args[i];
+ bool quotes = *a == '\0' || SDL_strpbrk(a, " \r\n\t\v") != NULL;
- /* two double quotes to surround an argument with */
- len += 2;
+ if (quotes) {
+ /* surround the argument with double quote if it is empty or contains whitespaces */
+ len += 2;
+ }
for (; *a; a++) {
switch (*a) {
@@ -116,8 +119,8 @@ static bool join_arguments(const char * const *args, LPWSTR *args_out)
len += 2;
break;
case '\\':
- /* only escape backslashes that precede a double quote */
- len += (a[1] == '"' || a[1] == '\0') ? 2 : 1;
+ /* only escape backslashes that precede a double quote (including the enclosing double quote) */
+ len += (a[1] == '"' || (quotes && a[1] == '\0')) ? 2 : 1;
break;
case ' ':
case '^':
@@ -149,8 +152,11 @@ static bool join_arguments(const char * const *args, LPWSTR *args_out)
i_out = 0;
for (i = 0; args[i]; i++) {
const char *a = args[i];
+ bool quotes = *a == '\0' || SDL_strpbrk(a, " \r\n\t\v") != NULL;
- result[i_out++] = '"';
+ if (quotes) {
+ result[i_out++] = '"';
+ }
for (; *a; a++) {
switch (*a) {
case '"':
@@ -163,7 +169,7 @@ static bool join_arguments(const char * const *args, LPWSTR *args_out)
break;
case '\\':
result[i_out++] = *a;
- if (a[1] == '"' || a[1] == '\0') {
+ if (a[1] == '"' || (quotes && a[1] == '\0')) {
result[i_out++] = *a;
}
break;
@@ -188,7 +194,9 @@ static bool join_arguments(const char * const *args, LPWSTR *args_out)
break;
}
}
- result[i_out++] = '"';
+ if (quotes) {
+ result[i_out++] = '"';
+ }
result[i_out++] = ' ';
}
SDL_assert(i_out == len);
@@ -237,6 +245,7 @@ static bool join_env(char **env, LPWSTR *env_out)
bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID props)
{
const char * const *args = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, NULL);
+ const char *cmdline = SDL_GetStringProperty(props, SDL_PROP_PROCESS_CREATE_CMDLINE_STRING, NULL);
SDL_Environment *env = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, SDL_GetEnvironment());
char **envp = NULL;
const char *working_directory = SDL_GetStringProperty(props, SDL_PROP_PROCESS_CREATE_WORKING_DIRECTORY_STRING, NULL);
@@ -286,7 +295,12 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
security_attributes.bInheritHandle = TRUE;
security_attributes.lpSecurityDescriptor = NULL;
- if (!join_arguments(args, &createprocess_cmdline)) {
+ if (cmdline) {
+ createprocess_cmdline = WIN_UTF8ToString(cmdline);
+ if (!createprocess_cmdline) {
+ goto done;
+ }
+ } else if (!join_arguments(args, &createprocess_cmdline)) {
goto done;
}
diff --git a/test/childprocess.c b/test/childprocess.c
index 772d86d760962..69a7cea2da3d9 100644
--- a/test/childprocess.c
+++ b/test/childprocess.c
@@ -2,6 +2,11 @@
#include <SDL3/SDL_main.h>
#include <SDL3/SDL_test.h>
+#ifdef SDL_PLATFORM_WINDOWS
+#include <io.h>
+#include <fcntl.h>
+#endif
+
#include <stdio.h>
#include <errno.h>
@@ -102,6 +107,11 @@ int main(int argc, char *argv[]) {
if (print_arguments) {
int print_i;
+#ifdef SDL_PLATFORM_WINDOWS
+ /* reopen stdout as binary to prevent newline conversion */
+ _setmode(_fileno(stdout), _O_BINARY);
+#endif
+
for (print_i = 0; i + print_i < argc; print_i++) {
fprintf(stdout, "|%d=%s|\r\n", print_i, argv[i + print_i]);
}
diff --git a/test/testprocess.c b/test/testprocess.c
index 425b445151835..4d6437d397727 100644
--- a/test/testprocess.c
+++ b/test/testprocess.c
@@ -82,7 +82,7 @@ static int SDLCALL process_testArguments(void *arg)
"",
" ",
"a b c",
- "a\tb\tc\t",
+ "a\tb\tc\t\v\r\n",
"\"a b\" c",
"'a' 'b' 'c'",
"%d%%%s",
@@ -965,6 +965,165 @@ static int process_testFileRedirection(void *arg)
return TEST_COMPLETED;
}
+static int process_testWindowsCmdline(void *arg)
+{
+ TestProcessData *data = (TestProcessData *)arg;
+ const char *process_args[] = {
+ data->childprocess_path,
+ "--print-arguments",
+ "--",
+ "",
+ " ",
+ "a b c",
+ "a\tb\tc\t",
+ "\"a b\" c",
+ "'a' 'b' 'c'",
+ "%d%%%s",
+ "\\t\\c",
+ "evil\\",
+ "a\\b\"c\\",
+ "\"\\^&|<>%", /* characters with a special meaning */
+ NULL
+ };
+ /* this will have the same result as process_args, but escaped in a different way */
+ const char *process_cmdline_template =
+ "%s "
+ "--print-arguments "
+ "-- "
+ "\"\" "
+ "\" \" "
+ "a\" \"b\" \"c\t" /* using tab as delimiter */
+ "\"a\tb\tc\t\" "
+ "\"\"\"\"a b\"\"\" c\" "
+ "\"'a' 'b' 'c'\" "
+ "%%d%%%%%%s " /* will be passed to sprintf */
+ "\\t\\c "
+ "evil\\ "
+ "a\\b\"\\\"\"c\\ "
+ "\\\"\\^&|<>%%";
+ char process_cmdline[65535];
+ SDL_PropertiesID props;
+ SDL_Process *process = NULL;
+ char *buffer;
+ int exit_code;
+ int i;
+ size_t total_read = 0;
+
+#ifndef SDL_PLATFORM_WINDOWS
+ SDLTest_AssertPass("SDL_PROP_PROCESS_CREATE_CMDLINE_STRING only works on Windows");
+ return TEST_SKIPPED;
+#endif
+
+ props = SDL_CreateProperties();
+ SDLTest_AssertCheck(props != 0, "SDL_CreateProperties()");
+ if (!props) {
+ goto failed;
+ }
+ SDL_SetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDIN_NUMBER, SDL_PROCESS_STDIO_APP);
+ SDL_SetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDOUT_NUMBER, SDL_PROCESS_STDIO_APP);
+ SDL_SetBooleanProperty(props, SDL_PROP_PROCESS_CREATE_STDERR_TO_STDOUT_BOOLEAN, true);
+
+ process = SDL_CreateProcessWithProperties(props);
+ SDLTest_AssertCheck(process == NULL, "SDL_CreateProcessWithProperties() should fail");
+
+ SDL_snprintf(process_cmdline, SDL_arraysize(process_cmdline), process_cmdline_template, data->childprocess_path);
+ SDL_SetStringProperty(props, SDL_PROP_PROCESS_CREATE_CMDLINE_STRING, process_cmdline);
+
+ process = SDL_CreateProcessWithProperties(props);
+ SDLTest_AssertCheck(process != NULL, "SDL_CreateProcessWithProperties()");
+ if (!process) {
+ goto failed;
+ }
+
+ exit_code = 0xdeadbeef;
+ buffer = (char *)SDL_ReadProcess(process, &total_read, &exit_code);
+ SDLTest_AssertCheck(buffer != NULL, "SDL_ReadProcess()");
+ SDLTest_AssertCheck(exit_code == 0, "Exit code should be 0, is %d", exit_code);
+ if (!buffer) {
+ goto failed;
+ }
+ SDLTest_LogEscapedString("stdout of process: ", buffer, total_read);
+
+ for (i = 3; process_args[i]; i++) {
+ char line[64];
+ SDL_snprintf(line, sizeof(line), "|%d=%s|", i - 3, process_args[i]);
+ SDLTest_AssertCheck(!!SDL_strstr(buffer, line), "Check %s is in output", line);
+ }
+ SDL_free(buffer);
+
+ SDLTest_AssertPass("About to destroy process");
+ SDL_DestroyProcess(process);
+
+ return TEST_COMPLETED;
+
+failed:
+ SDL_DestroyProcess(process);
+ return TEST_ABORTED;
+}
+
+static int process_testWindowsCmdlinePrecedence(void *arg)
+{
+ TestProcessData *data = (TestProcessData *)arg;
+ const char *process_args[] = {
+ data->childprocess_path,
+ "--print-arguments",
+ "--",
+ "argument 1",
+ NULL
+ };
+ const char *process_cmdline_template = "%s --print-arguments -- \"argument 2\"";
+ char process_cmdline[65535];
+ SDL_PropertiesID props;
+ SDL_Process *process = NULL;
+ char *buffer;
+ int exit_code;
+ size_t total_read = 0;
+
+#ifndef SDL_PLATFORM_WINDOWS
+ SDLTest_AssertPass("SDL_PROP_PROCESS_CREATE_CMDLINE_STRING only works on Windows");
+ return TEST_SKIPPED;
+#endif
+
+ props = SDL_CreateProperties();
+ SDLTest_AssertCheck(props != 0, "SDL_CreateProperties()");
+ if (!props) {
+ goto failed;
+ }
+
+ SDL_snprintf(process_cmdline, SDL_arraysize(process_cmdline), process_cmdline_template, data->childprocess_path);
+ SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, (void *)process_args);
+ SDL_SetStringProperty(props, SDL_PROP_PROCESS_CREATE_CMDLINE_STRING, (const char *)process_cmdline);
+ SDL_SetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDIN_NUMBER, SDL_PROCESS_STDIO_APP);
+ SDL_SetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDOUT_NUMBER, SDL_PROCESS_STDIO_APP);
+ SDL_SetBooleanProperty(props, SDL_PROP_PROCESS_CREATE_STDERR_TO_STDOUT_BOOLEAN, true);
+
+ process = SDL_CreateProcessWithProperties(props);
+ SDLTest_AssertCheck(process != NULL, "SDL_CreateProcessWithProperties()");
+ if (!process) {
+ goto failed;
+ }
+
+ exit_code = 0xdeadbeef;
+ buffer = (char *)SDL_ReadProcess(process, &total_read, &exit_code);
+ SDLTest_AssertCheck(buffer != NULL, "SDL_ReadProcess()");
+ SDLTest_AssertCheck(exit_code == 0, "Exit code should be 0, is %d", exit_code);
+ if (!buffer) {
+ goto failed;
+ }
+ SDLTest_LogEscapedString("stdout of process: ", buffer, total_read);
+ SDLTest_AssertCheck(!!SDL_strstr(buffer, "|0=argument 2|"), "Check |0=argument 2| is printed");
+ SDL_free(buffer);
+
+ SDLTest_AssertPass("About to destroy process");
+ SDL_DestroyProcess(process);
+
+ return TEST_COMPLETED;
+
+failed:
+ SDL_DestroyProcess(process);
+ return TEST_ABORTED;
+}
+
static const SDLTest_TestCaseReference processTestArguments = {
process_testArguments, "process_testArguments", "Test passing arguments to child process", TEST_ENABLED
};
@@ -1017,6 +1176,14 @@ static const SDLTest_TestCaseReference processTestFileRedirection = {
process_testFileRedirection, "process_testFileRedirection", "Test redirection from/to files", TEST_ENABLED
};
+static const SDLTest_TestCaseReference processTestWindowsCmdline = {
+ process_testWindowsCmdline, "process_testWindowsCmdline", "Test passing cmdline directly to CreateProcess", TEST_ENABLED
+};
+
+static const SDLTest_TestCaseReference processTestWindowsCmdlinePrecedence = {
+ process_testWindowsCmdlinePrecedence, "process_testWindowsCmdlinePrecedence", "Test SDL_PROP_PROCESS_CREATE_CMDLINE_STRING precedence over SDL_PROP_PROCESS_CREATE_ARGS_POINTER", TEST_ENABLED
+};
+
static const SDLTest_TestCaseReference *processTests[] = {
&processTestArguments,
&processTestExitCode,
@@ -1031,6 +1198,8 @@ static const SDLTest_TestCaseReference *processTests[] = {
&processTestNonExistingExecutable,
&processTestBatBadButVulnerability,
&processTestFileRedirection,
+ &processTestWindowsCmdline,
+ &processTestWindowsCmdlinePrecedence,
NULL
};