Commit 85782e11 authored by Nick Mathewson's avatar Nick Mathewson 🦀
Browse files

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

parents a9b046ce 02fc0a5e

changes/bug20988

deleted100644 → 0
+0 −4
Original line number Diff line number Diff line 
  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.

changes/bug21654

0 → 100644
+4 −0
Original line number Diff line number Diff line 
  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.

src/common/compat.c

+0 −39
Original line number Diff line number Diff line
@@ -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

src/common/compat.h

+0 −2
Original line number Diff line number Diff line
@@ -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"

src/common/util.c

+53 −93
Original line number Diff line number Diff line
@@ -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");

  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 (EAGAIN == errno) {

    } else if (result < 0 && errno == EAGAIN) {

      if (process)

        continue;

      else

        break;

        } else {

          log_warn(LD_GENERAL, "fgets() from handle failed: %s",

                   strerror(errno));

    } 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,54 +5332,30 @@ 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 */

  if (ret == 0)

    return IO_STREAM_CLOSED;

    } else {

      if (EAGAIN == errno) {

        /* Nothing more to read, try again next time */

  else if (ret < 0 && errno == EAGAIN)

    return IO_STREAM_EAGAIN;

      } else {

        /* There was a problem, abandon this child process */

  else if (ret < 0)

    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 (buf_out[len - 1] == '\n') {

  if (buf_out[ret - 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? */

    }

    buf_out[ret - 1] = '\0';

  } else

    buf_out[ret] = '\0';



  return IO_STREAM_OKAY;

}



  /* We should never get here */

  return IO_STREAM_TERM;

}



/** Parse a <b>line</b> from tor-fw-helper and issue an appropriate

 *  log message to our user. */

static void
@@ -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) {