Fix `//wsl$` paths not working in `MangleStartingDirectoryForWSL` (#12102)

This PR does two things, which are best viewed as atomic commits:
* e64ae7d: Move the `MangleStartingDirectoryForWSL` to `types/utils`. It doesn't _really_ make sense in `types`, since it's only really being used in a single place in TerminalConnection. However, TerminalConnection doesn't have tests, and types does. So this commit move the function there, and adds tests from #9223 to the types tests. 
* 42036c5: This actually fixes the bug in #11994. Unfortunately, `wsl --cd` will try to treat paths starting with `//wsl$` as a linux-relative path, when the user almost certainly wanted a windows-relative one. So we'll mangle that back into a path that looks like `\\wsl$\foo\bar`.
* [x] closes #11994
* [x] I work here
* [x] tests added 🎉
This commit is contained in:
Mike Griese 2022-01-11 12:30:40 -06:00 committed by GitHub
parent 6b657131d1
commit b87b809fa0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 239 additions and 67 deletions

View File

@ -2391,6 +2391,7 @@ TIcon
tif
tilunittests
Timeline
timelines
titlebar
TITLEISLINKNAME
TJson

View File

@ -61,72 +61,6 @@ 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
@ -233,7 +167,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
siEx.StartupInfo.lpTitle = mutableTitle.data();
}
auto [newCommandLine, newStartingDirectory] = _tryMangleStartingDirectoryForWSL(cmdline, _startingDirectory);
auto [newCommandLine, newStartingDirectory] = Utils::MangleStartingDirectoryForWSL(cmdline, _startingDirectory);
const wchar_t* const startingDirectory = newStartingDirectory.size() > 0 ? newStartingDirectory.c_str() : nullptr;
RETURN_IF_WIN32_BOOL_FALSE(CreateProcessW(

View File

@ -95,4 +95,13 @@ namespace Microsoft::Console::Utils
GUID CreateV5Uuid(const GUID& namespaceGuid, const gsl::span<const gsl::byte> name);
bool IsElevated();
// This function is only ever used by the ConPTY connection in
// TerminalConnection. However, that library does not have a good system of
// tests set up. Since this function has a plethora of edge cases that would
// be beneficial to have tests for, we're hosting it in this lib, so it can
// be easily tested.
std::tuple<std::wstring, std::wstring> MangleStartingDirectoryForWSL(std::wstring_view commandLine,
std::wstring_view startingDirectory);
}

View File

@ -26,6 +26,8 @@ class UtilsTests
TEST_METHOD(TestStringToUint);
TEST_METHOD(TestColorFromXTermColor);
TEST_METHOD(TestMangleWSLPaths);
void _VerifyXTermColorResult(const std::wstring_view wstr, DWORD colorValue);
void _VerifyXTermColorInvalid(const std::wstring_view wstr);
};
@ -332,3 +334,145 @@ void UtilsTests::_VerifyXTermColorInvalid(const std::wstring_view wstr)
std::optional<til::color> color = ColorFromXTermColor(wstr);
VERIFY_IS_FALSE(color.has_value());
}
void UtilsTests::TestMangleWSLPaths()
{
// Continue on failures
const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope;
const auto startingDirectory{ L"SENTINEL" };
// MUST MANGLE
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl" --cd "SENTINEL" )", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl -d X)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl" --cd "SENTINEL" -d X)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl -d X ~/bin/sh)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl" --cd "SENTINEL" -d X ~/bin/sh)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl.exe)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl.exe" --cd "SENTINEL" )", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl.exe -d X)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl.exe" --cd "SENTINEL" -d X)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl.exe -d X ~/bin/sh)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl.exe" --cd "SENTINEL" -d X ~/bin/sh)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"("wsl")", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl" --cd "SENTINEL" )", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"("wsl.exe")", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl.exe" --cd "SENTINEL" )", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"("wsl" -d X)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl" --cd "SENTINEL" -d X)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"("wsl.exe" -d X)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl.exe" --cd "SENTINEL" -d X)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"("C:\Windows\system32\wsl.exe" -d X)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("C:\Windows\system32\wsl.exe" --cd "SENTINEL" -d X)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"("C:\windows\system32\wsl" -d X)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("C:\windows\system32\wsl" --cd "SENTINEL" -d X)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl ~/bin)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("wsl" --cd "SENTINEL" ~/bin)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
// MUST NOT MANGLE
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"("C:\wsl.exe" -d X)", startingDirectory);
VERIFY_ARE_EQUAL(LR"("C:\wsl.exe" -d X)", commandline);
VERIFY_ARE_EQUAL(startingDirectory, path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(C:\wsl.exe)", startingDirectory);
VERIFY_ARE_EQUAL(LR"(C:\wsl.exe)", commandline);
VERIFY_ARE_EQUAL(startingDirectory, path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl --cd C:\)", startingDirectory);
VERIFY_ARE_EQUAL(LR"(wsl --cd C:\)", commandline);
VERIFY_ARE_EQUAL(startingDirectory, path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl ~)", startingDirectory);
VERIFY_ARE_EQUAL(LR"(wsl ~)", commandline);
VERIFY_ARE_EQUAL(startingDirectory, path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl ~ -d Ubuntu)", startingDirectory);
VERIFY_ARE_EQUAL(LR"(wsl ~ -d Ubuntu)", commandline);
VERIFY_ARE_EQUAL(startingDirectory, path);
}
{
// Test for GH#11994 - make sure `//wsl$/` paths get mangled back to
// `\\wsl$\`, to workaround a potential bug in `wsl --cd`
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl -d Ubuntu)", LR"(//wsl$/Ubuntu/home/user)");
VERIFY_ARE_EQUAL(LR"("wsl" --cd "\\wsl$\Ubuntu\home\user" -d Ubuntu)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl -d Ubuntu)", LR"(\\wsl$\Ubuntu\home\user)");
VERIFY_ARE_EQUAL(LR"("wsl" --cd "\\wsl$\Ubuntu\home\user" -d Ubuntu)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
// Same, but with `wsl.localhost`
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl -d Ubuntu)", LR"(//wsl.localhost/Ubuntu/home/user)");
VERIFY_ARE_EQUAL(LR"("wsl" --cd "\\wsl.localhost\Ubuntu\home\user" -d Ubuntu)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
{
auto [commandline, path] = MangleStartingDirectoryForWSL(LR"(wsl -d Ubuntu)", LR"(\\wsl.localhost\Ubuntu\home\user)");
VERIFY_ARE_EQUAL(LR"("wsl" --cd "\\wsl.localhost\Ubuntu\home\user" -d Ubuntu)", commandline);
VERIFY_ARE_EQUAL(L"", path);
}
}

View File

@ -6,6 +6,7 @@
#include "inc/colorTable.hpp"
#include <wil/token_helpers.h>
#include <til/string.h>
using namespace Microsoft::Console;
@ -591,3 +592,86 @@ bool Utils::IsElevated()
}();
return isElevated;
}
// 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.
std::tuple<std::wstring, std::wstring> Utils::MangleStartingDirectoryForWSL(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 (!til::equals_insensitive_ascii(executablePath.parent_path().c_str(), 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)
}
// GH#11994 - If the path starts with //wsl$, then the user is
// likely passing a Windows-style path to the WSL filesystem,
// but with forward slashes instead of backslashes.
// Unfortunately, `wsl --cd` will try to treat this as a
// linux-relative path, which will fail to do the expected
// thing.
//
// In that case, manually mangle the startingDirectory to use
// backslashes as the path separator instead.
std::wstring mangledDirectory{ startingDirectory };
if (til::starts_with(mangledDirectory, L"//wsl$") || til::starts_with(mangledDirectory, L"//wsl.localhost"))
{
mangledDirectory = std::filesystem::path{ startingDirectory }.make_preferred().wstring();
}
return {
fmt::format(LR"("{}" --cd "{}" {})", executablePath.wstring(), mangledDirectory, arguments),
std::wstring{}
};
}
}
} while (false);
return {
std::wstring{ commandLine },
std::wstring{ startingDirectory }
};
}