AuroraMiddleware/winpty
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Refactor mutable CreateProcess argument workarounds

Browse Source 
* Rename modifiableWString to vectorWithNulFromString.

 * Create a vectorFromString that omits the NUL terminator.

 * Stop triply-terminated non-empty environment blocks.

 * Stop adding an extra NUL terminator to environment blocks in
   handleStartProcessPacket.  (env.data() didn't necessarily add a
   terminator in C++03, but it does in C++11.)
This commit is contained in:
Ryan Prichard 2016-03-24 22:41:23 -05:00
parent 7baffb85b8
commit 87cfa4b83e
4 changed files with 90 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
src/agent/Agent.cc
View File

@ -313,45 +313,40 @@ void Agent::handlePacket(ReadBuffer &packet)
int Agent::handleStartProcessPacket(ReadBuffer &packet)
{
BOOL success;
ASSERT(m_childProcess == NULL);
std::wstring program = packet.getWString();
std::wstring cmdline = packet.getWString();
std::wstring cwd = packet.getWString();
std::wstring env = packet.getWString();
std::wstring desktop = packet.getWString();
const auto program = packet.getWString();
const auto cmdline = packet.getWString();
const auto cwd = packet.getWString();
const auto env = packet.getWString();
const auto desktop = packet.getWString();
packet.assertEof();
auto cmdlineV = vectorWithNulFromString(cmdline);
auto desktopV = vectorWithNulFromString(desktop);
auto envV = vectorFromString(env);
LPCWSTR programArg = program.empty() ? NULL : program.c_str();
std::vector<wchar_t> cmdlineCopy;
LPWSTR cmdlineArg = NULL;
if (!cmdline.empty()) {
cmdlineCopy.resize(cmdline.size() + 1);
cmdline.copy(&cmdlineCopy[0], cmdline.size());
cmdlineCopy[cmdline.size()] = L'\0';
cmdlineArg = &cmdlineCopy[0];
}
LPWSTR cmdlineArg = cmdline.empty() ? NULL : cmdlineV.data();
LPCWSTR cwdArg = cwd.empty() ? NULL : cwd.c_str();
LPCWSTR envArg = env.empty() ? NULL : env.data();
LPWSTR envArg = env.empty() ? NULL : envV.data();
STARTUPINFOW sui;
PROCESS_INFORMATION pi;
memset(&sui, 0, sizeof(sui));
memset(&pi, 0, sizeof(pi));
STARTUPINFOW sui = {};
PROCESS_INFORMATION pi = {};
sui.cb = sizeof(sui);
sui.lpDesktop = desktop.empty() ? NULL : (LPWSTR)desktop.c_str();
sui.lpDesktop = desktop.empty() ? NULL : desktopV.data();
success = CreateProcessW(programArg, cmdlineArg, NULL, NULL,
/*bInheritHandles=*/FALSE,
/*dwCreationFlags=*/CREATE_UNICODE_ENVIRONMENT |
/*CREATE_NEW_PROCESS_GROUP*/0,
(LPVOID)envArg, cwdArg, &sui, &pi);
int ret = success ? 0 : GetLastError();
const BOOL success =
CreateProcessW(programArg, cmdlineArg, NULL, NULL,
/*bInheritHandles=*/FALSE,
/*dwCreationFlags=*/CREATE_UNICODE_ENVIRONMENT |
/*CREATE_NEW_PROCESS_GROUP*/0,
envArg, cwdArg, &sui, &pi);
const int ret = success ? 0 : GetLastError();
trace("CreateProcess: %s %d",
trace("CreateProcess: %s %u",
(success ? "success" : "fail"),
(int)pi.dwProcessId);
static_cast<unsigned int>(pi.dwProcessId));
if (success) {
CloseHandle(pi.hThread);

84
src/libwinpty/winpty.cc
View File

@ -242,43 +242,34 @@ static std::wstring getDesktopFullName()
return getObjectName(station) + L"\\" + getObjectName(desktop);
}
static inline std::vector<wchar_t> modifiableWString(const std::wstring &str) {
std::vector<wchar_t> ret(str.size() + 1);
str.copy(ret.data(), str.size());
ret[str.size()] = L'\0';
return ret;
}
static void startAgentProcess(const BackgroundDesktop &desktop,
const std::wstring &controlPipeName,
const std::wstring &dataPipeName,
int cols, int rows)
{
bool success;
const std::wstring exePath = findAgentProgram();
const std::wstring cmdline =
(WStringBuilder(256)
<< L"\"" << exePath << L"\" "
<< controlPipeName << L' ' << dataPipeName << L' '
<< cols << L' ' << rows).str_moved();
auto cmdlineM = modifiableWString(cmdline);
auto cmdlineV = vectorWithNulFromString(cmdline);
auto desktopV = vectorWithNulFromString(desktop.desktopName);
// Start the agent.
STARTUPINFOW sui = {};
sui.cb = sizeof(sui);
auto desktopNameM = modifiableWString(desktop.desktopName);
if (desktop.station != NULL) {
sui.lpDesktop = desktopNameM.data();
}
sui.lpDesktop = desktop.station == NULL ? NULL : desktopV.data();
PROCESS_INFORMATION pi = {};
success = CreateProcessW(exePath.c_str(),
&cmdlineM[0],
NULL, NULL,
/*bInheritHandles=*/FALSE,
/*dwCreationFlags=*/CREATE_NEW_CONSOLE,
NULL, NULL,
&sui, &pi);
const BOOL success =
CreateProcessW(exePath.c_str(),
cmdlineV.data(),
NULL, NULL,
/*bInheritHandles=*/FALSE,
/*dwCreationFlags=*/CREATE_NEW_CONSOLE,
NULL, NULL,
&sui, &pi);
if (success) {
trace("Created agent successfully, pid=%u, cmdline=%s",
static_cast<unsigned int>(pi.dwProcessId),
@ -397,6 +388,42 @@ WINPTY_API winpty_t *winpty_open(int cols, int rows)
return pc;
}
// Return a std::wstring containing every character of the environment block.
// Typically, the block is non-empty, so the std::wstring returned ends with
// two NUL terminators. (These two terminators are counted in size(), so
// calling c_str() produces a triply-terminated string.)
static std::wstring wstringFromEnvBlock(const wchar_t *env)
{
std::wstring envStr;
if (env != NULL) {
const wchar_t *p = env;
while (*p != L'\0') {
p += wcslen(p) + 1;
}
p++;
envStr.assign(env, p);
// Assuming the environment was non-empty, envStr now ends with two NUL
// terminators.
//
// If the environment were empty, though, then envStr would only be
// singly terminated, but the MSDN documentation thinks an env block is
// always doubly-terminated, so add an extra NUL just in case it
// matters.
const auto envStrSz = envStr.size();
if (envStrSz == 1) {
ASSERT(envStr[0] == L'\0');
envStr.push_back(L'\0');
} else {
ASSERT(envStrSz >= 3);
ASSERT(envStr[envStrSz - 3] != L'\0');
ASSERT(envStr[envStrSz - 2] == L'\0');
ASSERT(envStr[envStrSz - 1] == L'\0');
}
}
return envStr;
}
WINPTY_API int winpty_start_process(winpty_t *pc,
const wchar_t *appname,
const wchar_t *cmdline,
@ -408,20 +435,7 @@ WINPTY_API int winpty_start_process(winpty_t *pc,
packet.putWString(appname ? appname : L"");
packet.putWString(cmdline ? cmdline : L"");
packet.putWString(cwd ? cwd : L"");
std::wstring envStr;
if (env != NULL) {
const wchar_t *p = env;
while (*p != L'\0') {
p += wcslen(p) + 1;
}
p++;
envStr.assign(env, p);
// Can a Win32 environment be empty? If so, does it end with one NUL or
// two? Add an extra NUL just in case it matters.
envStr.push_back(L'\0');
}
packet.putWString(envStr);
packet.putWString(wstringFromEnvBlock(env));
packet.putWString(getDesktopFullName());
writePacket(pc, packet);
return readInt32(pc);

2
src/shared/StringUtil.cc
View File

@ -22,8 +22,6 @@
#include <windows.h>
#include <vector>
#include "WinptyAssert.h"
// Workaround. MinGW (from mingw.org) does not have wcsnlen. MinGW-w64 *does*

18
src/shared/StringUtil.h
View File

@ -24,8 +24,26 @@
#include <stdlib.h>
#include <string>
#include <vector>
size_t winpty_wcsnlen(const wchar_t *s, size_t maxlen);
std::string utf8FromWide(const std::wstring &input);
// Return a vector containing each character in the string.
template <typename T>
std::vector<T> vectorFromString(const std::basic_string<T> &str) {
return std::vector<T>(str.begin(), str.end());
}
// Return a vector containing each character in the string, followed by a
// NUL terminator.
template <typename T>
std::vector<T> vectorWithNulFromString(const std::basic_string<T> &str) {
std::vector<T> ret;
ret.reserve(str.size() + 1);
ret.insert(ret.begin(), str.begin(), str.end());
ret.push_back('\0');
return ret;
}
#endif // STRING_UTIL_H