starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-03 21:25:34 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Bug Report: The conhost command line is not properly escaped #1457

New Issue
Closed
opened 2026-01-30 22:27:42 +00:00 by claunia · 10 comments
claunia commented 2026-01-30 22:27:42 +00:00
Owner
Copy Link

Originally created by @fcharlie on GitHub (Jun 1, 2019).

Environment

Windows build number: [run "ver" at a command prompt]
Windows Terminal version (if applicable):

Any other software?

Today I modified the Windows Terminal configuration file and wanted to create a Pwsh with the emoji title. However, I encountered some confusion in this process. I checked the task manager and found that conhost did not escape when the process was started, causing the command line to parse the exception.

Steps to reproduce

Expected behavior

I need to start pwsh by the following command line (note that the following command line is in cmd, or you can start pwsh normally using CreateProcessW.)

"C:\Program Files\PowerShell\7-preview\pwsh.exe" -NoExit -Command "$Host.UI.RawUI.WindowTitle=\"Windows Pwsh 💙 (7 Preview)\""

Here is the Windows Terminal configuration file (the command line has been properly escaped):

        {
            "startingDirectory": "C:\\Users\\CharlieInc",
            "guid": "{08a0be98-ff68-4e3a-a054-0fbd3969d3bb}",
            "name": "Windows Pwsh 💙 (7 Preview)",
            "colorscheme": "Campbell",
            "historySize": 9001,
            "snapOnInput": true,
            "cursorColor": "#FFFFFF",
            "cursorShape": "bar",
            "commandline": "\"C:\\Program Files\\PowerShell\\7-preview\\pwsh.exe\" -NoExit -Command \"$Host.UI.RawUI.WindowTitle=\\\"Windows Pwsh 💙 (7 Preview)\\\"\"",
            "fontFace": "Consolas",
            "fontSize": 12,
            "acrylicOpacity": 0.75,
            "useAcrylic": true,
            "closeOnExit": false,
            "padding": "0, 0, 0, 0",
            "icon": "ms-appdata:///roaming/pwsh-32.png"
        }

Actual behavior

As expected, Pwsh should set the title correctly, but pwsh reported the error:
image

Let's take a look at the command line:

image

image

When you see the red line, the command line that pwsh starts is incorrect, and when you start conhost, the command line is still correct!

Currently I have found a suspicious code, most likely this code caused an error:

71e19cd825/src/host/ConsoleArguments.cpp (L263-L288)

In this code, command line composition is simply a simple connection rather than a concatenated string. If the string contains spaces, this will result in an incorrect command line.

Below is a code for the command line escaping, which may be useful：

inline std::wstring escape_argument(std::wstring_view ac) {
  if (ac.empty()) {
    return L"\"\"";
  }
  bool hasspace = false;
  auto n = ac.size();
  for (auto c : ac) {
    switch (c) {
    case L'"':
    case L'\\':
      n++;
      break;
    case ' ':
    case '\t':
      hasspace = true;
      break;
    default:
      break;
    }
  }
  if (hasspace) {
    n += 2;
  }
  if (n == ac.size()) {
    return std::wstring(ac.data(), ac.size());
  }
  std::wstring buf;
  if (hasspace) {
    buf.push_back(L'"');
  }
  size_t slashes = 0;
  for (auto c : ac) {
    switch (c) {
    case L'\\':
      slashes++;
      buf.push_back(L'\\');
      break;
    case L'"': {
      for (; slashes > 0; slashes--) {
        buf.push_back(L'\\');
      }
      buf.push_back(L'\\');
      buf.push_back(c);
    } break;
    default:
      slashes = 0;
      buf.push_back(c);
      break;
    }
  }
  if (hasspace) {
    for (; slashes > 0; slashes--) {
      buf.push_back(L'\\');
    }
    buf.push_back(L'"');
  }
  return buf;
}
Originally created by @fcharlie on GitHub (Jun 1, 2019). <!-- This bug tracker is monitored by Windows Terminal development team and other technical folks. **Important: When reporting BSODs or security issues, DO NOT attach memory dumps, logs, or traces to Github issues**. Instead, send dumps/traces to secure@microsoft.com, referencing this GitHub issue. Please use this form and describe your issue, concisely but precisely, with as much detail as possible. --> # Environment ```none Windows build number: [run "ver" at a command prompt] Windows Terminal version (if applicable): Any other software? ``` Today I modified the Windows Terminal configuration file and wanted to create a Pwsh with the emoji title. However, I encountered some confusion in this process. I checked the task manager and found that conhost did not escape when the process was started, causing the command line to parse the exception. # Steps to reproduce <!-- A description of how to trigger this bug. --> # Expected behavior <!-- A description of what you're expecting, possibly containing screenshots or reference material. --> I need to start pwsh by the following command line (note that the following command line is in cmd, or you can start pwsh normally using CreateProcessW.) ``` "C:\Program Files\PowerShell\7-preview\pwsh.exe" -NoExit -Command "$Host.UI.RawUI.WindowTitle=\"Windows Pwsh 💙 (7 Preview)\"" ``` Here is the Windows Terminal configuration file (the command line has been properly escaped): ```json { "startingDirectory": "C:\\Users\\CharlieInc", "guid": "{08a0be98-ff68-4e3a-a054-0fbd3969d3bb}", "name": "Windows Pwsh 💙 (7 Preview)", "colorscheme": "Campbell", "historySize": 9001, "snapOnInput": true, "cursorColor": "#FFFFFF", "cursorShape": "bar", "commandline": "\"C:\\Program Files\\PowerShell\\7-preview\\pwsh.exe\" -NoExit -Command \"$Host.UI.RawUI.WindowTitle=\\\"Windows Pwsh 💙 (7 Preview)\\\"\"", "fontFace": "Consolas", "fontSize": 12, "acrylicOpacity": 0.75, "useAcrylic": true, "closeOnExit": false, "padding": "0, 0, 0, 0", "icon": "ms-appdata:///roaming/pwsh-32.png" } ``` # Actual behavior As expected, Pwsh should set the title correctly, but pwsh reported the error: ![image](https://user-images.githubusercontent.com/6904176/58741634-e7558300-844d-11e9-9b7e-dcdff905ffa3.png) Let's take a look at the command line: ![image](https://user-images.githubusercontent.com/6904176/58741674-49ae8380-844e-11e9-9ca4-43ce9bf4b1ab.png) ![image](https://user-images.githubusercontent.com/6904176/58741666-38657700-844e-11e9-8711-e813c22bc137.png) When you see the red line, the command line that pwsh starts is incorrect, and when you start conhost, the command line is still correct! <!-- What's actually happening? --> Currently I have found a suspicious code, most likely this code caused an error: https://github.com/microsoft/terminal/blob/71e19cd82528d66a0a7867cbed85990cfc1685f1/src/host/ConsoleArguments.cpp#L263-L288 In this code, command line composition is simply a simple connection rather than a concatenated string. If the string contains spaces, this will result in an incorrect command line. Below is a code for the command line escaping, which may be useful： ```cpp inline std::wstring escape_argument(std::wstring_view ac) { if (ac.empty()) { return L"\"\""; } bool hasspace = false; auto n = ac.size(); for (auto c : ac) { switch (c) { case L'"': case L'\\': n++; break; case ' ': case '\t': hasspace = true; break; default: break; } } if (hasspace) { n += 2; } if (n == ac.size()) { return std::wstring(ac.data(), ac.size()); } std::wstring buf; if (hasspace) { buf.push_back(L'"'); } size_t slashes = 0; for (auto c : ac) { switch (c) { case L'\\': slashes++; buf.push_back(L'\\'); break; case L'"': { for (; slashes > 0; slashes--) { buf.push_back(L'\\'); } buf.push_back(L'\\'); buf.push_back(c); } break; default: slashes = 0; buf.push_back(c); break; } } if (hasspace) { for (; slashes > 0; slashes--) { buf.push_back(L'\\'); } buf.push_back(L'"'); } return buf; } ```
claunia commented 2026-01-30 22:27:43 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Jun 3, 2019):

I'm inclined to think that this will be solved when we switch to the real conpty API. Right now, we're using the legacy method of launching conhost.exe as a conpty, and requesting that conhost launches the process on behalf of us. This results in the commandline needing to be escaped twice like this, which is pretty painful. We have some work planned to switch the Terminal back to using the API directly. When we make that change, then we won't need to double-escape the commandline, we'll just use the commandline to CreateProcess the process directly, and we'll attach that process to a conpty.

@zadjii-msft commented on GitHub (Jun 3, 2019): I'm inclined to think that this will be solved when we switch to the real conpty API. Right now, we're using the legacy method of launching conhost.exe as a conpty, and requesting that conhost launches the process on behalf of us. This results in the commandline needing to be escaped twice like this, which is pretty painful. We have some work planned to switch the Terminal back to using the API directly. When we make that change, then we won't need to double-escape the commandline, we'll just use the commandline to CreateProcess the process directly, and we'll attach that process to a conpty.
claunia commented 2026-01-30 22:27:43 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Jun 3, 2019):

@zadjii-msft is totally right. I'll file up with a Deliverable to use *PseudoConsole and tack that on to the Deliverable for make pseudoconsole API a framework package or something?.

@DHowett-MSFT commented on GitHub (Jun 3, 2019): @zadjii-msft is totally right. I'll file up with a Deliverable to use `*PseudoConsole` and tack that on to the Deliverable for _make pseudoconsole API a framework package or something?_.
claunia commented 2026-01-30 22:27:43 +00:00
Author
Owner
Copy Link

@fcharlie commented on GitHub (Jun 4, 2019):

@zadjii-msft @DHowett-MSFT Thanks.

Using the conpty API is correct, but I think conhost still needs to solve this problem.

@fcharlie commented on GitHub (Jun 4, 2019): @zadjii-msft @DHowett-MSFT Thanks. Using the conpty API is correct, but I think conhost still needs to solve this problem.
claunia commented 2026-01-30 22:27:43 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Jun 4, 2019):

conhost still needs to solve this problem

It will probably solve this problem by getting 100% out of the business of accepting commandlines of other applications to spawn as arguments.

@DHowett-MSFT commented on GitHub (Jun 4, 2019): > conhost still needs to solve this problem It will probably solve this problem by getting 100% out of the business of accepting commandlines of _other_ applications to spawn as arguments.
claunia commented 2026-01-30 22:27:44 +00:00
Author
Owner
Copy Link

@fcharlie commented on GitHub (Jun 4, 2019):

@DHowett-MSFT Good

@fcharlie commented on GitHub (Jun 4, 2019): @DHowett-MSFT Good
claunia commented 2026-01-30 22:27:44 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Jul 3, 2019):

(adding some notes from a related email thread:)

I'm hacking some integration between Windows Terminal and a new VS2019 feature but this doesn't work (at least not obviously/easily/with normal json)

"commandline": "C:\\Windows\\SysWOW64\\WindowsPowerShell\\v1.0\\powershell.exe -ExecutionPolicy Bypass -NoLogo -NoExit -File \"C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Preview\\Common7\\Tools\\VsDevPs.ps1\"",

image

Okay, so the problem is in conhost. When we’re starting conhost for the Terminal, we get the commandline already broken up into args. Conhost then takes all those args, and joins them with spaces, to call CreateProcess again, to launch the client app. Because CreateProcess has already tried to help here while launching `conhost -- `, it already grouped the entire last arg as a single arg. So, conhost never knew that last arg had quotes around it.

If you once again escape the quotes around the path, it should work:

"commandline": "C:\\Windows\\SysWOW64\\WindowsPowerShell\\v1.0\\powershell.exe -ExecutionPolicy Bypass -NoLogo -NoExit -File \\\"C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Preview\\Common7\\Tools\\VsDevPs.ps1\\\"",

image

@zadjii-msft commented on GitHub (Jul 3, 2019): (adding some notes from a related email thread:) <hr> I'm hacking some integration between Windows Terminal and a new VS2019 feature but this doesn't work (at least not obviously/easily/with normal json) `"commandline": "C:\\Windows\\SysWOW64\\WindowsPowerShell\\v1.0\\powershell.exe -ExecutionPolicy Bypass -NoLogo -NoExit -File \"C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Preview\\Common7\\Tools\\VsDevPs.ps1\"",` ![image](https://user-images.githubusercontent.com/18356694/60600196-5982f580-9d75-11e9-9d8c-64480c67d50e.png) <hr> Okay, so the problem is in conhost. When we’re starting conhost for the Terminal, we get the commandline already broken up into args. Conhost then takes all those args, and joins them with spaces, to call CreateProcess again, to launch the client app. Because CreateProcess has already tried to help here while launching `conhost -- <commandline>`, it already grouped the entire last arg as a single arg. So, conhost never knew that last arg had quotes around it. If you once again escape the quotes around the path, it should work: ``` "commandline": "C:\\Windows\\SysWOW64\\WindowsPowerShell\\v1.0\\powershell.exe -ExecutionPolicy Bypass -NoLogo -NoExit -File \\\"C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Preview\\Common7\\Tools\\VsDevPs.ps1\\\"", ``` ![image](https://user-images.githubusercontent.com/18356694/60600253-728ba680-9d75-11e9-82a5-a76c52961087.png) <hr>
claunia commented 2026-01-30 22:27:44 +00:00
Author
Owner
Copy Link

@fcharlie commented on GitHub (Jul 4, 2019):

@zadjii-msft I still recommend reassembling the command line arguments for use by CreateProcess using the previous escape_argument。

@fcharlie commented on GitHub (Jul 4, 2019): @zadjii-msft I still recommend reassembling the command line arguments for use by CreateProcess using the previous `escape_argument`。
claunia commented 2026-01-30 22:27:44 +00:00
Author
Owner
Copy Link

@fcharlie commented on GitHub (Jul 4, 2019):

@zadjii-msft @DHowett-MSFT
PR #1815 should fix this problem.

image

image

@fcharlie commented on GitHub (Jul 4, 2019): @zadjii-msft @DHowett-MSFT PR #1815 should fix this problem. ![image](https://user-images.githubusercontent.com/6904176/60672473-02ca0880-9ea8-11e9-9208-d6e647651e2d.png) ![image](https://user-images.githubusercontent.com/6904176/60672507-1ecdaa00-9ea8-11e9-87e3-c779216602f1.png)
claunia commented 2026-01-30 22:27:44 +00:00
Author
Owner
Copy Link

@fcharlie commented on GitHub (Jul 5, 2019):

Unfortunately, Windows UCRT's spawnvp spawnv and other functions do not correctly handle escaping command line arguments, which is the same as conhost.

Can someone feedback this question to the ucrt team?

See: C:\Program Files (x86)\Windows Kits\10\Source\10.0.18362.0\ucrt\exec\cenvarg.cpp +24

// Converts a main()-style argv arguments vector into a command line.  On success,
// returns a pointer to the newly constructed arguments block; the caller is
// responsible for freeing the string.  On failure, returns null and sets errno.
template <typename Character>
static errno_t __cdecl construct_command_line(
    Character const* const* const argv,
    Character**             const command_line_result
    ) throw()
{
    typedef __crt_char_traits<Character> traits;

    *command_line_result = nullptr;

    // Compute the number of bytes required to store the arguments in argv in a
    // command line string (including spaces between arguments and a terminator):
    size_t const command_line_count = [&]
    {
        size_t n = 0;
        for (Character const* const* it = argv; *it; n += traits::tcslen(*it++) + 1) { }

        // If there were no arguments, return 1 so that we can return an empty
        // string:
        return __max(n, 1);
    }();

    __crt_unique_heap_ptr<Character> command_line(_calloc_crt_t(Character, command_line_count));
    if (!command_line)
    {
        __acrt_errno_map_os_error(ERROR_NOT_ENOUGH_MEMORY);
        return errno = ENOMEM;
    }

    Character const* const* source_it = argv;
    Character*              result_it = command_line.get();

    // If there are no arguments, just return the empty string:
    if (*source_it == '\0')
    {
        *command_line_result = command_line.detach();
        return 0;
    }

    // Copy the arguments, separated by spaces:
    while (*source_it != '\0')
    {
        _ERRCHECK(traits::tcscpy_s(result_it, command_line_count - (result_it - command_line.get()), *source_it));
        result_it += traits::tcslen(*source_it);
        *result_it++ = ' ';
        ++source_it;
    }

    // Replace the last space with a terminator:
    result_it[-1] = '\0';

    *command_line_result = command_line.detach();
    return 0;
}
@fcharlie commented on GitHub (Jul 5, 2019): Unfortunately, Windows UCRT's `spawnvp` `spawnv` and other functions do not correctly handle escaping command line arguments, which is the same as conhost. Can someone feedback this question to the ucrt team? See: `C:\Program Files (x86)\Windows Kits\10\Source\10.0.18362.0\ucrt\exec\cenvarg.cpp` `+24` ```c++ // Converts a main()-style argv arguments vector into a command line. On success, // returns a pointer to the newly constructed arguments block; the caller is // responsible for freeing the string. On failure, returns null and sets errno. template <typename Character> static errno_t __cdecl construct_command_line( Character const* const* const argv, Character** const command_line_result ) throw() { typedef __crt_char_traits<Character> traits; *command_line_result = nullptr; // Compute the number of bytes required to store the arguments in argv in a // command line string (including spaces between arguments and a terminator): size_t const command_line_count = [&] { size_t n = 0; for (Character const* const* it = argv; *it; n += traits::tcslen(*it++) + 1) { } // If there were no arguments, return 1 so that we can return an empty // string: return __max(n, 1); }(); __crt_unique_heap_ptr<Character> command_line(_calloc_crt_t(Character, command_line_count)); if (!command_line) { __acrt_errno_map_os_error(ERROR_NOT_ENOUGH_MEMORY); return errno = ENOMEM; } Character const* const* source_it = argv; Character* result_it = command_line.get(); // If there are no arguments, just return the empty string: if (*source_it == '\0') { *command_line_result = command_line.detach(); return 0; } // Copy the arguments, separated by spaces: while (*source_it != '\0') { _ERRCHECK(traits::tcscpy_s(result_it, command_line_count - (result_it - command_line.get()), *source_it)); result_it += traits::tcslen(*source_it); *result_it++ = ' '; ++source_it; } // Replace the last space with a terminator: result_it[-1] = '\0'; *command_line_result = command_line.detach(); return 0; } ```
claunia commented 2026-01-30 22:27:45 +00:00
Author
Owner
Copy Link

@ghost commented on GitHub (Aug 3, 2019):

:tada:This issue was addressed in #1815, which has now been successfully released as Windows Terminal Preview v0.3.2142.0.🎉

Handy links:

@ghost commented on GitHub (Aug 3, 2019): :tada:This issue was addressed in #1815, which has now been successfully released as `Windows Terminal Preview v0.3.2142.0`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v0.3.2142.0) * [Store Download](https://www.microsoft.com/store/apps/9n0dx20hk701?cid=storebadge&ocid=badge)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-Interop Issue-Bug Needs-Tag-Fix Product-Conhost Product-Terminal Resolution-Fix-Committed
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#1457
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?