Properly escape constructed wt command-lines (#11314)

Ensures that command-lines constructed to invoke `wt` are escaped properly.

## PR Checklist
* [x] Closes #11273
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed

## Detailed Description of the Pull Request / Additional comments
This was broken in two places - when constructing the command-line in the shell extension and in `NewTerminalArgs::ToCommandline()`.

Both places now invoke a shared method to escape the command-line arguments that require it.

## Validation Steps Performed
Added a test and additionally:
* Invoked the shell extension from `D:\Downloads\With;Semicolon`.
* Added a `newWindow` action to `settings.json` as below and ensured the new window opened without erroring.
  ```json
  {
    "command": 
    {
      "action": "newWindow",
      "tabTitle": "\";foo\\"
    },
    "keys": "ctrl+shift+s"
  }
  ```
This commit is contained in:
Ian O'Neill 2021-09-24 17:17:16 +01:00 committed by GitHub
parent 0f122ca290
commit 9708a75131
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 63 additions and 11 deletions

View file

@ -465,6 +465,10 @@ namespace SettingsModelLocalTests
"name":"action7_startingDirectoryWithTrailingSlash",
"command": { "action": "newWindow", "startingDirectory":"C:\\", "commandline": "bar.exe" }
},
{
"name":"action8_tabTitleEscaping",
"command": { "action": "newWindow", "tabTitle":"\\\";foo\\" }
}
])" };
const auto commands0Json = VerifyParseSucceeded(commands0String);
@ -473,7 +477,7 @@ namespace SettingsModelLocalTests
VERIFY_ARE_EQUAL(0u, commands.Size());
auto warnings = implementation::Command::LayerJson(commands, commands0Json);
VERIFY_ARE_EQUAL(0u, warnings.size());
VERIFY_ARE_EQUAL(8u, commands.Size());
VERIFY_ARE_EQUAL(9u, commands.Size());
{
auto command = commands.Lookup(L"action0");
@ -586,5 +590,20 @@ namespace SettingsModelLocalTests
L"cmdline: \"%s\"", cmdline.c_str()));
VERIFY_ARE_EQUAL(L"--startingDirectory \"C:\\\\\" -- \"bar.exe\"", terminalArgs.ToCommandline());
}
{
auto command = commands.Lookup(L"action8_tabTitleEscaping");
VERIFY_IS_NOT_NULL(command);
VERIFY_IS_NOT_NULL(command.ActionAndArgs());
VERIFY_ARE_EQUAL(ShortcutAction::NewWindow, command.ActionAndArgs().Action());
const auto& realArgs = command.ActionAndArgs().Args().try_as<NewWindowArgs>();
VERIFY_IS_NOT_NULL(realArgs);
const auto& terminalArgs = realArgs.TerminalArgs();
VERIFY_IS_NOT_NULL(terminalArgs);
auto cmdline = terminalArgs.ToCommandline();
Log::Comment(NoThrowString().Format(
L"cmdline: \"%s\"", cmdline.c_str()));
VERIFY_ARE_EQUAL(LR"-(--title "\\\"\;foo\\")-", terminalArgs.ToCommandline());
}
}
}

View file

@ -55,9 +55,7 @@ HRESULT OpenTerminalHere::Invoke(IShellItemArray* psiItemArray,
STARTUPINFOEX siEx{ 0 };
siEx.StartupInfo.cb = sizeof(STARTUPINFOEX);
// Append a "\." to the given path, so that this will work in "C:\"
auto path{ wil::str_printf<std::wstring>(LR"-(%s\.)-", pszName.get()) };
auto cmdline{ wil::str_printf<std::wstring>(LR"-("%s" -d "%s")-", GetWtExePath().c_str(), path.c_str()) };
auto cmdline{ wil::str_printf<std::wstring>(LR"-("%s" -d %s)-", GetWtExePath().c_str(), QuoteAndEscapeCommandlineArg(pszName.get()).c_str()) };
RETURN_IF_WIN32_BOOL_FALSE(CreateProcessW(
nullptr, // lpApplicationName
cmdline.data(),
@ -66,7 +64,7 @@ HRESULT OpenTerminalHere::Invoke(IShellItemArray* psiItemArray,
false, // bInheritHandles
EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, // dwCreationFlags
nullptr, // lpEnvironment
path.data(),
pszName.get(),
&siEx.StartupInfo, // lpStartupInfo
&_piClient // lpProcessInformation
));

View file

@ -38,6 +38,7 @@
#include "MultipleActionsArgs.g.cpp"
#include <LibraryResources.h>
#include <WtExeUtils.h>
using namespace winrt::Microsoft::Terminal::Control;
@ -121,15 +122,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
if (!StartingDirectory().empty())
{
// If the directory ends in a '\', we need to add another one on so that the enclosing quote added
// afterwards isn't escaped
const auto trailingBackslashEscape = StartingDirectory().back() == L'\\' ? L"\\" : L"";
ss << fmt::format(L"--startingDirectory \"{}{}\" ", StartingDirectory(), trailingBackslashEscape);
ss << fmt::format(L"--startingDirectory {} ", QuoteAndEscapeCommandlineArg(StartingDirectory()));
}
if (!TabTitle().empty())
{
ss << fmt::format(L"--title \"{}\" ", TabTitle());
ss << fmt::format(L"--title {} ", QuoteAndEscapeCommandlineArg(TabTitle()));
}
if (TabColor())
@ -152,7 +150,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
if (!ColorScheme().empty())
{
ss << fmt::format(L"--colorScheme \"{}\" ", ColorScheme());
ss << fmt::format(L"--colorScheme {} ", QuoteAndEscapeCommandlineArg(ColorScheme()));
}
if (!Commandline().empty())

View file

@ -104,3 +104,40 @@ _TIL_INLINEPREFIX const std::wstring& GetWtExePath()
}();
return exePath;
}
// Method Description:
// - Quotes and escapes the given string so that it can be used as a command-line arg.
// - e.g. given `\";foo\` will return `"\\\"\;foo\\"` so that the caller can construct a command-line
// using something such as `fmt::format(L"wt --title {}", QuoteAndQuoteAndEscapeCommandlineArg(TabTitle()))`.
// Arguments:
// - arg - the command-line argument to quote and escape.
// Return Value:
// - the quoted and escaped command-line argument.
_TIL_INLINEPREFIX std::wstring QuoteAndEscapeCommandlineArg(const std::wstring_view& arg)
{
std::wstring out;
out.reserve(arg.size() + 2);
out.push_back(L'"');
size_t backslashes = 0;
for (const auto ch : arg)
{
if (ch == L'\\')
{
backslashes++;
}
else
{
if (ch == L';' || ch == L'"')
{
out.append(backslashes + 1, L'\\');
}
backslashes = 0;
}
out.push_back(ch);
}
out.append(backslashes, L'\\');
out.push_back(L'"');
return out;
}