Fix launching executables whose path can't be shortened on Windows

src/main/native/windows/process.cc

@@ -77,7 +77,9 @@ bool WaitableProcess::Create(const std::wstring& argv0,
   }
 
   std::wstring argv0short;
-  error_msg = AsExecutablePathForCreateProcess(argv0, &argv0short);
+  std::wstring extended_path;
+  error_msg =
+      AsExecutablePathForCreateProcess(argv0, &argv0short, &extended_path);
   if (!error_msg.empty()) {
     *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
                               L"WaitableProcess::Create", argv0, error_msg);
@@ -159,7 +161,8 @@ bool WaitableProcess::Create(const std::wstring& argv0,
   STARTUPINFOEXW info;
   attr_list->InitStartupInfoExW(&info);
   if (!CreateProcessW(
-          /* lpApplicationName */ nullptr,
+          /* lpApplicationName */ extended_path.empty() ? nullptr
+                                                        : extended_path.c_str(),
           /* lpCommandLine */ mutable_commandline.get(),
           /* lpProcessAttributes */ nullptr,
           /* lpThreadAttributes */ nullptr,

src/main/native/windows/util.cc

@@ -194,6 +194,15 @@ static bool Contains(const wstring& s, const WCHAR* substr) {
   return s.find(substr) != wstring::npos;
 }
 
+static bool IsBatchFile(const wstring& path) {
+  if (path.size() < 4) {
+    return false;
+  }
+  const wchar_t* ext = path.c_str() + (path.size() - 4);
+  return CompareStringOrdinal(ext, 4, L".bat", 4, TRUE) == CSTR_EQUAL ||
+         CompareStringOrdinal(ext, 4, L".cmd", 4, TRUE) == CSTR_EQUAL;
+}
+
 wstring AsShortPath(wstring path, wstring* result) {
   // Using `MAX_PATH` - 4 (256) instead of `MAX_PATH` to fix
   // https://github.com/bazelbuild/bazel/issues/12310
@@ -267,7 +276,8 @@ wstring AsShortPath(wstring path, wstring* result) {
   return L"";
 }
 
-wstring AsExecutablePathForCreateProcess(wstring path, wstring* result) {
+wstring AsExecutablePathForCreateProcess(wstring path, wstring* quoted_path,
+                                         wstring* extended_path) {
   if (path.empty()) {
     return MakeErrorMessage(WSTR(__FILE__), __LINE__,
                             L"AsExecutablePathForCreateProcess", path,
@@ -287,16 +297,34 @@ wstring AsExecutablePathForCreateProcess(wstring path, wstring* result) {
     }
     path = cwd + L"\\" + path;
   }
-  wstring error = AsShortPath(path, result);
-  if (!error.empty()) {
-    return MakeErrorMessage(WSTR(__FILE__), __LINE__,
-                            L"AsExecutablePathForCreateProcess", path, error);
+  wstring error = AsShortPath(path, quoted_path);
+  if (error.empty()) {
+    // Quote the path in case it's something like "c:\foo\app name.exe".
+    // Do this unconditionally, there's no harm in quoting. Quotes are not
+    // allowed inside paths so we don't need to escape quotes.
+    QuotePath(*quoted_path, quoted_path);
+    extended_path->clear();
+    return L"";
   }
-  // Quote the path in case it's something like "c:\foo\app name.exe".
-  // Do this unconditionally, there's no harm in quoting. Quotes are not
-  // allowed inside paths so we don't need to escape quotes.
-  QuotePath(*result, result);
-  return L"";
+  // Shortening might fail (https://github.com/bazelbuild/bazel/issues/19710),
+  // typically when 8.3 aliases are disabled on the volume, which is common in
+  // containers (https://github.com/microsoft/Windows-Containers/issues/507).
+  // As a fallback, set `quoted_path` as above and `extended_path` to the
+  // extended-length form of `path`, suitable for CreateProcessW's
+  // lpApplicationName: it is not subject to MAX_PATH, and providing it lifts
+  // that limit from the executable part of CreateProcessW's lpCommandLine too.
+  // This works only for a plain executable with an absolute, normalized path.
+  if (!IsBatchFile(path)) {
+    wstring native_path = path;
+    std::replace(native_path.begin(), native_path.end(), L'/', L'\\');
+    if (IsAbsoluteNormalizedWindowsPath(native_path)) {
+      QuotePath(native_path, quoted_path);
+      *extended_path = wstring(L"\\\\?\\") + native_path;
+      return L"";
+    }
+  }
+  return MakeErrorMessage(WSTR(__FILE__), __LINE__,
+                          L"AsExecutablePathForCreateProcess", path, error);
 }
 
 }  // namespace windows

src/main/native/windows/util.h

@@ -136,30 +136,39 @@ wstring GetLastErrorString(DWORD error_code);
 // Same as `AsExecutablePathForCreateProcess` except it won't quote the result.
 wstring AsShortPath(wstring path, wstring* result);
 
-// Computes a path suitable as the executable part in CreateProcessA's cmdline.
+// Computes a path suitable as the executable part in CreateProcessW's
+// lpCommandLine, plus an extended-length form suitable for CreateProcessW's
+// lpApplicationName when applicable.
 //
-// The null-terminated executable path for CreateProcessA has to fit into
-// MAX_PATH, therefore the limit for the executable's path is MAX_PATH - 1
-// (not including null terminator). This method attempts to convert the input
-// `path` to a short format to fit it into the MAX_PATH - 1 limit.
+// Unless CreateProcessW's lpApplicationName is populated, the null-terminated
+// executable path in CreateProcessW's lpCommandLine has to fit into MAX_PATH,
+// therefore the limit for the executable's path is MAX_PATH - 1 (not including
+// null terminator). This method attempts to convert the input `path` to a short
+// format to fit it into the MAX_PATH - 1 limit.
 //
 // `path` must be either an absolute, normalized, Windows-style path with drive
 // letter (e.g. "c:\foo\bar.exe", but no "\foo\bar.exe"), or must be just a file
 // name (e.g. "cmd.exe") that's shorter than MAX_PATH (without null-terminator).
 // In both cases, `path` must be unquoted.
 //
 // If this function succeeds, it returns an empty string (indicating no error),
-// and sets `result` to the resulting path, which is always quoted, and is
+// and sets `quoted_path` to the resulting path, which is always quoted, and is
 // always at most MAX_PATH + 1 long (MAX_PATH - 1 without null terminator, plus
-// two quotes). If there's any error, this function returns the error message.
+// two quotes), unless the `extended_path` fallback below applies. If there's
+// any error, this function returns the error message.
 //
 // If `path` is at most MAX_PATH - 1 long (not including null terminator), the
 // result will be that (plus quotes).
 // Otherwise this method attempts to compute an 8dot3 style short name for
 // `path`, and if that succeeds and the result is at most MAX_PATH - 1 long (not
 // including null terminator), then that will be the result (plus quotes).
+// Otherwise, if `path` is an absolute, normalized path to a plain executable
+// (not a batch file), `extended_path` is set to its extended-length form (the
+// "\\?\" prefix plus `path`) for use as CreateProcessW's lpApplicationName, and
+// `quoted_path` to the quoted `path`.
 // Otherwise this function fails and returns an error message.
-wstring AsExecutablePathForCreateProcess(wstring path, wstring* result);
+wstring AsExecutablePathForCreateProcess(wstring path, wstring* quoted_path,
+                                         wstring* extended_path);
 
 }  // namespace windows
 }  // namespace bazel

src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java

@@ -327,6 +327,25 @@ public void testExecutableNotFound() throws Exception {
     assertThat(readStdout(buf, 0, 1)).isEqualTo(0);
   }
 
+  @Test
+  public void testExecutableWithExtendedLengthPath() throws Exception {
+    StringBuilder dir = new StringBuilder(System.getenv("TEST_TMPDIR"));
+    for (char c = 'a'; c <= 'z'; c++) {
+      dir.append('\\').repeat(c, 8).append('.').repeat(c, 3);
+    }
+    String exe = dir + "\\cmd.exe";
+    assertThat(exe.length()).isGreaterThan(260); // Windows MAX_PATH
+    Files.createDirectories(Paths.get("\\\\?\\" + dir));
+    Files.copy(
+        Paths.get(System.getenv("SystemRoot"), "System32", "cmd.exe"),
+        Paths.get("\\\\?\\" + exe));
+
+    process = WindowsProcesses.createProcess(exe, "/c exit 42", null, null, null, null);
+    assertNoProcessError();
+    assertThat(WindowsProcesses.waitFor(process, -1)).isEqualTo(0);
+    assertThat(WindowsProcesses.getExitCode(process)).isEqualTo(42);
+  }
+
   @Test
   public void testReadingAndWritingAfterTermination() throws Exception {
     process =

src/test/native/windows/util_test.cc

@@ -236,21 +236,40 @@ static void DeleteDirsUnder(const wstring& basedir,
   }
 
 // This is a macro so the assertions will have the correct line number.
-#define ASSERT_SHORTENING_FAILS(/* const WCHAR* */ input,             \
-                                /* const WCHAR* */ error_msg)         \
-  {                                                                   \
-    wstring actual;                                                   \
-    wstring result(AsExecutablePathForCreateProcess(input, &actual)); \
-    ASSERT_CONTAINS(result, error_msg);                               \
+#define ASSERT_SHORTENING_FAILS(/* const WCHAR* */ input,                  \
+                                /* const WCHAR* */ error_msg)              \
+  {                                                                        \
+    wstring actual;                                                        \
+    wstring extended_path;                                                 \
+    wstring result(                                                        \
+        AsExecutablePathForCreateProcess(input, &actual, &extended_path)); \
+    ASSERT_CONTAINS(result, error_msg);                                    \
+    ASSERT_EQ(extended_path, L"");                                         \
   }
 
 // This is a macro so the assertions will have the correct line number.
-#define ASSERT_SHORTENING_SUCCEEDS(/* const WCHAR* */ input,             \
-                                   /* const wstring& */ expected_result) \
-  {                                                                      \
-    wstring actual;                                                      \
-    ASSERT_EQ(AsExecutablePathForCreateProcess(input, &actual), L"");    \
-    ASSERT_EQ(actual, expected_result);                                  \
+#define ASSERT_SHORTENING_SUCCEEDS(/* const WCHAR* */ input,              \
+                                   /* const wstring& */ expected_result)  \
+  {                                                                       \
+    wstring actual;                                                       \
+    wstring extended_path = L"stale";                                     \
+    ASSERT_EQ(                                                            \
+        AsExecutablePathForCreateProcess(input, &actual, &extended_path), \
+        L"");                                                             \
+    ASSERT_EQ(actual, expected_result);                                   \
+    ASSERT_EQ(extended_path, L"");                                        \
+  }
+
+// This is a macro so the assertions will have the correct line number.
+#define ASSERT_FALLBACK_SUCCEEDS(/* const WCHAR* */ input)                \
+  {                                                                       \
+    wstring actual;                                                       \
+    wstring extended_path;                                                \
+    ASSERT_EQ(                                                            \
+        AsExecutablePathForCreateProcess(input, &actual, &extended_path), \
+        L"");                                                             \
+    ASSERT_EQ(extended_path, wstring(L"\\\\?\\") + input);                \
+    ASSERT_EQ(actual, wstring(L"\"") + input + L"\"");                    \
   }
 
 TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessBadInputs) {
@@ -271,7 +290,9 @@ TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessBadInputs) {
   // Relative paths are fine, they are absolutized.
   std::wstring rel(L"foo\\bar.exe");
   std::wstring actual;
-  EXPECT_EQ(AsExecutablePathForCreateProcess(rel, &actual), L"");
+  std::wstring extended_path;
+  EXPECT_EQ(AsExecutablePathForCreateProcess(rel, &actual, &extended_path),
+            L"");
   EXPECT_GT(actual.size(), rel.size());
   EXPECT_EQ(actual.rfind(rel), actual.size() - rel.size() - 1);
 }
@@ -298,7 +319,7 @@ TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessConversions) {
     // When i=0 then `wfilename` is `kMaxPath` - 1 long, so
     // `AsExecutablePathForCreateProcess` will not attempt to shorten it, and
     // so it also won't notice that the file doesn't exist. If however we pass
-    // a non-existent path to CreateProcessA, then it'll fail, so we'll find out
+    // a non-existent path to CreateProcessW, then it'll fail, so we'll find out
     // about this error in production code.
     // When i>0 then `wfilename` is at least `kMaxPath` long, so
     // `AsExecutablePathForCreateProcess` will attempt to shorten it, but
@@ -349,5 +370,23 @@ TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessConversions) {
   DeleteDirsUnder(tmpdir, short_root);
 }
 
+TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessFallback) {
+  wstring dir = L"c:\\" + wstring(kMaxPath, L'a');
+
+  // A deep, absolute, normalized executable path that can't be shortened (here
+  // because it doesn't exist) takes the fallback: `extended_path` gets the
+  // extended-length form and `quoted_path` the quoted path.
+  ASSERT_FALLBACK_SUCCEEDS(dir + L"\\foo.exe");
+
+  // A batch file is denied the fallback even with an absolute, normalized path:
+  // CreateProcessW's lpApplicationName can only take a plain executable.
+  ASSERT_SHORTENING_FAILS(dir + L"\\foo.bat", L"GetShortPathNameW");
+
+  // A non-normalized path is denied the fallback because the "\\?\" prefix
+  // disables path normalization, so "." and ".." would reach the filesystem
+  // verbatim instead of being resolved.
+  ASSERT_SHORTENING_FAILS(dir + L"\\..\\foo.exe", L"path is not normalized");
+}
+
 }  // namespace windows
 }  // namespace bazel