diff --git a/CMakeLists.txt b/CMakeLists.txt index 35913d3a..3cc9815a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -523,6 +523,9 @@ if(BUILD_TESTING) ) if(TARGET test_process) target_link_libraries(test_process ${PROJECT_NAME}) + target_compile_definitions(test_process PRIVATE + "CMAKE_COMMAND=${CMAKE_COMMAND}") + file(TOUCH "${CMAKE_CURRENT_BINARY_DIR}/file with space.txt") endif() ament_add_gtest(test_logging_custom_env test/test_logging_custom_env.cpp diff --git a/src/process.c b/src/process.c index 42e8f6ac..d62c3295 100644 --- a/src/process.c +++ b/src/process.c @@ -113,6 +113,115 @@ char * rcutils_get_executable_name(rcutils_allocator_t allocator) return executable_name; } +#if defined _WIN32 || defined __CYGWIN__ +static +rcutils_ret_t +append_backslashes(rcutils_char_array_t * char_array, size_t num_backslashes) +{ + if (num_backslashes <= 0) { + return RCUTILS_RET_OK; + } + + size_t current_strlen; + if (char_array->buffer_length == 0) { + current_strlen = 0; + } else { + // The buffer length always contains the trailing \0, so the strlen is one less than that. + current_strlen = char_array->buffer_length - 1; + } + + size_t new_length = current_strlen + num_backslashes + 1; + rcutils_ret_t ret = rcutils_char_array_expand_as_needed(char_array, new_length); + if (RCUTILS_RET_OK != ret) { + return ret; + } + + memset(char_array->buffer + current_strlen, '\\', num_backslashes); + char_array->buffer[new_length - 1] = '\0'; + + char_array->buffer_length = new_length; + + return RCUTILS_RET_OK; +} + +/** + * This algorithm is based on the MSDN blog "everyone quotes command line arguments the wrong way". + * See https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way + */ +static +rcutils_ret_t +build_command_line( + const rcutils_string_array_t * string_array, + rcutils_char_array_t * char_array) +{ + rcutils_ret_t ret; + + for (size_t i = 0; i < string_array->size; i++) { + // Append argument separator + if (i != 0) { + ret = rcutils_char_array_strncat(char_array, " ", 1); + if (RCUTILS_RET_OK != ret) { + return ret; + } + } + + // Append argument which doesn't need to be quoted + size_t arg_length = strlen(string_array->data[i]); + if (strcspn(string_array->data[i], " \t\n\v\"") >= arg_length) { + ret = rcutils_char_array_strncat(char_array, string_array->data[i], arg_length); + if (RCUTILS_RET_OK != ret) { + return ret; + } + continue; + } + + // Append opening quote + ret = rcutils_char_array_strncat(char_array, "\"", 1); + if (RCUTILS_RET_OK != ret) { + return ret; + } + + for (const char * c = string_array->data[i]; ; c++) { + size_t backslash_count = 0; + while ('\\' == *c) { + backslash_count++; + c++; + } + + if ('\0' == *c) { + ret = append_backslashes(char_array, backslash_count * 2); + if (RCUTILS_RET_OK != ret) { + return ret; + } + break; + } + + if ('"' == *c) { + backslash_count = backslash_count * 2 + 1; + } + + ret = append_backslashes(char_array, backslash_count); + if (RCUTILS_RET_OK != ret) { + return ret; + } + + ret = rcutils_char_array_strncat(char_array, c, 1); + if (RCUTILS_RET_OK != ret) { + return ret; + } + } + + // Append closing quote + ret = rcutils_char_array_strncat(char_array, "\"", 1); + if (RCUTILS_RET_OK != ret) { + return ret; + } + } + + return RCUTILS_RET_OK; +} +#endif + rcutils_process_t * rcutils_start_process( const rcutils_string_array_t * args, @@ -135,8 +244,18 @@ rcutils_start_process( process->allocator = *allocator; #if defined _WIN32 || defined __CYGWIN__ - LPSTR cmd = rcutils_join(args, " ", *allocator); - if (NULL == cmd) { + rcutils_char_array_t cmd = rcutils_get_zero_initialized_char_array(); + rcutils_ret_t ret = rcutils_char_array_init(&cmd, 0, allocator); + if (RCUTILS_RET_OK != ret) { + rcutils_process_close(process); + return NULL; + } + + ret = build_command_line(args, &cmd); + if (RCUTILS_RET_OK != ret) { + if (RCUTILS_RET_OK != rcutils_char_array_fini(&cmd)) { + RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to fini array.\n"); + } rcutils_process_close(process); return NULL; } @@ -144,16 +263,20 @@ rcutils_start_process( STARTUPINFO start_info = {sizeof(start_info)}; PROCESS_INFORMATION process_info = {0}; if (!CreateProcess( - NULL, cmd, NULL, NULL, TRUE, 0, + NULL, cmd.buffer, NULL, NULL, TRUE, 0, NULL, NULL, &start_info, &process_info)) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("Failed to create process: %lu", GetLastError()); - allocator->deallocate(cmd, &allocator->state); + if (RCUTILS_RET_OK != rcutils_char_array_fini(&cmd)) { + RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to fini array.\n"); + } rcutils_process_close(process); return NULL; } - allocator->deallocate(cmd, &allocator->state); + if (RCUTILS_RET_OK != rcutils_char_array_fini(&cmd)) { + RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to fini array.\n"); + } CloseHandle(process_info.hThread); diff --git a/test/test_process.cpp b/test/test_process.cpp index 909bdff2..8dccde3b 100644 --- a/test/test_process.cpp +++ b/test/test_process.cpp @@ -20,6 +20,8 @@ #include "rcutils/error_handling.h" #include "rcutils/process.h" +static const char * const g_cmake_command = RCUTILS_STRINGIFY(CMAKE_COMMAND); + TEST(TestProcess, test_get_pid) { EXPECT_NE(rcutils_get_pid(), 0); } @@ -52,9 +54,12 @@ TEST(TestProcess, test_process_creation) { rcutils_ret_t ret = RCUTILS_RET_OK; int exit_code = 42; - ret = rcutils_string_array_init(&args, 1, &allocator); + ret = rcutils_string_array_init(&args, 4, &allocator); ASSERT_EQ(RCUTILS_RET_OK, ret); - args.data[0] = strdup("whoami"); + args.data[0] = strdup(g_cmake_command); + args.data[1] = strdup("-E"); + args.data[2] = strdup("echo"); + args.data[3] = strdup(""); EXPECT_EQ(nullptr, rcutils_start_process(NULL, &allocator)); rcutils_reset_error(); @@ -78,6 +83,40 @@ TEST(TestProcess, test_process_creation) { rcutils_process_close(process); + // cmake -E cat "file with space.txt" (returns 0) + ret = rcutils_string_array_resize(&args, 4); + ASSERT_EQ(RCUTILS_RET_OK, ret); + args.data[0] = strdup(g_cmake_command); + args.data[1] = strdup("-E"); + args.data[2] = strdup("cat"); + args.data[3] = strdup("file with space.txt"); + + process = rcutils_start_process(&args, &allocator); + EXPECT_NE(nullptr, process); + + ret = rcutils_process_wait(process, &exit_code); + EXPECT_EQ(RCUTILS_RET_OK, ret); + EXPECT_EQ(0, exit_code); + + rcutils_process_close(process); + + // cmake -E false (returns 1) + ret = rcutils_string_array_resize(&args, 3); + ASSERT_EQ(RCUTILS_RET_OK, ret); + args.data[0] = strdup(g_cmake_command); + args.data[1] = strdup("-E"); + allocator.deallocate(args.data[2], &allocator.state); + args.data[2] = strdup("false"); + + process = rcutils_start_process(&args, &allocator); + EXPECT_NE(nullptr, process); + + ret = rcutils_process_wait(process, &exit_code); + EXPECT_EQ(RCUTILS_RET_OK, ret); + EXPECT_EQ(1, exit_code); + + rcutils_process_close(process); + ret = rcutils_string_array_fini(&args); ASSERT_EQ(RCUTILS_RET_OK, ret); }