[Process API] Quoting enhancements (#12946)

Author: takase1121

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.

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) {

src/process/windows/SDL_windowsprocess.c

@@ -106,18 +106,21 @@ 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) {
             case '"':
                 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;
     }
 

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]);
         }

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
 };