Merge pull request #1243 from tonyabbott/develop

GH #1222 Escape command line arguments passed to Process::launch() on…

Foundation/src/Process_WIN32.cpp

@@ -107,14 +107,66 @@ void ProcessImpl::timesImpl(long& userTime, long& kernelTime)
 }
 
 
+static bool argNeedsEscaping(const std::string& arg)
+{
+	bool containsQuotableChar = arg.npos != arg.find_first_of(" \t\n\v\"");
+	// Assume args that start and end with quotes are already quoted and do not require further quoting.
+	// There is probably code out there written before launch() escaped the arguments that does its own
+	// escaping of arguments. This ensures we do not interfere with those arguments.
+	bool isAlreadyQuoted = arg.size() > 1 && '\"' == arg[0] && '\"' == arg[arg.size() - 1];
+	return containsQuotableChar && !isAlreadyQuoted;
+}
+
+
+// Based on code from https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
+static std::string escapeArg(const std::string& arg)
+{
+	if (argNeedsEscaping(arg))
+	{
+		std::string quotedArg("\"");
+		for (auto it = arg.begin(); ; ++it)
+		{
+			unsigned backslashCount = 0;
+			while (it != arg.end() && '\\' == *it)
+			{
+				++it;
+				++backslashCount;
+			}
+
+			if (it == arg.end())
+			{
+				quotedArg.append(2 * backslashCount, '\\');
+				break;
+			}
+			else if ('"' == *it)
+			{
+				quotedArg.append(2 * backslashCount + 1, '\\');
+				quotedArg.push_back('"');
+			}
+			else
+			{
+				quotedArg.append(backslashCount, '\\');
+				quotedArg.push_back(*it);
+			}
+		}
+		quotedArg.push_back('"');
+		return quotedArg;
+	}
+	else
+	{
+		return arg;
+	}
+}
+
+
 ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const ArgsImpl& args, const std::string& initialDirectory, Pipe* inPipe, Pipe* outPipe, Pipe* errPipe, const EnvImpl& env)
 {
 	std::string commandLine = command;
 	for (ArgsImpl::const_iterator it = args.begin(); it != args.end(); ++it)
 	{
 		commandLine.append(" ");
-		commandLine.append(*it);
-	}		
+		commandLine.append(escapeArg(*it));
+	}
 
 	STARTUPINFOA startupInfo;
 	GetStartupInfoA(&startupInfo); // take defaults from current process

Foundation/src/Process_WIN32U.cpp

@@ -108,13 +108,65 @@ void ProcessImpl::timesImpl(long& userTime, long& kernelTime)
 }
 
 
+static bool argNeedsEscaping(const std::string& arg)
+{
+	bool containsQuotableChar = arg.npos != arg.find_first_of(" \t\n\v\"");
+	// Assume args that start and end with quotes are already quoted and do not require further quoting.
+	// There is probably code out there written before launch() escaped the arguments that does its own
+	// escaping of arguments. This ensures we do not interfere with those arguments.
+	bool isAlreadyQuoted = arg.size() > 1 && '\"' == arg[0] && '\"' == arg[arg.size() - 1];
+	return containsQuotableChar && !isAlreadyQuoted;
+}
+
+
+// Based on code from https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
+static std::string escapeArg(const std::string& arg)
+{
+	if (argNeedsEscaping(arg))
+	{
+		std::string quotedArg("\"");
+		for (auto it = arg.begin(); ; ++it)
+		{
+			unsigned backslashCount = 0;
+			while (it != arg.end() && '\\' == *it)
+			{
+				++it;
+				++backslashCount;
+			}
+
+			if (it == arg.end())
+			{
+				quotedArg.append(2 * backslashCount, '\\');
+				break;
+			}
+			else if ('"' == *it)
+			{
+				quotedArg.append(2 * backslashCount + 1, '\\');
+				quotedArg.push_back('"');
+			}
+			else
+			{
+				quotedArg.append(backslashCount, '\\');
+				quotedArg.push_back(*it);
+			}
+		}
+		quotedArg.push_back('"');
+		return quotedArg;
+	}
+	else
+	{
+		return arg;
+	}
+}
+
+
 ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const ArgsImpl& args, const std::string& initialDirectory, Pipe* inPipe, Pipe* outPipe, Pipe* errPipe, const EnvImpl& env)
 {
 	std::string commandLine = command;
 	for (ArgsImpl::const_iterator it = args.begin(); it != args.end(); ++it)
 	{
 		commandLine.append(" ");
-		commandLine.append(*it);
+		commandLine.append(escapeArg(*it));
 	}		
 
 	std::wstring ucommandLine;

Foundation/testsuite/src/ProcessTest.cpp

@@ -147,6 +147,66 @@ void ProcessTest::testLaunchEnv()
 }
 
 
+void ProcessTest::testLaunchArgs()
+{
+#if !defined(_WIN32_WCE)
+	std::string name("TestApp");
+	std::string cmd;
+
+#if defined(POCO_OS_FAMILY_UNIX)
+	cmd = "./";
+	cmd += name;
+#else
+	cmd = name;
+#endif
+
+	std::vector<std::string> args;
+	args.push_back("-echo-args");
+	args.push_back("simple");
+	args.push_back("with space");
+	args.push_back("with\ttab");
+	args.push_back("with\vverticaltab");
+	// can't test newline here because TestApp -echo-args uses newline to separate the echoed args
+	//args.push_back("with\nnewline");
+	args.push_back("with \" quotes");
+	args.push_back("ends with \"quotes\"");
+	args.push_back("\"starts\" with quotes");
+	args.push_back("\"");
+	args.push_back("\\");
+	args.push_back("c:\\program files\\ends with backslash\\");
+	args.push_back("\"already quoted \\\" \\\\\"");
+	Pipe outPipe;
+	ProcessHandle ph = Process::launch(cmd, args, 0, &outPipe, 0);
+	PipeInputStream istr(outPipe);
+	std::string receivedArg;
+	int c = istr.get();
+	int argNumber = 1;
+	while (c != -1)
+	{
+		if ('\n' == c)
+		{
+			assert(argNumber < args.size());
+			std::string expectedArg = args[argNumber];
+			if (expectedArg.npos != expectedArg.find("already quoted")) {
+				expectedArg = "already quoted \" \\";
+			} 
+			assert(receivedArg == expectedArg);
+			++argNumber;
+			receivedArg = "";
+		}
+		else if ('\r' != c)
+		{
+			receivedArg += (char)c;
+		}
+		c = istr.get();
+	}
+	assert(argNumber == args.size());
+	int rc = ph.wait();
+	assert(rc == args.size());
+#endif // !defined(_WIN32_WCE)
+}
+
+
 void ProcessTest::testIsRunning()
 {
 #if !defined(_WIN32_WCE)
@@ -234,6 +294,7 @@ CppUnit::Test* ProcessTest::suite()
 	CppUnit_addTest(pSuite, ProcessTest, testLaunchRedirectIn);
 	CppUnit_addTest(pSuite, ProcessTest, testLaunchRedirectOut);
 	CppUnit_addTest(pSuite, ProcessTest, testLaunchEnv);
+	CppUnit_addTest(pSuite, ProcessTest, testLaunchArgs);
 	CppUnit_addTest(pSuite, ProcessTest, testIsRunning);
 	CppUnit_addTest(pSuite, ProcessTest, testIsRunningAllowsForTermination);
 	CppUnit_addTest(pSuite, ProcessTest, testSignalExitCode);

Foundation/testsuite/src/ProcessTest.h

@@ -30,6 +30,7 @@ public:
 	void testLaunchRedirectIn();
 	void testLaunchRedirectOut();
 	void testLaunchEnv();
+	void testLaunchArgs();
 	void testIsRunning();
 	void testIsRunningAllowsForTermination();
 	void testSignalExitCode();

Foundation/testsuite/src/TestApp.cpp

@@ -52,6 +52,13 @@ int main(int argc, char** argv)
 			std::signal(SIGINT, SIG_DFL);
 			std::raise(SIGINT);
 		}
+		else if (arg == "-echo-args")
+		{
+			for (int i = 2; i < argc; ++i)
+			{
+				std::cout << argv[i] << std::endl;
+			}
+		}
 	}
 	return argc - 1;
 }