Merge remote-tracking branch 'ahf/bugs/21654'

11 files changed, 138 insertions, 216 deletions

diff --git a/changes/bug20988 b/changes/bug20988
deleted file mode 100644
index b1d73f08e7..0000000000
--- a/changes/bug20988
+++ /dev/null
@@ -1,4 +0,0 @@
-  o Minor bugfixes (portability):
-    - Add Tor compatibility function for fgets(3) due to inconsistency of
-      returned values in different supported C libraries. This fixes unit test
-      failures reported on FreeBSD. Fixes bug 20988.
diff --git a/changes/bug21654 b/changes/bug21654
new file mode 100644
index 0000000000..fd1c650710
--- /dev/null
+++ b/changes/bug21654
@@ -0,0 +1,4 @@
+  o Code simplifications and refactoring
+    - Use unbuffered I/O for utility functions around the process_handle_t
+      type. This fixes unit test failures reported on OpenBSD and FreeBSD.
+      Fixes bug 21654.
diff --git a/src/common/compat.c b/src/common/compat.c
index da4283fbaa..0dbede6bed 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -3476,45 +3476,6 @@ tor_getpass(const char *prompt, char *output, size_t buflen)
 #endif
 }
 
-/** Read at most one less than the number of characters specified by
- * <b>size</b> from the given <b>stream</b> and store it in <b>str</b>.
- *
- * Reading stops when a newline character is found or at EOF or error. If any
- * characters are read and there's no error, a trailing NUL byte is appended to
- * the end of <b>str</b>.
- *
- * Upon successful completion, this function returns a pointer to the string
- * <b>str</b>. If EOF occurs before any characters are read the function will
- * return NULL and the content of <b>str</b> is unchanged. Upon error, the
- * function returns NULL and the caller must check for error using foef(3) and
- * ferror(3).
- */
-char *
-tor_fgets(char *str, int size, FILE *stream)
-{
-  char *ret;
-
-  /* Reset errno before our call to fgets(3) to avoid a situation where the
-   * caller is calling us again because we just returned NULL and errno ==
-   * EAGAIN, but when they call us again we will always return NULL because the
-   * error flag on the file handler remains set and errno is set to EAGAIN.
-   */
-  errno = 0;
-
-  ret = fgets(str, size, stream);
-
-  /* FreeBSD, OpenBSD, Linux (glibc), and Linux (musl) seem to disagree about
-   * what to do in the given situation. We check if the stream has been flagged
-   * with an error-bit and return NULL in that situation if errno is also set
-   * to EAGAIN.
-   */
-  if (ferror(stream) && errno == EAGAIN) {
-    return NULL;
-  }
-
-  return ret;
-}
-
 /** Return the amount of free disk space we have permission to use, in
  * bytes. Return -1 if the amount of free space can't be determined. */
 int64_t
diff --git a/src/common/compat.h b/src/common/compat.h
index 1f51ece61f..ee1c9454de 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -740,8 +740,6 @@ STATIC int tor_ersatz_socketpair(int family, int type, int protocol,
 
 ssize_t tor_getpass(const char *prompt, char *output, size_t buflen);
 
-char *tor_fgets(char *str, int size, FILE *stream);
-
 /* This needs some of the declarations above so we include it here. */
 #include "compat_threads.h"
 
diff --git a/src/common/util.c b/src/common/util.c
index a69d887e30..a8b44cbda7 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2118,7 +2118,7 @@ read_all(tor_socket_t fd, char *buf, size_t count, int isSocket)
     return -1;
   }
 
-  while (numread != count) {
+  while (numread < count) {
     if (isSocket)
       result = tor_socket_recv(fd, buf+numread, count-numread, 0);
     else
@@ -4175,10 +4175,10 @@ tor_process_get_stdout_pipe(process_handle_t *process_handle)
 }
 #else
 /* DOCDOC tor_process_get_stdout_pipe */
-FILE *
+int
 tor_process_get_stdout_pipe(process_handle_t *process_handle)
 {
-  return process_handle->stdout_handle;
+  return process_handle->stdout_pipe;
 }
 #endif
 
@@ -4609,10 +4609,6 @@ tor_spawn_background(const char *const filename, const char **argv,
     log_warn(LD_GENERAL, "Failed to set stderror/stdout/stdin pipes "
              "nonblocking in parent process: %s", strerror(errno));
   }
-  /* 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");
-  process_handle->stdin_handle = fdopen(process_handle->stdin_pipe, "r");
 
   *process_handle_out = process_handle;
   return process_handle->status;
@@ -4659,14 +4655,9 @@ tor_process_handle_destroy,(process_handle_t *process_handle,
   if (process_handle->stdin_pipe)
     CloseHandle(process_handle->stdin_pipe);
 #else
-  if (process_handle->stdout_handle)
-    fclose(process_handle->stdout_handle);
-
-  if (process_handle->stderr_handle)
-    fclose(process_handle->stderr_handle);
-
-  if (process_handle->stdin_handle)
-    fclose(process_handle->stdin_handle);
+  close(process_handle->stdout_pipe);
+  close(process_handle->stderr_pipe);
+  close(process_handle->stdin_pipe);
 
   clear_waitpid_callback(process_handle->waitpid_cb);
 #endif
@@ -4952,7 +4943,7 @@ tor_read_all_handle(HANDLE h, char *buf, size_t count,
   if (count > SIZE_T_CEILING || count > SSIZE_MAX)
     return -1;
 
-  while (numread != count) {
+  while (numread < count) {
     /* Check if there is anything to read */
     retval = PeekNamedPipe(h, NULL, 0, NULL, &byte_count, NULL);
     if (!retval) {
@@ -4998,19 +4989,19 @@ tor_read_all_handle(HANDLE h, char *buf, size_t count,
   return (ssize_t)numread;
 }
 #else
-/** Read from a handle <b>h</b> into <b>buf</b>, up to <b>count</b> bytes.  If
+/** Read from a handle <b>fd</b> into <b>buf</b>, up to <b>count</b> bytes.  If
  * <b>process</b> is NULL, the function will return immediately if there is
  * nothing more to read. Otherwise data will be read until end of file, or
  * <b>count</b> bytes are read.  Returns the number of bytes read, or -1 on
  * error. Sets <b>eof</b> to true if <b>eof</b> is not NULL and the end of the
  * file has been reached. */
 ssize_t
-tor_read_all_handle(FILE *h, char *buf, size_t count,
+tor_read_all_handle(int fd, char *buf, size_t count,
                     const process_handle_t *process,
                     int *eof)
 {
   size_t numread = 0;
-  char *retval;
+  ssize_t result;
 
   if (eof)
     *eof = 0;
@@ -5018,34 +5009,28 @@ tor_read_all_handle(FILE *h, char *buf, size_t count,
   if (count > SIZE_T_CEILING || count > SSIZE_MAX)
     return -1;
 
-  while (numread != count) {
-    /* Use fgets because that is what we use in log_from_pipe() */
-    retval = tor_fgets(buf+numread, (int)(count-numread), h);
-    if (NULL == retval) {
-      if (feof(h)) {
-        log_debug(LD_GENERAL, "fgets() reached end of file");
-        if (eof)
-          *eof = 1;
+  while (numread < count) {
+    result = read(fd, buf+numread, count-numread);
+
+    if (result == 0) {
+      log_debug(LD_GENERAL, "read() reached end of file");
+      if (eof)
+        *eof = 1;
+      break;
+    } else if (result < 0 && errno == EAGAIN) {
+      if (process)
+        continue;
+      else
         break;
-      } else {
-        if (EAGAIN == errno) {
-          if (process)
-            continue;
-          else
-            break;
-        } else {
-          log_warn(LD_GENERAL, "fgets() from handle failed: %s",
-                   strerror(errno));
-          return -1;
-        }
-      }
+    } else if (result < 0) {
+      log_warn(LD_GENERAL, "read() failed: %s", strerror(errno));
+      return -1;
     }
-    tor_assert(retval != NULL);
-    tor_assert(strlen(retval) + numread <= count);
-    numread += strlen(retval);
+
+    numread += result;
   }
 
-  log_debug(LD_GENERAL, "fgets() read %d bytes from handle", (int)numread);
+  log_debug(LD_GENERAL, "read() read %d bytes from handle", (int)numread);
   return (ssize_t)numread;
 }
 #endif
@@ -5059,7 +5044,7 @@ tor_read_all_from_process_stdout(const process_handle_t *process_handle,
   return tor_read_all_handle(process_handle->stdout_pipe, buf, count,
                              process_handle);
 #else
-  return tor_read_all_handle(process_handle->stdout_handle, buf, count,
+  return tor_read_all_handle(process_handle->stdout_pipe, buf, count,
                              process_handle, NULL);
 #endif
 }
@@ -5073,7 +5058,7 @@ tor_read_all_from_process_stderr(const process_handle_t *process_handle,
   return tor_read_all_handle(process_handle->stderr_pipe, buf, count,
                              process_handle);
 #else
-  return tor_read_all_handle(process_handle->stderr_handle, buf, count,
+  return tor_read_all_handle(process_handle->stderr_pipe, buf, count,
                              process_handle, NULL);
 #endif
 }
@@ -5267,11 +5252,10 @@ log_from_handle(HANDLE *pipe, int severity)
 #else
 
 /** Return a smartlist containing lines outputted from
- *  <b>handle</b>. Return NULL on error, and set
+ *  <b>fd</b>. Return NULL on error, and set
  *  <b>stream_status_out</b> appropriately. */
 MOCK_IMPL(smartlist_t *,
-tor_get_lines_from_handle, (FILE *handle,
-                            enum stream_status *stream_status_out))
+tor_get_lines_from_handle, (int fd, enum stream_status *stream_status_out))
 {
   enum stream_status stream_status;
   char stdout_buf[400];
@@ -5280,7 +5264,7 @@ tor_get_lines_from_handle, (FILE *handle,
   while (1) {
     memset(stdout_buf, 0, sizeof(stdout_buf));
 
-    stream_status = get_string_from_pipe(handle,
+    stream_status = get_string_from_pipe(fd,
                                          stdout_buf, sizeof(stdout_buf) - 1);
     if (stream_status != IO_STREAM_OKAY)
       goto done;
@@ -5294,20 +5278,20 @@ tor_get_lines_from_handle, (FILE *handle,
   return lines;
 }
 
-/** Read from stream, and send lines to log at the specified log level.
+/** Read from fd, 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
  * tor_spawn_background() specially.
  */
 static int
-log_from_pipe(FILE *stream, int severity, const char *executable,
+log_from_pipe(int fd, int severity, const char *executable,
               int *child_status)
 {
   char buf[256];
   enum stream_status r;
 
   for (;;) {
-    r = get_string_from_pipe(stream, buf, sizeof(buf) - 1);
+    r = get_string_from_pipe(fd, buf, sizeof(buf) - 1);
 
     if (r == IO_STREAM_CLOSED) {
       return 1;
@@ -5332,7 +5316,7 @@ log_from_pipe(FILE *stream, int severity, const char *executable,
 }
 #endif
 
-/** Reads from <b>stream</b> and stores input in <b>buf_out</b> making
+/** Reads from <b>fd</b> and stores input in <b>buf_out</b> making
  *  sure it's below <b>count</b> bytes.
  *  If the string has a trailing newline, we strip it off.
  *
@@ -5348,52 +5332,28 @@ log_from_pipe(FILE *stream, int severity, const char *executable,
  * IO_STREAM_OKAY: If everything went okay and we got a string
  *  in <b>buf_out</b>. */
 enum stream_status
-get_string_from_pipe(FILE *stream, char *buf_out, size_t count)
+get_string_from_pipe(int fd, char *buf_out, size_t count)
 {
-  char *retval;
-  size_t len;
+  ssize_t ret;
 
   tor_assert(count <= INT_MAX);
 
-  retval = tor_fgets(buf_out, (int)count, stream);
+  ret = read(fd, buf_out, count);
 
-  if (!retval) {
-    if (feof(stream)) {
-      /* Program has closed stream (probably it exited) */
-      /* TODO: check error */
-      return IO_STREAM_CLOSED;
-    } else {
-      if (EAGAIN == errno) {
-        /* Nothing more to read, try again next time */
-        return IO_STREAM_EAGAIN;
-      } else {
-        /* There was a problem, abandon this child process */
-        return IO_STREAM_TERM;
-      }
-    }
-  } else {
-    len = strlen(buf_out);
-    if (len == 0) {
-      /* this probably means we got a NUL at the start of the string. */
-      return IO_STREAM_EAGAIN;
-    }
+  if (ret == 0)
+    return IO_STREAM_CLOSED;
+  else if (ret < 0 && errno == EAGAIN)
+    return IO_STREAM_EAGAIN;
+  else if (ret < 0)
+    return IO_STREAM_TERM;
 
-    if (buf_out[len - 1] == '\n') {
-      /* Remove the trailing newline */
-      buf_out[len - 1] = '\0';
-    } else {
-      /* No newline; check whether we overflowed the buffer */
-      if (!feof(stream))
-        log_info(LD_GENERAL,
-                 "Line from stream was truncated: %s", buf_out);
-      /* TODO: What to do with this error? */
-    }
-
-    return IO_STREAM_OKAY;
-  }
+  if (buf_out[ret - 1] == '\n') {
+    /* Remove the trailing newline */
+    buf_out[ret - 1] = '\0';
+  } else
+    buf_out[ret] = '\0';
 
-  /* We should never get here */
-  return IO_STREAM_TERM;
+  return IO_STREAM_OKAY;
 }
 
 /** Parse a <b>line</b> from tor-fw-helper and issue an appropriate
@@ -5630,7 +5590,7 @@ tor_check_port_forwarding(const char *filename,
 #ifdef _WIN32
     stderr_status = log_from_handle(child_handle->stderr_pipe, LOG_INFO);
 #else
-    stderr_status = log_from_pipe(child_handle->stderr_handle,
+    stderr_status = log_from_pipe(child_handle->stderr_pipe,
                                   LOG_INFO, filename, &retval);
 #endif
     if (handle_fw_helper_output(filename, child_handle) < 0) {
diff --git a/src/common/util.h b/src/common/util.h
index 13fcc5142d..a8d7978c24 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -322,7 +322,7 @@ enum stream_status {
 
 const char *stream_status_to_string(enum stream_status stream_status);
 
-enum stream_status get_string_from_pipe(FILE *stream, char *buf, size_t count);
+enum stream_status get_string_from_pipe(int fd, char *buf, size_t count);
 
 MOCK_DECL(int,tor_unlink,(const char *pathname));
 
@@ -463,9 +463,6 @@ struct process_handle_t {
   int stdin_pipe;
   int stdout_pipe;
   int stderr_pipe;
-  FILE *stdin_handle;
-  FILE *stdout_handle;
-  FILE *stderr_handle;
   pid_t pid;
   /** If the process has not given us a SIGCHLD yet, this has the
    * waitpid_callback_t that gets invoked once it has. Otherwise this
@@ -488,7 +485,7 @@ int tor_split_lines(struct smartlist_t *sl, char *buf, int len);
 ssize_t tor_read_all_handle(HANDLE h, char *buf, size_t count,
                             const process_handle_t *process);
 #else
-ssize_t tor_read_all_handle(FILE *h, char *buf, size_t count,
+ssize_t tor_read_all_handle(int fd, char *buf, size_t count,
                             const process_handle_t *process,
                             int *eof);
 #endif
@@ -502,7 +499,7 @@ int tor_process_get_pid(process_handle_t *process_handle);
 #ifdef _WIN32
 HANDLE tor_process_get_stdout_pipe(process_handle_t *process_handle);
 #else
-FILE *tor_process_get_stdout_pipe(process_handle_t *process_handle);
+int tor_process_get_stdout_pipe(process_handle_t *process_handle);
 #endif
 
 #ifdef _WIN32
@@ -511,7 +508,7 @@ tor_get_lines_from_handle,(HANDLE *handle,
                            enum stream_status *stream_status));
 #else
 MOCK_DECL(struct smartlist_t *,
-tor_get_lines_from_handle,(FILE *handle,
+tor_get_lines_from_handle,(int fd,
                            enum stream_status *stream_status));
 #endif
 
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 2c3ce100df..f01668adcb 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -2701,7 +2701,7 @@ dirserv_read_measured_bandwidths(const char *from_file,
     return -1;
   }
 
-  if (!tor_fgets(line, sizeof(line), fp)
+  if (!fgets(line, sizeof(line), fp)
           || !strlen(line) || line[strlen(line)-1] != '\n') {
     log_warn(LD_DIRSERV, "Long or truncated time in bandwidth file: %s",
              escaped(line));
@@ -2731,7 +2731,7 @@ dirserv_read_measured_bandwidths(const char *from_file,
 
   while (!feof(fp)) {
     measured_bw_line_t parsed_line;
-    if (tor_fgets(line, sizeof(line), fp) && strlen(line)) {
+    if (fgets(line, sizeof(line), fp) && strlen(line)) {
       if (measured_bw_line_parse(&parsed_line, line) != -1) {
         /* Also cache the line for dirserv_get_bandwidth_for_router() */
         dirserv_cache_measured_bw(&parsed_line, file_time);
diff --git a/src/or/geoip.c b/src/or/geoip.c
index a8dc807c19..74811ea643 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -346,7 +346,7 @@ geoip_load_file(sa_family_t family, const char *filename)
              (family == AF_INET) ? "IPv4" : "IPv6", filename);
   while (!feof(f)) {
     char buf[512];
-    if (tor_fgets(buf, (int)sizeof(buf), f) == NULL)
+    if (fgets(buf, (int)sizeof(buf), f) == NULL)
       break;
     crypto_digest_add_bytes(geoip_digest_env, buf, strlen(buf));
     /* FFFF track full country name. */
diff --git a/src/test/test_pt.c b/src/test/test_pt.c
index f93019f1c4..f50c75ab48 100644
--- a/src/test/test_pt.c
+++ b/src/test/test_pt.c
@@ -284,13 +284,13 @@ test_pt_get_extrainfo_string(void *arg)
 }
 
 #ifdef _WIN32
-#define STDIN_HANDLE HANDLE
+#define STDIN_HANDLE HANDLE*
 #else
-#define STDIN_HANDLE FILE
+#define STDIN_HANDLE int
 #endif
 
 static smartlist_t *
-tor_get_lines_from_handle_replacement(STDIN_HANDLE *handle,
+tor_get_lines_from_handle_replacement(STDIN_HANDLE handle,
                                       enum stream_status *stream_status_out)
 {
   static int times_called = 0;
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 3e4d45d35b..130a019d5c 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -3940,17 +3940,13 @@ test_util_exit_status(void *ptr)
 #endif
 
 #ifndef _WIN32
-/* Check that fgets with a non-blocking pipe returns partial lines and sets
- * EAGAIN, returns full lines and sets no error, and returns NULL on EOF and
- * sets no error */
 static void
-test_util_fgets_eagain(void *ptr)
+test_util_string_from_pipe(void *ptr)
 {
   int test_pipe[2] = {-1, -1};
-  int retval;
+  int retval = 0;
+  enum stream_status status = IO_STREAM_TERM;
   ssize_t retlen;
-  char *retptr;
-  FILE *test_stream = NULL;
   char buf[4] = { 0 };
 
   (void)ptr;
@@ -3961,95 +3957,105 @@ test_util_fgets_eagain(void *ptr)
   retval = pipe(test_pipe);
   tt_int_op(retval, OP_EQ, 0);
 
-  /* Set up the read-end to be non-blocking */
-  retval = fcntl(test_pipe[0], F_SETFL, O_NONBLOCK);
-  tt_int_op(retval, OP_EQ, 0);
+  /* Send in a string. */
+  retlen = write(test_pipe[1], "ABC", 3);
+  tt_int_op(retlen, OP_EQ, 3);
 
-  /* Open it as a stdio stream */
-  test_stream = fdopen(test_pipe[0], "r");
-  tt_ptr_op(test_stream, OP_NE, NULL);
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
+  tt_int_op(errno, OP_EQ, 0);
+  tt_int_op(status, OP_EQ, IO_STREAM_OKAY);
+  tt_str_op(buf, OP_EQ, "ABC");
+  errno = 0;
 
-  /* Send in a partial line */
-  retlen = write(test_pipe[1], "A", 1);
-  tt_int_op(retlen, OP_EQ, 1);
-  retptr = tor_fgets(buf, sizeof(buf), test_stream);
-  tt_int_op(errno, OP_EQ, EAGAIN);
-  tt_ptr_op(retptr, OP_EQ, NULL);
-  tt_str_op(buf, OP_EQ, "A");
+  /* Send in a string that contains a nul. */
+  retlen = write(test_pipe[1], "AB\0", 3);
+  tt_int_op(retlen, OP_EQ, 3);
+
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
+  tt_int_op(errno, OP_EQ, 0);
+  tt_int_op(status, OP_EQ, IO_STREAM_OKAY);
+  tt_str_op(buf, OP_EQ, "AB");
   errno = 0;
 
-  /* Send in the rest */
-  retlen = write(test_pipe[1], "B\n", 2);
-  tt_int_op(retlen, OP_EQ, 2);
-  retptr = tor_fgets(buf, sizeof(buf), test_stream);
+  /* Send in a string that contains a nul only. */
+  retlen = write(test_pipe[1], "\0", 1);
+  tt_int_op(retlen, OP_EQ, 1);
+
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
   tt_int_op(errno, OP_EQ, 0);
-  tt_ptr_op(retptr, OP_EQ, buf);
-  tt_str_op(buf, OP_EQ, "B\n");
+  tt_int_op(status, OP_EQ, IO_STREAM_OKAY);
+  tt_str_op(buf, OP_EQ, "");
   errno = 0;
-  memset(buf, '\0', sizeof(buf));
 
-  /* Send in a full line */
-  retlen = write(test_pipe[1], "CD\n", 3);
+  /* Send in a string that contains a trailing newline. */
+  retlen = write(test_pipe[1], "AB\n", 3);
   tt_int_op(retlen, OP_EQ, 3);
-  retptr = tor_fgets(buf, sizeof(buf), test_stream);
+
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
   tt_int_op(errno, OP_EQ, 0);
-  tt_ptr_op(retptr, OP_EQ, buf);
-  tt_str_op(buf, OP_EQ, "CD\n");
+  tt_int_op(status, OP_EQ, IO_STREAM_OKAY);
+  tt_str_op(buf, OP_EQ, "AB");
   errno = 0;
-  memset(buf, '\0', sizeof(buf));
 
-  /* Send in a partial line */
-  retlen = write(test_pipe[1], "E", 1);
+  /* Send in a string that contains a newline only. */
+  retlen = write(test_pipe[1], "\n", 1);
   tt_int_op(retlen, OP_EQ, 1);
-  retptr = tor_fgets(buf, sizeof(buf), test_stream);
-  tt_int_op(errno, OP_EQ, EAGAIN);
-  tt_ptr_op(retptr, OP_EQ, NULL);
-  tt_str_op(buf, OP_EQ, "E");
+
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
+  tt_int_op(errno, OP_EQ, 0);
+  tt_int_op(status, OP_EQ, IO_STREAM_OKAY);
+  tt_str_op(buf, OP_EQ, "");
   errno = 0;
 
-  /* Send in the rest */
-  retlen = write(test_pipe[1], "F\n", 2);
-  tt_int_op(retlen, OP_EQ, 2);
-  retptr = tor_fgets(buf, sizeof(buf), test_stream);
+  /* Send in a string and check that we nul terminate return values. */
+  retlen = write(test_pipe[1], "AAA", 3);
+  tt_int_op(retlen, OP_EQ, 3);
+
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
   tt_int_op(errno, OP_EQ, 0);
-  tt_ptr_op(retptr, OP_EQ, buf);
-  tt_str_op(buf, OP_EQ, "F\n");
+  tt_int_op(status, OP_EQ, IO_STREAM_OKAY);
+  tt_str_op(buf, OP_EQ, "AAA");
+  tt_mem_op(buf, OP_EQ, "AAA\0", sizeof(buf));
   errno = 0;
-  memset(buf, '\0', sizeof(buf));
 
-  /* Send in a full line and close */
-  retlen = write(test_pipe[1], "GH", 2);
+  retlen = write(test_pipe[1], "B", 1);
+  tt_int_op(retlen, OP_EQ, 1);
+
+  memset(buf, '\xff', sizeof(buf));
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
+  tt_int_op(errno, OP_EQ, 0);
+  tt_int_op(status, OP_EQ, IO_STREAM_OKAY);
+  tt_str_op(buf, OP_EQ, "B");
+  tt_mem_op(buf, OP_EQ, "B\0\xff\xff", sizeof(buf));
+  errno = 0;
+
+  /* Send in a line and close */
+  retlen = write(test_pipe[1], "AB", 2);
   tt_int_op(retlen, OP_EQ, 2);
   retval = close(test_pipe[1]);
   tt_int_op(retval, OP_EQ, 0);
   test_pipe[1] = -1;
-  retptr = tor_fgets(buf, sizeof(buf), test_stream);
+
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
   tt_int_op(errno, OP_EQ, 0);
-  tt_ptr_op(retptr, OP_EQ, buf);
-  tt_str_op(buf, OP_EQ, "GH");
+  tt_int_op(status, OP_EQ, IO_STREAM_OKAY);
+  tt_str_op(buf, OP_EQ, "AB");
   errno = 0;
 
   /* Check for EOF */
-  retptr = tor_fgets(buf, sizeof(buf), test_stream);
+  status = get_string_from_pipe(test_pipe[0], buf, sizeof(buf)-1);
   tt_int_op(errno, OP_EQ, 0);
-  tt_ptr_op(retptr, OP_EQ, NULL);
-  retval = feof(test_stream);
-  tt_int_op(retval, OP_NE, 0);
+  tt_int_op(status, OP_EQ, IO_STREAM_CLOSED);
   errno = 0;
 
-  /* Check that buf is unchanged according to C99 and C11 */
-  tt_str_op(buf, OP_EQ, "GH");
-  memset(buf, '\0', sizeof(buf));
-
  done:
-  if (test_stream != NULL)
-    fclose(test_stream);
   if (test_pipe[0] != -1)
     close(test_pipe[0]);
   if (test_pipe[1] != -1)
     close(test_pipe[1]);
 }
-#endif
+
+#endif // _WIN32
 
 /**
  * Test for format_hex_number_sigsafe()
@@ -5723,7 +5729,7 @@ struct testcase_t util_tests[] = {
   UTIL_TEST(num_cpus, 0),
   UTIL_TEST_WIN_ONLY(load_win_lib, 0),
   UTIL_TEST_NO_WIN(exit_status, 0),
-  UTIL_TEST_NO_WIN(fgets_eagain, 0),
+  UTIL_TEST_NO_WIN(string_from_pipe, 0),
   UTIL_TEST(format_hex_number, 0),
   UTIL_TEST(format_dec_number, 0),
   UTIL_TEST(join_win_cmdline, 0),
diff --git a/src/test/test_util_slow.c b/src/test/test_util_slow.c
index 1e7160598c..96a2fa756b 100644
--- a/src/test/test_util_slow.c
+++ b/src/test/test_util_slow.c
@@ -242,7 +242,7 @@ test_util_spawn_background_partial_read_impl(int exit_early)
 #else
     /* Check that we didn't read the end of file last time */
     tt_assert(!eof);
-    pos = tor_read_all_handle(process_handle->stdout_handle, stdout_buf,
+    pos = tor_read_all_handle(process_handle->stdout_pipe, stdout_buf,
                               sizeof(stdout_buf) - 1, NULL, &eof);
 #endif
     log_info(LD_GENERAL, "tor_read_all_handle() returned %d", (int)pos);
@@ -273,7 +273,7 @@ test_util_spawn_background_partial_read_impl(int exit_early)
 #else
   if (!eof) {
     /* We should have got all the data, but maybe not the EOF flag */
-    pos = tor_read_all_handle(process_handle->stdout_handle, stdout_buf,
+    pos = tor_read_all_handle(process_handle->stdout_pipe, stdout_buf,
                               sizeof(stdout_buf) - 1,
                               process_handle, &eof);
     tt_int_op(0,OP_EQ, pos);