From 2a3eef4404415f1bb90a86c9cc396b6dee89039c Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Mon, 10 Sep 2018 13:05:01 +0200 Subject: Remove unused `int pid` member of `managed_proxy_t`. See: https://bugs.torproject.org/28179 --- src/feature/client/transports.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'src/feature/client') diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index d304dcd485..b80875c95c 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -97,8 +97,6 @@ typedef struct { /* A pointer to the process handle of this managed proxy. */ struct process_handle_t *process_handle; - int pid; /* The Process ID this managed proxy is using. */ - /** Boolean: We are re-parsing our config, and we are going to * remove this managed proxy if we don't find it any transport * plugins that use it. */ -- cgit v1.2.3-54-g00ecf From bfb94dd2ca8e04fb1fe8aba9ad48effbb8b70662 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 23 Nov 2018 05:56:41 +0100 Subject: Use process_t for managed proxies. This patch makes the managed proxy subsystem use the process_t data structure such that we can get events from the PT process while Tor is running and not just when the PT process is being configured. See: https://bugs.torproject.org/28179 --- src/core/mainloop/mainloop.c | 4 - src/feature/client/transports.c | 182 ++++++++++++++++++++++------------------ src/feature/client/transports.h | 12 ++- src/test/test_pt.c | 66 +++++++-------- 4 files changed, 141 insertions(+), 123 deletions(-) (limited to 'src/feature/client') diff --git a/src/core/mainloop/mainloop.c b/src/core/mainloop/mainloop.c index 2e2ae876d4..aaaa5009cb 100644 --- a/src/core/mainloop/mainloop.c +++ b/src/core/mainloop/mainloop.c @@ -1853,10 +1853,6 @@ second_elapsed_callback(time_t now, const or_options_t *options) run_connection_housekeeping(i, now); } - /* 11b. check pending unconfigured managed proxies */ - if (!net_is_disabled() && pt_proxies_configuration_pending()) - pt_configure_remaining_proxies(); - /* Run again in a second. */ return 1; } diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 85d4d2e080..f0400b713d 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -102,10 +102,11 @@ #include "feature/relay/ext_orport.h" #include "feature/control/control.h" -#include "lib/process/env.h" +#include "lib/process/process.h" #include "lib/process/subprocess.h" +#include "lib/process/env.h" -static process_environment_t * +static smartlist_t * create_managed_proxy_environment(const managed_proxy_t *mp); static inline int proxy_configuration_finished(const managed_proxy_t *mp); @@ -490,8 +491,8 @@ proxy_prepare_for_restart(managed_proxy_t *mp) tor_assert(mp->conf_state == PT_PROTO_COMPLETED); /* destroy the process handle and terminate the process. */ - tor_process_handle_destroy(mp->process_handle, 1); - mp->process_handle = NULL; + process_set_data(mp->process, NULL); + process_terminate(mp->process); /* destroy all its registered transports, since we will no longer use them. */ @@ -520,34 +521,35 @@ proxy_prepare_for_restart(managed_proxy_t *mp) static int launch_managed_proxy(managed_proxy_t *mp) { - int retval; - - process_environment_t *env = create_managed_proxy_environment(mp); - -#ifdef _WIN32 - /* Passing NULL as lpApplicationName makes Windows search for the .exe */ - retval = tor_spawn_background(NULL, - (const char **)mp->argv, - env, - &mp->process_handle); -#else /* !(defined(_WIN32)) */ - retval = tor_spawn_background(mp->argv[0], - (const char **)mp->argv, - env, - &mp->process_handle); -#endif /* defined(_WIN32) */ - - process_environment_free(env); - - if (retval == PROCESS_STATUS_ERROR) { - log_warn(LD_GENERAL, "Managed proxy at '%s' failed at launch.", + tor_assert(mp); + + smartlist_t *env = create_managed_proxy_environment(mp); + + /* Configure our process. */ + process_set_data(mp->process, mp); + process_set_stdout_read_callback(mp->process, managed_proxy_stdout_callback); + process_set_stderr_read_callback(mp->process, managed_proxy_stderr_callback); + process_set_exit_callback(mp->process, managed_proxy_exit_callback); + process_set_protocol(mp->process, PROCESS_PROTOCOL_LINE); + process_reset_environment(mp->process, env); + + /* Cleanup our env. */ + SMARTLIST_FOREACH(env, char *, x, tor_free(x)); + smartlist_free(env); + + /* Skip the argv[0] as we get that from process_new(argv[0]). */ + for (int i = 1; mp->argv[i] != NULL; ++i) + process_append_argument(mp->process, mp->argv[i]); + + if (process_exec(mp->process) != PROCESS_STATUS_RUNNING) { + log_warn(LD_CONFIG, "Managed proxy at '%s' failed at launch.", mp->argv[0]); return -1; } - log_info(LD_CONFIG, "Managed proxy at '%s' has spawned with PID '%d'.", - mp->argv[0], tor_process_get_pid(mp->process_handle)); - + log_info(LD_CONFIG, + "Managed proxy at '%s' has spawned with PID '%" PRIu64 "'.", + mp->argv[0], process_get_pid(mp->process)); mp->conf_state = PT_PROTO_LAUNCHED; return 0; @@ -615,10 +617,6 @@ pt_configure_remaining_proxies(void) STATIC int configure_proxy(managed_proxy_t *mp) { - int configuration_finished = 0; - smartlist_t *proxy_output = NULL; - enum stream_status stream_status = 0; - /* if we haven't launched the proxy yet, do it now */ if (mp->conf_state == PT_PROTO_INFANT) { if (launch_managed_proxy(mp) < 0) { /* launch fail */ @@ -629,45 +627,8 @@ configure_proxy(managed_proxy_t *mp) } tor_assert(mp->conf_state != PT_PROTO_INFANT); - tor_assert(mp->process_handle); - - proxy_output = - tor_get_lines_from_handle(tor_process_get_stdout_pipe(mp->process_handle), - &stream_status); - if (!proxy_output) { /* failed to get input from proxy */ - if (stream_status != IO_STREAM_EAGAIN) { /* bad stream status! */ - mp->conf_state = PT_PROTO_BROKEN; - log_warn(LD_GENERAL, "The communication stream of managed proxy '%s' " - "is '%s'. Most probably the managed proxy stopped running. " - "This might be a bug of the managed proxy, a bug of Tor, or " - "a misconfiguration. Please enable logging on your managed " - "proxy and check the logs for errors.", - mp->argv[0], stream_status_to_string(stream_status)); - } - - goto done; - } - - /* Handle lines. */ - SMARTLIST_FOREACH_BEGIN(proxy_output, const char *, line) { - handle_proxy_line(line, mp); - if (proxy_configuration_finished(mp)) - goto done; - } SMARTLIST_FOREACH_END(line); - - done: - /* if the proxy finished configuring, exit the loop. */ - if (proxy_configuration_finished(mp)) { - handle_finished_proxy(mp); - configuration_finished = 1; - } - - if (proxy_output) { - SMARTLIST_FOREACH(proxy_output, char *, cp, tor_free(cp)); - smartlist_free(proxy_output); - } - - return configuration_finished; + tor_assert(mp->process); + return mp->conf_state == PT_PROTO_COMPLETED; } /** Register server managed proxy mp transports to state */ @@ -748,8 +709,11 @@ managed_proxy_destroy(managed_proxy_t *mp, /* free the outgoing proxy URI */ tor_free(mp->proxy_uri); - tor_process_handle_destroy(mp->process_handle, also_terminate_process); - mp->process_handle = NULL; + /* do we want to terminate our process if it's still running? */ + if (also_terminate_process && mp->process) + process_terminate(mp->process); + + process_free(mp->process); tor_free(mp); } @@ -1257,7 +1221,7 @@ get_bindaddr_for_server_proxy(const managed_proxy_t *mp) /** Return a newly allocated process_environment_t * for mp's * process. */ -static process_environment_t * +static smartlist_t * create_managed_proxy_environment(const managed_proxy_t *mp) { const or_options_t *options = get_options(); @@ -1272,8 +1236,6 @@ create_managed_proxy_environment(const managed_proxy_t *mp) /* The final environment to be passed to mp. */ smartlist_t *merged_env_vars = get_current_process_environment_variables(); - process_environment_t *env; - { char *state_tmp = get_datadir_fname("pt_state/"); /* XXX temp */ smartlist_add_asprintf(envs, "TOR_PT_STATE_LOCATION=%s", state_tmp); @@ -1366,14 +1328,9 @@ create_managed_proxy_environment(const managed_proxy_t *mp) tor_free_, 1); } SMARTLIST_FOREACH_END(env_var); - env = process_environment_make(merged_env_vars); - smartlist_free(envs); - SMARTLIST_FOREACH(merged_env_vars, void *, x, tor_free(x)); - smartlist_free(merged_env_vars); - - return env; + return merged_env_vars; } /** Create and return a new managed proxy for transport using @@ -1392,6 +1349,7 @@ managed_proxy_create(const smartlist_t *with_transport_list, mp->argv = proxy_argv; mp->transports = smartlist_new(); mp->proxy_uri = get_pt_proxy_uri(); + mp->process = process_new(proxy_argv[0]); mp->transports_to_launch = smartlist_new(); SMARTLIST_FOREACH(with_transport_list, const char *, transport, @@ -1736,3 +1694,63 @@ tor_escape_str_for_pt_args(const char *string, const char *chars_to_escape) return new_string; } + +/** Callback function that is called when our PT process have data on its + * stdout. Our process can be found in process, the data can be found in + * line and the length of our line is given in size. */ +STATIC void +managed_proxy_stdout_callback(process_t *process, char *line, size_t size) +{ + tor_assert(process); + tor_assert(line); + + (void)size; + + managed_proxy_t *mp = process_get_data(process); + + handle_proxy_line(line, mp); + + if (proxy_configuration_finished(mp)) { + handle_finished_proxy(mp); + tor_assert(mp->conf_state == PT_PROTO_COMPLETED); + } +} + +/** Callback function that is called when our PT process have data on its + * stderr. Our process can be found in process, the data can be found in + * line and the length of our line is given in size. */ +STATIC void +managed_proxy_stderr_callback(process_t *process, char *line, size_t size) +{ + tor_assert(process); + tor_assert(line); + + (void)size; + + managed_proxy_t *mp = process_get_data(process); + + log_warn(LD_PT, "Managed proxy at '%s' reported: %s", mp->argv[0], line); +} + +/** Callback function that is called when our PT process terminates. The + * process exit code can be found in exit_code and our process can be + * found in process. */ +STATIC void +managed_proxy_exit_callback(process_t *process, process_exit_code_t exit_code) +{ + tor_assert(process); + + log_warn(LD_PT, + "Pluggable Transport process terminated with status code %" PRIu64, + exit_code); + + /* We detach ourself from the MP (if we are attached) and free ourself. */ + managed_proxy_t *mp = process_get_data(process); + + if (BUG(mp != NULL)) { + mp->process = NULL; + process_set_data(process, NULL); + } + + process_free(process); +} diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index b80875c95c..59df637d81 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -11,6 +11,8 @@ #ifndef TOR_TRANSPORTS_H #define TOR_TRANSPORTS_H +#include "lib/process/process.h" + /** Represents a pluggable transport used by a bridge. */ typedef struct transport_t { /** SOCKS version: One of PROXY_SOCKS4, PROXY_SOCKS5. */ @@ -81,7 +83,7 @@ enum pt_proto_state { PT_PROTO_FAILED_LAUNCH /* failed while launching */ }; -struct process_handle_t; +struct process_t; /** Structure containing information of a managed proxy. */ typedef struct { @@ -94,8 +96,8 @@ typedef struct { int is_server; /* is it a server proxy? */ - /* A pointer to the process handle of this managed proxy. */ - struct process_handle_t *process_handle; + /* A pointer to the process of this managed proxy. */ + struct process_t *process; /** Boolean: We are re-parsing our config, and we are going to * remove this managed proxy if we don't find it any transport @@ -140,6 +142,10 @@ STATIC char* get_pt_proxy_uri(void); STATIC void free_execve_args(char **arg); +STATIC void managed_proxy_stdout_callback(process_t *, char *, size_t); +STATIC void managed_proxy_stderr_callback(process_t *, char *, size_t); +STATIC void managed_proxy_exit_callback(process_t *, process_exit_code_t); + #endif /* defined(PT_PRIVATE) */ #endif /* !defined(TOR_TRANSPORTS_H) */ diff --git a/src/test/test_pt.c b/src/test/test_pt.c index d0160d1148..2501b867bb 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -9,6 +9,7 @@ #define STATEFILE_PRIVATE #define CONTROL_PRIVATE #define SUBPROCESS_PRIVATE +#define PROCESS_PRIVATE #include "core/or/or.h" #include "app/config/config.h" #include "app/config/confparse.h" @@ -20,6 +21,7 @@ #include "lib/process/subprocess.h" #include "lib/encoding/confline.h" #include "lib/net/resolve.h" +#include "lib/process/process.h" #include "app/config/or_state_st.h" @@ -151,6 +153,8 @@ test_pt_get_transport_options(void *arg) config_line_t *cl = NULL; (void)arg; + process_init(); + execve_args = tor_malloc(sizeof(char*)*2); execve_args[0] = tor_strdup("cheeseshop"); execve_args[1] = NULL; @@ -190,6 +194,7 @@ test_pt_get_transport_options(void *arg) config_free_lines(cl); managed_proxy_destroy(mp, 0); smartlist_free(transport_list); + process_free_all(); } static void @@ -253,6 +258,8 @@ test_pt_get_extrainfo_string(void *arg) char *s = NULL; (void) arg; + process_init(); + argv1 = tor_malloc_zero(sizeof(char*)*3); argv1[0] = tor_strdup("ewige"); argv1[1] = tor_strdup("Blumenkraft"); @@ -286,43 +293,25 @@ test_pt_get_extrainfo_string(void *arg) smartlist_free(t1); smartlist_free(t2); tor_free(s); + process_free_all(); } -#ifdef _WIN32 -#define STDIN_HANDLE HANDLE* -#else -#define STDIN_HANDLE int -#endif - -static smartlist_t * -tor_get_lines_from_handle_replacement(STDIN_HANDLE handle, - enum stream_status *stream_status_out) +static int +process_read_stdout_replacement(process_t *process, buf_t *buffer) { + (void)process; static int times_called = 0; - smartlist_t *retval_sl = smartlist_new(); - - (void) handle; - (void) stream_status_out; /* Generate some dummy CMETHOD lines the first 5 times. The 6th time, send 'CMETHODS DONE' to finish configuring the proxy. */ if (times_called++ != 5) { - smartlist_add_asprintf(retval_sl, "SMETHOD mock%d 127.0.0.1:555%d", + buf_add_printf(buffer, "SMETHOD mock%d 127.0.0.1:555%d\n", times_called, times_called); } else { - smartlist_add_strdup(retval_sl, "SMETHODS DONE"); + buf_add_string(buffer, "SMETHODS DONE\n"); } - return retval_sl; -} - -/* NOP mock */ -static void -tor_process_handle_destroy_replacement(process_handle_t *process_handle, - int also_terminate_process) -{ - (void) process_handle; - (void) also_terminate_process; + return (int)buf_datalen(buffer); } static or_state_t *dummy_state = NULL; @@ -355,12 +344,11 @@ test_pt_configure_proxy(void *arg) managed_proxy_t *mp = NULL; (void) arg; + process_init(); + dummy_state = tor_malloc_zero(sizeof(or_state_t)); - MOCK(tor_get_lines_from_handle, - tor_get_lines_from_handle_replacement); - MOCK(tor_process_handle_destroy, - tor_process_handle_destroy_replacement); + MOCK(process_read_stdout, process_read_stdout_replacement); MOCK(get_or_state, get_or_state_replacement); MOCK(queue_control_event_string, @@ -372,24 +360,34 @@ test_pt_configure_proxy(void *arg) mp->conf_state = PT_PROTO_ACCEPTING_METHODS; mp->transports = smartlist_new(); mp->transports_to_launch = smartlist_new(); - mp->process_handle = tor_malloc_zero(sizeof(process_handle_t)); mp->argv = tor_malloc_zero(sizeof(char*)*2); mp->argv[0] = tor_strdup(""); mp->is_server = 1; + /* Configure the process. */ + mp->process = process_new(""); + process_set_stdout_read_callback(mp->process, managed_proxy_stdout_callback); + process_set_data(mp->process, mp); + /* Test the return value of configure_proxy() by calling it some times while it is uninitialized and then finally finalizing its configuration. */ for (i = 0 ; i < 5 ; i++) { + /* force a read from our mocked stdout reader. */ + process_notify_event_stdout(mp->process); + /* try to configure our proxy. */ retval = configure_proxy(mp); /* retval should be zero because proxy hasn't finished configuring yet */ tt_int_op(retval, OP_EQ, 0); /* check the number of registered transports */ - tt_assert(smartlist_len(mp->transports) == i+1); + tt_int_op(smartlist_len(mp->transports), OP_EQ, i+1); /* check that the mp is still waiting for transports */ tt_assert(mp->conf_state == PT_PROTO_ACCEPTING_METHODS); } + /* Get the SMETHOD DONE written to the process. */ + process_notify_event_stdout(mp->process); + /* this last configure_proxy() should finalize the proxy configuration. */ retval = configure_proxy(mp); /* retval should be 1 since the proxy finished configuring */ @@ -435,8 +433,7 @@ test_pt_configure_proxy(void *arg) done: or_state_free(dummy_state); - UNMOCK(tor_get_lines_from_handle); - UNMOCK(tor_process_handle_destroy); + UNMOCK(process_read_stdout); UNMOCK(get_or_state); UNMOCK(queue_control_event_string); if (controlevent_msgs) { @@ -449,10 +446,11 @@ test_pt_configure_proxy(void *arg) smartlist_free(mp->transports); } smartlist_free(mp->transports_to_launch); - tor_free(mp->process_handle); + process_free(mp->process); tor_free(mp->argv[0]); tor_free(mp->argv); tor_free(mp); + process_free_all(); } /* Test the get_pt_proxy_uri() function. */ -- cgit v1.2.3-54-g00ecf From e3ceaebba25127a1d4a3da16e9128ab52491c080 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 23 Nov 2018 22:50:55 +0100 Subject: Add support for logging messages from pluggable transports. This patch adds support for the "LOG" protocol message from a pluggable transport. This allows pluggable transport developers to relay log messages from their binary to Tor, which will both emit them as log messages from the Tor process itself, but also pass them on via the control port. See: https://bugs.torproject.org/28180 See: https://bugs.torproject.org/28181 See: https://bugs.torproject.org/28182 --- src/feature/client/transports.c | 42 +++++++++++++++++++++++++++++++++++++++++ src/feature/client/transports.h | 1 + src/feature/control/control.c | 11 +++++++++++ src/feature/control/control.h | 5 ++++- src/test/test_pt.c | 19 +++++++++++++++++-- 5 files changed, 75 insertions(+), 3 deletions(-) (limited to 'src/feature/client') diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index f0400b713d..dc76e9c245 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -128,6 +128,7 @@ static void parse_method_error(const char *line, int is_server_method); #define PROTO_SMETHODS_DONE "SMETHODS DONE" #define PROTO_PROXY_DONE "PROXY DONE" #define PROTO_PROXY_ERROR "PROXY-ERROR" +#define PROTO_LOG "LOG" /** The first and only supported - at the moment - configuration protocol version. */ @@ -909,6 +910,9 @@ handle_proxy_line(const char *line, managed_proxy_t *mp) parse_proxy_error(line); goto err; + } else if (!strcmpstart(line, PROTO_LOG)) { + parse_log_line(line); + return; } else if (!strcmpstart(line, SPAWN_ERROR_MESSAGE)) { /* managed proxy launch failed: parse error message to learn why. */ int retval, child_state, saved_errno; @@ -1146,6 +1150,44 @@ parse_proxy_error(const char *line) line+strlen(PROTO_PROXY_ERROR)+1); } +/** Parses a LOG line and emit log events accordingly. */ +STATIC void +parse_log_line(const char *line) +{ + smartlist_t *items = smartlist_new(); + + if (strlen(line) < (strlen(PROTO_LOG) + 1)) { + log_warn(LD_PT, "Managed proxy sent us a %s line " + "with missing arguments.", PROTO_LOG); + goto done; + } + + const char *arguments = line + strlen(PROTO_LOG) + 1; + + /* The format is 'LOG '. We accept empty messages. */ + smartlist_split_string(items, arguments, NULL, + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 2); + + if (smartlist_len(items) < 2) { + log_warn(LD_PT, "Managed proxy sent us a %s line " + "with too few arguments.", PROTO_LOG); + goto done; + } + + const char *transport_name = smartlist_get(items, 0); + const char *message = smartlist_get(items, 1); + + log_info(LD_PT, "Managed proxy transport \"%s\" says: %s", + transport_name, message); + + /* Emit control port event. */ + control_event_transport_log(transport_name, message); + + done: + SMARTLIST_FOREACH(items, char *, s, tor_free(s)); + smartlist_free(items); +} + /** Return a newly allocated string that tor should place in * TOR_PT_SERVER_TRANSPORT_OPTIONS while configuring the server * manged proxy in mp. Return NULL if no such options are found. */ diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index 59df637d81..fbb720aac6 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -128,6 +128,7 @@ STATIC int parse_version(const char *line, managed_proxy_t *mp); STATIC void parse_env_error(const char *line); STATIC void parse_proxy_error(const char *line); STATIC void handle_proxy_line(const char *line, managed_proxy_t *mp); +STATIC void parse_log_line(const char *line); STATIC char *get_transport_options_for_server_proxy(const managed_proxy_t *mp); STATIC void managed_proxy_destroy(managed_proxy_t *mp, diff --git a/src/feature/control/control.c b/src/feature/control/control.c index 94679dfd22..b6505a85d6 100644 --- a/src/feature/control/control.c +++ b/src/feature/control/control.c @@ -7395,6 +7395,17 @@ control_event_transport_launched(const char *mode, const char *transport_name, mode, transport_name, fmt_addr(addr), port); } +/** A pluggable transport called transport_name has emitted a log + * message found in message. */ +void +control_event_transport_log(const char *transport_name, const char *message) +{ + send_control_event(EVENT_TRANSPORT_LOG, + "650 TRANSPORT_LOG %s %s\r\n", + transport_name, + message); +} + /** Convert rendezvous auth type to string for HS_DESC control events */ const char * diff --git a/src/feature/control/control.h b/src/feature/control/control.h index cd5402d455..eb2b5676ea 100644 --- a/src/feature/control/control.h +++ b/src/feature/control/control.h @@ -205,6 +205,8 @@ void control_event_clients_seen(const char *controller_str); void control_event_transport_launched(const char *mode, const char *transport_name, tor_addr_t *addr, uint16_t port); +void control_event_transport_log(const char *transport_name, + const char *message); const char *rend_auth_type_to_string(rend_auth_type_t auth_type); MOCK_DECL(const char *, node_describe_longname_by_id,(const char *id_digest)); void control_event_hs_descriptor_requested(const char *onion_address, @@ -293,7 +295,8 @@ void control_free_all(void); #define EVENT_HS_DESC 0x0021 #define EVENT_HS_DESC_CONTENT 0x0022 #define EVENT_NETWORK_LIVENESS 0x0023 -#define EVENT_MAX_ 0x0023 +#define EVENT_TRANSPORT_LOG 0x0024 +#define EVENT_MAX_ 0x0024 /* sizeof(control_connection_t.event_mask) in bits, currently a uint64_t */ #define EVENT_CAPACITY_ 0x0040 diff --git a/src/test/test_pt.c b/src/test/test_pt.c index 2501b867bb..c980276b14 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -304,11 +304,16 @@ process_read_stdout_replacement(process_t *process, buf_t *buffer) /* Generate some dummy CMETHOD lines the first 5 times. The 6th time, send 'CMETHODS DONE' to finish configuring the proxy. */ - if (times_called++ != 5) { + times_called++; + + if (times_called <= 5) { buf_add_printf(buffer, "SMETHOD mock%d 127.0.0.1:555%d\n", times_called, times_called); - } else { + } else if (times_called <= 6) { buf_add_string(buffer, "SMETHODS DONE\n"); + } else if (times_called <= 7) { + buf_add_string(buffer, "LOG mock3 Oh noes, something bad happened. " + "What do we do!?\n"); } return (int)buf_datalen(buffer); @@ -410,6 +415,16 @@ test_pt_configure_proxy(void *arg) tt_str_op(smartlist_get(controlevent_msgs, 4), OP_EQ, "650 TRANSPORT_LAUNCHED server mock5 127.0.0.1 5555\r\n"); + /* Get the log message out. */ + process_notify_event_stdout(mp->process); + + tt_int_op(controlevent_n, OP_EQ, 6); + tt_int_op(controlevent_event, OP_EQ, EVENT_TRANSPORT_LOG); + tt_int_op(smartlist_len(controlevent_msgs), OP_EQ, 6); + tt_str_op(smartlist_get(controlevent_msgs, 5), OP_EQ, + "650 TRANSPORT_LOG mock3 Oh noes, something bad happened. " + "What do we do!?\r\n"); + { /* check that the transport info were saved properly in the tor state */ config_line_t *transport_in_state = NULL; smartlist_t *transport_info_sl = smartlist_new(); -- cgit v1.2.3-54-g00ecf From f7d13425fc738d7d56d5582b9be8510ac236c076 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Mon, 26 Nov 2018 05:34:30 +0100 Subject: Delete old process_handle_t code. This patch removes the old process_handle_t code. Everything should by now be using the process_t interface. See: https://bugs.torproject.org/28179 --- .gitignore | 2 - src/feature/client/transports.c | 16 - src/lib/process/subprocess.c | 1078 --------------------------------------- src/lib/process/subprocess.h | 116 +---- src/test/Makefile.nmake | 7 +- src/test/include.am | 2 - src/test/test-child.c | 61 --- src/test/test.h | 1 - src/test/test_logging.c | 21 +- src/test/test_slow.c | 1 - src/test/test_util.c | 262 ---------- src/test/test_util_slow.c | 396 -------------- 12 files changed, 16 insertions(+), 1947 deletions(-) delete mode 100644 src/test/test-child.c delete mode 100644 src/test/test_util_slow.c (limited to 'src/feature/client') diff --git a/.gitignore b/.gitignore index df8db1113a..60635263ad 100644 --- a/.gitignore +++ b/.gitignore @@ -239,7 +239,6 @@ uptime-*.json /src/test/test /src/test/test-slow /src/test/test-bt-cl -/src/test/test-child /src/test/test-process /src/test/test-memwipe /src/test/test-ntor-cl @@ -250,7 +249,6 @@ uptime-*.json /src/test/test.exe /src/test/test-slow.exe /src/test/test-bt-cl.exe -/src/test/test-child.exe /src/test/test-process.exe /src/test/test-ntor-cl.exe /src/test/test-hs-ntor-cl.exe diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index dc76e9c245..fd4bec7807 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -913,22 +913,6 @@ handle_proxy_line(const char *line, managed_proxy_t *mp) } else if (!strcmpstart(line, PROTO_LOG)) { parse_log_line(line); return; - } else if (!strcmpstart(line, SPAWN_ERROR_MESSAGE)) { - /* managed proxy launch failed: parse error message to learn why. */ - int retval, child_state, saved_errno; - retval = tor_sscanf(line, SPAWN_ERROR_MESSAGE "%x/%x", - &child_state, &saved_errno); - if (retval == 2) { - log_warn(LD_GENERAL, - "Could not launch managed proxy executable at '%s' ('%s').", - mp->argv[0], strerror(saved_errno)); - } else { /* failed to parse error message */ - log_warn(LD_GENERAL,"Could not launch managed proxy executable at '%s'.", - mp->argv[0]); - } - - mp->conf_state = PT_PROTO_FAILED_LAUNCH; - return; } log_notice(LD_GENERAL, "Unknown line received by managed proxy (%s).", line); diff --git a/src/lib/process/subprocess.c b/src/lib/process/subprocess.c index 70851c15e0..c163790155 100644 --- a/src/lib/process/subprocess.c +++ b/src/lib/process/subprocess.c @@ -142,236 +142,6 @@ tor_join_win_cmdline(const char *argv[]) return joined_argv; } -#ifndef _WIN32 -/** Format child_state and saved_errno as a hex string placed in - * hex_errno. Called between fork and _exit, so must be signal-handler - * safe. - * - * hex_errno must have at least HEX_ERRNO_SIZE+1 bytes available. - * - * The format of hex_errno is: "CHILD_STATE/ERRNO\n", left-padded - * with spaces. CHILD_STATE indicates where - * in the process of starting the child process did the failure occur (see - * CHILD_STATE_* macros for definition), and SAVED_ERRNO is the value of - * errno when the failure occurred. - * - * On success return the number of characters added to hex_errno, not counting - * the terminating NUL; return -1 on error. - */ -STATIC int -format_helper_exit_status(unsigned char child_state, int saved_errno, - char *hex_errno) -{ - unsigned int unsigned_errno; - int written, left; - char *cur; - size_t i; - int res = -1; - - /* Fill hex_errno with spaces, and a trailing newline (memset may - not be signal handler safe, so we can't use it) */ - for (i = 0; i < (HEX_ERRNO_SIZE - 1); i++) - hex_errno[i] = ' '; - hex_errno[HEX_ERRNO_SIZE - 1] = '\n'; - - /* Convert errno to be unsigned for hex conversion */ - if (saved_errno < 0) { - // Avoid overflow on the cast to unsigned int when result is INT_MIN - // by adding 1 to the signed int negative value, - // then, after it has been negated and cast to unsigned, - // adding the original 1 back (the double-addition is intentional). - // Otherwise, the cast to signed could cause a temporary int - // to equal INT_MAX + 1, which is undefined. - unsigned_errno = ((unsigned int) -(saved_errno + 1)) + 1; - } else { - unsigned_errno = (unsigned int) saved_errno; - } - - /* - * Count how many chars of space we have left, and keep a pointer into the - * current point in the buffer. - */ - left = HEX_ERRNO_SIZE+1; - cur = hex_errno; - - /* Emit child_state */ - written = format_hex_number_sigsafe(child_state, cur, left); - - if (written <= 0) - goto err; - - /* Adjust left and cur */ - left -= written; - cur += written; - if (left <= 0) - goto err; - - /* Now the '/' */ - *cur = '/'; - - /* Adjust left and cur */ - ++cur; - --left; - if (left <= 0) - goto err; - - /* Need minus? */ - if (saved_errno < 0) { - *cur = '-'; - ++cur; - --left; - if (left <= 0) - goto err; - } - - /* Emit unsigned_errno */ - written = format_hex_number_sigsafe(unsigned_errno, cur, left); - - if (written <= 0) - goto err; - - /* Adjust left and cur */ - left -= written; - cur += written; - - /* Check that we have enough space left for a newline and a NUL */ - if (left <= 1) - goto err; - - /* Emit the newline and NUL */ - *cur++ = '\n'; - *cur++ = '\0'; - - res = (int)(cur - hex_errno - 1); - - goto done; - - err: - /* - * In error exit, just write a '\0' in the first char so whatever called - * this at least won't fall off the end. - */ - *hex_errno = '\0'; - - done: - return res; -} -#endif /* !defined(_WIN32) */ - -/* Maximum number of file descriptors, if we cannot get it via sysconf() */ -#define DEFAULT_MAX_FD 256 - -/** Terminate the process of process_handle, if that process has not - * already exited. - * - * Return 0 if we succeeded in terminating the process (or if the process - * already exited), and -1 if we tried to kill the process but failed. - * - * Based on code originally borrowed from Python's os.kill. */ -int -tor_terminate_process(process_handle_t *process_handle) -{ -#ifdef _WIN32 - if (tor_get_exit_code(process_handle, 0, NULL) == PROCESS_EXIT_RUNNING) { - HANDLE handle = process_handle->pid.hProcess; - - if (!TerminateProcess(handle, 0)) - return -1; - else - return 0; - } -#else /* !(defined(_WIN32)) */ - if (process_handle->waitpid_cb) { - /* We haven't got a waitpid yet, so we can just kill off the process. */ - return kill(process_handle->pid, SIGTERM); - } -#endif /* defined(_WIN32) */ - - return 0; /* We didn't need to kill the process, so report success */ -} - -/** Return the Process ID of process_handle. */ -int -tor_process_get_pid(process_handle_t *process_handle) -{ -#ifdef _WIN32 - return (int) process_handle->pid.dwProcessId; -#else - return (int) process_handle->pid; -#endif -} - -#ifdef _WIN32 -HANDLE -tor_process_get_stdout_pipe(process_handle_t *process_handle) -{ - return process_handle->stdout_pipe; -} -#else /* !(defined(_WIN32)) */ -/* DOCDOC tor_process_get_stdout_pipe */ -int -tor_process_get_stdout_pipe(process_handle_t *process_handle) -{ - return process_handle->stdout_pipe; -} -#endif /* defined(_WIN32) */ - -/* DOCDOC process_handle_new */ -static process_handle_t * -process_handle_new(void) -{ - process_handle_t *out = tor_malloc_zero(sizeof(process_handle_t)); - -#ifdef _WIN32 - out->stdin_pipe = INVALID_HANDLE_VALUE; - out->stdout_pipe = INVALID_HANDLE_VALUE; - out->stderr_pipe = INVALID_HANDLE_VALUE; -#else - out->stdin_pipe = -1; - out->stdout_pipe = -1; - out->stderr_pipe = -1; -#endif /* defined(_WIN32) */ - - return out; -} - -#ifndef _WIN32 -/** Invoked when a process that we've launched via tor_spawn_background() has - * been found to have terminated. - */ -static void -process_handle_waitpid_cb(int status, void *arg) -{ - process_handle_t *process_handle = arg; - - process_handle->waitpid_exit_status = status; - clear_waitpid_callback(process_handle->waitpid_cb); - if (process_handle->status == PROCESS_STATUS_RUNNING) - process_handle->status = PROCESS_STATUS_NOTRUNNING; - process_handle->waitpid_cb = 0; -} -#endif /* !defined(_WIN32) */ - -/** - * @name child-process states - * - * Each of these values represents a possible state that a child process can - * be in. They're used to determine what to say when telling the parent how - * far along we were before failure. - * - * @{ - */ -#define CHILD_STATE_INIT 0 -#define CHILD_STATE_PIPE 1 -#define CHILD_STATE_MAXFD 2 -#define CHILD_STATE_FORK 3 -#define CHILD_STATE_DUPOUT 4 -#define CHILD_STATE_DUPERR 5 -#define CHILD_STATE_DUPIN 6 -#define CHILD_STATE_CLOSEFD 7 -#define CHILD_STATE_EXEC 8 -#define CHILD_STATE_FAILEXEC 9 -/** @} */ /** * Boolean. If true, then Tor may call execve or CreateProcess via * tor_spawn_background. @@ -386,851 +156,3 @@ tor_disable_spawning_background_processes(void) { may_spawn_background_process = 0; } -/** Start a program in the background. If filename 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 filename. - * 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 argv 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 filename 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, const char **argv, - process_environment_t *env, - process_handle_t **process_handle_out) -{ - if (BUG(may_spawn_background_process == 0)) { - /* We should never reach this point if we're forbidden to spawn - * processes. Instead we should have caught the attempt earlier. */ - return PROCESS_STATUS_ERROR; - } - -#ifdef _WIN32 - HANDLE stdout_pipe_read = NULL; - HANDLE stdout_pipe_write = NULL; - HANDLE stderr_pipe_read = NULL; - HANDLE stderr_pipe_write = NULL; - HANDLE stdin_pipe_read = NULL; - HANDLE stdin_pipe_write = NULL; - process_handle_t *process_handle; - int status; - - STARTUPINFOA siStartInfo; - BOOL retval = FALSE; - - SECURITY_ATTRIBUTES saAttr; - char *joined_argv; - - 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 */ - 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 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 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 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 status; - } - - /* Set up pipe for stdin */ - if (!CreatePipe(&stdin_pipe_read, &stdin_pipe_write, &saAttr, 0)) { - log_warn(LD_GENERAL, - "Failed to create pipe for stdin communication with child process: %s", - format_win32_error(GetLastError())); - return status; - } - if (!SetHandleInformation(stdin_pipe_write, HANDLE_FLAG_INHERIT, 0)) { - log_warn(LD_GENERAL, - "Failed to configure pipe for stdin communication with child " - "process: %s", format_win32_error(GetLastError())); - return status; - } - - /* Create the child process */ - - /* Windows expects argv to be a whitespace delimited string, so join argv up - */ - joined_argv = tor_join_win_cmdline(argv); - - process_handle = process_handle_new(); - process_handle->status = status; - - 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 = stdin_pipe_read; - siStartInfo.dwFlags |= STARTF_USESTDHANDLES; - - /* Create the child process */ - - retval = CreateProcessA(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?) */ - CREATE_NO_WINDOW, // creation flags - (env==NULL) ? NULL : env->windows_environment_block, - 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())); - tor_free(process_handle); - } 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->stdin_pipe = stdin_pipe_write; - status = process_handle->status = PROCESS_STATUS_RUNNING; - } - - /* TODO: Close pipes on exit */ - *process_handle_out = process_handle; - return status; -#else /* !(defined(_WIN32)) */ - pid_t pid; - int stdout_pipe[2]; - int stderr_pipe[2]; - int stdin_pipe[2]; - int fd, retval; - process_handle_t *process_handle; - int status; - - const char *error_message = SPAWN_ERROR_MESSAGE; - size_t error_message_length; - - /* Represents where in the process of spawning the program is; - this is used for printing out the error message */ - unsigned char child_state = CHILD_STATE_INIT; - - char hex_errno[HEX_ERRNO_SIZE + 2]; /* + 1 should be sufficient actually */ - - static int max_fd = -1; - - 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); - - // child_state = CHILD_STATE_PIPE; - - /* Set up pipe for redirecting stdout, stderr, and stdin of child */ - retval = pipe(stdout_pipe); - if (-1 == retval) { - log_warn(LD_GENERAL, - "Failed to set up pipe for stdout communication with child process: %s", - strerror(errno)); - return status; - } - - retval = pipe(stderr_pipe); - if (-1 == retval) { - log_warn(LD_GENERAL, - "Failed to set up pipe for stderr communication with child process: %s", - strerror(errno)); - - close(stdout_pipe[0]); - close(stdout_pipe[1]); - - return status; - } - - retval = pipe(stdin_pipe); - if (-1 == retval) { - log_warn(LD_GENERAL, - "Failed to set up pipe for stdin communication with child process: %s", - strerror(errno)); - - close(stdout_pipe[0]); - close(stdout_pipe[1]); - close(stderr_pipe[0]); - close(stderr_pipe[1]); - - return status; - } - - // child_state = CHILD_STATE_MAXFD; - -#ifdef _SC_OPEN_MAX - if (-1 == max_fd) { - max_fd = (int) sysconf(_SC_OPEN_MAX); - if (max_fd == -1) { - max_fd = DEFAULT_MAX_FD; - log_warn(LD_GENERAL, - "Cannot find maximum file descriptor, assuming %d", max_fd); - } - } -#else /* !(defined(_SC_OPEN_MAX)) */ - max_fd = DEFAULT_MAX_FD; -#endif /* defined(_SC_OPEN_MAX) */ - - // child_state = CHILD_STATE_FORK; - - pid = fork(); - if (0 == pid) { - /* In child */ - -#if defined(HAVE_SYS_PRCTL_H) && defined(__linux__) - /* Attempt to have the kernel issue a SIGTERM if the parent - * goes away. Certain attributes of the binary being execve()ed - * will clear this during the execve() call, but it's better - * than nothing. - */ - prctl(PR_SET_PDEATHSIG, SIGTERM); -#endif /* defined(HAVE_SYS_PRCTL_H) && defined(__linux__) */ - - child_state = CHILD_STATE_DUPOUT; - - /* Link child stdout to the write end of the pipe */ - retval = dup2(stdout_pipe[1], STDOUT_FILENO); - if (-1 == retval) - goto error; - - child_state = CHILD_STATE_DUPERR; - - /* Link child stderr to the write end of the pipe */ - retval = dup2(stderr_pipe[1], STDERR_FILENO); - if (-1 == retval) - goto error; - - child_state = CHILD_STATE_DUPIN; - - /* Link child stdin to the read end of the pipe */ - retval = dup2(stdin_pipe[0], STDIN_FILENO); - if (-1 == retval) - goto error; - - // child_state = CHILD_STATE_CLOSEFD; - - close(stderr_pipe[0]); - close(stderr_pipe[1]); - close(stdout_pipe[0]); - close(stdout_pipe[1]); - close(stdin_pipe[0]); - close(stdin_pipe[1]); - - /* Close all other fds, including the read end of the pipe */ - /* XXX: We should now be doing enough FD_CLOEXEC setting to make - * this needless. */ - for (fd = STDERR_FILENO + 1; fd < max_fd; fd++) { - close(fd); - } - - // child_state = CHILD_STATE_EXEC; - - /* Call the requested program. We need the cast because - execvp doesn't define argv as const, even though it - does not modify the arguments */ - if (env) - execve(filename, (char *const *) argv, env->unixoid_environment_block); - else { - static char *new_env[] = { NULL }; - execve(filename, (char *const *) argv, new_env); - } - - /* If we got here, the exec or open(/dev/null) failed */ - - child_state = CHILD_STATE_FAILEXEC; - - error: - { - /* XXX: are we leaking fds from the pipe? */ - int n, err=0; - ssize_t nbytes; - - n = format_helper_exit_status(child_state, errno, hex_errno); - - if (n >= 0) { - /* 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); - err = (nbytes < 0); - nbytes = write(STDOUT_FILENO, hex_errno, n); - err += (nbytes < 0); - } - - _exit(err?254:255); // exit ok: in child. - } - - /* Never reached, but avoids compiler warning */ - return status; // LCOV_EXCL_LINE - } - - /* In parent */ - - if (-1 == pid) { - log_warn(LD_GENERAL, "Failed to fork child process: %s", strerror(errno)); - close(stdin_pipe[0]); - close(stdin_pipe[1]); - close(stdout_pipe[0]); - close(stdout_pipe[1]); - close(stderr_pipe[0]); - close(stderr_pipe[1]); - return status; - } - - process_handle = process_handle_new(); - process_handle->status = 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 */ - 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)); - } - - process_handle->waitpid_cb = set_waitpid_callback(pid, - process_handle_waitpid_cb, - process_handle); - - 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)); - } - - /* Return write end of the stdin pipe to caller, and close the read end */ - process_handle->stdin_pipe = stdin_pipe[1]; - retval = close(stdin_pipe[0]); - - if (-1 == retval) { - log_warn(LD_GENERAL, - "Failed to close read end of stdin pipe in parent process: %s", - strerror(errno)); - } - - status = process_handle->status = PROCESS_STATUS_RUNNING; - /* Set stdin/stdout/stderr pipes to be non-blocking */ - if (fcntl(process_handle->stdout_pipe, F_SETFL, O_NONBLOCK) < 0 || - fcntl(process_handle->stderr_pipe, F_SETFL, O_NONBLOCK) < 0 || - fcntl(process_handle->stdin_pipe, F_SETFL, O_NONBLOCK) < 0) { - log_warn(LD_GENERAL, "Failed to set stderror/stdout/stdin pipes " - "nonblocking in parent process: %s", strerror(errno)); - } - - *process_handle_out = process_handle; - return status; -#endif /* defined(_WIN32) */ -} - -/** Destroy all resources allocated by the process handle in - * process_handle. - * If also_terminate_process is true, also terminate the - * process of the process handle. */ -MOCK_IMPL(void, -tor_process_handle_destroy,(process_handle_t *process_handle, - int also_terminate_process)) -{ - if (!process_handle) - return; - - if (also_terminate_process) { - if (tor_terminate_process(process_handle) < 0) { - const char *errstr = -#ifdef _WIN32 - format_win32_error(GetLastError()); -#else - strerror(errno); -#endif - log_notice(LD_GENERAL, "Failed to terminate process with " - "PID '%d' ('%s').", tor_process_get_pid(process_handle), - errstr); - } else { - log_info(LD_GENERAL, "Terminated process with PID '%d'.", - tor_process_get_pid(process_handle)); - } - } - - process_handle->status = PROCESS_STATUS_NOTRUNNING; - -#ifdef _WIN32 - if (process_handle->stdout_pipe) - CloseHandle(process_handle->stdout_pipe); - - if (process_handle->stderr_pipe) - CloseHandle(process_handle->stderr_pipe); - - if (process_handle->stdin_pipe) - CloseHandle(process_handle->stdin_pipe); -#else /* !(defined(_WIN32)) */ - close(process_handle->stdout_pipe); - close(process_handle->stderr_pipe); - close(process_handle->stdin_pipe); - - clear_waitpid_callback(process_handle->waitpid_cb); -#endif /* defined(_WIN32) */ - - memset(process_handle, 0x0f, sizeof(process_handle_t)); - tor_free(process_handle); -} - -/** Get the exit code of a process specified by process_handle and store - * it in exit_code, if set to a non-NULL value. If block 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(process_handle_t *process_handle, - int block, int *exit_code) -{ -#ifdef _WIN32 - 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 /* !(defined(_WIN32)) */ - int stat_loc; - int retval; - - if (process_handle->waitpid_cb) { - /* We haven't processed a SIGCHLD yet. */ - retval = waitpid(process_handle->pid, &stat_loc, block?0:WNOHANG); - if (retval == process_handle->pid) { - clear_waitpid_callback(process_handle->waitpid_cb); - process_handle->waitpid_cb = NULL; - process_handle->waitpid_exit_status = stat_loc; - } - } else { - /* We already got a SIGCHLD for this process, and handled it. */ - retval = process_handle->pid; - stat_loc = process_handle->waitpid_exit_status; - } - - 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", - (int)process_handle->pid, strerror(errno)); - return PROCESS_EXIT_ERROR; - } - - if (!WIFEXITED(stat_loc)) { - log_warn(LD_GENERAL, "Process %d did not exit normally", - (int)process_handle->pid); - return PROCESS_EXIT_ERROR; - } - - if (exit_code != NULL) - *exit_code = WEXITSTATUS(stat_loc); -#endif /* defined(_WIN32) */ - - return PROCESS_EXIT_EXITED; -} - -#ifdef _WIN32 -/** Read from a handle h into buf, up to count bytes. If - * hProcess is NULL, the function will return immediately if there is - * nothing more to read. Otherwise hProcess should be set to the handle - * to the process owning the h. In this case, the function will exit - * only once the process has exited, or count 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, - const process_handle_t *process) -{ - size_t numread = 0; - BOOL retval; - DWORD byte_count; - BOOL process_exited = FALSE; - - if (count > SIZE_T_CEILING || count > SSIZE_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 == process) - 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 */ - tor_assert(process != NULL); - byte_count = WaitForSingleObject(process->pid.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; -} -#else /* !(defined(_WIN32)) */ -/** Read from a handle fd into buf, up to count bytes. If - * process is NULL, the function will return immediately if there is - * nothing more to read. Otherwise data will be read until end of file, or - * count bytes are read. Returns the number of bytes read, or -1 on - * error. Sets eof to true if eof is not NULL and the end of the - * file has been reached. */ -ssize_t -tor_read_all_handle(int fd, char *buf, size_t count, - const process_handle_t *process, - int *eof) -{ - size_t numread = 0; - ssize_t result; - - if (eof) - *eof = 0; - - if (count > SIZE_T_CEILING || count > SSIZE_MAX) - return -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 (result < 0) { - log_warn(LD_GENERAL, "read() failed: %s", strerror(errno)); - return -1; - } - - numread += result; - } - - log_debug(LD_GENERAL, "read() read %d bytes from handle", (int)numread); - return (ssize_t)numread; -} -#endif /* defined(_WIN32) */ - -/** 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 _WIN32 - return tor_read_all_handle(process_handle->stdout_pipe, buf, count, - process_handle); -#else - return tor_read_all_handle(process_handle->stdout_pipe, buf, count, - process_handle, NULL); -#endif /* defined(_WIN32) */ -} - -/** 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 _WIN32 - return tor_read_all_handle(process_handle->stderr_pipe, buf, count, - process_handle); -#else - return tor_read_all_handle(process_handle->stderr_pipe, buf, count, - process_handle, NULL); -#endif /* defined(_WIN32) */ -} - -/** Return a string corresponding to stream_status. */ -const char * -stream_status_to_string(enum stream_status stream_status) -{ - switch (stream_status) { - case IO_STREAM_OKAY: - return "okay"; - case IO_STREAM_EAGAIN: - return "temporarily unavailable"; - case IO_STREAM_TERM: - return "terminated"; - case IO_STREAM_CLOSED: - return "closed"; - default: - tor_fragile_assert(); - return "unknown"; - } -} - -/** Split buf into lines, and add to smartlist. The buffer buf will be - * modified. The resulting smartlist will consist of pointers to buf, so there - * is no need to free the contents of sl. buf must be a NUL-terminated - * string. len should be set to the length of the buffer excluding the - * NUL. Non-printable characters (including NUL) 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 nul */ - 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 NUL. cur points to - * the character after the NUL. */ - if (in_line) - smartlist_add(sl, (void *)(buf+start)); - in_line = 0; - } - return smartlist_len(sl); -} - -#ifdef _WIN32 - -/** Return a smartlist containing lines outputted from - * handle. Return NULL on error, and set - * stream_status_out appropriately. */ -MOCK_IMPL(smartlist_t *, -tor_get_lines_from_handle, (HANDLE *handle, - enum stream_status *stream_status_out)) -{ - int pos; - char stdout_buf[600] = {0}; - smartlist_t *lines = NULL; - - tor_assert(stream_status_out); - - *stream_status_out = IO_STREAM_TERM; - - pos = tor_read_all_handle(handle, stdout_buf, sizeof(stdout_buf) - 1, NULL); - if (pos < 0) { - *stream_status_out = IO_STREAM_TERM; - return NULL; - } - if (pos == 0) { - *stream_status_out = IO_STREAM_EAGAIN; - return NULL; - } - - /* End with a null even if there isn't a \r\n at the end */ - /* TODO: What if this is a partial line? */ - stdout_buf[pos] = '\0'; - - /* Split up the buffer */ - lines = smartlist_new(); - tor_split_lines(lines, stdout_buf, pos); - - /* Currently 'lines' is populated with strings residing on the - stack. Replace them with their exact copies on the heap: */ - SMARTLIST_FOREACH(lines, char *, line, - SMARTLIST_REPLACE_CURRENT(lines, line, tor_strdup(line))); - - *stream_status_out = IO_STREAM_OKAY; - - return lines; -} - -#else /* !(defined(_WIN32)) */ - -/** Return a smartlist containing lines outputted from - * fd. Return NULL on error, and set - * stream_status_out appropriately. */ -MOCK_IMPL(smartlist_t *, -tor_get_lines_from_handle, (int fd, enum stream_status *stream_status_out)) -{ - enum stream_status stream_status; - char stdout_buf[400]; - smartlist_t *lines = NULL; - - while (1) { - memset(stdout_buf, 0, sizeof(stdout_buf)); - - stream_status = get_string_from_pipe(fd, - stdout_buf, sizeof(stdout_buf) - 1); - if (stream_status != IO_STREAM_OKAY) - goto done; - - if (!lines) lines = smartlist_new(); - smartlist_split_string(lines, stdout_buf, "\n", 0, 0); - } - - done: - *stream_status_out = stream_status; - return lines; -} - -#endif /* defined(_WIN32) */ - -/** Reads from fd and stores input in buf_out making - * sure it's below count bytes. - * If the string has a trailing newline, we strip it off. - * - * This function is specifically created to handle input from managed - * proxies, according to the pluggable transports spec. Make sure it - * fits your needs before using it. - * - * Returns: - * IO_STREAM_CLOSED: If the stream is closed. - * IO_STREAM_EAGAIN: If there is nothing to read and we should check back - * later. - * IO_STREAM_TERM: If something is wrong with the stream. - * IO_STREAM_OKAY: If everything went okay and we got a string - * in buf_out. */ -enum stream_status -get_string_from_pipe(int fd, char *buf_out, size_t count) -{ - ssize_t ret; - - tor_assert(count <= INT_MAX); - - ret = read(fd, buf_out, count); - - 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[ret - 1] == '\n') { - /* Remove the trailing newline */ - buf_out[ret - 1] = '\0'; - } else - buf_out[ret] = '\0'; - - return IO_STREAM_OKAY; -} diff --git a/src/lib/process/subprocess.h b/src/lib/process/subprocess.h index 5b4318ef2b..eb91626343 100644 --- a/src/lib/process/subprocess.h +++ b/src/lib/process/subprocess.h @@ -11,124 +11,10 @@ #ifndef TOR_SUBPROCESS_H #define TOR_SUBPROCESS_H -#include "lib/cc/torint.h" -#include "lib/testsupport/testsupport.h" -#include -#ifdef _WIN32 -#include -#endif - -struct smartlist_t; - void tor_disable_spawning_background_processes(void); -typedef struct process_handle_t process_handle_t; -struct process_environment_t; -int tor_spawn_background(const char *const filename, const char **argv, - struct process_environment_t *env, - process_handle_t **process_handle_out); - -#define SPAWN_ERROR_MESSAGE "ERR: Failed to spawn background process - code " - -/** Status of an I/O stream. */ -enum stream_status { - IO_STREAM_OKAY, - IO_STREAM_EAGAIN, - IO_STREAM_TERM, - IO_STREAM_CLOSED -}; - -const char *stream_status_to_string(enum stream_status stream_status); - -enum stream_status get_string_from_pipe(int fd, char *buf, size_t count); - -/* Values of process_handle_t.status. */ -#define PROCESS_STATUS_NOTRUNNING 0 -#define PROCESS_STATUS_RUNNING 1 -#define PROCESS_STATUS_ERROR -1 - -#ifdef SUBPROCESS_PRIVATE -struct waitpid_callback_t; - -/** Structure to represent the state of a process with which Tor is - * communicating. The contents of this structure are private to util.c */ -struct process_handle_t { - /** One of the PROCESS_STATUS_* values */ - int status; -#ifdef _WIN32 - HANDLE stdin_pipe; - HANDLE stdout_pipe; - HANDLE stderr_pipe; - PROCESS_INFORMATION pid; -#else /* !(defined(_WIN32)) */ - int stdin_pipe; - int stdout_pipe; - int stderr_pipe; - 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 - * contains NULL. */ - struct waitpid_callback_t *waitpid_cb; - /** The exit status reported by waitpid. */ - int waitpid_exit_status; -#endif /* defined(_WIN32) */ -}; -#endif /* defined(SUBPROCESS_PRIVATE) */ - -/* 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(process_handle_t *process_handle, - int block, int *exit_code); -int tor_split_lines(struct smartlist_t *sl, char *buf, int len); -#ifdef _WIN32 -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(int fd, char *buf, size_t count, - const process_handle_t *process, - int *eof); -#endif /* defined(_WIN32) */ -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); +#ifndef _WIN32 char *tor_join_win_cmdline(const char *argv[]); - -int tor_process_get_pid(process_handle_t *process_handle); -#ifdef _WIN32 -HANDLE tor_process_get_stdout_pipe(process_handle_t *process_handle); -#else -int tor_process_get_stdout_pipe(process_handle_t *process_handle); #endif -#ifdef _WIN32 -MOCK_DECL(struct smartlist_t *, tor_get_lines_from_handle,(HANDLE *handle, - enum stream_status *stream_status)); -#else -MOCK_DECL(struct smartlist_t *, tor_get_lines_from_handle,(int fd, - enum stream_status *stream_status)); -#endif /* defined(_WIN32) */ - -int tor_terminate_process(process_handle_t *process_handle); - -MOCK_DECL(void, tor_process_handle_destroy,(process_handle_t *process_handle, - int also_terminate_process)); - -#ifdef SUBPROCESS_PRIVATE -/* Prototypes for private functions only used by util.c (and unit tests) */ - -#ifndef _WIN32 -STATIC int format_helper_exit_status(unsigned char child_state, - int saved_errno, char *hex_errno); - -/* Space for hex values of child state, a slash, saved_errno (with - leading minus) and newline (no null) */ -#define HEX_ERRNO_SIZE (sizeof(char) * 2 + 1 + \ - 1 + sizeof(int) * 2 + 1) -#endif /* !defined(_WIN32) */ - -#endif /* defined(SUBPROCESS_PRIVATE) */ - #endif diff --git a/src/test/Makefile.nmake b/src/test/Makefile.nmake index cfbe281b94..aa16a22b52 100644 --- a/src/test/Makefile.nmake +++ b/src/test/Makefile.nmake @@ -1,4 +1,4 @@ -all: test.exe test-child.exe bench.exe +all: test.exe bench.exe CFLAGS = /I ..\win32 /I ..\..\..\build-alpha\include /I ..\common /I ..\or \ /I ..\ext @@ -30,8 +30,5 @@ test.exe: $(TEST_OBJECTS) bench.exe: bench.obj $(CC) $(CFLAGS) bench.obj $(LIBS) ..\common\*.lib /Fe$@ -test-child.exe: test-child.obj - $(CC) $(CFLAGS) test-child.obj /Fe$@ - clean: - del *.obj *.lib test.exe bench.exe test-child.exe + del *.obj *.lib test.exe bench.exe diff --git a/src/test/include.am b/src/test/include.am index 9df2fd481d..e1f55e5d03 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -65,7 +65,6 @@ noinst_PROGRAMS+= \ src/test/test \ src/test/test-slow \ src/test/test-memwipe \ - src/test/test-child \ src/test/test-process \ src/test/test_workqueue \ src/test/test-switch-id \ @@ -204,7 +203,6 @@ src_test_test_slow_SOURCES += \ src/test/test_slow.c \ src/test/test_crypto_slow.c \ src/test/test_process_slow.c \ - src/test/test_util_slow.c \ src/test/testing_common.c \ src/test/testing_rsakeys.c \ src/ext/tinytest.c diff --git a/src/test/test-child.c b/src/test/test-child.c deleted file mode 100644 index 14df1a9b76..0000000000 --- a/src/test/test-child.c +++ /dev/null @@ -1,61 +0,0 @@ -/* Copyright (c) 2011-2018, The Tor Project, Inc. */ -/* See LICENSE for licensing information */ - -#include "orconfig.h" -#include -#ifdef _WIN32 -#define WINDOWS_LEAN_AND_MEAN -#include -#else -#include -#endif /* defined(_WIN32) */ -#include - -#ifdef _WIN32 -#define SLEEP(sec) Sleep((sec)*1000) -#else -#define SLEEP(sec) sleep(sec) -#endif - -/** Trivial test program which prints out its command line arguments so we can - * check if tor_spawn_background() works */ -int -main(int argc, char **argv) -{ - int i; - int delay = 1; - int fast = 0; - - if (argc > 1) { - if (!strcmp(argv[1], "--hang")) { - delay = 60; - } else if (!strcmp(argv[1], "--fast")) { - fast = 1; - delay = 0; - } - } - - fprintf(stdout, "OUT\n"); - fprintf(stderr, "ERR\n"); - for (i = 1; i < argc; i++) - fprintf(stdout, "%s\n", argv[i]); - if (!fast) - 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); - if (!fast) - SLEEP(1); - fprintf(stdout, "DONE\n"); - fflush(stdout); - if (fast) - return 0; - - while (--delay) { - SLEEP(1); - } - - return 0; -} - diff --git a/src/test/test.h b/src/test/test.h index 9f72623968..ce50fa89d9 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -271,7 +271,6 @@ extern struct testcase_t x509_tests[]; extern struct testcase_t slow_crypto_tests[]; extern struct testcase_t slow_process_tests[]; -extern struct testcase_t slow_util_tests[]; extern struct testgroup_t testgroups[]; diff --git a/src/test/test_logging.c b/src/test/test_logging.c index cd685a4af7..9d3ee5d55f 100644 --- a/src/test/test_logging.c +++ b/src/test/test_logging.c @@ -117,22 +117,27 @@ test_sigsafe_err(void *arg) content = read_file_to_str(fn, 0, NULL); tt_ptr_op(content, OP_NE, NULL); - tor_split_lines(lines, content, (int)strlen(content)); + smartlist_split_string(lines, content, "\n", 0, 0); tt_int_op(smartlist_len(lines), OP_GE, 5); - if (strstr(smartlist_get(lines, 0), "opening new log file")) + if (strstr(smartlist_get(lines, 0), "opening new log file")) { + void *item = smartlist_get(lines, 0); smartlist_del_keeporder(lines, 0); + tor_free(item); + } + tt_assert(strstr(smartlist_get(lines, 0), "Say, this isn't too cool")); - /* Next line is blank. */ - tt_assert(!strcmpstart(smartlist_get(lines, 1), "==============")); - tt_assert(!strcmpstart(smartlist_get(lines, 2), "Minimal.")); - /* Next line is blank. */ - tt_assert(!strcmpstart(smartlist_get(lines, 3), "==============")); - tt_str_op(smartlist_get(lines, 4), OP_EQ, + tt_str_op(smartlist_get(lines, 1), OP_EQ, ""); + tt_assert(!strcmpstart(smartlist_get(lines, 2), "==============")); + tt_assert(!strcmpstart(smartlist_get(lines, 3), "Minimal.")); + tt_str_op(smartlist_get(lines, 4), OP_EQ, ""); + tt_assert(!strcmpstart(smartlist_get(lines, 5), "==============")); + tt_str_op(smartlist_get(lines, 6), OP_EQ, "Testing any attempt to manually log from a signal."); done: tor_free(content); + SMARTLIST_FOREACH(lines, char *, x, tor_free(x)); smartlist_free(lines); } diff --git a/src/test/test_slow.c b/src/test/test_slow.c index 0c66eff930..97c2912af6 100644 --- a/src/test/test_slow.c +++ b/src/test/test_slow.c @@ -21,7 +21,6 @@ struct testgroup_t testgroups[] = { { "slow/crypto/", slow_crypto_tests }, { "slow/process/", slow_process_tests }, - { "slow/util/", slow_util_tests }, END_OF_GROUPS }; diff --git a/src/test/test_util.c b/src/test/test_util.c index bcface64fd..4fa67b6419 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -4301,204 +4301,6 @@ test_util_load_win_lib(void *ptr) } #endif /* defined(_WIN32) */ -#ifndef _WIN32 -static void -clear_hex_errno(char *hex_errno) -{ - memset(hex_errno, '\0', HEX_ERRNO_SIZE + 1); -} - -static void -test_util_exit_status(void *ptr) -{ - /* Leave an extra byte for a \0 so we can do string comparison */ - char hex_errno[HEX_ERRNO_SIZE + 1]; - int n; - - (void)ptr; - - clear_hex_errno(hex_errno); - tt_str_op("",OP_EQ, hex_errno); - - clear_hex_errno(hex_errno); - n = format_helper_exit_status(0, 0, hex_errno); - tt_str_op("0/0\n",OP_EQ, hex_errno); - tt_int_op(n,OP_EQ, strlen(hex_errno)); - -#if SIZEOF_INT == 4 - - clear_hex_errno(hex_errno); - n = format_helper_exit_status(0, 0x7FFFFFFF, hex_errno); - tt_str_op("0/7FFFFFFF\n",OP_EQ, hex_errno); - tt_int_op(n,OP_EQ, strlen(hex_errno)); - - clear_hex_errno(hex_errno); - n = format_helper_exit_status(0xFF, -0x80000000, hex_errno); - tt_str_op("FF/-80000000\n",OP_EQ, hex_errno); - tt_int_op(n,OP_EQ, strlen(hex_errno)); - tt_int_op(n,OP_EQ, HEX_ERRNO_SIZE); - -#elif SIZEOF_INT == 8 - - clear_hex_errno(hex_errno); - n = format_helper_exit_status(0, 0x7FFFFFFFFFFFFFFF, hex_errno); - tt_str_op("0/7FFFFFFFFFFFFFFF\n",OP_EQ, hex_errno); - tt_int_op(n,OP_EQ, strlen(hex_errno)); - - clear_hex_errno(hex_errno); - n = format_helper_exit_status(0xFF, -0x8000000000000000, hex_errno); - tt_str_op("FF/-8000000000000000\n",OP_EQ, hex_errno); - tt_int_op(n,OP_EQ, strlen(hex_errno)); - tt_int_op(n,OP_EQ, HEX_ERRNO_SIZE); - -#endif /* SIZEOF_INT == 4 || ... */ - - clear_hex_errno(hex_errno); - n = format_helper_exit_status(0x7F, 0, hex_errno); - tt_str_op("7F/0\n",OP_EQ, hex_errno); - tt_int_op(n,OP_EQ, strlen(hex_errno)); - - clear_hex_errno(hex_errno); - n = format_helper_exit_status(0x08, -0x242, hex_errno); - tt_str_op("8/-242\n",OP_EQ, hex_errno); - tt_int_op(n,OP_EQ, strlen(hex_errno)); - - clear_hex_errno(hex_errno); - tt_str_op("",OP_EQ, hex_errno); - - done: - ; -} -#endif /* !defined(_WIN32) */ - -#ifndef _WIN32 -static void -test_util_string_from_pipe(void *ptr) -{ - int test_pipe[2] = {-1, -1}; - int retval = 0; - enum stream_status status = IO_STREAM_TERM; - ssize_t retlen; - char buf[4] = { 0 }; - - (void)ptr; - - errno = 0; - - /* Set up a pipe to test on */ - retval = pipe(test_pipe); - 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); - - 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 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 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_int_op(status, OP_EQ, IO_STREAM_OKAY); - tt_str_op(buf, OP_EQ, ""); - errno = 0; - - /* Send in a string that contains a trailing newline. */ - retlen = write(test_pipe[1], "AB\n", 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 a string that contains a newline only. */ - retlen = write(test_pipe[1], "\n", 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_int_op(status, OP_EQ, IO_STREAM_OKAY); - tt_str_op(buf, OP_EQ, ""); - errno = 0; - - /* 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_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; - - 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 multiple lines. */ - retlen = write(test_pipe[1], "A\nB", 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, "A\nB"); - 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; - - 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; - - /* Check for EOF */ - 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_CLOSED); - errno = 0; - - done: - if (test_pipe[0] != -1) - close(test_pipe[0]); - if (test_pipe[1] != -1) - close(test_pipe[1]); -} - -#endif /* !defined(_WIN32) */ - /** * Test for format_hex_number_sigsafe() */ @@ -4651,67 +4453,6 @@ struct split_lines_test_t { 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}}, - {"line 1\r\nline 2", 14, {"line 1", "line 2", NULL}}, - {"line 1\r\n\r\nline 2", 16, {"line 1", "line 2", NULL}}, - {"line 1\r\n\r\r\r\nline 2", 18, {"line 1", "line 2", NULL}}, - {"line 1\r\n\n\n\n\rline 2", 18, {"line 1", "line 2", NULL}}, - {"line 1\r\n\r\t\r\nline 3", 18, {"line 1", ".", "line 3", NULL}}, - {"\n\t\r\t\nline 3", 11, {".", ".", "line 3", NULL}}, - {NULL, 0, { NULL }} - }; - - int i, j; - char *orig_line=NULL; - smartlist_t *sl=NULL; - - (void)ptr; - - for (i=0; tests[i].orig_line; i++) { - sl = smartlist_new(); - /* Allocate space for string and trailing NULL */ - orig_line = tor_memdup(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_BEGIN(sl, const char *, line) { - /* Check we have not got too many lines */ - tt_int_op(MAX_SPLIT_LINE_COUNT, OP_GT, j); - /* Check that there actually should be a line here */ - tt_ptr_op(tests[i].split_line[j], OP_NE, 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(line,OP_EQ, tests[i].split_line[j]); - j++; - } SMARTLIST_FOREACH_END(line); - /* Check that we didn't miss some lines */ - tt_ptr_op(NULL,OP_EQ, tests[i].split_line[j]); - tor_free(orig_line); - smartlist_free(sl); - sl = NULL; - } - - done: - tor_free(orig_line); - smartlist_free(sl); -} - static void test_util_di_ops(void *arg) { @@ -6483,12 +6224,9 @@ struct testcase_t util_tests[] = { UTIL_TEST(nowrap_math, 0), UTIL_TEST(num_cpus, 0), UTIL_TEST_WIN_ONLY(load_win_lib, 0), - UTIL_TEST_NO_WIN(exit_status, 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), - UTIL_TEST(split_lines, 0), UTIL_TEST(n_bits_set, 0), UTIL_TEST(eat_whitespace, 0), UTIL_TEST(sl_new_from_text_lines, 0), diff --git a/src/test/test_util_slow.c b/src/test/test_util_slow.c deleted file mode 100644 index c7b3e3e2a4..0000000000 --- a/src/test/test_util_slow.c +++ /dev/null @@ -1,396 +0,0 @@ -/* Copyright (c) 2001-2004, Roger Dingledine. - * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2018, The Tor Project, Inc. */ -/* See LICENSE for licensing information */ - -#include "orconfig.h" -#define UTIL_PRIVATE -#define SUBPROCESS_PRIVATE -#include "lib/crypt_ops/crypto_cipher.h" -#include "lib/log/log.h" -#include "lib/process/subprocess.h" -#include "lib/process/waitpid.h" -#include "lib/string/printf.h" -#include "lib/time/compat_time.h" -#include "test/test.h" - -#include -#include - -#ifndef BUILDDIR -#define BUILDDIR "." -#endif - -#ifdef _WIN32 -#define notify_pending_waitpid_callbacks() STMT_NIL -#define TEST_CHILD "test-child.exe" -#define EOL "\r\n" -#else -#define TEST_CHILD (BUILDDIR "/src/test/test-child") -#define EOL "\n" -#endif /* defined(_WIN32) */ - -#ifdef _WIN32 -/* I've assumed Windows doesn't have the gap between fork and exec - * that causes the race condition on unix-like platforms */ -#define MATCH_PROCESS_STATUS(s1,s2) ((s1) == (s2)) - -#else /* !(defined(_WIN32)) */ -/* work around a race condition of the timing of SIGCHLD handler updates - * to the process_handle's fields, and checks of those fields - * - * TODO: Once we can signal failure to exec, change PROCESS_STATUS_RUNNING to - * PROCESS_STATUS_ERROR (and similarly with *_OR_NOTRUNNING) */ -#define PROCESS_STATUS_RUNNING_OR_NOTRUNNING (PROCESS_STATUS_RUNNING+1) -#define IS_RUNNING_OR_NOTRUNNING(s) \ - ((s) == PROCESS_STATUS_RUNNING || (s) == PROCESS_STATUS_NOTRUNNING) -/* well, this is ugly */ -#define MATCH_PROCESS_STATUS(s1,s2) \ - ( (s1) == (s2) \ - ||((s1) == PROCESS_STATUS_RUNNING_OR_NOTRUNNING \ - && IS_RUNNING_OR_NOTRUNNING(s2)) \ - ||((s2) == PROCESS_STATUS_RUNNING_OR_NOTRUNNING \ - && IS_RUNNING_OR_NOTRUNNING(s1))) - -#endif /* defined(_WIN32) */ - -/** 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, - int expected_status) -{ - int retval, exit_code; - ssize_t pos; - process_handle_t *process_handle=NULL; - char stdout_buf[100], stderr_buf[100]; - int status; - - /* Start the program */ -#ifdef _WIN32 - status = tor_spawn_background(NULL, argv, NULL, &process_handle); -#else - status = tor_spawn_background(argv[0], argv, NULL, &process_handle); -#endif - - notify_pending_waitpid_callbacks(); - - /* the race condition doesn't affect status, - * because status isn't updated by the SIGCHLD handler, - * but we still need to handle PROCESS_STATUS_RUNNING_OR_NOTRUNNING */ - tt_assert(MATCH_PROCESS_STATUS(expected_status, status)); - if (status == PROCESS_STATUS_ERROR) { - tt_ptr_op(process_handle, OP_EQ, NULL); - return; - } - - tt_ptr_op(process_handle, OP_NE, NULL); - - /* When a spawned process forks, fails, then exits very quickly, - * (this typically occurs when exec fails) - * there is a race condition between the SIGCHLD handler - * updating the process_handle's fields, and this test - * checking the process status in those fields. - * The SIGCHLD update can occur before or after the code below executes. - * This causes intermittent failures in spawn_background_fail(), - * typically when the machine is under load. - * We use PROCESS_STATUS_RUNNING_OR_NOTRUNNING to avoid this issue. */ - - /* the race condition affects the change in - * process_handle->status from RUNNING to NOTRUNNING */ - tt_assert(MATCH_PROCESS_STATUS(expected_status, process_handle->status)); - -#ifndef _WIN32 - notify_pending_waitpid_callbacks(); - /* the race condition affects the change in - * process_handle->waitpid_cb to NULL, - * so we skip the check if expected_status is ambiguous, - * that is, PROCESS_STATUS_RUNNING_OR_NOTRUNNING */ - tt_assert(process_handle->waitpid_cb != NULL - || expected_status == PROCESS_STATUS_RUNNING_OR_NOTRUNNING); -#endif /* !defined(_WIN32) */ - -#ifdef _WIN32 - tt_assert(process_handle->stdout_pipe != INVALID_HANDLE_VALUE); - tt_assert(process_handle->stderr_pipe != INVALID_HANDLE_VALUE); - tt_assert(process_handle->stdin_pipe != INVALID_HANDLE_VALUE); -#else - tt_assert(process_handle->stdout_pipe >= 0); - tt_assert(process_handle->stderr_pipe >= 0); - tt_assert(process_handle->stdin_pipe >= 0); -#endif /* defined(_WIN32) */ - - /* Check stdout */ - 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(strlen(expected_out),OP_EQ, pos); - tt_str_op(expected_out,OP_EQ, stdout_buf); - - notify_pending_waitpid_callbacks(); - - /* Check it terminated correctly */ - retval = tor_get_exit_code(process_handle, 1, &exit_code); - tt_int_op(PROCESS_EXIT_EXITED,OP_EQ, retval); - tt_int_op(expected_exit,OP_EQ, exit_code); - // TODO: Make test-child exit with something other than 0 - -#ifndef _WIN32 - notify_pending_waitpid_callbacks(); - tt_ptr_op(process_handle->waitpid_cb, OP_EQ, NULL); -#endif - - /* 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(expected_err,OP_EQ, stderr_buf); - tt_int_op(strlen(expected_err),OP_EQ, pos); - - notify_pending_waitpid_callbacks(); - - done: - if (process_handle) - tor_process_handle_destroy(process_handle, 1); -} - -/** Check that we can launch a process and read the output */ -static void -test_util_spawn_background_ok(void *ptr) -{ - const char *argv[] = {TEST_CHILD, "--test", NULL}; - const char *expected_out = "OUT"EOL "--test"EOL "SLEEPING"EOL "DONE" EOL; - const char *expected_err = "ERR"EOL; - - (void)ptr; - - 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) -{ - const char *argv[] = {BUILDDIR "/src/test/no-such-file", "--test", NULL}; - const char *expected_err = ""; - char expected_out[1024]; - char code[32]; -#ifdef _WIN32 - const int expected_status = PROCESS_STATUS_ERROR; -#else - /* TODO: Once we can signal failure to exec, set this to be - * PROCESS_STATUS_RUNNING_OR_ERROR */ - const int expected_status = PROCESS_STATUS_RUNNING_OR_NOTRUNNING; -#endif /* defined(_WIN32) */ - - memset(expected_out, 0xf0, sizeof(expected_out)); - memset(code, 0xf0, sizeof(code)); - - (void)ptr; - - tor_snprintf(code, sizeof(code), "%x/%x", - 9 /* CHILD_STATE_FAILEXEC */ , ENOENT); - tor_snprintf(expected_out, sizeof(expected_out), - "ERR: Failed to spawn background process - code %s\n", code); - - 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_impl(int exit_early) -{ - const int expected_exit = 0; - const int expected_status = PROCESS_STATUS_RUNNING; - - int retval, exit_code; - ssize_t pos = -1; - process_handle_t *process_handle=NULL; - int status; - char stdout_buf[100], stderr_buf[100]; - - const char *argv[] = {TEST_CHILD, "--test", NULL}; - const char *expected_out[] = { "OUT" EOL "--test" EOL "SLEEPING" EOL, - "DONE" EOL, - NULL }; - const char *expected_err = "ERR" EOL; - -#ifndef _WIN32 - int eof = 0; -#endif - int expected_out_ctr; - - if (exit_early) { - argv[1] = "--hang"; - expected_out[0] = "OUT"EOL "--hang"EOL "SLEEPING" EOL; - } - - /* Start the program */ -#ifdef _WIN32 - status = tor_spawn_background(NULL, argv, NULL, &process_handle); -#else - status = tor_spawn_background(argv[0], argv, NULL, &process_handle); -#endif - tt_int_op(expected_status,OP_EQ, status); - tt_assert(process_handle); - tt_int_op(expected_status,OP_EQ, process_handle->status); - - /* Check stdout */ - for (expected_out_ctr = 0; expected_out[expected_out_ctr] != NULL;) { -#ifdef _WIN32 - pos = tor_read_all_handle(process_handle->stdout_pipe, stdout_buf, - sizeof(stdout_buf) - 1, NULL); -#else - /* Check that we didn't read the end of file last time */ - tt_assert(!eof); - pos = tor_read_all_handle(process_handle->stdout_pipe, stdout_buf, - sizeof(stdout_buf) - 1, NULL, &eof); -#endif /* defined(_WIN32) */ - 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(expected_out[expected_out_ctr],OP_EQ, stdout_buf); - tt_int_op(strlen(expected_out[expected_out_ctr]),OP_EQ, pos); - expected_out_ctr++; - } - - if (exit_early) { - tor_process_handle_destroy(process_handle, 1); - process_handle = NULL; - goto done; - } - - /* The process should have exited without writing more */ -#ifdef _WIN32 - pos = tor_read_all_handle(process_handle->stdout_pipe, stdout_buf, - sizeof(stdout_buf) - 1, - process_handle); - tt_int_op(0,OP_EQ, pos); -#else /* !(defined(_WIN32)) */ - if (!eof) { - /* We should have got all the data, but maybe not the EOF flag */ - 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); - tt_assert(eof); - } - /* Otherwise, we got the EOF on the last read */ -#endif /* defined(_WIN32) */ - - /* Check it terminated correctly */ - retval = tor_get_exit_code(process_handle, 1, &exit_code); - tt_int_op(PROCESS_EXIT_EXITED,OP_EQ, retval); - tt_int_op(expected_exit,OP_EQ, exit_code); - - // 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(expected_err,OP_EQ, stderr_buf); - tt_int_op(strlen(expected_err),OP_EQ, pos); - - done: - tor_process_handle_destroy(process_handle, 1); -} - -static void -test_util_spawn_background_partial_read(void *arg) -{ - (void)arg; - test_util_spawn_background_partial_read_impl(0); -} - -static void -test_util_spawn_background_exit_early(void *arg) -{ - (void)arg; - test_util_spawn_background_partial_read_impl(1); -} - -static void -test_util_spawn_background_waitpid_notify(void *arg) -{ - int retval, exit_code; - process_handle_t *process_handle=NULL; - int status; - int ms_timer; - - const char *argv[] = {TEST_CHILD, "--fast", NULL}; - - (void) arg; - -#ifdef _WIN32 - status = tor_spawn_background(NULL, argv, NULL, &process_handle); -#else - status = tor_spawn_background(argv[0], argv, NULL, &process_handle); -#endif - - tt_int_op(status, OP_EQ, PROCESS_STATUS_RUNNING); - tt_ptr_op(process_handle, OP_NE, NULL); - - /* We're not going to look at the stdout/stderr output this time. Instead, - * we're testing whether notify_pending_waitpid_calbacks() can report the - * process exit (on unix) and/or whether tor_get_exit_code() can notice it - * (on windows) */ - -#ifndef _WIN32 - ms_timer = 30*1000; - tt_ptr_op(process_handle->waitpid_cb, OP_NE, NULL); - while (process_handle->waitpid_cb && ms_timer > 0) { - tor_sleep_msec(100); - ms_timer -= 100; - notify_pending_waitpid_callbacks(); - } - tt_int_op(ms_timer, OP_GT, 0); - tt_ptr_op(process_handle->waitpid_cb, OP_EQ, NULL); -#endif /* !defined(_WIN32) */ - - ms_timer = 30*1000; - while (((retval = tor_get_exit_code(process_handle, 0, &exit_code)) - == PROCESS_EXIT_RUNNING) && ms_timer > 0) { - tor_sleep_msec(100); - ms_timer -= 100; - } - tt_int_op(ms_timer, OP_GT, 0); - - tt_int_op(retval, OP_EQ, PROCESS_EXIT_EXITED); - - done: - tor_process_handle_destroy(process_handle, 1); -} - -#undef TEST_CHILD -#undef EOL - -#undef MATCH_PROCESS_STATUS - -#ifndef _WIN32 -#undef PROCESS_STATUS_RUNNING_OR_NOTRUNNING -#undef IS_RUNNING_OR_NOTRUNNING -#endif - -#define UTIL_TEST(name, flags) \ - { #name, test_util_ ## name, flags, NULL, NULL } - -struct testcase_t slow_util_tests[] = { - UTIL_TEST(spawn_background_ok, 0), - UTIL_TEST(spawn_background_fail, 0), - UTIL_TEST(spawn_background_partial_read, 0), - UTIL_TEST(spawn_background_exit_early, 0), - UTIL_TEST(spawn_background_waitpid_notify, 0), - END_OF_TESTCASES -}; -- cgit v1.2.3-54-g00ecf From ccc1963890bc7448383cd4c45370139697973d64 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Mon, 26 Nov 2018 06:14:47 +0100 Subject: Move remaining code from subprocess.{h,c} to more appropriate places. This patch moves the remaining code from subprocess.{h,c} to more appropriate places in the process.c and process_win32.c module. We also delete the now empty subprocess module files. See: https://bugs.torproject.org/28179 --- src/app/config/config.c | 2 +- src/feature/client/transports.c | 1 - src/lib/process/include.am | 2 - src/lib/process/process.c | 19 +++++ src/lib/process/process.h | 2 + src/lib/process/process_win32.c | 105 +++++++++++++++++++++++++- src/lib/process/process_win32.h | 3 + src/lib/process/subprocess.c | 158 ---------------------------------------- src/lib/process/subprocess.h | 20 ----- src/test/test_logging.c | 1 - src/test/test_process.c | 40 ++++++++++ src/test/test_pt.c | 2 - src/test/test_util.c | 56 +------------- 13 files changed, 171 insertions(+), 240 deletions(-) delete mode 100644 src/lib/process/subprocess.c delete mode 100644 src/lib/process/subprocess.h (limited to 'src/feature/client') diff --git a/src/app/config/config.c b/src/app/config/config.c index 90eae50fdd..e33c6b26e8 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -142,7 +142,7 @@ #include "lib/process/pidfile.h" #include "lib/process/restrict.h" #include "lib/process/setuid.h" -#include "lib/process/subprocess.h" +#include "lib/process/process.h" #include "lib/net/gethostname.h" #include "lib/thread/numcpus.h" diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index fd4bec7807..0e326f90e9 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -103,7 +103,6 @@ #include "feature/control/control.h" #include "lib/process/process.h" -#include "lib/process/subprocess.h" #include "lib/process/env.h" static smartlist_t * diff --git a/src/lib/process/include.am b/src/lib/process/include.am index aa73356146..a2d54b6238 100644 --- a/src/lib/process/include.am +++ b/src/lib/process/include.am @@ -14,7 +14,6 @@ src_lib_libtor_process_a_SOURCES = \ src/lib/process/process_win32.c \ src/lib/process/restrict.c \ src/lib/process/setuid.c \ - src/lib/process/subprocess.c \ src/lib/process/waitpid.c \ src/lib/process/winprocess_sys.c @@ -32,6 +31,5 @@ noinst_HEADERS += \ src/lib/process/process_win32.h \ src/lib/process/restrict.h \ src/lib/process/setuid.h \ - src/lib/process/subprocess.h \ src/lib/process/waitpid.h \ src/lib/process/winprocess_sys.h diff --git a/src/lib/process/process.c b/src/lib/process/process.c index 915217e132..7275d0a21b 100644 --- a/src/lib/process/process.c +++ b/src/lib/process/process.c @@ -26,6 +26,12 @@ /** A list of all process_t instances currently allocated. */ static smartlist_t *processes; +/** + * Boolean. If true, then Tor may call execve or CreateProcess via + * tor_spawn_background. + **/ +static int may_spawn_background_process = 1; + /** Structure to represent a child process. */ struct process_t { /** Process status. */ @@ -114,6 +120,16 @@ process_protocol_to_string(process_protocol_t protocol) /* LCOV_EXCL_STOP */ } +/** + * Turn off may_spawn_background_process, so that all future calls to + * tor_spawn_background are guaranteed to fail. + **/ +void +tor_disable_spawning_background_processes(void) +{ + may_spawn_background_process = 0; +} + /** Initialize the Process subsystem. This function initializes the Process * subsystem's global state. For cleaning up, process_free_all() should * be called. */ @@ -234,6 +250,9 @@ process_exec(process_t *process) { tor_assert(process); + if (BUG(may_spawn_background_process == 0)) + return PROCESS_STATUS_ERROR; + process_status_t status = PROCESS_STATUS_NOT_RUNNING; log_info(LD_PROCESS, "Starting new process: %s", process->command); diff --git a/src/lib/process/process.h b/src/lib/process/process.h index 6092c2da7d..cb5bccbf7b 100644 --- a/src/lib/process/process.h +++ b/src/lib/process/process.h @@ -45,6 +45,8 @@ typedef enum { const char *process_protocol_to_string(process_protocol_t protocol); +void tor_disable_spawning_background_processes(void); + struct process_t; typedef struct process_t process_t; diff --git a/src/lib/process/process_win32.c b/src/lib/process/process_win32.c index 7b18903c70..73e19d518f 100644 --- a/src/lib/process/process_win32.c +++ b/src/lib/process/process_win32.c @@ -18,13 +18,16 @@ #include "lib/log/win32err.h" #include "lib/process/process.h" #include "lib/process/process_win32.h" -#include "lib/process/subprocess.h" #include "lib/process/env.h" #ifdef HAVE_SYS_TIME_H #include #endif +#ifdef HAVE_STRING_H +#include +#endif + #ifdef _WIN32 /** The size of our intermediate buffers. */ @@ -889,4 +892,104 @@ process_win32_handle_read_completion(process_win32_handle_t *handle, return false; } +/** Format a single argument for being put on a Windows command line. + * Returns a newly allocated string */ +STATIC char * +format_win_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_new(); + + /* 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 -#endif -#ifdef HAVE_SYS_PRCTL_H -#include -#endif -#ifdef HAVE_UNISTD_H -#include -#endif -#ifdef HAVE_SIGNAL_H -#include -#endif -#ifdef HAVE_FCNTL_H -#include -#endif -#ifdef HAVE_SYS_WAIT_H -#include -#endif -#include -#include - -/** Format a single argument for being put on a Windows command line. - * Returns a newly allocated string */ -static char * -format_win_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_new(); - - /* 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 diff --git a/src/test/test_process.c b/src/test/test_process.c index 2fcb3f6686..3640b86688 100644 --- a/src/test/test_process.c +++ b/src/test/test_process.c @@ -628,11 +628,51 @@ test_win32(void *arg) process_init(); process_t *process = process_new(""); + char *joined_argv = NULL; /* On Win32 all processes should have a Win32 process handle. */ tt_ptr_op(NULL, OP_NE, process_get_win32_process(process)); + /* Based on some test cases from "Parsing C++ Command-Line Arguments" in + * MSDN but we don't exercise all quoting rules because tor_join_win_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 + { NULL } // 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; + + for (i=0; cmdlines[i]!=NULL; i++) { + log_info(LD_GENERAL, "Joining argvs[%d], expecting <%s>", i, cmdlines[i]); + joined_argv = tor_join_win_cmdline(argvs[i]); + tt_str_op(cmdlines[i],OP_EQ, joined_argv); + tor_free(joined_argv); + } + done: + tor_free(joined_argv); process_free(process); process_free_all(); #endif diff --git a/src/test/test_pt.c b/src/test/test_pt.c index c980276b14..b5572ef80c 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -8,7 +8,6 @@ #define UTIL_PRIVATE #define STATEFILE_PRIVATE #define CONTROL_PRIVATE -#define SUBPROCESS_PRIVATE #define PROCESS_PRIVATE #include "core/or/or.h" #include "app/config/config.h" @@ -18,7 +17,6 @@ #include "core/or/circuitbuild.h" #include "app/config/statefile.h" #include "test/test.h" -#include "lib/process/subprocess.h" #include "lib/encoding/confline.h" #include "lib/net/resolve.h" #include "lib/process/process.h" diff --git a/src/test/test_util.c b/src/test/test_util.c index 4fa67b6419..a94153f2dc 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -10,7 +10,7 @@ #define UTIL_PRIVATE #define UTIL_MALLOC_PRIVATE #define SOCKET_PRIVATE -#define SUBPROCESS_PRIVATE +#define PROCESS_WIN32_PRIVATE #include "lib/testsupport/testsupport.h" #include "core/or/or.h" #include "lib/container/buffers.h" @@ -22,6 +22,7 @@ #include "test/test.h" #include "lib/memarea/memarea.h" #include "lib/process/waitpid.h" +#include "lib/process/process_win32.h" #include "test/log_test_helpers.h" #include "lib/compress/compress.h" #include "lib/compress/compress_zstd.h" @@ -30,7 +31,6 @@ #include "lib/fs/winlib.h" #include "lib/process/env.h" #include "lib/process/pidfile.h" -#include "lib/process/subprocess.h" #include "lib/intmath/weakrng.h" #include "lib/thread/numcpus.h" #include "lib/math/fp.h" @@ -4395,57 +4395,6 @@ test_util_format_dec_number(void *ptr) return; } -/** - * Test that we can properly format a Windows command line - */ -static void -test_util_join_win_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_win_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 - { NULL } // 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 = NULL; - - (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_win_cmdline(argvs[i]); - tt_str_op(cmdlines[i],OP_EQ, joined_argv); - tor_free(joined_argv); - } - - done: - tor_free(joined_argv); -} - #define MAX_SPLIT_LINE_COUNT 4 struct split_lines_test_t { const char *orig_line; // Line to be split (may contain \0's) @@ -6226,7 +6175,6 @@ struct testcase_t util_tests[] = { UTIL_TEST_WIN_ONLY(load_win_lib, 0), UTIL_TEST(format_hex_number, 0), UTIL_TEST(format_dec_number, 0), - UTIL_TEST(join_win_cmdline, 0), UTIL_TEST(n_bits_set, 0), UTIL_TEST(eat_whitespace, 0), UTIL_TEST(sl_new_from_text_lines, 0), -- cgit v1.2.3-54-g00ecf From 5585cbd08f54f732c32feea276c1a47ec8446c5e Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Wed, 28 Nov 2018 21:55:04 +0100 Subject: Change the Process exit_callback to return bool. This patch changes our process_t's exit_callback to return a boolean value. If the returned value is true, the process subsystem will call process_free() on the given process_t. See: https://bugs.torproject.org/28179 --- src/feature/client/transports.c | 11 +++++++--- src/feature/client/transports.h | 2 +- src/lib/process/process.c | 14 ++++++++---- src/lib/process/process.h | 4 ++-- src/lib/process/process_win32.c | 48 +++++++++++++++++++++++++++++++---------- src/lib/process/process_win32.h | 2 +- src/test/test_process.c | 5 +++-- src/test/test_process_slow.c | 5 +++-- 8 files changed, 65 insertions(+), 26 deletions(-) (limited to 'src/feature/client') diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 0e326f90e9..e3cc679411 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -1759,8 +1759,9 @@ managed_proxy_stderr_callback(process_t *process, char *line, size_t size) /** Callback function that is called when our PT process terminates. The * process exit code can be found in exit_code and our process can be - * found in process. */ -STATIC void + * found in process. Returns true iff we want the process subsystem to + * free our process_t handle for us. */ +STATIC bool managed_proxy_exit_callback(process_t *process, process_exit_code_t exit_code) { tor_assert(process); @@ -1772,10 +1773,14 @@ managed_proxy_exit_callback(process_t *process, process_exit_code_t exit_code) /* We detach ourself from the MP (if we are attached) and free ourself. */ managed_proxy_t *mp = process_get_data(process); + /* If we are still attached to the process, it is probably because our PT + * process crashed before we got to call process_set_data(p, NULL); */ if (BUG(mp != NULL)) { + /* FIXME(ahf): Our process stopped without us having told it to stop + * (crashed). Should we restart it here? */ mp->process = NULL; process_set_data(process, NULL); } - process_free(process); + return true; } diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index fbb720aac6..ba8cbf7105 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -145,7 +145,7 @@ STATIC void free_execve_args(char **arg); STATIC void managed_proxy_stdout_callback(process_t *, char *, size_t); STATIC void managed_proxy_stderr_callback(process_t *, char *, size_t); -STATIC void managed_proxy_exit_callback(process_t *, process_exit_code_t); +STATIC bool managed_proxy_exit_callback(process_t *, process_exit_code_t); #endif /* defined(PT_PRIVATE) */ diff --git a/src/lib/process/process.c b/src/lib/process/process.c index 7275d0a21b..75bffe35b9 100644 --- a/src/lib/process/process.c +++ b/src/lib/process/process.c @@ -612,7 +612,8 @@ process_notify_event_stdin(process_t *process) /** This function is called by the Process backend when a given process have * terminated. The exit status code is passed in exit_code. We mark the * process as no longer running and calls the exit_callback with - * information about the process termination. */ + * information about the process termination. The given process is + * free'd iff the exit_callback returns true. */ void process_notify_event_exit(process_t *process, process_exit_code_t exit_code) { @@ -626,9 +627,14 @@ process_notify_event_exit(process_t *process, process_exit_code_t exit_code) process->exit_code = exit_code; /* Call our exit callback, if it exists. */ - if (process->exit_callback) { - process->exit_callback(process, exit_code); - } + bool free_process_handle = false; + + /* The exit callback will tell us if we should process_free() our handle. */ + if (process->exit_callback) + free_process_handle = process->exit_callback(process, exit_code); + + if (free_process_handle) + process_free(process); } /** This function is called whenever the Process backend have notified us that diff --git a/src/lib/process/process.h b/src/lib/process/process.h index cb5bccbf7b..4b0fae4250 100644 --- a/src/lib/process/process.h +++ b/src/lib/process/process.h @@ -56,8 +56,8 @@ typedef uint64_t process_pid_t; typedef void (*process_read_callback_t)(process_t *, char *, size_t); -typedef void (*process_exit_callback_t)(process_t *, - process_exit_code_t); +typedef bool +(*process_exit_callback_t)(process_t *, process_exit_code_t); void process_init(void); void process_free_all(void); diff --git a/src/lib/process/process_win32.c b/src/lib/process/process_win32.c index e75328f3e8..911bad3933 100644 --- a/src/lib/process/process_win32.c +++ b/src/lib/process/process_win32.c @@ -474,28 +474,47 @@ process_win32_timer_callback(periodic_timer_t *timer, void *data) tor_assert(timer == periodic_timer); tor_assert(data == NULL); - log_debug(LD_PROCESS, "Windows Process I/O timer ticked"); - /* Move the process into an alertable state. */ process_win32_trigger_completion_callbacks(); /* Check if our processes are still alive. */ - const smartlist_t *processes = process_get_all_processes(); - SMARTLIST_FOREACH(processes, process_t *, p, - process_win32_timer_test_process(p)); + /* Since the call to process_win32_timer_test_process() might call + * process_notify_event_exit() which again might call process_free() which + * updates the list of processes returned by process_get_all_processes() it + * is important here that we make sure to not touch the list of processes if + * the call to process_win32_timer_test_process() returns true. */ + bool done; + + do { + const smartlist_t *processes = process_get_all_processes(); + done = true; + + SMARTLIST_FOREACH_BEGIN(processes, process_t *, process) { + /* If process_win32_timer_test_process() returns true, it means that + * smartlist_remove() might have been called on the list returned by + * process_get_all_processes(). We start the loop over again until we + * have a succesful run over the entire list where the list was not + * modified. */ + if (process_win32_timer_test_process(process)) { + done = false; + break; + } + } SMARTLIST_FOREACH_END(process); + } while (! done); } /** Test whether a given process is still alive. Notify the Process subsystem - * if our process have died. */ -STATIC void + * if our process have died. Returns true iff the given process have + * terminated. */ +STATIC bool process_win32_timer_test_process(process_t *process) { tor_assert(process); /* No need to look at processes that don't claim they are running. */ if (process_get_status(process) != PROCESS_STATUS_RUNNING) - return; + return false; process_win32_t *win32_process = process_get_win32_process(process); BOOL ret = FALSE; @@ -508,12 +527,19 @@ process_win32_timer_test_process(process_t *process) if (! ret) { log_warn(LD_PROCESS, "GetExitCodeProcess() failed: %s", format_win32_error(GetLastError())); - return; + return false; } - /* Notify our process_t that our process have terminated. */ - if (exit_code != STILL_ACTIVE) + /* Notify our process_t that our process have terminated. Since our + * exit_callback might decide to process_free() our process handle it is very + * important that we do not touch the process_t after the call to + * process_notify_event_exit(). */ + if (exit_code != STILL_ACTIVE) { process_notify_event_exit(process, exit_code); + return true; + } + + return false; } /** Create a new overlapped named pipe. This function creates a new connected, diff --git a/src/lib/process/process_win32.h b/src/lib/process/process_win32.h index 3c809dfd23..00de8c949b 100644 --- a/src/lib/process/process_win32.h +++ b/src/lib/process/process_win32.h @@ -49,7 +49,7 @@ STATIC void process_win32_timer_start(void); STATIC void process_win32_timer_stop(void); STATIC bool process_win32_timer_running(void); STATIC void process_win32_timer_callback(periodic_timer_t *, void *); -STATIC void process_win32_timer_test_process(process_t *); +STATIC bool process_win32_timer_test_process(process_t *); /* I/O pipe handling. */ struct process_win32_handle_t; diff --git a/src/test/test_process.c b/src/test/test_process.c index 3640b86688..17481097be 100644 --- a/src/test/test_process.c +++ b/src/test/test_process.c @@ -125,7 +125,7 @@ process_stderr_callback(process_t *process, char *data, size_t size) return; } -static void +static bool process_exit_callback(process_t *process, process_exit_code_t exit_code) { tt_ptr_op(process, OP_NE, NULL); @@ -134,7 +134,8 @@ process_exit_callback(process_t *process, process_exit_code_t exit_code) process_data->exit_code = exit_code; done: - return; + /* Do not free up our process_t. */ + return false; } static void diff --git a/src/test/test_process_slow.c b/src/test/test_process_slow.c index ad84127bba..b23492e972 100644 --- a/src/test/test_process_slow.c +++ b/src/test/test_process_slow.c @@ -94,7 +94,7 @@ process_stderr_callback(process_t *process, char *data, size_t size) return; } -static void +static bool process_exit_callback(process_t *process, process_exit_code_t exit_code) { tt_ptr_op(process, OP_NE, NULL); @@ -106,7 +106,8 @@ process_exit_callback(process_t *process, process_exit_code_t exit_code) tor_shutdown_event_loop_and_exit(0); done: - return; + /* Do not free up our process_t. */ + return false; } #ifdef _WIN32 -- cgit v1.2.3-54-g00ecf From ec2ae3ed8b542950dc774223607f14f524375a50 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Thu, 29 Nov 2018 16:42:56 +0100 Subject: Change EVENT_TRANSPORT_LOG to EVENT_PT_LOG. This patch changes our EVENT_TRANSPORT_LOG event to be EVENT_PT_LOG. The new message includes the path to the PT executable instead of the transport name, since one PT binary can include multiple transport they sometimes might need to log messages that are not specific to a given transport. See: https://bugs.torproject.org/28179 --- src/feature/client/transports.c | 33 ++++++++++----------------------- src/feature/client/transports.h | 2 +- src/feature/control/control.c | 10 +++++----- src/feature/control/control.h | 5 ++--- src/test/test_pt.c | 6 +++--- 5 files changed, 21 insertions(+), 35 deletions(-) (limited to 'src/feature/client') diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index e3cc679411..df7991846c 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -910,7 +910,7 @@ handle_proxy_line(const char *line, managed_proxy_t *mp) parse_proxy_error(line); goto err; } else if (!strcmpstart(line, PROTO_LOG)) { - parse_log_line(line); + parse_log_line(line, mp); return; } @@ -1135,40 +1135,27 @@ parse_proxy_error(const char *line) /** Parses a LOG line and emit log events accordingly. */ STATIC void -parse_log_line(const char *line) +parse_log_line(const char *line, managed_proxy_t *mp) { - smartlist_t *items = smartlist_new(); + tor_assert(line); + tor_assert(mp); if (strlen(line) < (strlen(PROTO_LOG) + 1)) { log_warn(LD_PT, "Managed proxy sent us a %s line " - "with missing arguments.", PROTO_LOG); - goto done; - } - - const char *arguments = line + strlen(PROTO_LOG) + 1; - - /* The format is 'LOG '. We accept empty messages. */ - smartlist_split_string(items, arguments, NULL, - SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 2); - - if (smartlist_len(items) < 2) { - log_warn(LD_PT, "Managed proxy sent us a %s line " - "with too few arguments.", PROTO_LOG); + "with missing argument.", PROTO_LOG); goto done; } - const char *transport_name = smartlist_get(items, 0); - const char *message = smartlist_get(items, 1); + const char *message = line + strlen(PROTO_LOG) + 1; - log_info(LD_PT, "Managed proxy transport \"%s\" says: %s", - transport_name, message); + log_info(LD_PT, "Managed proxy \"%s\" says: %s", + mp->argv[0], message); /* Emit control port event. */ - control_event_transport_log(transport_name, message); + control_event_pt_log(mp->argv[0], message); done: - SMARTLIST_FOREACH(items, char *, s, tor_free(s)); - smartlist_free(items); + return; } /** Return a newly allocated string that tor should place in diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index ba8cbf7105..88735a7211 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -128,7 +128,7 @@ STATIC int parse_version(const char *line, managed_proxy_t *mp); STATIC void parse_env_error(const char *line); STATIC void parse_proxy_error(const char *line); STATIC void handle_proxy_line(const char *line, managed_proxy_t *mp); -STATIC void parse_log_line(const char *line); +STATIC void parse_log_line(const char *line, managed_proxy_t *mp); STATIC char *get_transport_options_for_server_proxy(const managed_proxy_t *mp); STATIC void managed_proxy_destroy(managed_proxy_t *mp, diff --git a/src/feature/control/control.c b/src/feature/control/control.c index b6505a85d6..ebda51781a 100644 --- a/src/feature/control/control.c +++ b/src/feature/control/control.c @@ -7395,14 +7395,14 @@ control_event_transport_launched(const char *mode, const char *transport_name, mode, transport_name, fmt_addr(addr), port); } -/** A pluggable transport called transport_name has emitted a log +/** A pluggable transport called pt_name has emitted a log * message found in message. */ void -control_event_transport_log(const char *transport_name, const char *message) +control_event_pt_log(const char *pt_name, const char *message) { - send_control_event(EVENT_TRANSPORT_LOG, - "650 TRANSPORT_LOG %s %s\r\n", - transport_name, + send_control_event(EVENT_PT_LOG, + "650 PT_LOG %s %s\r\n", + pt_name, message); } diff --git a/src/feature/control/control.h b/src/feature/control/control.h index eb2b5676ea..c554c32539 100644 --- a/src/feature/control/control.h +++ b/src/feature/control/control.h @@ -205,8 +205,7 @@ void control_event_clients_seen(const char *controller_str); void control_event_transport_launched(const char *mode, const char *transport_name, tor_addr_t *addr, uint16_t port); -void control_event_transport_log(const char *transport_name, - const char *message); +void control_event_pt_log(const char *pt_name, const char *message); const char *rend_auth_type_to_string(rend_auth_type_t auth_type); MOCK_DECL(const char *, node_describe_longname_by_id,(const char *id_digest)); void control_event_hs_descriptor_requested(const char *onion_address, @@ -295,7 +294,7 @@ void control_free_all(void); #define EVENT_HS_DESC 0x0021 #define EVENT_HS_DESC_CONTENT 0x0022 #define EVENT_NETWORK_LIVENESS 0x0023 -#define EVENT_TRANSPORT_LOG 0x0024 +#define EVENT_PT_LOG 0x0024 #define EVENT_MAX_ 0x0024 /* sizeof(control_connection_t.event_mask) in bits, currently a uint64_t */ diff --git a/src/test/test_pt.c b/src/test/test_pt.c index b5572ef80c..8fcdd5c1e8 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -310,7 +310,7 @@ process_read_stdout_replacement(process_t *process, buf_t *buffer) } else if (times_called <= 6) { buf_add_string(buffer, "SMETHODS DONE\n"); } else if (times_called <= 7) { - buf_add_string(buffer, "LOG mock3 Oh noes, something bad happened. " + buf_add_string(buffer, "LOG Oh noes, something bad happened. " "What do we do!?\n"); } @@ -417,10 +417,10 @@ test_pt_configure_proxy(void *arg) process_notify_event_stdout(mp->process); tt_int_op(controlevent_n, OP_EQ, 6); - tt_int_op(controlevent_event, OP_EQ, EVENT_TRANSPORT_LOG); + tt_int_op(controlevent_event, OP_EQ, EVENT_PT_LOG); tt_int_op(smartlist_len(controlevent_msgs), OP_EQ, 6); tt_str_op(smartlist_get(controlevent_msgs, 5), OP_EQ, - "650 TRANSPORT_LOG mock3 Oh noes, something bad happened. " + "650 PT_LOG Oh noes, something bad happened. " "What do we do!?\r\n"); { /* check that the transport info were saved properly in the tor state */ -- cgit v1.2.3-54-g00ecf From cacdd290872420d51f880d75466f6d6e22430466 Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 7 Dec 2018 02:09:22 +0100 Subject: Use `const char *` instead of `char *` for line parameter for process callbacks. This patch changes the type definition of the process callbacks to use `const char *` instead of `char *`. See: https://bugs.torproject.org/28179 --- src/feature/client/transports.c | 8 ++++++-- src/feature/client/transports.h | 4 ++-- src/lib/process/process.h | 2 +- src/test/test_process.c | 4 ++-- src/test/test_process_slow.c | 4 ++-- 5 files changed, 13 insertions(+), 9 deletions(-) (limited to 'src/feature/client') diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index df7991846c..0f456ba1e6 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -1711,7 +1711,9 @@ tor_escape_str_for_pt_args(const char *string, const char *chars_to_escape) * stdout. Our process can be found in process, the data can be found in * line and the length of our line is given in size. */ STATIC void -managed_proxy_stdout_callback(process_t *process, char *line, size_t size) +managed_proxy_stdout_callback(process_t *process, + const char *line, + size_t size) { tor_assert(process); tor_assert(line); @@ -1732,7 +1734,9 @@ managed_proxy_stdout_callback(process_t *process, char *line, size_t size) * stderr. Our process can be found in process, the data can be found in * line and the length of our line is given in size. */ STATIC void -managed_proxy_stderr_callback(process_t *process, char *line, size_t size) +managed_proxy_stderr_callback(process_t *process, + const char *line, + size_t size) { tor_assert(process); tor_assert(line); diff --git a/src/feature/client/transports.h b/src/feature/client/transports.h index 88735a7211..a3994a0099 100644 --- a/src/feature/client/transports.h +++ b/src/feature/client/transports.h @@ -143,8 +143,8 @@ STATIC char* get_pt_proxy_uri(void); STATIC void free_execve_args(char **arg); -STATIC void managed_proxy_stdout_callback(process_t *, char *, size_t); -STATIC void managed_proxy_stderr_callback(process_t *, char *, size_t); +STATIC void managed_proxy_stdout_callback(process_t *, const char *, size_t); +STATIC void managed_proxy_stderr_callback(process_t *, const char *, size_t); STATIC bool managed_proxy_exit_callback(process_t *, process_exit_code_t); #endif /* defined(PT_PRIVATE) */ diff --git a/src/lib/process/process.h b/src/lib/process/process.h index 4b0fae4250..179db19aeb 100644 --- a/src/lib/process/process.h +++ b/src/lib/process/process.h @@ -54,7 +54,7 @@ typedef uint64_t process_exit_code_t; typedef uint64_t process_pid_t; typedef void (*process_read_callback_t)(process_t *, - char *, + const char *, size_t); typedef bool (*process_exit_callback_t)(process_t *, process_exit_code_t); diff --git a/src/test/test_process.c b/src/test/test_process.c index 17481097be..9b62862f04 100644 --- a/src/test/test_process.c +++ b/src/test/test_process.c @@ -98,7 +98,7 @@ process_mocked_write_stdin(process_t *process, buf_t *buffer) } static void -process_stdout_callback(process_t *process, char *data, size_t size) +process_stdout_callback(process_t *process, const char *data, size_t size) { tt_ptr_op(process, OP_NE, NULL); tt_ptr_op(data, OP_NE, NULL); @@ -112,7 +112,7 @@ process_stdout_callback(process_t *process, char *data, size_t size) } static void -process_stderr_callback(process_t *process, char *data, size_t size) +process_stderr_callback(process_t *process, const char *data, size_t size) { tt_ptr_op(process, OP_NE, NULL); tt_ptr_op(data, OP_NE, NULL); diff --git a/src/test/test_process_slow.c b/src/test/test_process_slow.c index b23492e972..cc7e8c5a9c 100644 --- a/src/test/test_process_slow.c +++ b/src/test/test_process_slow.c @@ -67,7 +67,7 @@ process_data_free(process_data_t *process_data) } static void -process_stdout_callback(process_t *process, char *data, size_t size) +process_stdout_callback(process_t *process, const char *data, size_t size) { tt_ptr_op(process, OP_NE, NULL); tt_ptr_op(data, OP_NE, NULL); @@ -81,7 +81,7 @@ process_stdout_callback(process_t *process, char *data, size_t size) } static void -process_stderr_callback(process_t *process, char *data, size_t size) +process_stderr_callback(process_t *process, const char *data, size_t size) { tt_ptr_op(process, OP_NE, NULL); tt_ptr_op(data, OP_NE, NULL); -- cgit v1.2.3-54-g00ecf From 651cdd05b7071bff4da7c3335f697d90c82c9f3e Mon Sep 17 00:00:00 2001 From: Alexander Færøy Date: Fri, 14 Dec 2018 02:17:00 +0100 Subject: Add an additional space when we check for the PROTO_LOG handler. See: https://bugs.torproject.org/28179 --- src/feature/client/transports.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src/feature/client') diff --git a/src/feature/client/transports.c b/src/feature/client/transports.c index 0f456ba1e6..de53fe3469 100644 --- a/src/feature/client/transports.c +++ b/src/feature/client/transports.c @@ -909,7 +909,11 @@ handle_proxy_line(const char *line, managed_proxy_t *mp) parse_proxy_error(line); goto err; - } else if (!strcmpstart(line, PROTO_LOG)) { + + /* We check for the additional " " after the PROTO_LOG string to make sure + * we can later extend this big if/else-if table with something that begins + * with "LOG" without having to get the order right. */ + } else if (!strcmpstart(line, PROTO_LOG " ")) { parse_log_line(line, mp); return; } -- cgit v1.2.3-54-g00ecf