From 069b26b2d985bca636c19b4a86875cb91988a368 Mon Sep 17 00:00:00 2001 From: Ulf Lamping Date: Mon, 13 Mar 2006 00:30:51 +0000 Subject: fix bug #803: sync pipe on Win32 wasn't set to binary mode, so error message transport failed between Ethereal and dumpcap. I've also changed the way the secondary error message is transported from former "header message 0 secondary 0" to "header header message 0 header secondary 0" as that might be a bit more clearer, and I'll need it for further development anyway. I was using this while debugging and not recognizing the real problem - for about four hours :-(. I'll need this feature when doing the interface (and link layer type) browsing later (transferring this data from dumpcap to Ethereal) to get a full blown privilege seperation. svn path=/trunk/; revision=17608 --- capture.c | 3 +- capture.h | 2 +- capture_loop.c | 3 +- capture_sync.c | 91 +++++++++++++++++++++++++++++++++++++--------------------- dumpcap.c | 81 ++++++++++++++++++++++++++------------------------- 5 files changed, 104 insertions(+), 76 deletions(-) diff --git a/capture.c b/capture.c index 4ddf910acd..2285299be5 100644 --- a/capture.c +++ b/capture.c @@ -374,9 +374,8 @@ capture_input_drops(capture_options *capture_opts, int dropped) The secondary message might be a null string. */ void -capture_input_error_message(capture_options *capture_opts, char *error_msg) +capture_input_error_message(capture_options *capture_opts, char *error_msg, char *secondary_error_msg) { - char *secondary_error_msg = strchr(error_msg, '\0') + 1; gchar *safe_error_msg; gchar *safe_secondary_error_msg; diff --git a/capture.h b/capture.h index f36dd50348..0bc6ed75e6 100644 --- a/capture.h +++ b/capture.h @@ -68,7 +68,7 @@ extern void capture_input_drops(capture_options *capture_opts, int dropped); /** * Capture child told us that an error has occurred while starting the capture. */ -extern void capture_input_error_message(capture_options *capture_opts, char *error_message); +extern void capture_input_error_message(capture_options *capture_opts, char *error_message, char *secondary_error_msg); /** * Capture child told us that an error has occurred while parsing a diff --git a/capture_loop.c b/capture_loop.c index a908c5b1a7..f3c8564167 100644 --- a/capture_loop.c +++ b/capture_loop.c @@ -556,7 +556,8 @@ capture_loop_open_input(capture_options *capture_opts, loop_data *ld, Do, however, warn about the lack of 64-bit support, and warn that WAN devices aren't supported. */ g_snprintf(errmsg, errmsg_len, -"The capture session could not be initiated (%s).", +"The capture session could not be initiated.\n" +"\"%s\"", open_err_str); g_snprintf(secondary_errmsg, secondary_errmsg_len, "\n" diff --git a/capture_sync.c b/capture_sync.c index 39bf34f18f..631f870df6 100644 --- a/capture_sync.c +++ b/capture_sync.c @@ -215,34 +215,24 @@ signal_pipe_capquit_to_child(capture_options *capture_opts) } #endif - - -/* read a message from the sending pipe in the standard format - (1-byte message indicator, 3-byte message length (excluding length - and indicator field), and the rest is the message) */ static int -pipe_read_block(int pipe, char *indicator, int len, char *msg) { - int required; +pipe_read_bytes(int pipe, char *bytes, int required) { int newly; - guchar header[4]; - int offset; + int offset = 0; - /* read header (indicator and 3-byte length) */ - required = 4; - offset = 0; while(required) { - newly = read(pipe, &header[offset], required); + newly = read(pipe, &bytes[offset], required); if (newly == 0) { /* EOF */ g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, - "read %d header empty (capture closed)", pipe); + "read from pipe %d: EOF (capture closed?)", pipe); return newly; } if (newly < 0) { /* error */ g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, - "read %d header error: %s", pipe, strerror(errno)); + "read from pipe %d: error(%u): %s", pipe, errno, strerror(errno)); return newly; } @@ -250,9 +240,40 @@ pipe_read_block(int pipe, char *indicator, int len, char *msg) { offset += newly; } + return newly; +} + +/* convert header values (indicator and 4-byte length) */ +static void +pipe_convert_header(const guchar *header, int header_len, char *indicator, int *block_len) { + + g_assert(header_len == 4); + /* convert header values */ *indicator = header[0]; - required = header[1]<<16 | header[2]<<8 | header[3]; + *block_len = header[1]<<16 | header[2]<<8 | header[3]; +} + +/* read a message from the sending pipe in the standard format + (1-byte message indicator, 3-byte message length (excluding length + and indicator field), and the rest is the message) */ +static int +pipe_read_block(int pipe, char *indicator, int len, char *msg) { + int required; + int newly; + guchar header[4]; + + + /* read header (indicator and 3-byte length) */ + newly = pipe_read_bytes(pipe, header, 4); + if(newly != 4) { + g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, + "read %d failed to read header: %u", pipe, newly); + return -1; + } + + /* convert header values */ + pipe_convert_header(header, 4, indicator, &required); /* only indicator with no value? */ if(required == 0) { @@ -261,6 +282,7 @@ pipe_read_block(int pipe, char *indicator, int len, char *msg) { return 4; } + /* does the data fit into the given buffer? */ if(required > len) { g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "read %d length error, required %d > len %d, indicator: %u", @@ -274,26 +296,17 @@ pipe_read_block(int pipe, char *indicator, int len, char *msg) { } len = required; - /* read value */ - offset = 0; - while(required) { - newly = read(pipe, &msg[offset], required); - if (newly == -1) { - /* error */ - g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, - "read %d value error: %s, indicator: %u", pipe, - strerror(errno), *indicator); - return newly; - } - - required -= newly; - offset += newly; + /* read the actual block data */ + newly = pipe_read_bytes(pipe, msg, required); + if(newly != required) { + g_warning("Unknown message from dumpcap, try to show it as a string: %s", msg); + return -1; } g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "read %d ok indicator: %c len: %u msg: %s", pipe, *indicator, len, msg); - return len + 4; + return newly + 4; } @@ -633,7 +646,7 @@ sync_pipe_start(capture_options *capture_opts) { execv(exename, argv); g_snprintf(errmsg, sizeof errmsg, "Couldn't run %s in child process: %s", exename, strerror(errno)); - sync_pipe_errmsg_to_parent(errmsg, NULL); + sync_pipe_errmsg_to_parent(errmsg, ""); /* Exit with "_exit()", so that we don't close the connection to the X server (and cause stuff buffered up by our parent but @@ -697,6 +710,10 @@ sync_pipe_input_cb(gint source, gpointer user_data) char buffer[SP_MAX_MSG_LEN+1]; int nread; char indicator; + int primary_len; + char * primary_msg; + int secondary_len; + char * secondary_msg; nread = pipe_read_block(source, &indicator, SP_MAX_MSG_LEN, buffer); @@ -746,8 +763,16 @@ sync_pipe_input_cb(gint source, gpointer user_data) capture_input_new_packets(capture_opts, nread); break; case SP_ERROR_MSG: - capture_input_error_message(capture_opts, buffer); + /* convert primary message */ + pipe_convert_header(buffer, 4, &indicator, &primary_len); + primary_msg = buffer+4; + /* convert secondary message */ + pipe_convert_header(primary_msg + primary_len, 4, &indicator, &secondary_len); + secondary_msg = primary_msg + primary_len + 4; + /* message output */ + capture_input_error_message(capture_opts, primary_msg, secondary_msg); /* the capture child will close the sync_pipe, nothing to do for now */ + /* (an error message doesn't mean we have to stop capturing) */ break; case SP_BAD_FILTER: capture_input_cfilter_error_message(capture_opts, buffer); diff --git a/dumpcap.c b/dumpcap.c index 268f32615e..a972486141 100644 --- a/dumpcap.c +++ b/dumpcap.c @@ -337,6 +337,10 @@ main(int argc, char *argv[]) /*** hidden option: Ethereal child mode (using binary output messages) ***/ case 'Z': capture_child = TRUE; +#ifdef _WIN32 + /* set output pipe to binary mode, to avoid ugly text conversions */ + _setmode(1, O_BINARY); +#endif break; /*** all non capture option specific ***/ @@ -505,51 +509,51 @@ console_log_handler(const char *log_domain, GLogLevelFlags log_level, /* * Maximum length of sync pipe message data. Must be < 2^24, as the * message length is 3 bytes. - * XXX - this must be large enough to handle a Really Big Filter + * XXX - this should be large enough to handle a Really Big Filter * Expression, as the error message for an incorrect filter expression * is a bit larger than the filter expression. */ #define SP_MAX_MSG_LEN 4096 +/* write a single message header to the recipient pipe */ +static int +pipe_write_header(int pipe, char indicator, int length) +{ + guchar header[1+3]; /* indicator + 3-byte len */ + + + g_assert(length <= SP_MAX_MSG_LEN); + + /* write header (indicator + 3-byte len) */ + header[0] = indicator; + header[1] = (length >> 16) & 0xFF; + header[2] = (length >> 8) & 0xFF; + header[3] = (length >> 0) & 0xFF; + + /* write header */ + return write(pipe, header, sizeof header); +} /* write a message to the recipient pipe in the standard format (3 digit message length (excluding length and indicator field), 1 byte message indicator and the rest is the message). - If msg is NULL, the message has only a length and indicator. - Otherwise, if secondary_msg isn't NULL, send both msg and - secondary_msg as null-terminated strings, otherwise just send - msg as a null-terminated string and follow it with an empty string. */ + If msg is NULL, the message has only a length and indicator. */ static void -pipe_write_block(int pipe, char indicator, const char *msg, - const char *secondary_msg) +pipe_write_block(int pipe, char indicator, const char *msg) { - guchar header[3+1]; /* indicator + 3-byte len */ int ret; - size_t len, secondary_len, total_len; + size_t len; /*g_warning("write %d enter", pipe);*/ - len = 0; - secondary_len = 0; - total_len = 0; if(msg != NULL) { - len = strlen(msg) + 1; /* include the terminating '\0' */ - total_len = len; - if(secondary_msg == NULL) - secondary_msg = ""; /* default to an empty string */ - secondary_len = strlen(secondary_msg) + 1; - total_len += secondary_len; + len = strlen(msg) + 1; /* including the terminating '\0'! */ + } else { + len = 0; } - g_assert(indicator < '0' || indicator > '9'); - g_assert(total_len <= SP_MAX_MSG_LEN); /* write header (indicator + 3-byte len) */ - header[0] = indicator; - header[1] = (total_len >> 16) & 0xFF; - header[2] = (total_len >> 8) & 0xFF; - header[3] = (total_len >> 0) & 0xFF; - - ret = write(pipe, header, sizeof header); + ret = pipe_write_header(pipe, indicator, len); if(ret == -1) { return; } @@ -561,10 +565,6 @@ pipe_write_block(int pipe, char indicator, const char *msg, if(ret == -1) { return; } - ret = write(pipe, secondary_msg, secondary_len); - if(ret == -1) { - return; - } } else { /*g_warning("write %d indicator: %c no value", pipe, indicator);*/ } @@ -583,7 +583,7 @@ sync_pipe_packet_count_to_parent(int packet_count) if(capture_child) { g_snprintf(tmp, sizeof(tmp), "%d", packet_count); g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "Packets: %s", tmp); - pipe_write_block(1, SP_PACKET_COUNT, tmp, NULL); + pipe_write_block(1, SP_PACKET_COUNT, tmp); } else { count += packet_count; fprintf(stderr, "\rPackets: %u ", count); @@ -600,7 +600,7 @@ sync_pipe_filename_to_parent(const char *filename) g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, "File: %s", filename); if(capture_child) { - pipe_write_block(1, SP_FILE, filename, NULL); + pipe_write_block(1, SP_FILE, filename); } } @@ -612,7 +612,7 @@ sync_pipe_cfilter_error_to_parent(const char *cfilter _U_, const char *errmsg) g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "Capture filter error: %s", errmsg); if (capture_child) { - pipe_write_block(1, SP_BAD_FILTER, errmsg, NULL); + pipe_write_block(1, SP_BAD_FILTER, errmsg); } } @@ -620,13 +620,16 @@ void sync_pipe_errmsg_to_parent(const char *error_msg, const char *secondary_error_msg) { - g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, "Error: %s", error_msg); - if (secondary_error_msg != NULL) - g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, "Secondary error: %s", - secondary_error_msg); + g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, + "Primary Error: %s", error_msg); + g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, + "Secondary Error: %s", secondary_error_msg); if(capture_child) { - pipe_write_block(1, SP_ERROR_MSG, error_msg, secondary_error_msg); + /* first write a "master header" with the length of the two messages plus their "slave headers" */ + pipe_write_header(1, SP_ERROR_MSG, strlen(error_msg) + 1 + 4 + strlen(secondary_error_msg) + 1 + 4); + pipe_write_block(1, SP_ERROR_MSG, error_msg); + pipe_write_block(1, SP_ERROR_MSG, secondary_error_msg); } } @@ -640,7 +643,7 @@ sync_pipe_drops_to_parent(int drops) g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_MESSAGE, "Packets dropped: %s", tmp); if(capture_child) { - pipe_write_block(1, SP_DROPS, tmp, NULL); + pipe_write_block(1, SP_DROPS, tmp); } } -- cgit v1.2.3