win/spawn: optionally run executable paths with no file extension (li…

…buv#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 <[email protected]>
bradking · Feb 5, 2024 · 3f7191e · 3f7191e
1 parent 535efdf
commit 3f7191e
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 8 deletions.
diff --git 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
@@ -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
+                       "$<TARGET_FILE:uv_run_tests>"
+                       "$<TARGET_FILE_DIR:uv_run_tests>/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
+                       "$<TARGET_FILE:uv_run_tests_a>"
+                       "$<TARGET_FILE_DIR:uv_run_tests_a>/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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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