Merge remote-tracking branch 'sjmurdoch/bug2046'

8 files changed, 907 insertions, 89 deletions

diff --git a/configure.in b/configure.in
index 4a89df6e04..26792a6677 100644
--- a/configure.in
+++ b/configure.in
@@ -325,6 +325,7 @@ dnl Where do you live, libevent?  And how do we call you?
 
 if test "$bwin32" = true; then
   TOR_LIB_WS32=-lws2_32
+  TOR_LIB_IPHLPAPI=-liphlpapi
   # Some of the cargo-cults recommend -lwsock32 as well, but I don't
   # think it's actually necessary.
   TOR_LIB_GDI=-lgdi32
@@ -334,6 +335,7 @@ else
 fi
 AC_SUBST(TOR_LIB_WS32)
 AC_SUBST(TOR_LIB_GDI)
+AC_SUBST(TOR_LIB_IPHLPAPI)
 
 dnl We need to do this before we try our disgusting hack below.
 AC_CHECK_HEADERS([sys/types.h])
@@ -559,7 +561,7 @@ dnl There are no packages for Debian or Redhat as of this patch
 
 if test "$upnp" = "true"; then
     AC_DEFINE(MINIUPNPC, 1, [Define to 1 if we are building with UPnP.])
-    TOR_SEARCH_LIBRARY(libminiupnpc, $trylibminiupnpcdir, [-lminiupnpc],
+    TOR_SEARCH_LIBRARY(libminiupnpc, $trylibminiupnpcdir, [-lminiupnpc $TOR_LIB_WS32 $TOR_LIB_IPHLPAPI],
         [#include <miniupnpc/miniwget.h>
          #include <miniupnpc/miniupnpc.h>
          #include <miniupnpc/upnpcommands.h>],
diff --git a/src/common/util.c b/src/common/util.c
index 81d31f6026..6093662b82 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2957,6 +2957,105 @@ load_windows_system_library(const TCHAR *library_name)
 }
 #endif
 
+/* Format a single argument for being put on a Windows command line.
+ * Returns a newly allocated string */
+static char *
+format_cmdline_argument(const char *arg)
+{
+  char *formatted_arg;
+  char need_quotes;
+  const char *c;
+  int i;
+  int bs_counter = 0;
+  /* Backslash we can point to when one is inserted into the string */
+  const char backslash = '\\';
+
+  /* Smartlist of *char */
+  smartlist_t *arg_chars;
+  arg_chars = smartlist_create();
+
+  /* Quote string if it contains whitespace or is empty */
+  need_quotes = (strchr(arg, ' ') || strchr(arg, '\t') || '\0' == arg[0]);
+
+  /* Build up smartlist of *chars */
+  for (c=arg; *c != '\0'; c++) {
+    if ('"' == *c) {
+      /* Double up backslashes preceding a quote */
+      for (i=0; i<(bs_counter*2); i++)
+        smartlist_add(arg_chars, (void*)&backslash);
+      bs_counter = 0;
+      /* Escape the quote */
+      smartlist_add(arg_chars, (void*)&backslash);
+      smartlist_add(arg_chars, (void*)c);
+    } else if ('\\' == *c) {
+      /* Count backslashes until we know whether to double up */
+      bs_counter++;
+    } else {
+      /* Don't double up slashes preceding a non-quote */
+      for (i=0; i<bs_counter; i++)
+        smartlist_add(arg_chars, (void*)&backslash);
+      bs_counter = 0;
+      smartlist_add(arg_chars, (void*)c);
+    }
+  }
+  /* Don't double up trailing backslashes */
+  for (i=0; i<bs_counter; i++)
+    smartlist_add(arg_chars, (void*)&backslash);
+
+  /* Allocate space for argument, quotes (if needed), and terminator */
+  formatted_arg = tor_malloc(sizeof(char) *
+      (smartlist_len(arg_chars) + (need_quotes?2:0) + 1));
+
+  /* Add leading quote */
+  i=0;
+  if (need_quotes)
+    formatted_arg[i++] = '"';
+
+  /* Add characters */
+  SMARTLIST_FOREACH(arg_chars, char*, c,
+  {
+    formatted_arg[i++] = *c;
+  });
+
+  /* Add trailing quote */
+  if (need_quotes)
+    formatted_arg[i++] = '"';
+  formatted_arg[i] = '\0';
+
+  smartlist_free(arg_chars);
+  return formatted_arg;
+}
+
+/* Format a command line for use on Windows, which takes the command as a
+ * string rather than string array. Follows the rules from "Parsing C++
+ * Command-Line Arguments" in MSDN. Algorithm based on list2cmdline in the
+ * Python subprocess module. Returns a newly allocated string */
+char *
+tor_join_cmdline(const char *argv[])
+{
+  smartlist_t *argv_list;
+  char *joined_argv;
+  int i;
+
+  /* Format each argument and put the result in a smartlist */
+  argv_list = smartlist_create();
+  for (i=0; argv[i] != NULL; i++) {
+    smartlist_add(argv_list, (void *)format_cmdline_argument(argv[i]));
+  }
+
+  /* Join the arguments with whitespace */
+  joined_argv = smartlist_join_strings(argv_list, " ", 0, NULL);
+
+  /* Free the newly allocated arguments, and the smartlist */
+  SMARTLIST_FOREACH(argv_list, char *, arg,
+  {
+    tor_free(arg);
+  });
+  smartlist_free(argv_list);
+
+  return joined_argv;
+}
+
 /** Format <b>child_state</b> and <b>saved_errno</b> as a hex string placed in
  * <b>hex_errno</b>.  Called between fork and _exit, so must be signal-handler
  * safe.
@@ -3038,28 +3137,129 @@ format_helper_exit_status(unsigned char child_state, int saved_errno,
 
 #define SPAWN_ERROR_MESSAGE "ERR: Failed to spawn background process - code "
 
-/** Start a program in the background. If <b>filename</b> contains a '/',
- * then it will be treated as an absolute or relative path.  Otherwise the
- * system path will be searched for <b>filename</b>. The strings in
- * <b>argv</b> will be passed as the command line arguments of the child
- * program (following convention, argv[0] should normally be the filename of
- * the executable). The last element of argv must be NULL. If the child
- * program is launched, the PID will be returned and <b>stdout_read</b> and
- * <b>stdout_err</b> will be set to file descriptors from which the stdout
- * and stderr, respectively, output of the child program can be read, and the
- * stdin of the child process shall be set to /dev/null.  Otherwise returns
- * -1.  Some parts of this code are based on the POSIX subprocess module from
- * Python.
+/** Start a program in the background. If <b>filename</b> contains a '/', then
+ * it will be treated as an absolute or relative path.  Otherwise, on
+ * non-Windows systems, the system path will be searched for <b>filename</b>.
+ * On Windows, only the current directory will be searched. Here, to search the
+ * system path (as well as the application directory, current working
+ * directory, and system directories), set filename to NULL.
+ *
+ * The strings in <b>argv</b> will be passed as the command line arguments of
+ * the child program (following convention, argv[0] should normally be the
+ * filename of the executable, and this must be the case if <b>filename</b> is
+ * NULL). The last element of argv must be NULL. A handle to the child process
+ * will be returned in process_handle (which must be non-NULL). Read
+ * process_handle.status to find out if the process was successfully launched.
+ * For convenience, process_handle.status is returned by this function.
+ *
+ * Some parts of this code are based on the POSIX subprocess module from
+ * Python, and example code from
+ * http://msdn.microsoft.com/en-us/library/ms682499%28v=vs.85%29.aspx.
  */
+
 int
-tor_spawn_background(const char *const filename, int *stdout_read,
-                     int *stderr_read, const char **argv)
+tor_spawn_background(const char *const filename, const char **argv,
+                     process_handle_t *process_handle)
 {
 #ifdef MS_WINDOWS
-  (void) filename; (void) stdout_read; (void) stderr_read; (void) argv;
-  log_warn(LD_BUG, "not yet implemented on Windows.");
-  return -1;
-#else
+  HANDLE stdout_pipe_read = NULL;
+  HANDLE stdout_pipe_write = NULL;
+  HANDLE stderr_pipe_read = NULL;
+  HANDLE stderr_pipe_write = NULL;
+
+  STARTUPINFO siStartInfo;
+  BOOL retval = FALSE;
+
+  SECURITY_ATTRIBUTES saAttr;
+  char *joined_argv;
+
+  /* process_handle must not be NULL */
+  tor_assert(process_handle != NULL);
+
+  saAttr.nLength = sizeof(SECURITY_ATTRIBUTES);
+  saAttr.bInheritHandle = TRUE;
+  /* TODO: should we set explicit security attributes? (#2046, comment 5) */
+  saAttr.lpSecurityDescriptor = NULL;
+
+  /* Assume failure to start process */
+  memset(process_handle, 0, sizeof(process_handle_t));
+  process_handle->status = PROCESS_STATUS_ERROR;
+
+  /* Set up pipe for stdout */
+  if (!CreatePipe(&stdout_pipe_read, &stdout_pipe_write, &saAttr, 0)) {
+    log_warn(LD_GENERAL,
+      "Failed to create pipe for stdout communication with child process: %s",
+      format_win32_error(GetLastError()));
+    return process_handle->status;
+  }
+  if (!SetHandleInformation(stdout_pipe_read, HANDLE_FLAG_INHERIT, 0)) {
+    log_warn(LD_GENERAL,
+      "Failed to configure pipe for stdout communication with child "
+      "process: %s", format_win32_error(GetLastError()));
+    return process_handle->status;
+  }
+
+  /* Set up pipe for stderr */
+  if (!CreatePipe(&stderr_pipe_read, &stderr_pipe_write, &saAttr, 0)) {
+    log_warn(LD_GENERAL,
+      "Failed to create pipe for stderr communication with child process: %s",
+      format_win32_error(GetLastError()));
+    return process_handle->status;
+  }
+  if (!SetHandleInformation(stderr_pipe_read, HANDLE_FLAG_INHERIT, 0)) {
+    log_warn(LD_GENERAL,
+      "Failed to configure pipe for stderr communication with child "
+      "process: %s", format_win32_error(GetLastError()));
+    return process_handle->status;
+  }
+
+  /* Create the child process */
+
+  /* Windows expects argv to be a whitespace delimited string, so join argv up
+   */
+  joined_argv = tor_join_cmdline(argv);
+
+  ZeroMemory(&(process_handle->pid), sizeof(PROCESS_INFORMATION));
+  ZeroMemory(&siStartInfo, sizeof(STARTUPINFO));
+  siStartInfo.cb = sizeof(STARTUPINFO);
+  siStartInfo.hStdError = stderr_pipe_write;
+  siStartInfo.hStdOutput = stdout_pipe_write;
+  siStartInfo.hStdInput = NULL;
+  siStartInfo.dwFlags |= STARTF_USESTDHANDLES;
+
+  /* Create the child process */
+
+  retval = CreateProcess(filename,      // module name
+                 joined_argv,   // command line
+  /* TODO: should we set explicit security attributes? (#2046, comment 5) */
+                 NULL,          // process security attributes
+                 NULL,          // primary thread security attributes
+                 TRUE,          // handles are inherited
+  /*(TODO: set CREATE_NEW CONSOLE/PROCESS_GROUP to make GetExitCodeProcess()
+   * work?) */
+                 0,             // creation flags
+                 NULL,          // use parent's environment
+                 NULL,          // use parent's current directory
+                 &siStartInfo,  // STARTUPINFO pointer
+                 &(process_handle->pid));  // receives PROCESS_INFORMATION
+
+  tor_free(joined_argv);
+
+  if (!retval) {
+    log_warn(LD_GENERAL,
+      "Failed to create child process %s: %s", filename?filename:argv[0],
+      format_win32_error(GetLastError()));
+  } else  {
+    /* TODO: Close hProcess and hThread in process_handle->pid? */
+    process_handle->stdout_pipe = stdout_pipe_read;
+    process_handle->stderr_pipe = stderr_pipe_read;
+    process_handle->status = PROCESS_STATUS_RUNNING;
+  }
+
+  /* TODO: Close pipes on exit */
+
+  return process_handle->status;
+#else // MS_WINDOWS
   pid_t pid;
   int stdout_pipe[2];
   int stderr_pipe[2];
@@ -3077,6 +3277,10 @@ tor_spawn_background(const char *const filename, int *stdout_read,
 
   static int max_fd = -1;
 
+  /* Assume failure to start */
+  memset(process_handle, 0, sizeof(process_handle_t));
+  process_handle->status = PROCESS_STATUS_ERROR;
+
   /* We do the strlen here because strlen() is not signal handler safe,
      and we are not allowed to use unsafe functions between fork and exec */
   error_message_length = strlen(error_message);
@@ -3089,7 +3293,7 @@ tor_spawn_background(const char *const filename, int *stdout_read,
     log_warn(LD_GENERAL,
       "Failed to set up pipe for stdout communication with child process: %s",
        strerror(errno));
-    return -1;
+    return process_handle->status;
   }
 
   retval = pipe(stderr_pipe);
@@ -3097,7 +3301,7 @@ tor_spawn_background(const char *const filename, int *stdout_read,
     log_warn(LD_GENERAL,
       "Failed to set up pipe for stderr communication with child process: %s",
       strerror(errno));
-    return -1;
+    return process_handle->status;
   }
 
   child_state = CHILD_STATE_MAXFD;
@@ -3176,13 +3380,15 @@ tor_spawn_background(const char *const filename, int *stdout_read,
 
     /* Write the error message. GCC requires that we check the return
        value, but there is nothing we can do if it fails */
+    /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */
     nbytes = write(STDOUT_FILENO, error_message, error_message_length);
     nbytes = write(STDOUT_FILENO, hex_errno, sizeof(hex_errno));
 
     (void) nbytes;
 
     _exit(255);
-    return -1; /* Never reached, but avoids compiler warning */
+    /* Never reached, but avoids compiler warning */
+    return process_handle->status;
   }
 
   /* In parent */
@@ -3193,36 +3399,309 @@ tor_spawn_background(const char *const filename, int *stdout_read,
     close(stdout_pipe[1]);
     close(stderr_pipe[0]);
     close(stderr_pipe[1]);
-    return -1;
+    return process_handle->status;
   }
 
+  process_handle->pid = pid;
+
+  /* TODO: If the child process forked but failed to exec, waitpid it */
+
   /* Return read end of the pipes to caller, and close write end */
-  *stdout_read = stdout_pipe[0];
+  process_handle->stdout_pipe = stdout_pipe[0];
   retval = close(stdout_pipe[1]);
 
   if (-1 == retval) {
     log_warn(LD_GENERAL,
             "Failed to close write end of stdout pipe in parent process: %s",
             strerror(errno));
-    /* Do not return -1, because the child is running, so the parent
-       needs to know about the pid in order to reap it later */
   }
 
-  *stderr_read = stderr_pipe[0];
+  process_handle->stderr_pipe = stderr_pipe[0];
   retval = close(stderr_pipe[1]);
 
   if (-1 == retval) {
     log_warn(LD_GENERAL,
             "Failed to close write end of stderr pipe in parent process: %s",
             strerror(errno));
-    /* Do not return -1, because the child is running, so the parent
-       needs to know about the pid in order to reap it later */
   }
 
-  return pid;
+  process_handle->status = PROCESS_STATUS_RUNNING;
+  /* Set stdout/stderr pipes to be non-blocking */
+  fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK);
+  fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK);
+  /* Open the buffered IO streams */
+  process_handle->stdout_handle = fdopen(process_handle->stdout_pipe, "r");
+  process_handle->stderr_handle = fdopen(process_handle->stderr_pipe, "r");
+
+  return process_handle->status;
+#endif // MS_WINDOWS
+}
+
+/* Get the exit code of a process specified by <b>process_handle</b> and store
+ * it in <b>exit_code</b>, if set to a non-NULL value.  If <b>block</b> is set
+ * to true, the call will block until the process has exited.  Otherwise if
+ * the process is still running, the function will return
+ * PROCESS_EXIT_RUNNING, and exit_code will be left unchanged. Returns
+ * PROCESS_EXIT_EXITED if the process did exit. If there is a failure,
+ * PROCESS_EXIT_ERROR will be returned and the contents of exit_code (if
+ * non-NULL) will be undefined. N.B. Under *nix operating systems, this will
+ * probably not work in Tor, because waitpid() is called in main.c to reap any
+ * terminated child processes.*/
+int
+tor_get_exit_code(const process_handle_t process_handle,
+                  int block, int *exit_code)
+{
+#ifdef MS_WINDOWS
+  DWORD retval;
+  BOOL success;
+
+  if (block) {
+    /* Wait for the process to exit */
+    retval = WaitForSingleObject(process_handle.pid.hProcess, INFINITE);
+    if (retval != WAIT_OBJECT_0) {
+      log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s",
+              (int)retval, format_win32_error(GetLastError()));
+      return PROCESS_EXIT_ERROR;
+    }
+  } else {
+    retval = WaitForSingleObject(process_handle.pid.hProcess, 0);
+    if (WAIT_TIMEOUT == retval) {
+      /* Process has not exited */
+      return PROCESS_EXIT_RUNNING;
+    } else if (retval != WAIT_OBJECT_0) {
+      log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s",
+               (int)retval, format_win32_error(GetLastError()));
+      return PROCESS_EXIT_ERROR;
+    }
+  }
+
+  if (exit_code != NULL) {
+    success = GetExitCodeProcess(process_handle.pid.hProcess,
+                                 (PDWORD)exit_code);
+    if (!success) {
+      log_warn(LD_GENERAL, "GetExitCodeProcess() failed: %s",
+               format_win32_error(GetLastError()));
+      return PROCESS_EXIT_ERROR;
+    }
+  }
+#else
+  int stat_loc;
+  int retval;
+
+  retval = waitpid(process_handle.pid, &stat_loc, block?0:WNOHANG);
+  if (!block && 0 == retval) {
+    /* Process has not exited */
+    return PROCESS_EXIT_RUNNING;
+  } else if (retval != process_handle.pid) {
+    log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle.pid,
+             strerror(errno));
+    return PROCESS_EXIT_ERROR;
+  }
+
+  if (!WIFEXITED(stat_loc)) {
+    log_warn(LD_GENERAL, "Process %d did not exit normally",
+             process_handle.pid);
+    return PROCESS_EXIT_ERROR;
+  }
+
+  if (exit_code != NULL)
+    *exit_code = WEXITSTATUS(stat_loc);
+#endif // MS_WINDOWS
+
+  return PROCESS_EXIT_EXITED;
+}
+
+#ifdef MS_WINDOWS
+/** Read from a handle <b>h</b> into <b>buf</b>, up to <b>count</b> bytes.  If
+ * <b>hProcess</b> is NULL, the function will return immediately if there is
+ * nothing more to read. Otherwise <b>hProcess</b> should be set to the handle
+ * to the process owning the <b>h</b>. In this case, the function will exit
+ * only once the process has exited, or <b>count</b> bytes are read. Returns
+ * the number of bytes read, or -1 on error. */
+ssize_t
+tor_read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess)
+{
+  size_t numread = 0;
+  BOOL retval;
+  DWORD byte_count;
+  BOOL process_exited = FALSE;
+
+  if (count > SIZE_T_CEILING || count > SSIZE_T_MAX)
+    return -1;
+
+  while (numread != count) {
+    /* Check if there is anything to read */
+    retval = PeekNamedPipe(h, NULL, 0, NULL, &byte_count, NULL);
+    if (!retval) {
+      log_warn(LD_GENERAL,
+        "Failed to peek from handle: %s",
+        format_win32_error(GetLastError()));
+      return -1;
+    } else if (0 == byte_count) {
+      /* Nothing available: process exited or it is busy */
+
+      /* Exit if we don't know whether the process is running */
+      if (NULL == hProcess)
+        break;
+
+      /* The process exited and there's nothing left to read from it */
+      if (process_exited)
+        break;
+
+      /* If process is not running, check for output one more time in case
+         it wrote something after the peek was performed. Otherwise keep on
+         waiting for output */
+      byte_count = WaitForSingleObject(hProcess, 0);
+      if (WAIT_TIMEOUT != byte_count)
+        process_exited = TRUE;
+
+      continue;
+    }
+
+    /* There is data to read; read it */
+    retval = ReadFile(h, buf+numread, count-numread, &byte_count, NULL);
+    tor_assert(byte_count + numread <= count);
+    if (!retval) {
+      log_warn(LD_GENERAL, "Failed to read from handle: %s",
+        format_win32_error(GetLastError()));
+      return -1;
+    } else if (0 == byte_count) {
+      /* End of file */
+      break;
+    }
+    numread += byte_count;
+  }
+  return (ssize_t)numread;
+}
+#endif
+
+/* Read from stdout of a process until the process exits. */
+ssize_t
+tor_read_all_from_process_stdout(const process_handle_t process_handle,
+                                char *buf, size_t count)
+{
+#ifdef MS_WINDOWS
+  return tor_read_all_handle(process_handle.stdout_pipe, buf, count,
+                             process_handle.pid.hProcess);
+#else
+  return read_all(process_handle.stdout_pipe, buf, count, 0);
+#endif
+}
+
+/* Read from stdout of a process until the process exits. */
+ssize_t
+tor_read_all_from_process_stderr(const process_handle_t process_handle,
+                                 char *buf, size_t count)
+{
+#ifdef MS_WINDOWS
+  return tor_read_all_handle(process_handle.stderr_pipe, buf, count,
+                             process_handle.pid.hProcess);
+#else
+  return read_all(process_handle.stderr_pipe, buf, count, 0);
 #endif
 }
 
+/* Split buf into lines, and add to smartlist. The buffer <b>buf</b> will be
+ * modified. The resulting smartlist will consist of pointers to buf, so there
+ * is no need to free the contents of sl. <b>buf</b> must be a NULL terminated
+ * string. <b>len</b> should be set to the length of the buffer excluding the
+ * NULL. Non-printable characters (including NULL) will be replaced with "." */
+
+int
+tor_split_lines(smartlist_t *sl, char *buf, int len)
+{
+  /* Index in buf of the start of the current line */
+  int start = 0;
+  /* Index in buf of the current character being processed */
+  int cur = 0;
+  /* Are we currently in a line */
+  char in_line = 0;
+
+  /* Loop over string */
+  while (cur < len) {
+    /* Loop until end of line or end of string */
+    for (; cur < len; cur++) {
+      if (in_line) {
+        if ('\r' == buf[cur] || '\n' == buf[cur]) {
+          /* End of line */
+          buf[cur] = '\0';
+          /* Point cur to the next line */
+          cur++;
+          /* Line starts at start and ends with a null */
+          break;
+        } else {
+          if (!TOR_ISPRINT(buf[cur]))
+            buf[cur] = '.';
+        }
+      } else {
+        if ('\r' == buf[cur] || '\n' == buf[cur]) {
+          /* Skip leading vertical space */
+          ;
+        } else {
+          in_line = 1;
+          start = cur;
+          if (!TOR_ISPRINT(buf[cur]))
+            buf[cur] = '.';
+        }
+      }
+    }
+    /* We are at the end of the line or end of string. If in_line is true there
+     * is a line which starts at buf+start and ends at a NULL. cur points to
+     * the character after the NULL. */
+    if (in_line)
+      smartlist_add(sl, (void *)(buf+start));
+    in_line = 0;
+  }
+  return smartlist_len(sl);
+}
+
+#ifdef MS_WINDOWS
+/** Read from stream, and send lines to log at the specified log level.
+ * Returns -1 if there is a error reading, and 0 otherwise.
+ * If the generated stream is flushed more often than on new lines, or
+ * a read exceeds 256 bytes, lines will be truncated. This should be fixed,
+ * along with the corresponding problem on *nix (see bug #2045).
+ */
+static int
+log_from_handle(HANDLE *pipe, int severity)
+{
+  char buf[256];
+  int pos;
+  smartlist_t *lines;
+
+  pos = tor_read_all_handle(pipe, buf, sizeof(buf) - 1, NULL);
+  if (pos < 0) {
+    /* Error */
+    log_warn(LD_GENERAL, "Failed to read data from subprocess");
+    return -1;
+  }
+
+  if (0 == pos) {
+    /* There's nothing to read (process is busy or has exited) */
+    log_debug(LD_GENERAL, "Subprocess had nothing to say");
+    return 0;
+  }
+
+  /* End with a null even if there isn't a \r\n at the end */
+  /* TODO: What if this is a partial line? */
+  buf[pos] = '\0';
+  log_debug(LD_GENERAL, "Subprocess had %d bytes to say", pos);
+
+  /* Split up the buffer */
+  lines = smartlist_create();
+  tor_split_lines(lines, buf, pos);
+
+  /* Log each line */
+  SMARTLIST_FOREACH(lines, char *, line,
+  {
+    log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", line);
+  });
+  smartlist_free(lines);
+
+  return 0;
+}
+
+#else
 /** Read from stream, and send lines to log at the specified log level.
  * Returns 1 if stream is closed normally, -1 if there is a error reading, and
  * 0 otherwise. Handles lines from tor-fw-helper and
@@ -3298,26 +3777,22 @@ log_from_pipe(FILE *stream, int severity, const char *executable,
   /* We should never get here */
   return -1;
 }
+#endif
 
 void
 tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
                           time_t now)
 {
-#ifdef MS_WINDOWS
-  (void) filename; (void) dir_port; (void) or_port; (void) now;
-  (void) tor_spawn_background;
-  (void) log_from_pipe;
-  log_warn(LD_GENERAL, "Sorry, port forwarding is not yet supported "
-           "on windows.");
-#else
 /* When fw-helper succeeds, how long do we wait until running it again */
 #define TIME_TO_EXEC_FWHELPER_SUCCESS 300
-/* When fw-helper fails, how long do we wait until running it again */
+/* When fw-helper failed to start, how long do we wait until running it again
+ */
 #define TIME_TO_EXEC_FWHELPER_FAIL 60
 
-  static int child_pid = -1;
-  static FILE *stdout_read = NULL;
-  static FILE *stderr_read = NULL;
+  /* Static variables are initialized to zero, so child_handle.status=0
+   * which corresponds to it not running on startup */
+  static process_handle_t child_handle;
+
   static time_t time_to_run_helper = 0;
   int stdout_status, stderr_status, retval;
   const char *argv[10];
@@ -3342,37 +3817,48 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
   argv[9] = NULL;
 
   /* Start the child, if it is not already running */
-  if (-1 == child_pid &&
+  if (child_handle.status != PROCESS_STATUS_RUNNING &&
       time_to_run_helper < now) {
-    int fd_out=-1, fd_err=-1;
-
     /* Assume tor-fw-helper will succeed, start it later*/
     time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS;
 
-    child_pid = tor_spawn_background(filename, &fd_out, &fd_err, argv);
-    if (child_pid < 0) {
+#ifdef MS_WINDOWS
+    /* Passing NULL as lpApplicationName makes Windows search for the .exe */
+    tor_spawn_background(NULL, argv, &child_handle);
+#else
+    tor_spawn_background(filename, argv, &child_handle);
+#endif
+    if (PROCESS_STATUS_ERROR == child_handle.status) {
       log_warn(LD_GENERAL, "Failed to start port forwarding helper %s",
               filename);
-      child_pid = -1;
+      time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL;
       return;
     }
-    /* Set stdout/stderr pipes to be non-blocking */
-    fcntl(fd_out, F_SETFL, O_NONBLOCK);
-    fcntl(fd_err, F_SETFL, O_NONBLOCK);
-    /* Open the buffered IO streams */
-    stdout_read = fdopen(fd_out, "r");
-    stderr_read = fdopen(fd_err, "r");
-
+#ifdef MS_WINDOWS
     log_info(LD_GENERAL,
-      "Started port forwarding helper (%s) with pid %d", filename, child_pid);
+      "Started port forwarding helper (%s)", filename);
+#else
+    log_info(LD_GENERAL,
+      "Started port forwarding helper (%s) with pid %d", filename,
+      child_handle.pid);
+#endif
   }
 
   /* If child is running, read from its stdout and stderr) */
-  if (child_pid > 0) {
+  if (PROCESS_STATUS_RUNNING == child_handle.status) {
     /* Read from stdout/stderr and log result */
     retval = 0;
-    stdout_status = log_from_pipe(stdout_read, LOG_INFO, filename, &retval);
-    stderr_status = log_from_pipe(stderr_read, LOG_WARN, filename, &retval);
+#ifdef MS_WINDOWS
+    stdout_status = log_from_handle(child_handle.stdout_pipe, LOG_INFO);
+    stderr_status = log_from_handle(child_handle.stderr_pipe, LOG_WARN);
+    /* If we got this far (on Windows), the process started */
+    retval = 0;
+#else
+    stdout_status = log_from_pipe(child_handle.stdout_handle,
+                    LOG_INFO, filename, &retval);
+    stderr_status = log_from_pipe(child_handle.stderr_handle,
+                    LOG_WARN, filename, &retval);
+#endif
     if (retval) {
       /* There was a problem in the child process */
       time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL;
@@ -3382,9 +3868,22 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
     if (-1 == stdout_status || -1 == stderr_status)
       /* There was a failure */
       retval = -1;
+#ifdef MS_WINDOWS
+    else if (tor_get_exit_code(child_handle, 0, NULL) !=
+             PROCESS_EXIT_RUNNING) {
+      /* process has exited or there was an error */
+      /* TODO: Do something with the process return value */
+      /* TODO: What if the process output something since
+       * between log_from_handle and tor_get_exit_code? */
+      retval = 1;
+    }
+#else
     else if (1 == stdout_status || 1 == stderr_status)
-      /* stdout or stderr was closed */
+      /* stdout or stderr was closed, the process probably
+       * exited. It will be reaped by waitpid() in main.c */
+      /* TODO: Do something with the process return value */
       retval = 1;
+#endif
     else
       /* Both are fine */
       retval = 0;
@@ -3393,15 +3892,15 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
     if (0 != retval) {
       if (1 == retval) {
         log_info(LD_GENERAL, "Port forwarding helper terminated");
+        child_handle.status = PROCESS_STATUS_NOTRUNNING;
       } else {
         log_warn(LD_GENERAL, "Failed to read from port forwarding helper");
+        child_handle.status = PROCESS_STATUS_ERROR;
       }
 
       /* TODO: The child might not actually be finished (maybe it failed or
          closed stdout/stderr), so maybe we shouldn't start another? */
-      child_pid = -1;
     }
   }
-#endif
 }
 
diff --git a/src/common/util.h b/src/common/util.h
index a1def6cc3f..853ac8db5c 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -354,8 +354,47 @@ HANDLE load_windows_system_library(const TCHAR *library_name);
 
 #ifdef UTIL_PRIVATE
 /* Prototypes for private functions only used by util.c (and unit tests) */
-int tor_spawn_background(const char *const filename, int *stdout_read,
-                         int *stderr_read, const char **argv);
+
+/* Values of process_handle_t.status. PROCESS_STATUS_NOTRUNNING must be
+ * 0 because tor_check_port_forwarding depends on this being the initial
+ * statue of the static instance of process_handle_t */
+#define PROCESS_STATUS_NOTRUNNING 0
+#define PROCESS_STATUS_RUNNING 1
+#define PROCESS_STATUS_ERROR -1
+typedef struct process_handle_s {
+  int status;
+#ifdef MS_WINDOWS
+  HANDLE stdout_pipe;
+  HANDLE stderr_pipe;
+  PROCESS_INFORMATION pid;
+#else
+  int stdout_pipe;
+  int stderr_pipe;
+  FILE *stdout_handle;
+  FILE *stderr_handle;
+  pid_t pid;
+#endif // MS_WINDOWS
+} process_handle_t;
+
+int tor_spawn_background(const char *const filename, const char **argv,
+                         process_handle_t *process_handle);
+
+/* Return values of tor_get_exit_code() */
+#define PROCESS_EXIT_RUNNING 1
+#define PROCESS_EXIT_EXITED 0
+#define PROCESS_EXIT_ERROR -1
+int tor_get_exit_code(const process_handle_t process_handle,
+                      int block, int *exit_code);
+int tor_split_lines(struct smartlist_t *sl, char *buf, int len);
+#ifdef MS_WINDOWS
+ssize_t tor_read_all_handle(HANDLE h, char *buf, size_t count,
+                            HANDLE hProcess);
+#endif
+ssize_t tor_read_all_from_process_stdout(const process_handle_t process_handle,
+                                        char *buf, size_t count);
+ssize_t tor_read_all_from_process_stderr(const process_handle_t process_handle,
+                                         char *buf, size_t count);
+char *tor_join_cmdline(const char *argv[]);
 void format_helper_exit_status(unsigned char child_state,
                                int saved_errno, char *hex_errno);
 
diff --git a/src/test/test-child.c b/src/test/test-child.c
index ca52750c2f..1b9c5e3d57 100644
--- a/src/test/test-child.c
+++ b/src/test/test-child.c
@@ -1,4 +1,11 @@
 #include <stdio.h>
+#include "orconfig.h"
+#ifdef MS_WINDOWS
+#define WINDOWS_LEAN_AND_MEAN
+#include <windows.h>
+#else
+#include <unistd.h>
+#endif
 
 /** Trivial test program which prints out its command line arguments so we can
  * check if tor_spawn_background() works */
@@ -11,7 +18,22 @@ main(int argc, char **argv)
   fprintf(stderr, "ERR\n");
   for (i = 1; i < argc; i++)
     fprintf(stdout, "%s\n", argv[i]);
+  fprintf(stdout, "SLEEPING\n");
+  /* We need to flush stdout so that test_util_spawn_background_partial_read()
+     succeed. Otherwise ReadFile() will get the entire output in one */
+  // XXX: Can we make stdio flush on newline?
+  fflush(stdout);
+#ifdef MS_WINDOWS
+  Sleep(1000);
+#else
+  sleep(1);
+#endif
   fprintf(stdout, "DONE\n");
+#ifdef MS_WINDOWS
+  Sleep(1000);
+#else
+  sleep(1);
+#endif
 
   return 0;
 }
diff --git a/src/test/test_util.c b/src/test/test_util.c
index c4769e6407..a3bc003b2c 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -1376,46 +1376,54 @@ test_util_fgets_eagain(void *ptr)
 }
 #endif
 
-#ifndef MS_WINDOWS
 /** Helper function for testing tor_spawn_background */
 static void
 run_util_spawn_background(const char *argv[], const char *expected_out,
-                          const char *expected_err, int expected_exit)
+                          const char *expected_err, int expected_exit,
+                          int expected_status)
 {
-  int stdout_pipe=-1, stderr_pipe=-1;
-  int retval, stat_loc;
-  pid_t pid;
+  int retval, exit_code;
   ssize_t pos;
+  process_handle_t process_handle;
   char stdout_buf[100], stderr_buf[100];
 
   /* Start the program */
-  retval = tor_spawn_background(argv[0], &stdout_pipe, &stderr_pipe, argv);
-  tt_int_op(retval, >, 0);
-  tt_int_op(stdout_pipe, >, 0);
-  tt_int_op(stderr_pipe, >, 0);
-  pid = retval;
+#ifdef MS_WINDOWS
+  tor_spawn_background(NULL, argv, &process_handle);
+#else
+  tor_spawn_background(argv[0], argv, &process_handle);
+#endif
+
+  tt_int_op(process_handle.status, ==, expected_status);
+
+  /* If the process failed to start, don't bother continuing */
+  if (process_handle.status == PROCESS_STATUS_ERROR)
+    return;
+
+  tt_int_op(process_handle.stdout_pipe, >, 0);
+  tt_int_op(process_handle.stderr_pipe, >, 0);
 
   /* Check stdout */
-  pos = read_all(stdout_pipe, stdout_buf, sizeof(stdout_buf) - 1, 0);
+  pos = tor_read_all_from_process_stdout(process_handle, stdout_buf,
+                                        sizeof(stdout_buf) - 1);
   tt_assert(pos >= 0);
   stdout_buf[pos] = '\0';
-  tt_int_op(pos, ==, strlen(expected_out));
   tt_str_op(stdout_buf, ==, expected_out);
+  tt_int_op(pos, ==, strlen(expected_out));
 
   /* Check it terminated correctly */
-  retval = waitpid(pid, &stat_loc, 0);
-  tt_int_op(retval, ==, pid);
-  tt_assert(WIFEXITED(stat_loc));
-  tt_int_op(WEXITSTATUS(stat_loc), ==, expected_exit);
-  tt_assert(!WIFSIGNALED(stat_loc));
-  tt_assert(!WIFSTOPPED(stat_loc));
+  retval = tor_get_exit_code(process_handle, 1, &exit_code);
+  tt_int_op(retval, ==, PROCESS_EXIT_EXITED);
+  tt_int_op(exit_code, ==, expected_exit);
+  // TODO: Make test-child exit with something other than 0
 
   /* Check stderr */
-  pos = read_all(stderr_pipe, stderr_buf, sizeof(stderr_buf) - 1, 0);
+  pos = tor_read_all_from_process_stderr(process_handle, stderr_buf,
+                                         sizeof(stderr_buf) - 1);
   tt_assert(pos >= 0);
   stderr_buf[pos] = '\0';
-  tt_int_op(pos, ==, strlen(expected_err));
   tt_str_op(stderr_buf, ==, expected_err);
+  tt_int_op(pos, ==, strlen(expected_err));
 
  done:
   ;
@@ -1425,30 +1433,239 @@ run_util_spawn_background(const char *argv[], const char *expected_out,
 static void
 test_util_spawn_background_ok(void *ptr)
 {
+#ifdef MS_WINDOWS
+  const char *argv[] = {"test-child.exe", "--test", NULL};
+  const char *expected_out = "OUT\r\n--test\r\nSLEEPING\r\nDONE\r\n";
+  const char *expected_err = "ERR\r\n";
+#else
   const char *argv[] = {BUILDDIR "/src/test/test-child", "--test", NULL};
-  const char *expected_out = "OUT\n--test\nDONE\n";
+  const char *expected_out = "OUT\n--test\nSLEEPING\nDONE\n";
   const char *expected_err = "ERR\n";
+#endif
 
   (void)ptr;
 
-  run_util_spawn_background(argv, expected_out, expected_err, 0);
+  run_util_spawn_background(argv, expected_out, expected_err, 0,
+                            PROCESS_STATUS_RUNNING);
 }
 
 /** Check that failing to find the executable works as expected */
 static void
 test_util_spawn_background_fail(void *ptr)
 {
+#ifdef MS_WINDOWS
   const char *argv[] = {BUILDDIR "/src/test/no-such-file", "--test", NULL};
   const char *expected_out = "ERR: Failed to spawn background process "
                              "- code          9/2\n";
   const char *expected_err = "";
+  const int expected_status = PROCESS_STATUS_ERROR;
+#else
+  const char *argv[] = {BUILDDIR "/src/test/no-such-file", "--test", NULL};
+  const char *expected_out = "ERR: Failed to spawn background process "
+                             "- code          9/2\n";
+  const char *expected_err = "";
+  /* TODO: Once we can signal failure to exec, set this to be
+   * PROCESS_STATUS_ERROR */
+  const int expected_status = PROCESS_STATUS_RUNNING;
+#endif
 
   (void)ptr;
 
-  run_util_spawn_background(argv, expected_out, expected_err, 255);
+  run_util_spawn_background(argv, expected_out, expected_err, 255,
+                            expected_status);
 }
+
+/** Test that reading from a handle returns a partial read rather than
+ * blocking */
+static void
+test_util_spawn_background_partial_read(void *ptr)
+{
+  const int expected_exit = 0;
+  const int expected_status = PROCESS_STATUS_RUNNING;
+
+  int retval, exit_code;
+  ssize_t pos;
+  process_handle_t process_handle;
+  char stdout_buf[100], stderr_buf[100];
+#ifdef MS_WINDOWS
+  const char *argv[] = {"test-child.exe", "--test", NULL};
+  const char *expected_out[] = { "OUT\r\n--test\r\nSLEEPING\r\n",
+                                 "DONE\r\n",
+                                 NULL };
+  const char *expected_err = "ERR\r\n";
+  int expected_out_ctr;
+#else
+  const char *argv[] = {BUILDDIR "/src/test/test-child", "--test", NULL};
+  const char *expected_out = "OUT\n--test\nSLEEPING\nDONE\n";
+  const char *expected_err = "ERR\r\n";
+#endif
+  (void)ptr;
+
+  /* Start the program */
+  tor_spawn_background(NULL, argv, &process_handle);
+  tt_int_op(process_handle.status, ==, expected_status);
+
+  /* Check stdout */
+#ifdef MS_WINDOWS
+  for (expected_out_ctr =0; expected_out[expected_out_ctr] != NULL;) {
+    pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf,
+                              sizeof(stdout_buf) - 1, NULL);
+    log_info(LD_GENERAL, "tor_read_all_handle() returned %d", (int)pos);
+
+    /* We would have blocked, keep on trying */
+    if (0 == pos)
+      continue;
+
+    tt_assert(pos >= 0);
+    stdout_buf[pos] = '\0';
+    tt_str_op(stdout_buf, ==, expected_out[expected_out_ctr]);
+    tt_int_op(pos, ==, strlen(expected_out[expected_out_ctr]));
+    expected_out_ctr++;
+  }
+  /* The process should have exited without writing more */
+  pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf,
+                            sizeof(stdout_buf) - 1,
+                            process_handle.pid.hProcess);
+  tt_int_op(pos, ==, 0);
+#else
+  pos = tor_read_all_from_process_stdout(process_handle, stdout_buf,
+                                         sizeof(stdout_buf) - 1);
+  tt_assert(pos >= 0);
+  stdout_buf[pos] = '\0';
+  tt_str_op(stdout_buf, ==, expected_out);
+  tt_int_op(pos, ==, strlen(expected_out));
 #endif
 
+  /* Check it terminated correctly */
+  retval = tor_get_exit_code(process_handle, 1, &exit_code);
+  tt_int_op(retval, ==, PROCESS_EXIT_EXITED);
+  tt_int_op(exit_code, ==, expected_exit);
+  // TODO: Make test-child exit with something other than 0
+
+  /* Check stderr */
+  pos = tor_read_all_from_process_stderr(process_handle, stderr_buf,
+                                         sizeof(stderr_buf) - 1);
+  tt_assert(pos >= 0);
+  stderr_buf[pos] = '\0';
+  tt_str_op(stderr_buf, ==, expected_err);
+  tt_int_op(pos, ==, strlen(expected_err));
+
+ done:
+  ;
+}
+
+/**
+ * Test that we can properly format q Windows command line
+ */
+static void
+test_util_join_cmdline(void *ptr)
+{
+  /* Based on some test cases from "Parsing C++ Command-Line Arguments" in MSDN
+   * but we don't exercise all quoting rules because tor_join_cmdline will try
+   * to only generate simple cases for the child process to parse; i.e. we
+   * never embed quoted strings in arguments. */
+
+  const char *argvs[][4] = {
+    {"a", "bb", "CCC", NULL}, // Normal
+    {NULL, NULL, NULL, NULL}, // Empty argument list
+    {"", NULL, NULL, NULL}, // Empty argument
+    {"\"a", "b\"b", "CCC\"", NULL}, // Quotes
+    {"a\tbc", "dd  dd", "E", NULL}, // Whitespace
+    {"a\\\\\\b", "de fg", "H", NULL}, // Backslashes
+    {"a\\\"b", "\\c", "D\\", NULL}, // Backslashes before quote
+    {"a\\\\b c", "d", "E", NULL}, // Backslashes not before quote
+    {} // Terminator
+  };
+
+  const char *cmdlines[] = {
+    "a bb CCC",
+    "",
+    "\"\"",
+    "\\\"a b\\\"b CCC\\\"",
+    "\"a\tbc\" \"dd  dd\" E",
+    "a\\\\\\b \"de fg\" H",
+    "a\\\\\\\"b \\c D\\",
+    "\"a\\\\b c\" d E",
+    NULL // Terminator
+  };
+
+  int i;
+  char *joined_argv;
+
+  (void)ptr;
+
+  for (i=0; cmdlines[i]!=NULL; i++) {
+    log_info(LD_GENERAL, "Joining argvs[%d], expecting <%s>", i, cmdlines[i]);
+    joined_argv = tor_join_cmdline(argvs[i]);
+    tt_str_op(joined_argv, ==, cmdlines[i]);
+    tor_free(joined_argv);
+  }
+
+ done:
+  ;
+}
+
+#define MAX_SPLIT_LINE_COUNT 3
+struct split_lines_test_t {
+  const char *orig_line; // Line to be split (may contain \0's)
+  int orig_length; // Length of orig_line
+  const char *split_line[MAX_SPLIT_LINE_COUNT]; // Split lines
+};
+
+/**
+ * Test that we properly split a buffer into lines
+ */
+static void
+test_util_split_lines(void *ptr)
+{
+  /* Test cases. orig_line of last test case must be NULL.
+   * The last element of split_line[i] must be NULL. */
+  struct split_lines_test_t tests[] = {
+    {"", 0, {NULL}},
+    {"foo", 3, {"foo", NULL}},
+    {"\n\rfoo\n\rbar\r\n", 12, {"foo", "bar", NULL}},
+    {"fo o\r\nb\tar", 10, {"fo o", "b.ar", NULL}},
+    {"\x0f""f\0o\0\n\x01""b\0r\0\r", 12, {".f.o.", ".b.r.", NULL}},
+    {NULL, 0, {}}
+  };
+
+  int i, j;
+  char *orig_line;
+  smartlist_t *sl;
+
+  (void)ptr;
+
+  for (i=0; tests[i].orig_line; i++) {
+    sl = smartlist_create();
+    orig_line = tor_malloc(tests[i].orig_length);
+    memcpy(orig_line, tests[i].orig_line, tests[i].orig_length + 1);
+    tor_split_lines(sl, orig_line, tests[i].orig_length);
+
+    j = 0;
+    log_info(LD_GENERAL, "Splitting test %d of length %d",
+             i, tests[i].orig_length);
+    SMARTLIST_FOREACH(sl, const char *, line,
+    {
+      /* Check we have not got too many lines */
+      tt_int_op(j, <, MAX_SPLIT_LINE_COUNT);
+      /* Check that there actually should be a line here */
+      tt_assert(tests[i].split_line[j] != NULL);
+      log_info(LD_GENERAL, "Line %d of test %d, should be <%s>",
+               j, i, tests[i].split_line[j]);
+      /* Check that the line is as expected */
+      tt_str_op(tests[i].split_line[j], ==, line);
+      j++;
+    });
+    /* Check that we didn't miss some lines */
+    tt_assert(tests[i].split_line[j] == NULL);
+    tor_free(orig_line);
+    smartlist_free(sl);
+  }
+
+ done:
+  ;
+}
+
 static void
 test_util_di_ops(void)
 {
@@ -1533,9 +1750,12 @@ struct testcase_t util_tests[] = {
   UTIL_TEST(exit_status, 0),
 #ifndef MS_WINDOWS
   UTIL_TEST(fgets_eagain, TT_SKIP),
+#endif
   UTIL_TEST(spawn_background_ok, 0),
   UTIL_TEST(spawn_background_fail, 0),
-#endif
+  UTIL_TEST(spawn_background_partial_read, 0),
+  UTIL_TEST(join_cmdline, 0),
+  UTIL_TEST(split_lines, 0),
   END_OF_TESTCASES
 };
 
diff --git a/src/tools/tor-fw-helper/Makefile.am b/src/tools/tor-fw-helper/Makefile.am
index 77ff63fc36..8f64ad2ba3 100644
--- a/src/tools/tor-fw-helper/Makefile.am
+++ b/src/tools/tor-fw-helper/Makefile.am
@@ -25,7 +25,7 @@ endif
 
 if MINIUPNPC
 miniupnpc_ldflags = @TOR_LDFLAGS_libminiupnpc@
-miniupnpc_ldadd = -lminiupnpc -lm
+miniupnpc_ldadd = -lminiupnpc -lm @TOR_LIB_IPHLPAPI@
 miniupnpc_cppflags = @TOR_CPPFLAGS_libminiupnpc@
 else
 miniupnpc_ldflags =
diff --git a/src/tools/tor-fw-helper/tor-fw-helper-upnp.c b/src/tools/tor-fw-helper/tor-fw-helper-upnp.c
index 18ca56394f..c4b14a84e2 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper-upnp.c
+++ b/src/tools/tor-fw-helper/tor-fw-helper-upnp.c
@@ -9,6 +9,9 @@
 
 #include "orconfig.h"
 #ifdef MINIUPNPC
+#ifdef MS_WINDOWS
+#define STATICLIB
+#endif
 #include <stdint.h>
 #include <string.h>
 #include <stdio.h>
diff --git a/src/tools/tor-fw-helper/tor-fw-helper.c b/src/tools/tor-fw-helper/tor-fw-helper.c
index 20d60d7ba6..002239745a 100644
--- a/src/tools/tor-fw-helper/tor-fw-helper.c
+++ b/src/tools/tor-fw-helper/tor-fw-helper.c
@@ -13,6 +13,7 @@
  * later date.
  */
 
+#include "orconfig.h"
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -20,7 +21,10 @@
 #include <time.h>
 #include <string.h>
 
-#include "orconfig.h"
+#ifdef MS_WINDOWS
+#include <winsock2.h>
+#endif
+
 #include "tor-fw-helper.h"
 #ifdef NAT_PMP
 #include "tor-fw-helper-natpmp.h"
@@ -219,6 +223,30 @@ tor_fw_add_dir_port(tor_fw_options_t *tor_fw_options,
   }
 }
 
+/** Called before we make any calls to network-related functions.
+ * (Some operating systems require their network libraries to be
+ * initialized.) (from common/compat.c) */
+static int
+network_init(void)
+{
+#ifdef MS_WINDOWS
+  /* This silly exercise is necessary before windows will allow
+   * gethostbyname to work. */
+  WSADATA WSAData;
+  int r;
+  r = WSAStartup(0x101, &WSAData);
+  if (r) {
+    fprintf(stderr, "E: Error initializing Windows network layer "
+            "- code was %d", r);
+    return -1;
+  }
+  /* WSAData.iMaxSockets might show the max sockets we're allowed to use.
+   * We might use it to complain if we're trying to be a server but have
+   * too few sockets available. */
+#endif
+  return 0;
+}
+
 int
 main(int argc, char **argv)
 {
@@ -229,6 +257,7 @@ main(int argc, char **argv)
   backends_t backend_state;
 
   memset(&tor_fw_options, 0, sizeof(tor_fw_options));
+  memset(&backend_state, 0, sizeof(backend_state));
 
   while (1) {
     int option_index = 0;
@@ -329,6 +358,10 @@ main(int argc, char **argv)
             tor_fw_options.public_dir_port);
   }
 
+  // Initialize networking
+  if (network_init())
+    exit(1);
+
   // Initalize the various fw-helper backend helpers
   r = init_backends(&tor_fw_options, &backend_state);
   if (r)