Skip to content

Commit

Permalink
Revert "Disable path validation, add warning (#10045)"
Browse files Browse the repository at this point in the history
This reverts commit 3c30877.
  • Loading branch information
DHowett committed May 28, 2021
1 parent 646904c commit 4a3f173
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 29 deletions.
14 changes: 1 addition & 13 deletions src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,20 +294,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
// EXIT POINT
const auto hr = wil::ResultFromCaughtException();

winrt::hstring failureText{ fmt::format(std::wstring_view{ RS_(L"ProcessFailedToLaunch") },
gsl::narrow_cast<unsigned long>(hr),
_commandline) };
winrt::hstring failureText{ fmt::format(std::wstring_view{ RS_(L"ProcessFailedToLaunch") }, gsl::narrow_cast<unsigned long>(hr), _commandline) };
_TerminalOutputHandlers(failureText);

// If the path was invalid, let's present an informative message to the user
if (hr == HRESULT_FROM_WIN32(ERROR_DIRECTORY))
{
winrt::hstring badPathText{ fmt::format(std::wstring_view{ RS_(L"BadPathText") },
_startingDirectory) };
_TerminalOutputHandlers(L"\r\n");
_TerminalOutputHandlers(badPathText);
}

_transitionToState(ConnectionState::Failed);

// Tear down any state we may have accumulated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,4 @@
<comment>The first argument {0...} is the hexadecimal error code. The second argument {1} is the user-specified path to a program.
If this string is broken to multiple lines, it will not be displayed properly.</comment>
</data>
<data name="BadPathText" xml:space="preserve">
<value>Could not access starting directory "{0}"</value>
<comment>The first argument {0} is a path to a directory on the filesystem, as provided by the user.</comment>
</data>
</root>
</root>
26 changes: 15 additions & 11 deletions src/cascadia/TerminalSettingsModel/Profile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,17 +413,21 @@ std::wstring Profile::EvaluateStartingDirectory(const std::wstring& directory)
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%.
//
// However, that could cause hangs when combined with WSL. When the WSL
// filesystem is slow to respond, we'll end up waiting indefinitely for
// their filesystem driver to respond. This can result in the whole terminal
// becoming unresponsive.
//
// 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);
// Validate that the resulting path is legitimate
const DWORD dwFileAttributes = GetFileAttributes(evaluatedPath.get());
if ((dwFileAttributes != INVALID_FILE_ATTRIBUTES) && (WI_IsFlagSet(dwFileAttributes, FILE_ATTRIBUTE_DIRECTORY)))
{
return std::wstring(evaluatedPath.get(), numCharsInput);
}
else
{
// In the event where the user supplied a path that can't be resolved, use a reasonable default (in this case, %userprofile%)
const DWORD numCharsDefault = ExpandEnvironmentStrings(DEFAULT_STARTING_DIRECTORY.c_str(), nullptr, 0);
std::unique_ptr<wchar_t[]> defaultPath = std::make_unique<wchar_t[]>(numCharsDefault);
THROW_LAST_ERROR_IF(0 == ExpandEnvironmentStrings(DEFAULT_STARTING_DIRECTORY.c_str(), defaultPath.get(), numCharsDefault));

return std::wstring(defaultPath.get(), numCharsDefault);
}
}

// Function Description:
Expand Down

0 comments on commit 4a3f173

Please sign in to comment.