From: Dominik Gabi <dominik@fb.com>
Date: Thu, 15 Sep 2016 14:57:13 +0000 (-0700)
Subject: deprecate `folly::Subprocess(std::string, ...)`
X-Git-Tag: v2016.09.19.00~12
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=8f27d856f0e5287b54533efe44d0797d5e50bc1e;p=folly.git

deprecate `folly::Subprocess(std::string, ...)`

Summary:
introducing `Subprocess::shellify` to get around this. Still thinking
about how to make sure people escape arguments passing into that function. I'd
like to have something along the lines of Phabricator's `csprintf` here.

Reviewed By: yfeldblum

Differential Revision: D3857827

fbshipit-source-id: 8afbc9f1c62c62e0fc91782e11b808145b370933
---

diff --git a/folly/Subprocess.cpp b/folly/Subprocess.cpp
index d7d28d9f..d6883392 100644
--- a/folly/Subprocess.cpp
+++ b/folly/Subprocess.cpp
@@ -182,17 +182,25 @@ Subprocess::Subprocess(
   if (options.usePath_) {
     throw std::invalid_argument("usePath() not allowed when running in shell");
   }
+
+  auto argv = Subprocess::shellify(cmd);
+  spawn(cloneStrings(argv), argv[0].c_str(), options, env);
+}
+
+/* static */ std::vector<std::string> Subprocess::shellify(
+    const std::string& cmd) {
+  std::vector<std::string> argv;
+
   const char* shell = getenv("SHELL");
   if (!shell) {
     shell = "/bin/sh";
   }
 
-  std::unique_ptr<const char*[]> argv(new const char*[4]);
-  argv[0] = shell;
-  argv[1] = "-c";
-  argv[2] = cmd.c_str();
-  argv[3] = nullptr;
-  spawn(std::move(argv), shell, options, env);
+  argv.push_back(shell);
+  argv.push_back("-c");
+  argv.push_back(cmd);
+
+  return argv;
 }
 
 Subprocess::~Subprocess() {
diff --git a/folly/Subprocess.h b/folly/Subprocess.h
index 8dabfc09..0300fd8e 100644
--- a/folly/Subprocess.h
+++ b/folly/Subprocess.h
@@ -456,11 +456,14 @@ class Subprocess {
    * The shell to use is taken from the environment variable $SHELL,
    * or /bin/sh if $SHELL is unset.
    */
+  FOLLY_DEPRECATED("Prefer not running in a shell or use `shellify`.")
   explicit Subprocess(
       const std::string& cmd,
       const Options& options = Options(),
       const std::vector<std::string>* env = nullptr);
 
+  static std::vector<std::string> shellify(const std::string& cmd);
+
   ////
   //// The methods below only manipulate the process state, and do not
   //// affect its communication pipes.
diff --git a/folly/test/SubprocessTest.cpp b/folly/test/SubprocessTest.cpp
index 66b18319..419db068 100644
--- a/folly/test/SubprocessTest.cpp
+++ b/folly/test/SubprocessTest.cpp
@@ -531,3 +531,9 @@ TEST(CommunicateSubprocessTest, TakeOwnershipOfPipes) {
   buf[2] = 0;
   EXPECT_EQ("3\n", std::string(buf));
 }
+
+TEST(Subprocess, Shellify) {
+  auto argv = Subprocess::shellify("rm -rf /");
+  EXPECT_EQ(argv[1], "-c");
+  EXPECT_EQ(argv[2], "rm -rf /");
+}