From 1adc98b9b55a28a6785a6faf8e5b3369981d8dbb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 May 2014 21:22:40 -0400 Subject: [PATCH 1/2] Split portfw-error-logging code into a new function. No code has changed; only moved. Part of a fix for 9781. --- src/common/util.c | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 6524be3ed8..d82677d5a7 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -4540,6 +4540,30 @@ stream_status_to_string(enum stream_status stream_status) } } +/* DOCDOC */ +static void +log_portfw_spawn_error_message(const char *buf, + const char *executable, int *child_status) +{ + /* Parse error message */ + int retval, child_state, saved_errno; + retval = tor_sscanf(buf, SPAWN_ERROR_MESSAGE "%x/%x", + &child_state, &saved_errno); + if (retval == 2) { + log_warn(LD_GENERAL, + "Failed to start child process \"%s\" in state %d: %s", + executable, child_state, strerror(saved_errno)); + if (child_status) + *child_status = 1; + } else { + /* Failed to parse message from child process, log it as a + warning */ + log_warn(LD_GENERAL, + "Unexpected message from port forwarding helper \"%s\": %s", + executable, buf); + } +} + #ifdef _WIN32 /** Return a smartlist containing lines outputted from @@ -4687,23 +4711,7 @@ log_from_pipe(FILE *stream, int severity, const char *executable, /* Check if buf starts with SPAWN_ERROR_MESSAGE */ if (strcmpstart(buf, SPAWN_ERROR_MESSAGE) == 0) { - /* Parse error message */ - int retval, child_state, saved_errno; - retval = tor_sscanf(buf, SPAWN_ERROR_MESSAGE "%x/%x", - &child_state, &saved_errno); - if (retval == 2) { - log_warn(LD_GENERAL, - "Failed to start child process \"%s\" in state %d: %s", - executable, child_state, strerror(saved_errno)); - if (child_status) - *child_status = 1; - } else { - /* Failed to parse message from child process, log it as a - warning */ - log_warn(LD_GENERAL, - "Unexpected message from port forwarding helper \"%s\": %s", - executable, buf); - } + log_portfw_spawn_error_message(buf, executable, child_status); } else { log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", buf); } From b5e142cb1ba2dcdcf0c7586cdb3bd1efa2bd78b7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 6 May 2014 21:34:40 -0400 Subject: [PATCH 2/2] Log an error reply from tor-fw-helper correctly. Fix for bug 9781; bugfix on cd05f35d2cdf50 in 0.2.4.2-alpha. --- changes/bug9781 | 5 +++++ src/common/util.c | 21 +++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 changes/bug9781 diff --git a/changes/bug9781 b/changes/bug9781 new file mode 100644 index 0000000000..0a3c557cb8 --- /dev/null +++ b/changes/bug9781 @@ -0,0 +1,5 @@ + o Minor bugfixes (tor-fw-helper): + + - Give a correct log message when tor-fw-helper fails to launch. + (Previously, we would say something like "tor-fw-helper sent us a + string we could not parse".) Fixes bug 9781; bugfix on 0.2.4.2-alpha. diff --git a/src/common/util.c b/src/common/util.c index d82677d5a7..a27fe4bf00 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -4789,7 +4789,7 @@ get_string_from_pipe(FILE *stream, char *buf_out, size_t count) /** Parse a line from tor-fw-helper and issue an appropriate * log message to our user. */ static void -handle_fw_helper_line(const char *line) +handle_fw_helper_line(const char *executable, const char *line) { smartlist_t *tokens = smartlist_new(); char *message = NULL; @@ -4800,6 +4800,19 @@ handle_fw_helper_line(const char *line) int port = 0; int success = 0; + if (strcmpstart(line, SPAWN_ERROR_MESSAGE) == 0) { + /* We need to check for SPAWN_ERROR_MESSAGE again here, since it's + * possible that it got sent after we tried to read it in log_from_pipe. + * + * XXX Ideally, we should be using one of stdout/stderr for the real + * output, and one for the output of the startup code. We used to do that + * before cd05f35d2c. + */ + int child_status; + log_portfw_spawn_error_message(line, executable, &child_status); + goto done; + } + smartlist_split_string(tokens, line, NULL, SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, -1); @@ -4879,7 +4892,7 @@ handle_fw_helper_line(const char *line) /** Read what tor-fw-helper has to say in its stdout and handle it * appropriately */ static int -handle_fw_helper_output(process_handle_t *process_handle) +handle_fw_helper_output(const char *executable, process_handle_t *process_handle) { smartlist_t *fw_helper_output = NULL; enum stream_status stream_status = 0; @@ -4894,7 +4907,7 @@ handle_fw_helper_output(process_handle_t *process_handle) /* Handle the lines we got: */ SMARTLIST_FOREACH_BEGIN(fw_helper_output, char *, line) { - handle_fw_helper_line(line); + handle_fw_helper_line(executable, line); tor_free(line); } SMARTLIST_FOREACH_END(line); @@ -5009,7 +5022,7 @@ tor_check_port_forwarding(const char *filename, stderr_status = log_from_pipe(child_handle->stderr_handle, LOG_INFO, filename, &retval); #endif - if (handle_fw_helper_output(child_handle) < 0) { + if (handle_fw_helper_output(filename, child_handle) < 0) { log_warn(LD_GENERAL, "Failed to handle fw helper output."); stdout_status = -1; retval = -1;