From 25dff2b7dbab54bd97b5ec22a05e594504555a12 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Wed, 15 Mar 2017 14:40:47 +0100
Subject: [PATCH] runProgram(): Distinguish between empty input and no input

For example, if we call brotli with an empty input, it shouldn't read
from the caller's stdin.
---
 src/libstore/download.cc                 |  2 +-
 src/libutil/compression.cc               |  2 +-
 src/libutil/util.cc                      | 33 +++++++++++++++++-------
 src/libutil/util.hh                      |  4 ++-
 src/nix-prefetch-url/nix-prefetch-url.cc |  4 +--
 5 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/src/libstore/download.cc b/src/libstore/download.cc
index da29b2fc6..22bde086e 100644
--- a/src/libstore/download.cc
+++ b/src/libstore/download.cc
@@ -671,7 +671,7 @@ Path Downloader::downloadCached(ref<Store> store, const string & url_, bool unpa
             Path tmpDir = createTempDir();
             AutoDelete autoDelete(tmpDir, true);
             // FIXME: this requires GNU tar for decompression.
-            runProgram("tar", true, {"xf", storePath, "-C", tmpDir, "--strip-components", "1"}, "");
+            runProgram("tar", true, {"xf", storePath, "-C", tmpDir, "--strip-components", "1"});
             unpackedStorePath = store->addToStore(name, tmpDir, true, htSHA256, defaultPathFilter, false);
         }
         replaceSymlink(unpackedStorePath, unpackedLink);
diff --git a/src/libutil/compression.cc b/src/libutil/compression.cc
index 5df97e739..8ffd55efb 100644
--- a/src/libutil/compression.cc
+++ b/src/libutil/compression.cc
@@ -92,7 +92,7 @@ static ref<std::string> decompressBzip2(const std::string & in)
 static ref<std::string> decompressBrotli(const std::string & in)
 {
     // FIXME: use libbrotli
-    return make_ref<std::string>(runProgram(BRO, true, {"-d"}, in));
+    return make_ref<std::string>(runProgram(BRO, true, {"-d"}, {in}));
 }
 
 ref<std::string> compress(const std::string & method, const std::string & in)
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index bc66b0c53..d2d32782d 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -1,6 +1,7 @@
 #include "util.hh"
 #include "affinity.hh"
 #include "sync.hh"
+#include "finally.hh"
 
 #include <cctype>
 #include <cerrno>
@@ -10,6 +11,7 @@
 #include <iostream>
 #include <sstream>
 #include <thread>
+#include <future>
 
 #include <sys/wait.h>
 #include <unistd.h>
@@ -837,23 +839,21 @@ std::vector<char *> stringsToCharPtrs(const Strings & ss)
 
 
 string runProgram(Path program, bool searchPath, const Strings & args,
-    const string & input)
+    const std::experimental::optional<std::string> & input)
 {
     checkInterrupt();
 
     /* Create a pipe. */
     Pipe out, in;
     out.create();
-    if (!input.empty()) in.create();
+    if (input) in.create();
 
     /* Fork. */
     Pid pid = startProcess([&]() {
         if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1)
             throw SysError("dupping stdout");
-        if (!input.empty()) {
-            if (dup2(in.readSide.get(), STDIN_FILENO) == -1)
-                throw SysError("dupping stdin");
-        }
+        if (input && dup2(in.readSide.get(), STDIN_FILENO) == -1)
+            throw SysError("dupping stdin");
 
         Strings args_(args);
         args_.push_front(program);
@@ -872,10 +872,23 @@ string runProgram(Path program, bool searchPath, const Strings & args,
 
     std::thread writerThread;
 
-    if (!input.empty()) {
+    std::promise<void> promise;
+
+    Finally doJoin([&]() {
+        if (writerThread.joinable())
+            writerThread.join();
+    });
+
+
+    if (input) {
         in.readSide = -1;
         writerThread = std::thread([&]() {
-            writeFull(in.writeSide.get(), input);
+            try {
+                writeFull(in.writeSide.get(), *input);
+                promise.set_value();
+            } catch (...) {
+                promise.set_exception(std::current_exception());
+            }
             in.writeSide = -1;
         });
     }
@@ -888,8 +901,8 @@ string runProgram(Path program, bool searchPath, const Strings & args,
         throw ExecError(status, format("program ‘%1%’ %2%")
             % program % statusToString(status));
 
-    if (!input.empty())
-        writerThread.join();
+    /* Wait for the writer thread to finish. */
+    if (input) promise.get_future().get();
 
     return result;
 }
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index b74c1d417..4e3a011b3 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -14,6 +14,7 @@
 #include <cstdio>
 #include <map>
 #include <sstream>
+#include <experimental/optional>
 
 #ifndef HAVE_STRUCT_DIRENT_D_TYPE
 #define DT_UNKNOWN 0
@@ -232,7 +233,8 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = P
 /* Run a program and return its stdout in a string (i.e., like the
    shell backtick operator). */
 string runProgram(Path program, bool searchPath = false,
-    const Strings & args = Strings(), const string & input = "");
+    const Strings & args = Strings(),
+    const std::experimental::optional<std::string> & input = {});
 
 class ExecError : public Error
 {
diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc
index acf603025..b3b2fcac7 100644
--- a/src/nix-prefetch-url/nix-prefetch-url.cc
+++ b/src/nix-prefetch-url/nix-prefetch-url.cc
@@ -170,10 +170,10 @@ int main(int argc, char * * argv)
                 Path unpacked = (Path) tmpDir + "/unpacked";
                 createDirs(unpacked);
                 if (hasSuffix(baseNameOf(uri), ".zip"))
-                    runProgram("unzip", true, {"-qq", tmpFile, "-d", unpacked}, "");
+                    runProgram("unzip", true, {"-qq", tmpFile, "-d", unpacked});
                 else
                     // FIXME: this requires GNU tar for decompression.
-                    runProgram("tar", true, {"xf", tmpFile, "-C", unpacked}, "");
+                    runProgram("tar", true, {"xf", tmpFile, "-C", unpacked});
 
                 /* If the archive unpacks to a single file/directory, then use
                    that as the top-level. */