Repository: microsoft/terminal (language: cpp)

Issue:
`CreatePseudoConsole` crashes due to long path to conpty.dll
### Windows Terminal version

1.19.10573.0

### Windows build number

10.0.22631.0

### Other Software

_No response_

### Steps to reproduce

1. Install [Windows Terminal v1.19.10573.0](https://github.com/microsoft/terminal/releases/tag/v1.19.10573.0) to a directory with long full path: the parent directory of `WindowsTerminal.exe` should have length 167 chars or higher.
2. Run `WindowsTerminal.exe`

Please see https://github.com/segrey/CreatePseudoConsole-crash-with-long-path, it automates the reproduction of this issue.

### Expected Behavior

Windows Terminal runs normally.

### Actual Behavior

Windows Terminal crashes at startup and finishes with exit code -1073740791 (hex: c0000409)

Event Viewer (Windows Log | Application) contains the following error:
```text
Faulting application name: WindowsTerminal.exe, version: 1.19.2402.26003, time stamp: 0x65dce3ea
Faulting module name: ucrtbase.dll, version: 10.0.22621.2506, time stamp: 0x097c794c
Exception code: 0xc0000409
Fault offset: 0x000000000007df28
Faulting process id: 0x0x4424
Faulting application start time: 0x0x1DA739C0BB31B88
Faulting application path: D:\CreatePseudoConsole-crash-with-long-path\tmp\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaa\terminal-1.19.10573.0\WindowsTerminal.exe
Faulting module path: C:\WINDOWS\System32\ucrtbase.dll
Report Id: 201c4d23-ff42-474b-8764-703686bc06cc
Faulting package full name: 
Faulting package-relative application ID: 
```

Running the `WindowsTerminal.exe` with WinDbg shows that the problem is in `CreatePseudoConsole` function:

![image](https://github.com/microsoft/terminal/assets/607109/04736caa-de74-4e02-9d8e-4151f4c0124f)

Looks like it's caused by crashing [`swprintf_s`](https://github.com/microsoft/terminal/blob/a38388615e299658072f906578acd60e976fe787/src/winconpty/winconpty.cpp#L142), because `MAX_PATH` (260) is too small to accomodate the full path. This is indirectly confirmed by the following: `WindowsTerminal.exe` doesn't crash when it's located in a directory with full path length 162 chars -- in this case the command line of OpenConsole.exe is 259 chars and everything works correctly:

`"D:\CreatePseudoConsole-crash-with-long-path\tmp\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aaaaaaaa\aa\terminal-1.19.10573.0\OpenConsole.exe" --headless --resizeQuirk --width 120 --height 30 --signal 0xd28 --server 0xd20`

It'd be great to fix this issue, because sometimes conpty.dll / OpenConsole.exe are located deep inside the file system.


Discussion / hints:
Wow, great catch!
Im wondering, how did you even discover this in the first place? Are you one of those people that name their directories like Java Classes?
@a-usr you may check the linked issue https://github.com/JetBrains/pty4j/issues/145 for the history
> @a-usr you may check the linked issue [JetBrains/pty4j#145](https://github.com/JetBrains/pty4j/issues/145) for the history

To be honest I thought there was more to it. But thanks for the link!
Nope, just tests involving running from gradle long paths failed all the sudden
@a-usr Initially, it was discovered by IntelliJ plugin developers. Also, please see [IDEA-347968](https://youtrack.jetbrains.com/issue/IDEA-347968/Terminal-IDE-fatal-crash-Execution-failed-for-task-runIde-or-if-installation-path-155-characters#focus=Comments-27-9510169.0-0) for more details.
Anyhow, awesome that you found/helped to find this bug!

Fix patch:
diff --git a/src/winconpty/winconpty.cpp b/src/winconpty/winconpty.cpp
index 60c6c442f2a..5f10758ae6a 100644
--- a/src/winconpty/winconpty.cpp
+++ b/src/winconpty/winconpty.cpp
@@ -93,7 +93,7 @@ static bool _HandleIsValid(HANDLE h) noexcept
     return (h != INVALID_HANDLE_VALUE) && (h != nullptr);
 }
 
-HRESULT _CreatePseudoConsole(const HANDLE hToken,
+HRESULT _CreatePseudoConsole(HANDLE hToken,
                              const COORD size,
                              const HANDLE hInput,
                              const HANDLE hOutput,
@@ -109,6 +109,12 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken,
         return E_INVALIDARG;
     }
 
+    // CreateProcessAsUserW expects the token to be either valid or null.
+    if (hToken == INVALID_HANDLE_VALUE)
+    {
+        hToken = nullptr;
+    }
+
     wil::unique_handle serverHandle;
     RETURN_IF_NTSTATUS_FAILED(CreateServerHandle(serverHandle.addressof(), TRUE));
 
@@ -132,9 +138,6 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken,
     RETURN_IF_WIN32_BOOL_FALSE(CreatePipe(signalPipeConhostSide.addressof(), signalPipeOurSide.addressof(), &sa, 0));
     RETURN_IF_WIN32_BOOL_FALSE(SetHandleInformation(signalPipeConhostSide.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT));
 
-    // GH4061: Ensure that the path to executable in the format is escaped so C:\Program.exe cannot collide with C:\Program Files
-    // This is plenty of space to hold the formatted string
-    wchar_t cmd[MAX_PATH]{};
     const BOOL bInheritCursor = (dwFlags & PSEUDOCONSOLE_INHERIT_CURSOR) == PSEUDOCONSOLE_INHERIT_CURSOR;
 
     const wchar_t* textMeasurement;
@@ -154,16 +157,21 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken,
         break;
     }
 
-    swprintf_s(cmd,
-               MAX_PATH,
-               L"\"%s\" --headless %s%s--width %hd --height %hd --signal 0x%tx --server 0x%tx",
-               _ConsoleHostPath(),
-               bInheritCursor ? L"--inheritcursor " : L"",
-               textMeasurement,
-               size.X,
-               size.Y,
-               std::bit_cast<uintptr_t>(signalPipeConhostSide.get()),
-               std::bit_cast<uintptr_t>(serverHandle.get()));
+    const auto conhostPath = _ConsoleHostPath();
+
+    // GH4061: Ensure that the path to executable in the format is escaped so C:\Program.exe cannot collide with C:\Program Files
+    // This is plenty of space to hold the formatted string
+    wil::unique_process_heap_string cmd;
+    RETURN_IF_FAILED(wil::str_printf_nothrow(
+        cmd,
+        L"\"%s\" --headless %s%s--width %hd --height %hd --signal 0x%tx --server 0x%tx",
+        conhostPath,
+        bInheritCursor ? L"--inheritcursor " : L"",
+        textMeasurement,
+        size.X,
+        size.Y,
+        std::bit_cast<uintptr_t>(signalPipeConhostSide.get()),
+        std::bit_cast<uintptr_t>(serverHandle.get())));
 
     STARTUPINFOEXW siEx{ 0 };
     siEx.StartupInfo.cb = sizeof(STARTUPINFOEXW);
@@ -205,7 +213,8 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken,
                                                          nullptr,
                                                          nullptr));
     wil::unique_process_information pi;
-    { // wow64 disabled filesystem redirection scope
+    {
+        // wow64 disabled filesystem redirection scope
 #if defined(BUILD_WOW6432)
         PVOID RedirectionFlag;
         RETURN_IF_NTSTATUS_FAILED(RtlWow64EnableFsRedirectionEx(
@@ -215,35 +224,20 @@ HRESULT _CreatePseudoConsole(const HANDLE hToken,
             RtlWow64EnableFsRedirectionEx(RedirectionFlag, &RedirectionFlag);
         });
 #endif
-        if (hToken == INVALID_HANDLE_VALUE || hToken == nullptr)
-        {
-            // Call create process
-            RETURN_IF_WIN32_BOOL_FALSE(CreateProcessW(_ConsoleHostPath(),
-                                                      cmd,
-                                                      nullptr,
-                                                      nullptr,
-                                                      TRUE,
-                                                      EXTENDED_STARTUPINFO_PRESENT,
-                                                      nullptr,
-                                                      nullptr,
-                                                      &siEx.StartupInfo,
-                                                      pi.addressof()));
-        }
-        else
-        {
-            // Call create process
-            RETURN_IF_WIN32_BOOL_FALSE(CreateProcessAsUserW(hToken,
-                                                            _ConsoleHostPath(),
-                                                            cmd,
-                                                            nullptr,
-                                                            nullptr,
-                                                            TRUE,
-                                                            EXTENDED_STARTUPINFO_PRESENT,
-                                                            nullptr,
-                                                            nullptr,
-                                                            &siEx.StartupInfo,
-                                                            pi.addressof()));
-        }
+
+        // Call create process
+        RETURN_IF_WIN32_BOOL_FALSE(CreateProcessAsUserW(
+            hToken,
+            conhostPath,
+            cmd.get(),
+            nullptr,
+            nullptr,
+            TRUE,
+            EXTENDED_STARTUPINFO_PRESENT,
+            nullptr,
+            nullptr,
+            &siEx.StartupInfo,
+            pi.addressof()));
     }
 
     pPty->hSignal = signalPipeOurSide.release();


Test patch: