Repository: microsoft/terminal (language: cpp)

Issue:
Escape sequences are not stripped from the tab title
### Windows Terminal version

1.12.3472.0

### Windows build number

10.0.19041.1415

### Other Software

_No response_

### Steps to reproduce

1. Edit the settings.json file.
2. Set the default tab title with an escape sequence like this: `"tabTitle": "\u001b[41m"`
3. Start a cmd shell


### Expected Behavior

The escape character should be stripped from the title.

### Actual Behavior

The escape character is left in the title when propagated over conpty, so it ends up being interpreted as a background color change in the terminal.

![image](https://user-images.githubusercontent.com/4181424/150380055-929963d1-dbb2-469c-9c4f-d8914c5cb626.png)

We do already sanitize the title in `DoSrvSetConsoleTitleW`, which covers title changes via the console API, but in this case the `CONSOLE_INFORMATION::SetTitle` method is being called directly from the `AllocateConsole` method on startup:

https://github.com/microsoft/terminal/blob/7061c54ac59c8469a00a6ce36334ba379ff704e9/src/host/consoleInformation.cpp#L130

My suggestion is we move the sanitization from `DoSrvSetConsoleTitleW` into the `CONSOLE_INFORMATION::SetTitle` method, so we can be sure it's applied in all cases.


Discussion / hints:
That's a great catch, and the suggestion sounds fine to me!


\> inb4 some enterprise has been intentionally putting an escape char in their titles for god knows what reason
🤣 
> inb4 some enterprise has been intentionally putting an escape char in their titles for god knows what reason

I did actually wonder about that, but the current sanitization code only runs when we're in pty mode, and I was planning to retain that condition, so crazy enterprise code should still work as usual on conhost if it comes to that.

Fix patch:
diff --git a/src/host/ConsoleArguments.cpp b/src/host/ConsoleArguments.cpp
index 148a6d321d2..333f9e1d859 100644
--- a/src/host/ConsoleArguments.cpp
+++ b/src/host/ConsoleArguments.cpp
@@ -667,7 +667,7 @@ bool ConsoleArguments::IsWin32InputModeEnabled() const
 #ifdef UNIT_TESTING
 // Method Description:
 // - This is a test helper method. It can be used to trick us into thinking
-//   we're headless (in conpty mode), even without parsing any arguments.
+//   we're in conpty mode, even without parsing any arguments.
 // Arguments:
 // - <none>
 // Return Value:
@@ -675,5 +675,7 @@ bool ConsoleArguments::IsWin32InputModeEnabled() const
 void ConsoleArguments::EnableConptyModeForTests()
 {
     _headless = true;
+    _vtInHandle = GetStdHandle(STD_INPUT_HANDLE);
+    _vtOutHandle = GetStdHandle(STD_OUTPUT_HANDLE);
 }
 #endif
diff --git a/src/host/consoleInformation.cpp b/src/host/consoleInformation.cpp
index 9489ff59094..7313132b73b 100644
--- a/src/host/consoleInformation.cpp
+++ b/src/host/consoleInformation.cpp
@@ -240,6 +240,20 @@ InputBuffer* const CONSOLE_INFORMATION::GetActiveInputBuffer() const
 void CONSOLE_INFORMATION::SetTitle(const std::wstring_view newTitle)
 {
     _Title = std::wstring{ newTitle.begin(), newTitle.end() };
+
+    // Sanitize the input if we're in pty mode. No control chars - this string
+    //      will get emitted back to the TTY in a VT sequence, and we don't want
+    //      to embed control characters in that string. Note that we can't use
+    //      IsInVtIoMode for this test, because the VT I/O thread won't have
+    //      been created when the title is first set during startup.
+    if (ServiceLocator::LocateGlobals().launchArgs.InConptyMode())
+    {
+        _Title.erase(std::remove_if(_Title.begin(), _Title.end(), [](auto ch) {
+                         return ch < UNICODE_SPACE || (ch > UNICODE_DEL && ch < UNICODE_NBSP);
+                     }),
+                     _Title.end());
+    }
+
     _TitleAndPrefix = _Prefix + _Title;
 
     auto* const pRender = ServiceLocator::LocateGlobals().pRender;
diff --git a/src/host/getset.cpp b/src/host/getset.cpp
index 82241ea09e1..0e18bddebc2 100644
--- a/src/host/getset.cpp
+++ b/src/host/getset.cpp
@@ -1798,26 +1798,7 @@ void DoSrvPrivateRefreshWindow(_In_ const SCREEN_INFORMATION& screenInfo)
 [[nodiscard]] HRESULT DoSrvSetConsoleTitleW(const std::wstring_view title) noexcept
 {
     CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
-
-    // Sanitize the input if we're in pty mode. No control chars - this string
-    //      will get emitted back to the TTY in a VT sequence, and we don't want
-    //      to embed control characters in that string.
-    if (gci.IsInVtIoMode())
-    {
-        std::wstring sanitized{ title };
-        sanitized.erase(std::remove_if(sanitized.begin(), sanitized.end(), [](auto ch) {
-                            return ch < UNICODE_SPACE || (ch > UNICODE_DEL && ch < UNICODE_NBSP);
-                        }),
-                        sanitized.end());
-
-        gci.SetTitle({ sanitized });
-    }
-    else
-    {
-        // SetTitle will trigger the renderer to update the titlebar for us.
-        gci.SetTitle(title);
-    }
-
+    gci.SetTitle(title);
     return S_OK;
 }
 


Test patch: