From 3f7191e5c27a0e1852fe046a5ec0512a47e4a409 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 5 Feb 2024 11:04:05 -0500 Subject: [PATCH] win/spawn: optionally run executable paths with no file extension (#4292) Add a process options flag to enable the optional behavior. Most users are likely recommended to set this flag by default, but it was deemed potentially breaking to set it by default in libuv. Co-authored-by: Kyle Edwards --- .github/workflows/CI-win.yml | 2 +- CMakeLists.txt | 12 +++++++ Makefile.am | 6 ++++ docs/src/process.rst | 12 ++++++- include/uv.h | 9 +++++- src/unix/process.c | 1 + src/win/process.c | 14 ++++++--- test/test-list.h | 4 +++ test/test-spawn.c | 61 ++++++++++++++++++++++++++++++++++++ 9 files changed, 113 insertions(+), 8 deletions(-) diff --git a/.github/workflows/CI-win.yml b/.github/workflows/CI-win.yml index 7dc3fdcab..79a5abf4f 100644 --- a/.github/workflows/CI-win.yml +++ b/.github/workflows/CI-win.yml @@ -93,7 +93,7 @@ jobs: cmake --install build --prefix "`pwd`/build/usr" mkdir -p build/usr/test build/usr/bin cp -av test/fixtures build/usr/test - cp -av build/uv_run_tests_a.exe build/uv_run_tests.exe \ + cp -av build/uv_run_tests_a.exe build/uv_run_tests.exe build/uv_run_tests_a_no_ext build/uv_run_tests_no_ext \ `${{ matrix.config.arch }}-w64-mingw32-gcc -print-file-name=libgcc_s_${{ matrix.config.libgcc }}-1.dll` \ `${{ matrix.config.arch }}-w64-mingw32-gcc -print-file-name=libwinpthread-1.dll` \ `${{ matrix.config.arch }}-w64-mingw32-gcc -print-file-name=libatomic-1.dll` \ diff --git a/CMakeLists.txt b/CMakeLists.txt index 3914f0d68..5e8e0166d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -707,6 +707,12 @@ if(LIBUV_BUILD_TESTS) set_tests_properties(uv_test PROPERTIES ENVIRONMENT "LIBPATH=${CMAKE_BINARY_DIR}:$ENV{LIBPATH}") endif() + if(WIN32) + add_custom_command(TARGET uv_run_tests POST_BUILD + COMMAND "${CMAKE_COMMAND}" -E copy + "$" + "$/uv_run_tests_no_ext") + endif() add_executable(uv_run_tests_a ${uv_test_sources} uv_win_longpath.manifest) target_compile_definitions(uv_run_tests_a PRIVATE ${uv_defines}) target_compile_options(uv_run_tests_a PRIVATE ${uv_cflags}) @@ -723,6 +729,12 @@ if(LIBUV_BUILD_TESTS) set_target_properties(uv_run_tests PROPERTIES LINKER_LANGUAGE CXX) set_target_properties(uv_run_tests_a PROPERTIES LINKER_LANGUAGE CXX) endif() + if(WIN32) + add_custom_command(TARGET uv_run_tests_a POST_BUILD + COMMAND "${CMAKE_COMMAND}" -E copy + "$" + "$/uv_run_tests_a_no_ext") + endif() endif() # Now for some gibbering horrors from beyond the stars... diff --git a/Makefile.am b/Makefile.am index ff6f1b8a6..a14228da3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -136,6 +136,12 @@ TESTS = test/run-tests check_PROGRAMS = test/run-tests test_run_tests_CFLAGS = $(AM_CFLAGS) +if WINNT +check-am: test/run-tests_no_ext +test/run-tests_no_ext: test/run-tests$(EXEEXT) + cp test/run-tests$(EXEEXT) test/run-tests_no_ext +endif + if SUNOS # Can't be turned into a CC_CHECK_CFLAGS in configure.ac, it makes compilers # on other platforms complain that the argument is unused during compilation. diff --git a/docs/src/process.rst b/docs/src/process.rst index 8acf7db3d..8d2fdb3e4 100644 --- a/docs/src/process.rst +++ b/docs/src/process.rst @@ -85,7 +85,14 @@ Data types * option is only meaningful on Windows systems. On Unix it is silently * ignored. */ - UV_PROCESS_WINDOWS_HIDE_GUI = (1 << 6) + UV_PROCESS_WINDOWS_HIDE_GUI = (1 << 6), + /* + * On Windows, if the path to the program to execute, specified in + * uv_process_options_t's file field, has a directory component, + * search for the exact file name before trying variants with + * extensions like '.exe' or '.cmd'. + */ + UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME = (1 << 7) }; .. c:type:: uv_stdio_container_t @@ -262,6 +269,9 @@ API .. versionchanged:: 1.24.0 Added `UV_PROCESS_WINDOWS_HIDE_CONSOLE` and `UV_PROCESS_WINDOWS_HIDE_GUI` flags. + .. versionchanged:: 1.48.0 Added the + `UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME` flag. + .. c:function:: int uv_process_kill(uv_process_t* handle, int signum) Sends the specified signal to the given process handle. Check the documentation diff --git a/include/uv.h b/include/uv.h index b1e58e6cb..a62b3fa69 100644 --- a/include/uv.h +++ b/include/uv.h @@ -1106,7 +1106,14 @@ enum uv_process_flags { * option is only meaningful on Windows systems. On Unix it is silently * ignored. */ - UV_PROCESS_WINDOWS_HIDE_GUI = (1 << 6) + UV_PROCESS_WINDOWS_HIDE_GUI = (1 << 6), + /* + * On Windows, if the path to the program to execute, specified in + * uv_process_options_t's file field, has a directory component, + * search for the exact file name before trying variants with + * extensions like '.exe' or '.cmd'. + */ + UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME = (1 << 7) }; /* diff --git a/src/unix/process.c b/src/unix/process.c index dd58c18d9..4812a90f2 100644 --- a/src/unix/process.c +++ b/src/unix/process.c @@ -972,6 +972,7 @@ int uv_spawn(uv_loop_t* loop, assert(!(options->flags & ~(UV_PROCESS_DETACHED | UV_PROCESS_SETGID | UV_PROCESS_SETUID | + UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME | UV_PROCESS_WINDOWS_HIDE | UV_PROCESS_WINDOWS_HIDE_CONSOLE | UV_PROCESS_WINDOWS_HIDE_GUI | diff --git a/src/win/process.c b/src/win/process.c index 50161b14b..400722bd4 100644 --- a/src/win/process.c +++ b/src/win/process.c @@ -304,8 +304,9 @@ static WCHAR* path_search_walk_ext(const WCHAR *dir, * - If there's really only a filename, check the current directory for file, * then search all path directories. * - * - If filename specified has *any* extension, search for the file with the - * specified extension first. + * - If filename specified has *any* extension, or already contains a path + * and the UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME flag is specified, + * search for the file with the exact specified filename first. * * - If the literal filename is not found in a directory, try *appending* * (not replacing) .com first and then .exe. @@ -331,7 +332,8 @@ static WCHAR* path_search_walk_ext(const WCHAR *dir, */ static WCHAR* search_path(const WCHAR *file, WCHAR *cwd, - const WCHAR *path) { + const WCHAR *path, + unsigned int flags) { int file_has_dir; WCHAR* result = NULL; WCHAR *file_name_start; @@ -372,7 +374,7 @@ static WCHAR* search_path(const WCHAR *file, file, file_name_start - file, file_name_start, file_len - (file_name_start - file), cwd, cwd_len, - name_has_ext); + name_has_ext || (flags & UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME)); } else { dir_end = path; @@ -935,6 +937,7 @@ int uv_spawn(uv_loop_t* loop, assert(!(options->flags & ~(UV_PROCESS_DETACHED | UV_PROCESS_SETGID | UV_PROCESS_SETUID | + UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME | UV_PROCESS_WINDOWS_HIDE | UV_PROCESS_WINDOWS_HIDE_CONSOLE | UV_PROCESS_WINDOWS_HIDE_GUI | @@ -1014,7 +1017,8 @@ int uv_spawn(uv_loop_t* loop, application_path = search_path(application, cwd, - path); + path, + options->flags); if (application_path == NULL) { /* Not found. */ err = ERROR_FILE_NOT_FOUND; diff --git a/test/test-list.h b/test/test-list.h index e97b941f6..d30f02faa 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -507,6 +507,8 @@ TEST_DECLARE (listen_no_simultaneous_accepts) TEST_DECLARE (fs_stat_root) TEST_DECLARE (spawn_with_an_odd_path) TEST_DECLARE (spawn_no_path) +TEST_DECLARE (spawn_no_ext) +TEST_DECLARE (spawn_path_no_ext) TEST_DECLARE (ipc_listen_after_bind_twice) TEST_DECLARE (win32_signum_number) #else @@ -1027,6 +1029,8 @@ TASK_LIST_START TEST_ENTRY (fs_stat_root) TEST_ENTRY (spawn_with_an_odd_path) TEST_ENTRY (spawn_no_path) + TEST_ENTRY (spawn_no_ext) + TEST_ENTRY (spawn_path_no_ext) TEST_ENTRY (ipc_listen_after_bind_twice) TEST_ENTRY (win32_signum_number) #else diff --git a/test/test-spawn.c b/test/test-spawn.c index 33552717f..6a8487470 100644 --- a/test/test-spawn.c +++ b/test/test-spawn.c @@ -1397,6 +1397,67 @@ TEST_IMPL(spawn_no_path) { MAKE_VALGRIND_HAPPY(uv_default_loop()); return 0; } + + +TEST_IMPL(spawn_no_ext) { + char new_exepath[1024]; + + init_process_options("spawn_helper1", exit_cb); + options.flags |= UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME; + snprintf(new_exepath, sizeof(new_exepath), "%.*s_no_ext", + (int) (exepath_size - sizeof(".exe") + 1), + exepath); + options.file = options.args[0] = new_exepath; + + ASSERT_OK(uv_spawn(uv_default_loop(), &process, &options)); + ASSERT_OK(uv_run(uv_default_loop(), UV_RUN_DEFAULT)); + + ASSERT_EQ(1, exit_cb_called); + ASSERT_EQ(1, close_cb_called); + + MAKE_VALGRIND_HAPPY(uv_default_loop()); + return 0; +} + + +TEST_IMPL(spawn_path_no_ext) { + int r; + int len; + int file_len; + char file[64]; + char path[1024]; + char* env[2]; + + /* Set up the process, but make sure that the file to run is relative and + * requires a lookup into PATH. */ + init_process_options("spawn_helper1", exit_cb); + options.flags |= UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME; + + /* Set up the PATH env variable */ + for (len = strlen(exepath), file_len = 0; + exepath[len - 1] != '/' && exepath[len - 1] != '\\'; + len--, file_len++); + snprintf(file, sizeof(file), "%.*s_no_ext", + (int) (file_len - sizeof(".exe") + 1), + exepath + len); + exepath[len] = 0; + snprintf(path, sizeof(path), "PATH=%s", exepath); + + env[0] = path; + env[1] = NULL; + + options.file = options.args[0] = file; + options.env = env; + + r = uv_spawn(uv_default_loop(), &process, &options); + ASSERT(r == UV_ENOENT || r == UV_EACCES); + ASSERT_OK(uv_is_active((uv_handle_t*) &process)); + uv_close((uv_handle_t*) &process, NULL); + ASSERT_OK(uv_run(uv_default_loop(), UV_RUN_DEFAULT)); + + MAKE_VALGRIND_HAPPY(uv_default_loop()); + return 0; +} #endif #ifndef _WIN32