maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

When launching wsl, promote the starting directory to --cd (#9223)

Browse source 
This commit introduces a hack to ConptyConnection for launching WSL.
When we detect that WSL is being launched (either "wsl" or "wsl.exe",
unqialified or _specifically_ from the current OS's System32 directory),
we will promote the startingDirectory specified at launch time into a
commandline argument.

Why do we want to switch to `--cd`?
With the current design of ConptyConnection and WSL, there are some
significant limitations:
* `startingDirectory` cannot be a WSL path, which forces users to
  use weird tricks such as setting the starting directory to
  `\\wsl$\Distro\home\user`.
* WSL occasionally fails to launch in time to handle a `\\wsl$` path,
  which makes us spawn in a strange location (or no location at all).

(This fix will only address the second one until a WSL update is
released that adds support for `--cd $LINUX_PATH`.)

We will not do the promotion if any of the following are true:
* the commandline contains `--cd` already
* the commandline contains a bare `~`
   * This was a commonly-used workaround that forced wsl to start in the
     user's home directory. It conflicts with --cd.
* wsl is not spelled properly (`WSL` and `WSL.EXE` are unacceptable)
* an absolute path to wsl outside the system32 directory is provided

We chose the do this trick in the connection layer, the latest possible
point, because it captures the most use cases.

We could have done it earlier, but the options were quite limiting.
They are:

* Generate WSL profiles with startingDirectory set to the home folder
   * We can't do this because we do not know the user's home folder
     path.
* Generate WSL profiles with `--cd` in them.
   * This only works for unmodified profiles.
   * This only works for generated profiles.
   * Users cannot override the commandline without breaking it.
   * Users cannot specify a startingDirectory (!) since the one on the
     commandline wins.
* Set a flag on generated WSL profiles to request this trick
   * This only works for generated profiles. Users who create their own
     WSL profiles couldn't set startingDirectory and have it work the
     same.

Patching the commandline, hacky though it may be, seemed to be the most
compatible option. Eventually, we can even support `wt -d ~ wsl`!

## Validation Steps Performed

Manual validation for the following cases:

```c++
// MUST MANGLE
auto a01 = _tryMangleStartingDirectoryForWSL(LR"(wsl)", L"SENTINEL");
auto a02 = _tryMangleStartingDirectoryForWSL(LR"(wsl -d X)", L"SENTINEL");
auto a03 = _tryMangleStartingDirectoryForWSL(LR"(wsl -d X ~/bin/sh)", L"SENTINEL");
auto a04 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe)", L"SENTINEL");
auto a05 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe -d X)", L"SENTINEL");
auto a06 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe -d X ~/bin/sh)", L"SENTINEL");
auto a07 = _tryMangleStartingDirectoryForWSL(LR"("wsl")", L"SENTINEL");
auto a08 = _tryMangleStartingDirectoryForWSL(LR"("wsl.exe")", L"SENTINEL");
auto a09 = _tryMangleStartingDirectoryForWSL(LR"("wsl" -d X)", L"SENTINEL");
auto a10 = _tryMangleStartingDirectoryForWSL(LR"("wsl.exe" -d X)", L"SENTINEL");
auto a11 = _tryMangleStartingDirectoryForWSL(LR"("C:\Windows\system32\wsl.exe" -d X)", L"SENTINEL");
auto a12 = _tryMangleStartingDirectoryForWSL(LR"("C:\windows\system32\wsl" -d X)", L"SENTINEL");
auto a13 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~/bin)", L"SENTINEL");

// MUST NOT MANGLE
auto a14 = _tryMangleStartingDirectoryForWSL(LR"("C:\wsl.exe" -d X)", L"SENTINEL");
auto a15 = _tryMangleStartingDirectoryForWSL(LR"(C:\wsl.exe)", L"SENTINEL");
auto a16 = _tryMangleStartingDirectoryForWSL(LR"(wsl --cd C:\)", L"SENTINEL");
auto a17 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~)", L"SENTINEL");
auto a18 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~ -d Ubuntu)", L"SENTINEL");
```

We don't have anywhere to put TerminalConnection unit tests :|

Closes #592.
This commit is contained in:
Dustin L. Howett 2021-08-02 15:39:11 -05:00 committed by GitHub
parent 6936ee15fe
commit a2a605050f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 80 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
src/cascadia/TerminalApp/TerminalPage.cpp
View file

@ -837,13 +837,19 @@ namespace winrt::TerminalApp::implementation
// construction, because the connection might not spawn the child
// process until later, on another thread, after we've already
// restored the CWD to it's original value.
std::wstring cwdString{ wil::GetCurrentDirectoryW<std::wstring>() };
std::filesystem::path cwd{ cwdString };
cwd /= settings.StartingDirectory().c_str();
winrt::hstring newWorkingDirectory{ settings.StartingDirectory() };
if (newWorkingDirectory.size() <= 1 ||
!(newWorkingDirectory[0] == L'~' || newWorkingDirectory[0] == L'/'))
{ // We only want to resolve the new WD against the CWD if it doesn't look like a Linux path (see GH#592)
std::wstring cwdString{ wil::GetCurrentDirectoryW<std::wstring>() };
std::filesystem::path cwd{ cwdString };
cwd /= settings.StartingDirectory().c_str();
newWorkingDirectory = winrt::hstring{ cwd.wstring() };
}
auto conhostConn = TerminalConnection::ConptyConnection();
conhostConn.Initialize(TerminalConnection::ConptyConnection::CreateSettings(settings.Commandline(),
winrt::hstring{ cwd.wstring() },
newWorkingDirectory,
settings.StartingTitle(),
envMap.GetView(),
::base::saturated_cast<uint32_t>(settings.InitialRows()),

71
src/cascadia/TerminalConnection/ConptyConnection.cpp
View file

@ -57,6 +57,72 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
return S_OK;
}
// Function Description:
// - Promotes a starting directory provided to a WSL invocation to a commandline argument.
// This is necessary because WSL has some modicum of support for linux-side directories (!) which
// CreateProcess never will.
static std::tuple<std::wstring, std::wstring> _tryMangleStartingDirectoryForWSL(std::wstring_view commandLine, std::wstring_view startingDirectory)
{
do
{
if (startingDirectory.size() > 0 && commandLine.size() >= 3)
{ // "wsl" is three characters; this is a safe bet. no point in doing it if there's no starting directory though!
// Find the first space, quote or the end of the string -- we'll look for wsl before that.
const auto terminator{ commandLine.find_first_of(LR"(" )", 1) }; // look past the first character in case it starts with "
const auto start{ til::at(commandLine, 0) == L'"' ? 1 : 0 };
const std::filesystem::path executablePath{ commandLine.substr(start, terminator - start) };
const auto executableFilename{ executablePath.filename().wstring() };
if (executableFilename == L"wsl" || executableFilename == L"wsl.exe")
{
// We've got a WSL -- let's just make sure it's the right one.
if (executablePath.has_parent_path())
{
std::wstring systemDirectory{};
if (FAILED(wil::GetSystemDirectoryW(systemDirectory)))
{
break; // just bail out.
}
if (executablePath.parent_path().wstring() != systemDirectory)
{
break; // it wasn't in system32!
}
}
else
{
// assume that unqualified WSL is the one in system32 (minor danger)
}
const auto arguments{ terminator == std::wstring_view::npos ? std::wstring_view{} : commandLine.substr(terminator + 1) };
if (arguments.find(L"--cd") != std::wstring_view::npos)
{
break; // they've already got a --cd!
}
const auto tilde{ arguments.find_first_of(L'~') };
if (tilde != std::wstring_view::npos)
{
if (tilde + 1 == arguments.size() || til::at(arguments, tilde + 1) == L' ')
{
// We want to suppress --cd if they have added a bare ~ to their commandline (they conflict).
break;
}
// Tilde followed by non-space should be okay (like, wsl -d Debian ~/blah.sh)
}
return {
fmt::format(LR"("{}" --cd "{}" {})", executablePath.wstring(), startingDirectory, arguments),
std::wstring{}
};
}
}
} while (false);
return {
std::wstring{ commandLine },
std::wstring{ startingDirectory }
};
}
// Function Description:
// - launches the client application attached to the new pseudoconsole
HRESULT ConptyConnection::_LaunchAttachedClient() noexcept
@ -163,11 +229,12 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
siEx.StartupInfo.lpTitle = mutableTitle.data();
}
const wchar_t* const startingDirectory = _startingDirectory.size() > 0 ? _startingDirectory.c_str() : nullptr;
auto [newCommandLine, newStartingDirectory] = _tryMangleStartingDirectoryForWSL(cmdline, _startingDirectory);
const wchar_t* const startingDirectory = newStartingDirectory.size() > 0 ? newStartingDirectory.c_str() : nullptr;
RETURN_IF_WIN32_BOOL_FALSE(CreateProcessW(
nullptr,
cmdline.data(),
newCommandLine.data(),
nullptr, // lpProcessAttributes
nullptr, // lpThreadAttributes
false, // bInheritHandles

7
src/cascadia/TerminalSettingsModel/Profile.cpp
View file

@ -448,11 +448,6 @@ winrt::Microsoft::Terminal::Settings::Model::FontConfig Profile::FontInfo()
// - the function returns an evaluated version of %userprofile% to avoid blocking the session from starting.
std::wstring Profile::EvaluateStartingDirectory(const std::wstring& directory)
{
// First expand path
DWORD numCharsInput = ExpandEnvironmentStrings(directory.c_str(), nullptr, 0);
std::unique_ptr<wchar_t[]> evaluatedPath = std::make_unique<wchar_t[]>(numCharsInput);
THROW_LAST_ERROR_IF(0 == ExpandEnvironmentStrings(directory.c_str(), evaluatedPath.get(), numCharsInput));
// Prior to GH#9541, we'd validate that the user's startingDirectory existed
// here. If it was invalid, we'd gracefully fall back to %USERPROFILE%.
//
@ -463,7 +458,7 @@ std::wstring Profile::EvaluateStartingDirectory(const std::wstring& directory)
//
// If the path is eventually invalid, we'll display warning in the
// ConptyConnection when the process fails to launch.
return std::wstring(evaluatedPath.get(), numCharsInput);
return wil::ExpandEnvironmentStringsW<std::wstring>(directory.c_str());
}
// Function Description: