From: Phil Willoughby <philwill@fb.com>
Date: Tue, 31 Jan 2017 09:37:27 +0000 (-0800)
Subject: Require compile-time constant format strings to `shellify`
X-Git-Tag: v2017.03.06.00~67
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=84d872be72aa1909dfad98b0f7644d2c157c47a9;p=folly.git

Require compile-time constant format strings to `shellify`

Summary:
Because it's impossible to require that a parameter to a function is a compile-time constant string this replaces `shellify()` with the user-defined-literal suffix `_shellify()`.

It's trivial to convert previously-correct code: `shellify("whatever {}", A)` => `"whatever {}"_shellify(A)`

The previous `folly::shellify()` API is still present as a transition measure. Compilers will issue a deprecation warning if it is used.

Reviewed By: yfeldblum

Differential Revision: D4435512

fbshipit-source-id: 6639cd91280dc72108e47a8a7775c5160a4e644f
---

diff --git a/folly/Shell.h b/folly/Shell.h
index a78692ff..0a58aa45 100644
--- a/folly/Shell.h
+++ b/folly/Shell.h
@@ -38,25 +38,56 @@ namespace folly {
  */
 std::string shellQuote(StringPiece argument);
 
+namespace detail {
+template <typename... Arguments>
+std::vector<std::string> shellify(
+    StringPiece format,
+    Arguments&&... arguments) {
+  auto command = sformat(
+      format,
+      shellQuote(to<std::string>(std::forward<Arguments>(arguments)))...);
+  return {"/bin/sh", "-c", command};
+}
+
+struct ShellCmdFormat {
+  StringPiece format;
+  template <typename... Arguments>
+  std::vector<std::string> operator()(Arguments&&... arguments) const {
+    return ::folly::detail::shellify(
+        format, std::forward<Arguments>(arguments)...);
+  }
+};
+
+} // namespace detail
+
+inline namespace literals {
+inline namespace shell_literals {
+constexpr detail::ShellCmdFormat operator"" _shellify(
+    char const* name,
+    std::size_t length) {
+  return {folly::StringPiece(name, length)};
+}
+} // inline namespace shell_literals
+} // inline namespace literals
+
 /**
   * Create argument array for `Subprocess()` for a process running in a
   * shell.
   *
   * The shell to use is always going to be `/bin/sh`.
   *
-  * The format string should always be a string literal to protect against
-  * shell injections. Arguments will automatically be escaped with `'`.
-  *
-  * TODO(dominik): find a way to ensure statically determined format strings.
+  * This is deprecated in favour of the user-defined-literal `_shellify`
+  * from namespace `folly::shell_literals` because that requires that the format
+  * string is a compile-time constant which can be inspected during code reviews
   */
 template <typename... Arguments>
+FOLLY_DEPRECATED(
+    "Use `\"command {} {} ...\"_shellify(argument1, argument2 ...)` from "
+    "namespace `folly::literals::shell_literals`")
 std::vector<std::string> shellify(
-    const StringPiece format,
+    StringPiece format,
     Arguments&&... arguments) {
-  auto command = sformat(
-      format,
-      shellQuote(to<std::string>(std::forward<Arguments>(arguments)))...);
-  return {"/bin/sh", "-c", command};
+  return detail::shellify(format, std::forward<Arguments>(arguments)...);
 }
 
 } // folly
diff --git a/folly/test/ShellTest.cpp b/folly/test/ShellTest.cpp
index 9f6cb10d..697ecd8e 100644
--- a/folly/test/ShellTest.cpp
+++ b/folly/test/ShellTest.cpp
@@ -28,6 +28,22 @@ TEST(Shell, ShellQuote) {
 }
 
 TEST(Shell, Shellify) {
+  auto command = "rm -rf /"_shellify();
+  EXPECT_EQ(command[0], "/bin/sh");
+  EXPECT_EQ(command[1], "-c");
+  EXPECT_EQ(command[2], "rm -rf /");
+
+  command = "rm -rf {}"_shellify("someFile.txt");
+  EXPECT_EQ(command[2], "rm -rf 'someFile.txt'");
+
+  command = "rm -rf {}"_shellify(5);
+  EXPECT_EQ(command[2], "rm -rf '5'");
+
+  command = "ls {}"_shellify("blah'; rm -rf /");
+  EXPECT_EQ(command[2], "ls 'blah'\\''; rm -rf /'");
+}
+
+TEST(Shell, Shellify_deprecated) {
   auto command = shellify("rm -rf /");
   EXPECT_EQ(command[0], "/bin/sh");
   EXPECT_EQ(command[1], "-c");