From 73087d6fb486a61e8eb0886a02e6c6403534fa36 Mon Sep 17 00:00:00 2001 From: David Perry Date: Thu, 29 Apr 2021 07:23:21 -0400 Subject: [PATCH] Use wtap_blocks for packet comments Mostly functioning proof of concept for #14329. This work is intended to allow Wireshark to support multiple packet comments per packet. Uses and expands upon the `wtap_block` API in `wiretap/wtap_opttypes.h`. It attaches a `wtap_block` structure to `wtap_rec` in place of its current `opt_comment` and `packet_verdict` members to hold OPT_COMMENT and OPT_PKT_VERDICT option values. --- cfile.h | 6 +- debian/libwireshark0.symbols | 2 +- debian/libwiretap0.symbols | 15 +- editcap.c | 9 +- epan/dissectors/packet-frame.c | 175 +++++---- epan/epan.c | 18 +- epan/epan.h | 5 +- epan/frame_data.c | 4 +- epan/frame_data.h | 4 +- epan/packet.c | 38 +- epan/packet.h | 5 +- epan/wslua/wslua_dumper.c | 19 +- epan/wslua/wslua_file_common.h | 1 + epan/wslua/wslua_file_handler.c | 8 +- epan/wslua/wslua_frame_info.c | 124 +++++- extcap/androiddump.c | 3 +- file.c | 107 ++--- file.h | 16 +- file_packet_provider.c | 20 +- frame_tvbuff.c | 5 +- sharkd.c | 52 ++- sharkd.h | 7 +- sharkd_session.c | 50 ++- test/lua/acme_file.lua | 1 - test/lua/globals_2.2.txt | 1 - test/suite_sharkd.py | 2 +- tshark.c | 2 +- ui/qt/capture_file_properties_dialog.cpp | 23 +- ui/qt/main_window.cpp | 2 + ui/qt/main_window.h | 8 +- ui/qt/main_window.ui | 18 +- ui/qt/main_window_slots.cpp | 90 ++++- ui/qt/packet_list.cpp | 87 +++-- ui/qt/packet_list.h | 5 +- ui/tap_export_pdu.c | 15 +- wiretap/erf.c | 23 +- wiretap/pcapng.c | 430 ++++++++++---------- wiretap/wtap.c | 30 +- wiretap/wtap.h | 17 +- wiretap/wtap_opttypes.c | 477 ++++++++++++++++++++++- wiretap/wtap_opttypes.h | 195 ++++++++- 41 files changed, 1566 insertions(+), 553 deletions(-) diff --git a/cfile.h b/cfile.h index 2d2d37ed18..61d9e43837 100644 --- a/cfile.h +++ b/cfile.h @@ -59,7 +59,7 @@ struct packet_provider_data { frame_data *prev_dis; frame_data *prev_cap; frame_data_sequence *frames; /* Sequence of frames, if we're keeping that information */ - GTree *frames_user_comments; /* BST with user comments for frames (key = frame_data) */ + GTree *frames_edited_blocks; /* BST with changed blocks for frames (key = frame_data) */ }; typedef struct _capture_file { @@ -131,8 +131,8 @@ extern void cap_file_init(capture_file *cf); const char *cap_file_provider_get_interface_name(struct packet_provider_data *prov, guint32 interface_id); const char *cap_file_provider_get_interface_description(struct packet_provider_data *prov, guint32 interface_id); -const char *cap_file_provider_get_user_comment(struct packet_provider_data *prov, const frame_data *fd); -void cap_file_provider_set_user_comment(struct packet_provider_data *prov, frame_data *fd, const char *new_comment); +wtap_block_t cap_file_provider_get_user_block(struct packet_provider_data *prov, const frame_data *fd); +void cap_file_provider_set_user_block(struct packet_provider_data *prov, frame_data *fd, const wtap_block_t new_block); #ifdef __cplusplus } diff --git a/debian/libwireshark0.symbols b/debian/libwireshark0.symbols index 7b52712fb9..30d847ca20 100644 --- a/debian/libwireshark0.symbols +++ b/debian/libwireshark0.symbols @@ -559,7 +559,7 @@ libwireshark.so.0 libwireshark0 #MINVER# epan_get_interface_description@Base 2.3.0 epan_get_interface_name@Base 1.99.2 epan_get_runtime_version_info@Base 1.9.1 - epan_get_user_comment@Base 1.99.2 + epan_get_user_block@Base 3.5.0 epan_get_version@Base 1.9.1 epan_get_version_number@Base 2.5.0 epan_init@Base 2.9.0 diff --git a/debian/libwiretap0.symbols b/debian/libwiretap0.symbols index a89446a2a0..7e1a6b12e8 100644 --- a/debian/libwiretap0.symbols +++ b/debian/libwiretap0.symbols @@ -23,39 +23,52 @@ libwiretap.so.0 libwiretap0 #MINVER# wtap_add_generated_idb@Base 3.3.0 wtap_addrinfo_list_empty@Base 2.5.0 wtap_block_add_custom_option@Base 3.5.0 + wtap_block_add_bytes_option@Base 3.5.0 + wtap_block_add_bytes_option_borrow@Base 3.5.0 wtap_block_add_if_filter_option@Base 3.5.0 wtap_block_add_ipv4_option@Base 2.1.2 wtap_block_add_ipv6_option@Base 2.1.2 wtap_block_add_string_option@Base 2.1.2 wtap_block_add_string_option_format@Base 2.1.2 + wtap_block_add_uint32_option@Base 3.5.0 wtap_block_add_uint64_option@Base 2.1.2 wtap_block_add_uint8_option@Base 2.1.2 wtap_block_array_free@Base 2.1.2 wtap_block_copy@Base 2.1.2 + wtap_block_count_option@Base 3.5.0 wtap_block_create@Base 2.1.2 wtap_block_foreach_option@Base 2.1.2 - wtap_block_free@Base 2.1.2 + wtap_block_get_bytes_option_value@Base 3.5.0 wtap_block_get_if_filter_option_value@Base 3.5.0 wtap_block_get_ipv4_option_value@Base 2.1.2 wtap_block_get_ipv6_option_value@Base 2.1.2 wtap_block_get_mandatory_data@Base 2.1.2 + wtap_block_get_nth_bytes_option_value@Base 3.5.0 wtap_block_get_nth_string_option_value@Base 2.1.2 + wtap_block_get_options_size_padded@Base 3.5.0 wtap_block_get_string_option_value@Base 2.1.2 wtap_block_get_type@Base 3.5.0 + wtap_block_get_uint32_option_value@Base 3.5.0 wtap_block_get_uint64_option_value@Base 2.1.2 wtap_block_get_uint8_option_value@Base 2.1.2 wtap_block_make_copy@Base 3.3.2 + wtap_block_option_get_value_size@Base 3.5.0 + wtap_block_ref@Base 3.5.0 wtap_block_remove_nth_option_instance@Base 2.2.0 wtap_block_remove_option@Base 2.2.0 + wtap_block_set_bytes_option_value@Base 3.5.0 wtap_block_set_if_filter_option_value@Base 3.5.0 wtap_block_set_ipv4_option_value@Base 2.1.2 wtap_block_set_ipv6_option_value@Base 2.1.2 + wtap_block_set_nth_bytes_option_value@Base 3.5.0 wtap_block_set_nth_string_option_value@Base 2.1.2 wtap_block_set_nth_string_option_value_format@Base 3.5.0 wtap_block_set_string_option_value@Base 2.1.2 wtap_block_set_string_option_value_format@Base 2.1.2 + wtap_block_set_uint32_option_value@Base 3.5.0 wtap_block_set_uint64_option_value@Base 2.1.2 wtap_block_set_uint8_option_value@Base 2.1.2 + wtap_block_unref@Base 3.5.0 wtap_cleanup@Base 2.3.0 wtap_cleareof@Base 1.9.1 wtap_close@Base 1.9.1 diff --git a/editcap.c b/editcap.c index caa49faba4..851dca5561 100644 --- a/editcap.c +++ b/editcap.c @@ -1260,7 +1260,6 @@ main(int argc, char *argv[]) case LONGOPT_CAPTURE_COMMENT: { /* pcapng supports multiple comments, so support them here too. - * Wireshark only sees the first capture comment though. */ if (!capture_comments) { capture_comments = g_ptr_array_new_with_free_func(g_free); @@ -2186,13 +2185,13 @@ main(int argc, char *argv[]) /* Copy and change rather than modify returned rec */ temp_rec = *rec; /* The comment is not modified by dumper, cast away. */ - temp_rec.opt_comment = (char *)comment; - temp_rec.has_comment_changed = TRUE; + wtap_block_add_string_option(rec->block, OPT_COMMENT, (char *)comment, strlen((char *)comment)); + temp_rec.has_block_changed = TRUE; rec = &temp_rec; } else { /* Copy and change rather than modify returned rec */ temp_rec = *rec; - temp_rec.has_comment_changed = FALSE; + temp_rec.has_block_changed = FALSE; rec = &temp_rec; } } @@ -2278,7 +2277,7 @@ clean_exit: if (idbs_seen != NULL) { for (guint b = 0; b < idbs_seen->len; b++) { wtap_block_t if_data = g_array_index(idbs_seen, wtap_block_t, b); - wtap_block_free(if_data); + wtap_block_unref(if_data); } g_array_free(idbs_seen, TRUE); } diff --git a/epan/dissectors/packet-frame.c b/epan/dissectors/packet-frame.c index 9a306b8a40..1742a69d20 100644 --- a/epan/dissectors/packet-frame.c +++ b/epan/dissectors/packet-frame.c @@ -177,6 +177,15 @@ static dissector_table_t wtap_fts_rec_dissector_table; #define OPT_VERDICT_TYPE_TC 1 #define OPT_VERDICT_TYPE_XDP 2 +/* Structure for passing as userdata to wtap_block_foreach_option */ +typedef struct fr_foreach_s { + proto_item *item; + proto_tree *tree; + tvbuff_t *tvb; + packet_info *pinfo; + guint n_changes; +} fr_foreach_t; + static const char * get_verdict_type_string(guint8 type) { @@ -191,29 +200,6 @@ get_verdict_type_string(guint8 type) return "Unknown"; } -static void -format_verdict_summary(wtap_rec *rec, char *buffer, size_t n) -{ - buffer[0] = 0; - - for(guint i = 0; i < rec->packet_verdict->len; i++) { - char *format = i ? ", %s (%u)" : "%s (%u)"; - size_t offset = strlen(buffer); - GBytes *verdict = (GBytes *) g_ptr_array_index(rec->packet_verdict, i); - const guint8 *data; - - if (verdict == NULL) - continue; - - if (offset >= n) - return; - - data = (const guint8 *) g_bytes_get_data(verdict, NULL); - snprintf(buffer + offset, n - offset, - format, get_verdict_type_string(data[0]), data[0]); - } -} - static void ensure_tree_item(proto_tree *tree, guint count) { @@ -276,10 +262,80 @@ call_frame_end_routine(gpointer routine) (*func)(); } +static gboolean +frame_add_comment(wtap_block_t block _U_, guint option_id, wtap_opttype_e option_type _U_, wtap_optval_t *option, void *user_data) +{ + fr_foreach_t *fr_user_data = (fr_foreach_t *)user_data; + proto_item *comment_item; + + if (option_id == OPT_COMMENT) { + comment_item = proto_tree_add_string_format(fr_user_data->tree, hf_comments_text, + fr_user_data->tvb, 0, 0, + option->stringval, + "%s", option->stringval); + expert_add_info_format(fr_user_data->pinfo, comment_item, &ei_comments_text, + "%s", option->stringval); + } + fr_user_data->n_changes++; + return TRUE; +} + +static gboolean +frame_add_verdict(wtap_block_t block _U_, guint option_id, wtap_opttype_e option_type _U_, wtap_optval_t *option, void *user_data) +{ + fr_foreach_t *fr_user_data = (fr_foreach_t *)user_data; + + if (option_id == OPT_PKT_VERDICT) { + GBytes *verdict = option->byteval; + const guint8 *verdict_data; + gsize len; + char *format = fr_user_data->n_changes ? ", %s (%u)" : "%s (%u)"; + + if (verdict == NULL) + return TRUE; + + verdict_data = (const guint8 *) g_bytes_get_data(verdict, &len); + + if (len == 0) + return TRUE; + + proto_item_append_text(fr_user_data->item, format, + get_verdict_type_string(verdict_data[0]), verdict_data[0]); + + len -= 1; + switch(verdict_data[0]) { + case OPT_VERDICT_TYPE_HW: + proto_tree_add_bytes_with_length(fr_user_data->tree, hf_frame_verdict_hardware, + fr_user_data->tvb, 0, 0, verdict_data + 1, (gint) len); + break; + case OPT_VERDICT_TYPE_TC: + if (len == 8) { + gint64 val; + memcpy(&val, verdict_data + 1, sizeof(val)); + proto_tree_add_int64(fr_user_data->tree, hf_frame_verdict_tc, fr_user_data->tvb, 0, 0, val); + } + break; + case OPT_VERDICT_TYPE_XDP: + if (len == 8) { + gint64 val; + memcpy(&val, verdict_data + 1, sizeof(val)); + proto_tree_add_int64(fr_user_data->tree, hf_frame_verdict_xdp, fr_user_data->tvb, 0, 0, val); + } + break; + default: + proto_tree_add_bytes_with_length(fr_user_data->tree, hf_frame_verdict_unknown, + fr_user_data->tvb, 0, 0, verdict_data, (gint) len + 1); + break; + } + } + fr_user_data->n_changes++; + return TRUE; +} + static int dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* data) { - proto_item *volatile ti = NULL, *comment_item; + proto_item *volatile ti = NULL; guint cap_len = 0, frame_len = 0; proto_tree *volatile tree; proto_tree *comments_tree; @@ -289,6 +345,7 @@ dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* frame_data_t *fr_data = (frame_data_t*)data; const color_filter_t *color_filter; dissector_handle_t dissector_handle; + fr_foreach_t fr_user_data; tree=parent_tree; @@ -397,17 +454,15 @@ dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* DISSECTOR_ASSERT_NOT_REACHED(); break; } - - if (fr_data->pkt_comment) { + if (wtap_block_count_option(fr_data->pkt_block, OPT_COMMENT) > 0) { item = proto_tree_add_item(tree, proto_pkt_comment, tvb, 0, 0, ENC_NA); comments_tree = proto_item_add_subtree(item, ett_comments); - comment_item = proto_tree_add_string_format(comments_tree, hf_comments_text, tvb, 0, 0, - fr_data->pkt_comment, "%s", - fr_data->pkt_comment); - expert_add_info_format(pinfo, comment_item, &ei_comments_text, - "%s", fr_data->pkt_comment); - - + fr_user_data.item = item; + fr_user_data.tree = comments_tree; + fr_user_data.pinfo = pinfo; + fr_user_data.tvb = tvb; + fr_user_data.n_changes = 0; + wtap_block_foreach_option(fr_data->pkt_block, frame_add_comment, (void *)&fr_user_data); } /* if FRAME is not referenced from any filters we don't need to worry about @@ -593,56 +648,18 @@ dissect_frame(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* proto_tree_add_uint64(fh_tree, hf_frame_packet_id, tvb, 0, 0, pinfo->rec->rec_header.packet_header.packet_id); - if (pinfo->rec->presence_flags & WTAP_HAS_VERDICT && - pinfo->rec->packet_verdict != NULL) { - char line_buffer[128]; + if (wtap_block_count_option(fr_data->pkt_block, OPT_PKT_VERDICT) > 0) { proto_tree *verdict_tree; proto_item *verdict_item; - format_verdict_summary(pinfo->rec, line_buffer, sizeof(line_buffer)); - verdict_item = proto_tree_add_string(fh_tree, hf_frame_verdict, tvb, 0, 0, line_buffer); + verdict_item = proto_tree_add_string(fh_tree, hf_frame_verdict, tvb, 0, 0, ""); verdict_tree = proto_item_add_subtree(verdict_item, ett_verdict); - - for(guint i = 0; i < pinfo->rec->packet_verdict->len; i++) { - - GBytes *verdict = (GBytes *) g_ptr_array_index(pinfo->rec->packet_verdict, i); - const guint8 *verdict_data; - gsize len; - - if (verdict == NULL) - continue; - - verdict_data = (const guint8 *) g_bytes_get_data(verdict, &len); - - if (len == 0) - continue; - - len -= 1; - switch(verdict_data[0]) { - case OPT_VERDICT_TYPE_HW: - proto_tree_add_bytes_with_length(verdict_tree, hf_frame_verdict_hardware, - tvb, 0, 0, verdict_data + 1, (gint) len); - break; - case OPT_VERDICT_TYPE_TC: - if (len == 8) { - gint64 val; - memcpy(&val, verdict_data + 1, sizeof(val)); - proto_tree_add_int64(verdict_tree, hf_frame_verdict_tc, tvb, 0, 0, val); - } - break; - case OPT_VERDICT_TYPE_XDP: - if (len == 8) { - gint64 val; - memcpy(&val, verdict_data + 1, sizeof(val)); - proto_tree_add_int64(verdict_tree, hf_frame_verdict_xdp, tvb, 0, 0, val); - } - break; - default: - proto_tree_add_bytes_with_length(verdict_tree, hf_frame_verdict_unknown, - tvb, 0, 0, verdict_data, (gint) len + 1); - break; - } - } + fr_user_data.item = verdict_item; + fr_user_data.tree = verdict_tree; + fr_user_data.pinfo = pinfo; + fr_user_data.tvb = tvb; + fr_user_data.n_changes = 0; + wtap_block_foreach_option(pinfo->rec->block, frame_add_verdict, (void *)&fr_user_data); } if (pinfo->rec->rec_type == REC_TYPE_PACKET) diff --git a/epan/epan.c b/epan/epan.c index 1eb2581487..bd17666f79 100644 --- a/epan/epan.c +++ b/epan/epan.c @@ -448,11 +448,11 @@ epan_new(struct packet_provider_data *prov, return session; } -const char * -epan_get_user_comment(const epan_t *session, const frame_data *fd) +wtap_block_t +epan_get_user_block(const epan_t *session, const frame_data *fd) { - if (session->funcs.get_user_comment) - return session->funcs.get_user_comment(session->prov, fd); + if (session->funcs.get_user_block) + return session->funcs.get_user_block(session->prov, fd); return NULL; } @@ -558,6 +558,8 @@ epan_dissect_reset(epan_dissect_t *edt) ws_assert(edt); + wtap_block_unref(edt->pi.rec->block); + g_slist_free(edt->pi.proto_data); g_slist_free(edt->pi.dependent_frames); @@ -611,6 +613,8 @@ epan_dissect_run(epan_dissect_t *edt, int file_type_subtype, /* free all memory allocated */ wmem_leave_packet_scope(); + wtap_block_unref(rec->block); + rec->block = NULL; } void @@ -625,6 +629,8 @@ epan_dissect_run_with_taps(epan_dissect_t *edt, int file_type_subtype, /* free all memory allocated */ wmem_leave_packet_scope(); + wtap_block_unref(rec->block); + rec->block = NULL; } void @@ -639,6 +645,8 @@ epan_dissect_file_run(epan_dissect_t *edt, wtap_rec *rec, /* free all memory allocated */ wmem_leave_packet_scope(); + wtap_block_unref(rec->block); + rec->block = NULL; } void @@ -652,6 +660,8 @@ epan_dissect_file_run_with_taps(epan_dissect_t *edt, wtap_rec *rec, /* free all memory allocated */ wmem_leave_packet_scope(); + wtap_block_unref(rec->block); + rec->block = NULL; } void diff --git a/epan/epan.h b/epan/epan.h index b5c9c13fbb..dc42541d93 100644 --- a/epan/epan.h +++ b/epan/epan.h @@ -16,6 +16,7 @@ #include #include #include +#include #include "ws_symbol_export.h" #ifdef __cplusplus @@ -49,7 +50,7 @@ struct packet_provider_funcs { const nstime_t *(*get_frame_ts)(struct packet_provider_data *prov, guint32 frame_num); const char *(*get_interface_name)(struct packet_provider_data *prov, guint32 interface_id); const char *(*get_interface_description)(struct packet_provider_data *prov, guint32 interface_id); - const char *(*get_user_comment)(struct packet_provider_data *prov, const frame_data *fd); + wtap_block_t (*get_user_block)(struct packet_provider_data *prov, const frame_data *fd); }; /** @@ -153,7 +154,7 @@ typedef struct epan_session epan_t; WS_DLL_PUBLIC epan_t *epan_new(struct packet_provider_data *prov, const struct packet_provider_funcs *funcs); -WS_DLL_PUBLIC const char *epan_get_user_comment(const epan_t *session, const frame_data *fd); +WS_DLL_PUBLIC wtap_block_t epan_get_user_block(const epan_t *session, const frame_data *fd); WS_DLL_PUBLIC const char *epan_get_interface_name(const epan_t *session, guint32 interface_id); diff --git a/epan/frame_data.c b/epan/frame_data.c index 047554136c..8cc35ee374 100644 --- a/epan/frame_data.c +++ b/epan/frame_data.c @@ -218,8 +218,8 @@ frame_data_init(frame_data *fdata, guint32 num, const wtap_rec *rec, ws_assert(rec->tsprec <= 0xF); fdata->tsprec = (unsigned int)rec->tsprec; fdata->abs_ts = rec->ts; - fdata->has_phdr_comment = (rec->opt_comment != NULL); - fdata->has_user_comment = 0; + fdata->has_phdr_block = (rec->block != NULL); + fdata->has_user_block = 0; fdata->need_colorize = 0; fdata->color_filter = NULL; fdata->shift_offset.secs = 0; diff --git a/epan/frame_data.h b/epan/frame_data.h index d7ea7cdbc1..00d7585ef3 100644 --- a/epan/frame_data.h +++ b/epan/frame_data.h @@ -84,8 +84,8 @@ typedef struct _frame_data { unsigned int ref_time : 1; /**< 1 = marked as a reference time frame, 0 = normal */ unsigned int ignored : 1; /**< 1 = ignore this frame, 0 = normal */ unsigned int has_ts : 1; /**< 1 = has time stamp, 0 = no time stamp */ - unsigned int has_phdr_comment : 1; /** 1 = there's comment for this packet */ - unsigned int has_user_comment : 1; /** 1 = user set (also deleted) comment for this packet */ + unsigned int has_phdr_block : 1; /** 1 = there's a block (possibly with options) for this packet */ + unsigned int has_user_block : 1; /** 1 = user changed block (or its options) for this packet */ unsigned int need_colorize : 1; /**< 1 = need to (re-)calculate packet color */ unsigned int tsprec : 4; /**< Time stamp precision -2^tsprec gives up to femtoseconds */ nstime_t abs_ts; /**< Absolute timestamp */ diff --git a/epan/packet.c b/epan/packet.c index 369731db60..969c77b056 100644 --- a/epan/packet.c +++ b/epan/packet.c @@ -583,13 +583,16 @@ dissect_record(epan_dissect_t *edt, int file_type_subtype, frame_delta_abs_time(edt->session, fd, fd->frame_ref_num, &edt->pi.rel_ts); - /* pkt comment use first user, later from rec */ - if (fd->has_user_comment) - frame_dissector_data.pkt_comment = epan_get_user_comment(edt->session, fd); - else if (fd->has_phdr_comment) - frame_dissector_data.pkt_comment = rec->opt_comment; - else - frame_dissector_data.pkt_comment = NULL; + /* pkt block use first user, later from rec */ + if (fd->has_user_block) { + frame_dissector_data.pkt_block = epan_get_user_block(edt->session, fd); + } + else if (fd->has_phdr_block) { + frame_dissector_data.pkt_block = rec->block; + } + else { + frame_dissector_data.pkt_block = NULL; + } frame_dissector_data.file_type_subtype = file_type_subtype; frame_dissector_data.color_edt = edt; /* Used strictly for "coloring rules" */ @@ -612,6 +615,8 @@ dissect_record(epan_dissect_t *edt, int file_type_subtype, record_type); } ENDTRY; + wtap_block_unref(rec->block); + rec->block = NULL; fd->visited = 1; } @@ -652,13 +657,16 @@ dissect_file(epan_dissect_t *edt, wtap_rec *rec, TRY { - /* pkt comment use first user, later from rec */ - if (fd->has_user_comment) - file_dissector_data.pkt_comment = epan_get_user_comment(edt->session, fd); - else if (fd->has_phdr_comment) - file_dissector_data.pkt_comment = rec->opt_comment; - else - file_dissector_data.pkt_comment = NULL; + /* pkt block use first user, later from rec */ + if (fd->has_user_block) { + file_dissector_data.pkt_block = epan_get_user_block(edt->session, fd); + } + else if (fd->has_phdr_block) { + file_dissector_data.pkt_block = rec->block; + } + else { + file_dissector_data.pkt_block = NULL; + } file_dissector_data.color_edt = edt; /* Used strictly for "coloring rules" */ @@ -680,6 +688,8 @@ dissect_file(epan_dissect_t *edt, wtap_rec *rec, "[Malformed Record: Packet Length]"); } ENDTRY; + wtap_block_unref(rec->block); + rec->block = NULL; fd->visited = 1; } diff --git a/epan/packet.h b/epan/packet.h index 96f64e8722..6cf5e975a3 100644 --- a/epan/packet.h +++ b/epan/packet.h @@ -11,6 +11,7 @@ #ifndef __PACKET_H__ #define __PACKET_H__ +#include #include "proto.h" #include "tvbuff.h" #include "epan.h" @@ -747,7 +748,7 @@ WS_DLL_PUBLIC void mark_frame_as_depended_upon(packet_info *pinfo, guint32 frame typedef struct frame_data_s { int file_type_subtype; - const gchar *pkt_comment; /**< NULL if not available */ + wtap_block_t pkt_block; /**< NULL if not available */ struct epan_dissect *color_edt; /** Used strictly for "coloring rules" */ } frame_data_t; @@ -755,7 +756,7 @@ typedef struct frame_data_s /* Structure passed to the file dissector */ typedef struct file_data_s { - const gchar *pkt_comment; /**< NULL if not available */ + wtap_block_t pkt_block; /**< NULL if not available */ struct epan_dissect *color_edt; /** Used strictly for "coloring rules" */ } file_data_t; diff --git a/epan/wslua/wslua_dumper.c b/epan/wslua/wslua_dumper.c index 0d50a9b9a5..b733a19578 100644 --- a/epan/wslua/wslua_dumper.c +++ b/epan/wslua/wslua_dumper.c @@ -14,6 +14,7 @@ #include "config.h" +#include #include /* WSLUA_MODULE Dumper Saving Capture Files @@ -384,8 +385,8 @@ WSLUA_METHOD Dumper_dump(lua_State* L) { rec.rec_header.packet_header.pseudo_header = *ph->wph; } - /* TODO: Can we get access to pinfo->pkt_comment here somehow? We - * should be copying it to pkthdr.opt_comment if we can. */ + /* TODO: Can we get access to pinfo->rec->block here somehow? We + * should be copying it to pkthdr.pkt_block if we can. */ if (! wtap_dump(d, &rec, ba->data, &err, &err_info)) { switch (err) { @@ -539,15 +540,15 @@ WSLUA_METHOD Dumper_dump_current(lua_State* L) { rec.rec_header.packet_header.pseudo_header = *lua_pinfo->pseudo_header; /* - * wtap_dump does not modify rec.opt_comment, so it should be possible to - * pass epan_get_user_comment() or lua_pinfo->rec->opt_comment directly. + * wtap_dump does not modify rec.block, so it should be possible to + * pass epan_get_user_block() or lua_pinfo->rec->block directly. * Temporarily duplicating the memory should not hurt though. */ - if (lua_pinfo->fd->has_user_comment) { - rec.opt_comment = wmem_strdup(wmem_packet_scope(), epan_get_user_comment(lua_pinfo->epan, lua_pinfo->fd)); - rec.has_comment_changed = TRUE; - } else if (lua_pinfo->fd->has_phdr_comment) { - rec.opt_comment = wmem_strdup(wmem_packet_scope(), lua_pinfo->rec->opt_comment); + if (lua_pinfo->fd->has_user_block) { + rec.block = epan_get_user_block(lua_pinfo->epan, lua_pinfo->fd); + rec.has_block_changed = TRUE; + } else if (lua_pinfo->fd->has_phdr_block) { + rec.block = lua_pinfo->rec->block; } data = (const guchar *)tvb_memdup(wmem_packet_scope(),tvb,0,rec.rec_header.packet_header.caplen); diff --git a/epan/wslua/wslua_file_common.h b/epan/wslua/wslua_file_common.h index 06fe77114b..30b85a719a 100644 --- a/epan/wslua/wslua_file_common.h +++ b/epan/wslua/wslua_file_common.h @@ -19,6 +19,7 @@ #include "config.h" #include "wslua.h" +#include #include /* this is way overkill for this one member, but in case we need to add diff --git a/epan/wslua/wslua_file_handler.c b/epan/wslua/wslua_file_handler.c index 92dd7e267a..9507d004a4 100644 --- a/epan/wslua/wslua_file_handler.c +++ b/epan/wslua/wslua_file_handler.c @@ -258,8 +258,8 @@ wslua_filehandler_read(wtap *wth, wtap_rec *rec, Buffer *buf, *err = errno = 0; } - g_free(rec->opt_comment); - rec->opt_comment = NULL; + wtap_block_unref(rec->block); + rec->block = NULL; fp = push_File(L, wth->fh); fc = push_CaptureInfo(L, wth, FALSE); @@ -316,8 +316,8 @@ wslua_filehandler_seek_read(wtap *wth, gint64 seek_off, *err = errno = 0; } - g_free(rec->opt_comment); - rec->opt_comment = NULL; + wtap_block_unref(rec->block); + rec->block = NULL; fp = push_File(L, wth->random_fh); fc = push_CaptureInfo(L, wth, FALSE); diff --git a/epan/wslua/wslua_frame_info.c b/epan/wslua/wslua_frame_info.c index 03d2eddf5f..b67abf5710 100644 --- a/epan/wslua/wslua_frame_info.c +++ b/epan/wslua/wslua_frame_info.c @@ -14,6 +14,7 @@ */ #include "wslua_file_common.h" +#include /* WSLUA_CONTINUE_MODULE File */ @@ -55,8 +56,8 @@ WSLUA_METAMETHOD FrameInfo__tostring(lua_State* L) { lua_pushstring(L,"FrameInfo pointer is NULL!"); } else { if (fi->rec) - lua_pushfstring(L, "FrameInfo: rec_type=%u, presence_flags=%d, caplen=%d, len=%d, pkt_encap=%d, opt_comment='%s'", - fi->rec->rec_type, fi->rec->presence_flags, fi->rec->rec_header.packet_header.caplen, fi->rec->rec_header.packet_header.len, fi->rec->rec_header.packet_header.pkt_encap, fi->rec->opt_comment); + lua_pushfstring(L, "FrameInfo: rec_type=%u, presence_flags=%d, caplen=%d, len=%d, pkt_encap=%d, block='%p'", + fi->rec->rec_type, fi->rec->presence_flags, fi->rec->rec_header.packet_header.caplen, fi->rec->rec_header.packet_header.len, fi->rec->rec_header.packet_header.pkt_encap, fi->rec->block); else lua_pushstring(L, "FrameInfo rec pointer is NULL!"); } @@ -103,6 +104,82 @@ static int FrameInfo__gc(lua_State* L) { return 0; } +/* WSLUA_ATTRIBUTE FrameInfo_comment RW table of comments in this frame. */ +static int FrameInfo_get_comment (lua_State* L) { + FrameInfo fi = checkFrameInfo(L,1); +#define FRAMEINFO_COMMENTS_TABLE 2 + gchar *comment = NULL; + wtap_block_t block = NULL; + guint i = 0; + guint n_comments = 0; + + block = fi->rec->block; + // XXX - how to get the user-edited block, if any? + n_comments = wtap_block_count_option(block, OPT_COMMENT); + lua_createtable(L, n_comments, 0); + for (i = 0; i < n_comments; i++) { + comment = NULL; + lua_pushnumber(L, i+1); + if (WTAP_OPTTYPE_SUCCESS == + wtap_block_get_nth_string_option_value(block, OPT_COMMENT, i, &comment)) { + lua_pushstring(L, comment); + } + else { + lua_pushnil(L); + } + lua_settable(L, FRAMEINFO_COMMENTS_TABLE); + } + + return 1; +} + +static int FrameInfo_set_comment (lua_State* L) { + FrameInfo fi = checkFrameInfo(L,1); +#define FRAMEINFO_COMMENTS_NEWTABLE 2 +#define FRAMEINFO_COMMENTS_NEWCOMMENT 2 + size_t len = 0; + gchar *comment = NULL; + wtap_block_t block = NULL; + guint i = 0; + guint n_comments = 0; + + if(fi->rec->block != NULL) { + block = fi->rec->block; + } + else { + block = wtap_block_create(WTAP_BLOCK_PACKET); + fi->rec->block = block; + } + + /* Strip off old comments */ + n_comments = wtap_block_count_option(block, OPT_COMMENT); + for (i = 0; i < n_comments; i++) { + wtap_block_remove_nth_option_instance(block, OPT_COMMENT, 0); + } + + /* Add new comment(s) */ + if (lua_istable(L, FRAMEINFO_COMMENTS_NEWTABLE)) { + for (lua_pushnil(L); lua_next(L, FRAMEINFO_COMMENTS_NEWTABLE); ) { + if (lua_isstring(L,-1)) { + comment = (gchar *)luaL_checklstring(L,-1,&len); + wtap_block_add_string_option(block, OPT_COMMENT, comment, len); + } else if (! lua_isnil(L,-1) ) { + return luaL_error(L,"only strings should be in the table"); + } + lua_pop(L, 1); + } + } + else if (lua_isstring(L, FRAMEINFO_COMMENTS_NEWCOMMENT)) { + comment = (gchar *)luaL_checklstring(L,FRAMEINFO_COMMENTS_NEWCOMMENT,&len); + wtap_block_add_string_option(block, OPT_COMMENT, comment, len); + } + else { + return luaL_error(L,"comment must be either a string or an array of strings"); + } + + return 0; +} + /* WSLUA_ATTRIBUTE FrameInfo_time RW The packet timestamp as an NSTime object. Note: Set the `FileHandler.time_precision` to the appropriate `wtap_file_tsprec` value as well. @@ -207,12 +284,6 @@ WSLUA_ATTRIBUTE_NAMED_NUMBER_SETTER(FrameInfo,original_length,rec->rec_header.pa WSLUA_ATTRIBUTE_NAMED_NUMBER_GETTER(FrameInfo,encap,rec->rec_header.packet_header.pkt_encap); WSLUA_ATTRIBUTE_NAMED_NUMBER_SETTER(FrameInfo,encap,rec->rec_header.packet_header.pkt_encap,int); -// rec->opt_comment will be freed by wtap_sequential_close -> wtap_rec_cleanup. -/* WSLUA_ATTRIBUTE FrameInfo_comment RW A string comment for the packet, if the - `wtap_presence_flags.COMMENTS` was set in the presence flags; nil if there is no comment. */ -WSLUA_ATTRIBUTE_NAMED_STRING_GETTER(FrameInfo,comment,rec->opt_comment); -WSLUA_ATTRIBUTE_NAMED_STRING_SETTER(FrameInfo,comment,rec->opt_comment,TRUE); - /* This table is ultimately registered as a sub-table of the class' metatable, * and if __index/__newindex is invoked then it calls the appropriate function * from this table for getting/setting the members. @@ -269,8 +340,8 @@ WSLUA_METAMETHOD FrameInfoConst__tostring(lua_State* L) { lua_pushstring(L,"FrameInfo pointer is NULL!"); } else { if (fi->rec && !fi->expired) - lua_pushfstring(L, "FrameInfo: rec_type=%u, presence_flags=%d, caplen=%d, len=%d, pkt_encap=%d, opt_comment='%s'", - fi->rec->rec_type, fi->rec->presence_flags, fi->rec->rec_header.packet_header.caplen, fi->rec->rec_header.packet_header.len, fi->rec->rec_header.packet_header.pkt_encap, fi->rec->opt_comment); + lua_pushfstring(L, "FrameInfo: rec_type=%u, presence_flags=%d, caplen=%d, len=%d, pkt_encap=%d, block='%p'", + fi->rec->rec_type, fi->rec->presence_flags, fi->rec->rec_header.packet_header.caplen, fi->rec->rec_header.packet_header.len, fi->rec->rec_header.packet_header.pkt_encap, fi->rec->block); else lua_pushfstring(L, "FrameInfo has %s", fi->rec?"expired":"null rec pointer"); } @@ -315,6 +386,36 @@ static int FrameInfoConst__gc(lua_State* L) { return 0; } +/* WSLUA_ATTRIBUTE FrameInfoConst_comment RO The first string comment for the packet, if any; + nil if there is no comment. */ +static int FrameInfoConst_get_comment (lua_State* L) { + FrameInfoConst fi = checkFrameInfoConst(L,1); +#define FRAMEINFOCONST_COMMENTS_TABLE 2 + gchar *comment = NULL; + wtap_block_t block = NULL; + guint i = 0; + guint n_comments = 0; + + block = fi->rec->block; + // XXX - how to get the user-edited block, if any? + n_comments = wtap_block_count_option(block, OPT_COMMENT); + lua_createtable(L, n_comments, 0); + for (i = 0; i < n_comments; i++) { + comment = NULL; + lua_pushnumber(L, i+1); + if (WTAP_OPTTYPE_SUCCESS == + wtap_block_get_nth_string_option_value(block, OPT_COMMENT, i, &comment)) { + lua_pushstring(L, comment); + } + else { + lua_pushnil(L); + } + lua_settable(L, FRAMEINFOCONST_COMMENTS_TABLE); + } + + return 1; +} + /* WSLUA_ATTRIBUTE FrameInfoConst_time RO The packet timestamp as an NSTime object. */ static int FrameInfoConst_get_time (lua_State* L) { FrameInfoConst fi = checkFrameInfoConst(L,1); @@ -358,9 +459,6 @@ WSLUA_ATTRIBUTE_NAMED_NUMBER_GETTER(FrameInfoConst,original_length,rec->rec_head See `wtap_encaps` in `init.lua` for possible packet encapsulation types to use as the value for this field. */ WSLUA_ATTRIBUTE_NAMED_NUMBER_GETTER(FrameInfoConst,encap,rec->rec_header.packet_header.pkt_encap); -/* WSLUA_ATTRIBUTE FrameInfoConst_comment RO A comment for the packet; nil if there is none. */ -WSLUA_ATTRIBUTE_NAMED_STRING_GETTER(FrameInfoConst,comment,rec->opt_comment); - WSLUA_ATTRIBUTES FrameInfoConst_attributes[] = { WSLUA_ATTRIBUTE_ROREG(FrameInfoConst,rec_type), WSLUA_ATTRIBUTE_ROREG(FrameInfoConst,flags), diff --git a/extcap/androiddump.c b/extcap/androiddump.c index 7a08db63d3..9323e1fc57 100644 --- a/extcap/androiddump.c +++ b/extcap/androiddump.c @@ -503,8 +503,7 @@ static gboolean extcap_dumper_dump(struct extcap_dumper extcap_dumper, rec.ts.secs = seconds; rec.ts.nsecs = (int) nanoseconds; - rec.opt_comment = 0; - rec.opt_comment = NULL; + rec.block = NULL; rec.rec_header.packet_header.drop_count = 0; rec.rec_header.packet_header.pack_flags = 0; diff --git a/file.c b/file.c index 9218b6d37a..44f2958686 100644 --- a/file.c +++ b/file.c @@ -252,7 +252,7 @@ ws_epan_new(capture_file *cf) ws_get_frame_ts, cap_file_provider_get_interface_name, cap_file_provider_get_interface_description, - cap_file_provider_get_user_comment + cap_file_provider_get_user_block }; return epan_new(&cf->provider, &funcs); @@ -407,9 +407,9 @@ cf_close(capture_file *cf) free_frame_data_sequence(cf->provider.frames); cf->provider.frames = NULL; } - if (cf->provider.frames_user_comments) { - g_tree_destroy(cf->provider.frames_user_comments); - cf->provider.frames_user_comments = NULL; + if (cf->provider.frames_edited_blocks) { + g_tree_destroy(cf->provider.frames_edited_blocks); + cf->provider.frames_edited_blocks = NULL; } cf_unselect_packet(cf); /* nothing to select */ cf->first_displayed = 0; @@ -1285,8 +1285,8 @@ read_record(capture_file *cf, wtap_rec *rec, Buffer *buf, dfilter_t *dfcode, fdata = frame_data_sequence_add(cf->provider.frames, &fdlocal); cf->count++; - if (rec->opt_comment != NULL) - cf->packet_comment_count++; + if (rec->block != NULL) + cf->packet_comment_count += wtap_block_count_option(rec->block, OPT_COMMENT); cf->f_datalen = offset + fdlocal.cap_len; /* When a redissection is in progress (or queued), do not process packets. @@ -3995,23 +3995,23 @@ cf_update_section_comment(capture_file *cf, gchar *comment) } /* - * Get the comment on a packet (record). - * If the comment has been edited, it returns the result of the edit, - * otherwise it returns the comment from the file. + * Get the packet block for a packet (record). + * If the block has been edited, it returns the result of the edit, + * otherwise it returns the block from the file. + * NB. Caller must wtap_block_unref() the result when done. */ -char * -cf_get_packet_comment(capture_file *cf, const frame_data *fd) +wtap_block_t +cf_get_packet_block(capture_file *cf, const frame_data *fd) { - char *comment; + /* fetch user block */ + if (fd->has_user_block) + return wtap_block_ref(cap_file_provider_get_user_block(&cf->provider, fd)); - /* fetch user comment */ - if (fd->has_user_comment) - return g_strdup(cap_file_provider_get_user_comment(&cf->provider, fd)); - - /* fetch phdr comment */ - if (fd->has_phdr_comment) { + /* fetch phdr block */ + if (fd->has_phdr_block) { wtap_rec rec; /* Record metadata */ Buffer buf; /* Record data */ + wtap_block_t block; wtap_rec_init(&rec); ws_buffer_init(&buf, 1514); @@ -4019,41 +4019,50 @@ cf_get_packet_comment(capture_file *cf, const frame_data *fd) if (!cf_read_record(cf, fd, &rec, &buf)) { /* XXX, what we can do here? */ } - /* rec.opt_comment is owned by the record, copy it before it is gone. */ - comment = g_strdup(rec.opt_comment); + /* rec.block is owned by the record, steal it before it is gone. */ + block = wtap_block_ref(rec.block); + wtap_rec_cleanup(&rec); ws_buffer_free(&buf); - return comment; + return block; } return NULL; } /* - * Update(replace) the comment on a capture from a frame + * Update(replace) the block on a capture from a frame */ gboolean -cf_set_user_packet_comment(capture_file *cf, frame_data *fd, const gchar *new_comment) +cf_set_user_packet_block(capture_file *cf, frame_data *fd, const wtap_block_t new_block) { - char *pkt_comment = cf_get_packet_comment(cf, fd); + wtap_block_t pkt_block = cf_get_packet_block(cf, fd); - /* Check if the comment has changed */ - if (!g_strcmp0(pkt_comment, new_comment)) { - g_free(pkt_comment); - return FALSE; + /* It's possible to edit the user block "in place" by doing a call to + * cf_get_packet_block() that returns an already created user block, + * editing that, and calling this function. + * If the caller did that, then the block pointers will be equal. + */ + if (pkt_block == new_block) { + /* No need to save anything here, the caller changes went right + * onto the block. + * Unfortunately we don't have a way to know how many comments were in the block + * before the caller edited it. + */ } - g_free(pkt_comment); + else { + if (pkt_block) + cf->packet_comment_count -= wtap_block_count_option(pkt_block, OPT_COMMENT); - if (pkt_comment) - cf->packet_comment_count--; + if (new_block) + cf->packet_comment_count += wtap_block_count_option(new_block, OPT_COMMENT); - if (new_comment) - cf->packet_comment_count++; + cap_file_provider_set_user_block(&cf->provider, fd, new_block); - cap_file_provider_set_user_comment(&cf->provider, fd, new_comment); + expert_update_comment_count(cf->packet_comment_count); + } - expert_update_comment_count(cf->packet_comment_count); - - /* OK, we have unsaved changes. */ + /* Either way, we have unsaved changes. */ + wtap_block_unref(pkt_block); cf->unsaved_changes = TRUE; return TRUE; } @@ -4131,19 +4140,19 @@ save_record(capture_file *cf, frame_data *fdata, wtap_rec *rec, wtap_rec new_rec; int err; gchar *err_info; - const char *pkt_comment; + wtap_block_t pkt_block; /* Copy the record information from what was read in from the file. */ new_rec = *rec; /* Make changes based on anything that the user has done but that hasn't been saved yet. */ - if (fdata->has_user_comment) - pkt_comment = cap_file_provider_get_user_comment(&cf->provider, fdata); + if (fdata->has_user_block) + pkt_block = cap_file_provider_get_user_block(&cf->provider, fdata); else - pkt_comment = rec->opt_comment; - new_rec.opt_comment = g_strdup(pkt_comment); - new_rec.has_comment_changed = fdata->has_user_comment ? TRUE : FALSE; + pkt_block = rec->block; + new_rec.block = pkt_block; + new_rec.has_block_changed = fdata->has_user_block ? TRUE : FALSE; /* XXX - what if times have been shifted? */ /* and save the packet */ @@ -4153,7 +4162,6 @@ save_record(capture_file *cf, frame_data *fdata, wtap_rec *rec, return FALSE; } - g_free(new_rec.opt_comment); return TRUE; } @@ -4760,13 +4768,14 @@ cf_save_records(capture_file *cf, const char *fname, guint save_format, for (framenum = 1; framenum <= cf->count; framenum++) { fdata = frame_data_sequence_find(cf->provider.frames, framenum); - fdata->has_phdr_comment = FALSE; - fdata->has_user_comment = FALSE; + // XXX: This also ignores non-comment options like verdict + fdata->has_phdr_block = FALSE; + fdata->has_user_block = FALSE; } - if (cf->provider.frames_user_comments) { - g_tree_destroy(cf->provider.frames_user_comments); - cf->provider.frames_user_comments = NULL; + if (cf->provider.frames_edited_blocks) { + g_tree_destroy(cf->provider.frames_edited_blocks); + cf->provider.frames_edited_blocks = NULL; } cf->packet_comment_count = 0; diff --git a/file.h b/file.h index c9a5614b68..295ce03d5b 100644 --- a/file.h +++ b/file.h @@ -689,24 +689,24 @@ cf_merge_files_to_tempfile(gpointer pd_window, char **out_filenamep, void cf_update_section_comment(capture_file *cf, gchar *comment); /* - * Get the comment on a packet (record). - * If the comment has been edited, it returns the result of the edit, - * otherwise it returns the comment from the file. + * Get the packet block for a packet (record). + * If the block has been edited, it returns the result of the edit, + * otherwise it returns the block from the file. * * @param cf the capture file * @param fd the frame_data structure for the frame - * @returns A comment (use g_free to free) or NULL if there is none. + * @returns A block (use wtap_block_unref to free) or NULL if there is none. */ -char *cf_get_packet_comment(capture_file *cf, const frame_data *fd); +wtap_block_t cf_get_packet_block(capture_file *cf, const frame_data *fd); /** - * Update(replace) the comment on a capture from a frame + * Update(replace) the block on a capture from a frame * * @param cf the capture file * @param fd the frame_data structure for the frame - * @param new_comment the string replacing the old comment + * @param new_block the block replacing the old block */ -gboolean cf_set_user_packet_comment(capture_file *cf, frame_data *fd, const gchar *new_comment); +gboolean cf_set_user_packet_block(capture_file *cf, frame_data *fd, wtap_block_t new_block); /** * What types of comments does this file have? diff --git a/file_packet_provider.c b/file_packet_provider.c index c57eb00fd8..2411ac5481 100644 --- a/file_packet_provider.c +++ b/file_packet_provider.c @@ -69,24 +69,24 @@ cap_file_provider_get_interface_description(struct packet_provider_data *prov, g return NULL; } -const char * -cap_file_provider_get_user_comment(struct packet_provider_data *prov, const frame_data *fd) +wtap_block_t +cap_file_provider_get_user_block(struct packet_provider_data *prov, const frame_data *fd) { - if (prov->frames_user_comments) - return (const char *)g_tree_lookup(prov->frames_user_comments, fd); + if (prov->frames_edited_blocks) + return (wtap_block_t)g_tree_lookup(prov->frames_edited_blocks, fd); /* ws_warning? */ return NULL; } void -cap_file_provider_set_user_comment(struct packet_provider_data *prov, frame_data *fd, const char *new_comment) +cap_file_provider_set_user_block(struct packet_provider_data *prov, frame_data *fd, wtap_block_t new_block) { - if (!prov->frames_user_comments) - prov->frames_user_comments = g_tree_new_full(frame_cmp, NULL, NULL, g_free); + if (!prov->frames_edited_blocks) + prov->frames_edited_blocks = g_tree_new_full(frame_cmp, NULL, NULL, (GDestroyNotify)wtap_block_unref); - /* insert new packet comment */ - g_tree_replace(prov->frames_user_comments, fd, g_strdup(new_comment)); + /* insert new packet block */ + g_tree_replace(prov->frames_edited_blocks, fd, (gpointer)new_block); - fd->has_user_comment = TRUE; + fd->has_user_block = TRUE; } diff --git a/frame_tvbuff.c b/frame_tvbuff.c index 17625c56f6..cb8426fbf1 100644 --- a/frame_tvbuff.c +++ b/frame_tvbuff.c @@ -36,6 +36,7 @@ frame_read(struct tvb_frame *frame_tvb, wtap_rec *rec, Buffer *buf) { int err; gchar *err_info; + gboolean ok = TRUE; /* XXX, what if phdr->caplen isn't equal to * frame_tvb->tvb.length + frame_tvb->offset? @@ -45,11 +46,11 @@ frame_read(struct tvb_frame *frame_tvb, wtap_rec *rec, Buffer *buf) switch (err) { case WTAP_ERR_BAD_FILE: g_free(err_info); + ok = FALSE; break; } - return FALSE; } - return TRUE; + return ok; } static GPtrArray *buffer_cache = NULL; diff --git a/sharkd.c b/sharkd.c index 21a410e6a9..deac44b06f 100644 --- a/sharkd.c +++ b/sharkd.c @@ -48,6 +48,7 @@ #include "ui/filter_files.h" #include "ui/tap_export_pdu.h" #include "ui/failure_message.h" +#include "wtap.h" #include #include #include @@ -240,7 +241,7 @@ sharkd_epan_new(capture_file *cf) sharkd_get_frame_ts, cap_file_provider_get_interface_name, cap_file_provider_get_interface_description, - cap_file_provider_get_user_comment + cap_file_provider_get_user_block }; return epan_new(&cf->provider, &funcs); @@ -767,16 +768,55 @@ sharkd_filter(const char *dftext, guint8 **result) return framenum; } -const char * -sharkd_get_user_comment(const frame_data *fd) +/* + * Get the user block if available, nothing otherwise. + * Must be cloned if changes desired. + */ +wtap_block_t +sharkd_get_user_block(const frame_data *fd) { - return cap_file_provider_get_user_comment(&cfile.provider, fd); + return cap_file_provider_get_user_block(&cfile.provider, fd); +} + +/* + * Gets the user block if available, otherwise the packet's default block, + * or a new packet block. + * User must wtap_block_unref() it when done. + */ +wtap_block_t +sharkd_get_packet_block(const frame_data *fd) +{ + if (fd->has_user_block) + return wtap_block_ref(cap_file_provider_get_user_block(&cfile.provider, fd)); + if (fd->has_phdr_block) + { + wtap_rec rec; /* Record metadata */ + Buffer buf; /* Record data */ + wtap_block_t block; + int err; + gchar *err_info; + + wtap_rec_init(&rec); + ws_buffer_init(&buf, 1514); + + if (!wtap_seek_read(cfile.provider.wth, fd->file_off, &rec, &buf, &err, &err_info)) + { /* XXX, what we can do here? */ } + + /* rec.block is owned by the record, steal it before it is gone. */ + block = wtap_block_ref(rec.block); + + wtap_rec_cleanup(&rec); + ws_buffer_free(&buf); + return block; + } + else + return wtap_block_create(WTAP_BLOCK_PACKET); } int -sharkd_set_user_comment(frame_data *fd, const gchar *new_comment) +sharkd_set_user_block(frame_data *fd, wtap_block_t new_block) { - cap_file_provider_set_user_comment(&cfile.provider, fd, new_comment); + cap_file_provider_set_user_block(&cfile.provider, fd, new_block); return 0; } diff --git a/sharkd.h b/sharkd.h index dae4993cfb..5d21d61a04 100644 --- a/sharkd.h +++ b/sharkd.h @@ -13,6 +13,7 @@ #define __SHARKD_H #include +#include #define SHARKD_DISSECT_FLAG_NULL 0x00u #define SHARKD_DISSECT_FLAG_BYTES 0x01u @@ -35,8 +36,10 @@ int sharkd_filter(const char *dftext, guint8 **result); frame_data *sharkd_get_frame(guint32 framenum); int sharkd_dissect_columns(frame_data *fdata, guint32 frame_ref_num, guint32 prev_dis_num, column_info *cinfo, gboolean dissect_color); int sharkd_dissect_request(guint32 framenum, guint32 frame_ref_num, guint32 prev_dis_num, sharkd_dissect_func_t cb, guint32 dissect_flags, void *data); -const char *sharkd_get_user_comment(const frame_data *fd); -int sharkd_set_user_comment(frame_data *fd, const gchar *new_comment); +wtap_block_t sharkd_get_user_block(const frame_data *fd); +wtap_block_t sharkd_get_packet_block(const frame_data *fd); +int sharkd_set_user_block(frame_data *fd, wtap_block_t new_block); +const char *sharkd_version(void); /* sharkd_daemon.c */ int sharkd_init(int argc, char **argv); diff --git a/sharkd_session.c b/sharkd_session.c index 25aafd03e4..c282a6a439 100644 --- a/sharkd_session.c +++ b/sharkd_session.c @@ -9,6 +9,7 @@ * SPDX-License-Identifier: GPL-2.0-or-later */ +#include "wtap_opttypes.h" #include #include @@ -1443,9 +1444,9 @@ sharkd_session_process_frames(const char *buf, const jsmntok_t *tokens, int coun sharkd_json_value_anyf("num", "%u", framenum); - if (fdata->has_user_comment || fdata->has_phdr_comment) + if (fdata->has_user_block || fdata->has_phdr_block) { - if (!fdata->has_user_comment || sharkd_get_user_comment(fdata) != NULL) + if (!fdata->has_user_block || sharkd_get_user_block(fdata) != NULL) sharkd_json_value_anyf("ct", "true"); } @@ -3347,20 +3348,34 @@ sharkd_session_process_frame_cb(epan_dissect_t *edt, proto_tree *tree, struct ep { packet_info *pi = &edt->pi; frame_data *fdata = pi->fd; - const char *pkt_comment = NULL; + wtap_block_t pkt_block = NULL; const struct sharkd_frame_request_data * const req_data = (const struct sharkd_frame_request_data * const) data; const gboolean display_hidden = (req_data) ? req_data->display_hidden : FALSE; sharkd_json_result_prologue(rpcid); - if (fdata->has_user_comment) - pkt_comment = sharkd_get_user_comment(fdata); - else if (fdata->has_phdr_comment) - pkt_comment = pi->rec->opt_comment; + if (fdata->has_user_block) + pkt_block = sharkd_get_user_block(fdata); + else if (fdata->has_phdr_block) + pkt_block = pi->rec->block; - if (pkt_comment) - sharkd_json_value_string("comment", pkt_comment); + if (pkt_block) + { + guint i; + guint n; + gchar *comment; + + n = wtap_block_count_option(pkt_block, OPT_COMMENT); + + sharkd_json_array_open("comment"); + for (i = 0; i < n; i++) { + if (WTAP_OPTTYPE_SUCCESS == wtap_block_get_nth_string_option_value(pkt_block, OPT_COMMENT, i, &comment)) { + sharkd_json_value_string(NULL, comment); + } + } + sharkd_json_array_close(); + } if (tree) { @@ -4179,6 +4194,9 @@ sharkd_session_process_complete(char *buf, const jsmntok_t *tokens, int count) * * Output object with attributes: * (m) err - error code: 0 succeed + * + * Note: + * For now, adds comments, doesn't remove or replace them. */ static void sharkd_session_process_setcomment(char *buf, const jsmntok_t *tokens, int count) @@ -4188,7 +4206,8 @@ sharkd_session_process_setcomment(char *buf, const jsmntok_t *tokens, int count) guint32 framenum; frame_data *fdata; - int ret; + wtap_opttype_return_val ret; + wtap_block_t pkt_block = NULL; if (!tok_frame || !ws_strtou32(tok_frame, NULL, &framenum) || framenum == 0) { @@ -4209,15 +4228,22 @@ sharkd_session_process_setcomment(char *buf, const jsmntok_t *tokens, int count) return; } - ret = sharkd_set_user_comment(fdata, tok_comment); + pkt_block = sharkd_get_packet_block(fdata); - if (ret) + ret = wtap_block_add_string_option(pkt_block, OPT_COMMENT, tok_comment, strlen(tok_comment)); + + if (ret != WTAP_OPTTYPE_SUCCESS) + { sharkd_json_error( rpcid, -3003, NULL, "Unable to set the comment" ); + } else + { + sharkd_set_user_block(fdata, pkt_block); sharkd_json_simple_ok(rpcid); + } } diff --git a/test/lua/acme_file.lua b/test/lua/acme_file.lua index 66d12d6c6d..f159ba2d45 100644 --- a/test/lua/acme_file.lua +++ b/test/lua/acme_file.lua @@ -666,7 +666,6 @@ function Packet:set_wslua_fields(frame) frame.flags = wtap_presence_flags.TS -- for timestamp if self.comment then frame.comment = self.comment - frame.flags = frame.flags + wtap_presence_flags.COMMENTS -- comment flag end return true end diff --git a/test/lua/globals_2.2.txt b/test/lua/globals_2.2.txt index 9f83d033c0..3ae1adba7c 100644 --- a/test/lua/globals_2.2.txt +++ b/test/lua/globals_2.2.txt @@ -1217,7 +1217,6 @@ }, ["wtap_presence_flags"] = { ["CAP_LEN"] = 2, - ["COMMENTS"] = 8, ["DROP_COUNT"] = 16, ["INTERFACE_ID"] = 4, ["PACK_FLAGS"] = 32, diff --git a/test/suite_sharkd.py b/test/suite_sharkd.py index b5348cdaf2..c89c30689b 100644 --- a/test/suite_sharkd.py +++ b/test/suite_sharkd.py @@ -438,7 +438,7 @@ class case_sharkd(subprocesstest.SubprocessTestCase): {"jsonrpc":"2.0","id":1,"result":{"status":"OK"}}, {"jsonrpc":"2.0","id":2,"error":{"code":-3002,"message":"Frame number is out of range"}}, {"jsonrpc":"2.0","id":3,"result":{"status":"OK"}}, - {"jsonrpc":"2.0","id":4,"result":{"comment":"foo\nbar","fol": MatchAny(list)}}, + {"jsonrpc":"2.0","id":4,"result":{"comment":["foo\nbar"],"fol": MatchAny(list)}}, )) def test_sharkd_req_setconf_bad(self, check_sharkd_session): diff --git a/tshark.c b/tshark.c index 4204c27f5d..7c6f02ce14 100644 --- a/tshark.c +++ b/tshark.c @@ -3537,7 +3537,7 @@ process_cap_file(capture_file *cf, char *save_file, int out_file_type, /* If we don't have an application name add Tshark */ if (wtap_block_get_string_option_value(g_array_index(params.shb_hdrs, wtap_block_t, 0), OPT_SHB_USERAPPL, &shb_user_appl) != WTAP_OPTTYPE_SUCCESS) { - /* this is free'd by wtap_block_free() later */ + /* this is free'd by wtap_block_unref() later */ wtap_block_add_string_option_format(g_array_index(params.shb_hdrs, wtap_block_t, 0), OPT_SHB_USERAPPL, "%s", get_appname_and_version()); } diff --git a/ui/qt/capture_file_properties_dialog.cpp b/ui/qt/capture_file_properties_dialog.cpp index 39241dcc1a..8a38b90987 100644 --- a/ui/qt/capture_file_properties_dialog.cpp +++ b/ui/qt/capture_file_properties_dialog.cpp @@ -562,17 +562,24 @@ void CaptureFilePropertiesDialog::fillDetails() for (guint32 framenum = 1; framenum <= cap_file_.capFile()->count ; framenum++) { frame_data *fdata = frame_data_sequence_find(cap_file_.capFile()->provider.frames, framenum); - char *pkt_comment = cf_get_packet_comment(cap_file_.capFile(), fdata); + wtap_block_t pkt_block = cf_get_packet_block(cap_file_.capFile(), fdata); - if (pkt_comment) { - QString frame_comment_html = tr("

Frame %1: ").arg(framenum); - QString raw_comment = gchar_free_to_qstring(pkt_comment); + if (pkt_block) { + guint n_comments = wtap_block_count_option(pkt_block, OPT_COMMENT); + for (guint i = 0; i < n_comments; i++) { + char *comment_text; + if (WTAP_OPTTYPE_SUCCESS == wtap_block_get_nth_string_option_value(pkt_block, OPT_COMMENT, i, &comment_text)) { + QString frame_comment_html = tr("

Frame %1: ").arg(framenum); + QString raw_comment = comment_text; - frame_comment_html += html_escape(raw_comment).replace('\n', "
"); - frame_comment_html += "

\n"; - cursor.insertBlock(); - cursor.insertHtml(frame_comment_html); + frame_comment_html += html_escape(raw_comment).replace('\n', "
"); + frame_comment_html += "

\n"; + cursor.insertBlock(); + cursor.insertHtml(frame_comment_html); + } + } } + wtap_block_unref(pkt_block); } } ui->detailsTextEdit->verticalScrollBar()->setValue(0); diff --git a/ui/qt/main_window.cpp b/ui/qt/main_window.cpp index 6ff429918a..5281d39f7a 100644 --- a/ui/qt/main_window.cpp +++ b/ui/qt/main_window.cpp @@ -510,6 +510,8 @@ MainWindow::MainWindow(QWidget *parent) : connect(packet_list_, SIGNAL(framesSelected(QList)), this, SLOT(setMenusForSelectedPacket())); connect(packet_list_, SIGNAL(framesSelected(QList)), this, SIGNAL(framesSelected(QList))); + connect(main_ui_->menuPacketComment, SIGNAL(aboutToShow()), this, SLOT(setEditCommentsMenu())); + proto_tree_ = new ProtoTree(&master_split_); proto_tree_->installEventFilter(this); diff --git a/ui/qt/main_window.h b/ui/qt/main_window.h index 1a5adf4be5..fcd4fdb131 100644 --- a/ui/qt/main_window.h +++ b/ui/qt/main_window.h @@ -394,6 +394,10 @@ private slots: void mainStackChanged(int); void updateRecentCaptures(); void recentActionTriggered(); + void actionAddPacketComment(); + void actionEditPacketComment(); + void actionDeletePacketComment(); + void setEditCommentsMenu(); void setMenusForSelectedPacket(); void setMenusForSelectedTreeRow(FieldInformation *fi = NULL); void interfaceSelectionChanged(); @@ -508,8 +512,8 @@ private slots: void on_actionEditPreviousTimeReference_triggered(); void on_actionEditTimeShift_triggered(); void editTimeShiftFinished(int); - void on_actionEditPacketComment_triggered(); - void editPacketCommentFinished(PacketCommentDialog* pc_dialog, int result); + void addPacketCommentFinished(PacketCommentDialog* pc_dialog, int result); + void editPacketCommentFinished(PacketCommentDialog* pc_dialog, int result, guint nComment); void on_actionDeleteAllPacketComments_triggered(); void deleteAllPacketCommentsFinished(int result); void on_actionEditConfigurationProfiles_triggered(); diff --git a/ui/qt/main_window.ui b/ui/qt/main_window.ui index 2f1a8a3ec8..662f77296c 100644 --- a/ui/qt/main_window.ui +++ b/ui/qt/main_window.ui @@ -665,7 +665,12 @@ - + + + Packet Comments + + + @@ -1579,17 +1584,6 @@ Ctrl+Shift+T - - - Packet Comment… - - - Add or change a packet comment - - - Ctrl+Alt+C - - Delete All Packet Comments diff --git a/ui/qt/main_window_slots.cpp b/ui/qt/main_window_slots.cpp index 4e6eb12743..ded526d013 100644 --- a/ui/qt/main_window_slots.cpp +++ b/ui/qt/main_window_slots.cpp @@ -1114,6 +1114,45 @@ void MainWindow::recentActionTriggered() { } } +void MainWindow::setEditCommentsMenu() +{ + main_ui_->menuPacketComment->clear(); + main_ui_->menuPacketComment->addAction(tr("Add New Comment…"), this, SLOT(actionAddPacketComment()), QKeySequence(Qt::CTRL + Qt::ALT + Qt::Key_C)); + if (selectedRows().count() == 1) { + const int thisRow = selectedRows().first(); + frame_data * current_frame = frameDataForRow(thisRow); + wtap_block_t pkt_block = cf_get_packet_block(capture_file_.capFile(), current_frame); + guint nComments = wtap_block_count_option(pkt_block, OPT_COMMENT); + if (nComments > 0) { + QAction *aPtr; + main_ui_->menuPacketComment->addSeparator(); + for (guint i = 0; i < nComments; i++) { + QString comment = packet_list_->getPacketComment(i).trimmed(); + if (comment.size() > 40) { + comment.truncate(40); + comment += "…"; + } + aPtr = main_ui_->menuPacketComment->addAction(tr("Edit \"%1\"", "edit packet comment").arg(comment), + this, SLOT(actionEditPacketComment())); + aPtr->setData(i); + } + + main_ui_->menuPacketComment->addSeparator(); + for (guint i = 0; i < nComments; i++) { + QString comment = packet_list_->getPacketComment(i).trimmed(); + if (comment.size() > 40) { + comment.truncate(40); + comment += "…"; + } + aPtr = main_ui_->menuPacketComment->addAction(tr("Delete \"%1\"", "delete packet comment").arg(comment), + this, SLOT(actionDeletePacketComment())); + aPtr->setData(i); + } + } + wtap_block_unref(pkt_block); + } +} + void MainWindow::setMenusForSelectedPacket() { gboolean is_ip = FALSE, is_tcp = FALSE, is_udp = FALSE, is_dccp = FALSE, is_sctp = FALSE, is_tls = FALSE, is_rtp = FALSE, is_lte_rlc = FALSE, @@ -1213,8 +1252,9 @@ void MainWindow::setMenusForSelectedPacket() if (capture_file_.capFile() && capture_file_.capFile()->linktypes) linkTypes = capture_file_.capFile()->linktypes; - main_ui_->actionEditPacketComment->setEnabled(frame_selected && linkTypes && wtap_dump_can_write(capture_file_.capFile()->linktypes, WTAP_COMMENT_PER_PACKET)); - main_ui_->actionDeleteAllPacketComments->setEnabled(linkTypes && wtap_dump_can_write(capture_file_.capFile()->linktypes, WTAP_COMMENT_PER_PACKET)); + bool enableEditComments = frame_selected && linkTypes && wtap_dump_can_write(capture_file_.capFile()->linktypes, WTAP_COMMENT_PER_PACKET); + main_ui_->menuPacketComment->setEnabled(enableEditComments); + main_ui_->actionDeleteAllPacketComments->setEnabled(enableEditComments); main_ui_->actionEditIgnorePacket->setEnabled(frame_selected || multi_selection); main_ui_->actionEditIgnoreAllDisplayed->setEnabled(have_filtered); @@ -2156,7 +2196,7 @@ void MainWindow::editTimeShiftFinished(int) } } -void MainWindow::on_actionEditPacketComment_triggered() +void MainWindow::actionAddPacketComment() { QList rows = selectedRows(); if (rows.count() != 1) @@ -2167,21 +2207,57 @@ void MainWindow::on_actionEditPacketComment_triggered() return; PacketCommentDialog* pc_dialog; - pc_dialog = new PacketCommentDialog(fdata->num, this, packet_list_->packetComment()); - connect(pc_dialog, &QDialog::finished, std::bind(&MainWindow::editPacketCommentFinished, this, pc_dialog, std::placeholders::_1)); + pc_dialog = new PacketCommentDialog(fdata->num, this, NULL); + connect(pc_dialog, &QDialog::finished, std::bind(&MainWindow::addPacketCommentFinished, this, pc_dialog, std::placeholders::_1)); pc_dialog->setWindowModality(Qt::ApplicationModal); pc_dialog->setAttribute(Qt::WA_DeleteOnClose); pc_dialog->show(); } -void MainWindow::editPacketCommentFinished(PacketCommentDialog* pc_dialog, int result) +void MainWindow::addPacketCommentFinished(PacketCommentDialog* pc_dialog _U_, int result _U_) { if (result == QDialog::Accepted) { - packet_list_->setPacketComment(pc_dialog->text()); + packet_list_->addPacketComment(pc_dialog->text()); updateForUnsavedChanges(); } } +void MainWindow::actionEditPacketComment() +{ + QList rows = selectedRows(); + if (rows.count() != 1) + return; + + frame_data * fdata = frameDataForRow(rows.at(0)); + if (! fdata) + return; + + QAction *ra = qobject_cast(sender()); + guint nComment = ra->data().toUInt(); + PacketCommentDialog* pc_dialog; + pc_dialog = new PacketCommentDialog(fdata->num, this, packet_list_->getPacketComment(nComment)); + connect(pc_dialog, &QDialog::finished, std::bind(&MainWindow::editPacketCommentFinished, this, pc_dialog, std::placeholders::_1, nComment)); + pc_dialog->setWindowModality(Qt::ApplicationModal); + pc_dialog->setAttribute(Qt::WA_DeleteOnClose); + pc_dialog->show(); +} + +void MainWindow::editPacketCommentFinished(PacketCommentDialog* pc_dialog _U_, int result _U_, guint nComment) +{ + if (result == QDialog::Accepted) { + packet_list_->setPacketComment(nComment, pc_dialog->text()); + updateForUnsavedChanges(); + } +} + +void MainWindow::actionDeletePacketComment() +{ + QAction *ra = qobject_cast(sender()); + guint nComment = ra->data().toUInt(); + packet_list_->setPacketComment(nComment, QString("")); + updateForUnsavedChanges(); +} + void MainWindow::on_actionDeleteAllPacketComments_triggered() { QMessageBox *msg_dialog = new QMessageBox(); diff --git a/ui/qt/packet_list.cpp b/ui/qt/packet_list.cpp index 94f27ab77a..0862ff8227 100644 --- a/ui/qt/packet_list.cpp +++ b/ui/qt/packet_list.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -34,6 +35,7 @@ #include #include "ui/util.h" +#include "wiretap/wtap_opttypes.h" #include "wsutil/str_util.h" #include @@ -645,7 +647,7 @@ void PacketList::contextMenuEvent(QContextMenuEvent *event) ctx_menu->addAction(window()->findChild("actionEditIgnorePacket")); ctx_menu->addAction(window()->findChild("actionEditSetTimeReference")); ctx_menu->addAction(window()->findChild("actionEditTimeShift")); - ctx_menu->addAction(window()->findChild("actionEditPacketComment")); + ctx_menu->addMenu(window()->findChild("menuPacketComment")); ctx_menu->addSeparator(); @@ -1373,11 +1375,13 @@ void PacketList::resetColorized() update(); } -QString PacketList::packetComment() +QString PacketList::getPacketComment(guint c_number) { int row = currentIndex().row(); const frame_data *fdata; char *pkt_comment; + wtap_opttype_return_val result; + QString ret_val = NULL; if (!cap_file_ || !packet_list_model_) return NULL; @@ -1385,17 +1389,41 @@ QString PacketList::packetComment() if (!fdata) return NULL; - pkt_comment = cf_get_packet_comment(cap_file_, fdata); - if (!pkt_comment) return NULL; - - return gchar_free_to_qstring(pkt_comment); + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + result = wtap_block_get_nth_string_option_value(pkt_block, OPT_COMMENT, c_number, &pkt_comment); + if (result == WTAP_OPTTYPE_SUCCESS) { + ret_val = QString(pkt_comment); + } + wtap_block_unref(pkt_block); + return ret_val; } -void PacketList::setPacketComment(QString new_comment) +void PacketList::addPacketComment(QString new_comment) +{ + int row = currentIndex().row(); + frame_data *fdata; + + if (!cap_file_ || !packet_list_model_) return; + if (new_comment.isEmpty()) return; + + fdata = packet_list_model_->getRowFdata(row); + + if (!fdata) return; + + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + + QByteArray ba = new_comment.toLocal8Bit(); + wtap_block_add_string_option(pkt_block, OPT_COMMENT, ba.data(), ba.size()); + + cf_set_user_packet_block(cap_file_, fdata, pkt_block); + + redrawVisiblePackets(); +} + +void PacketList::setPacketComment(guint c_number, QString new_comment) { int row = currentIndex().row(); frame_data *fdata; - gchar *new_packet_comment; if (!cap_file_ || !packet_list_model_) return; @@ -1403,15 +1431,17 @@ void PacketList::setPacketComment(QString new_comment) if (!fdata) return; + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + /* Check if we are clearing the comment */ if (new_comment.isEmpty()) { - new_packet_comment = NULL; + wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, c_number); } else { - new_packet_comment = qstring_strdup(new_comment); + QByteArray ba = new_comment.toLocal8Bit(); + wtap_block_set_nth_string_option_value(pkt_block, OPT_COMMENT, c_number, ba.data(), ba.size()); } - cf_set_user_packet_comment(cap_file_, fdata, new_packet_comment); - g_free(new_packet_comment); + cf_set_user_packet_block(cap_file_, fdata, pkt_block); redrawVisiblePackets(); } @@ -1427,16 +1457,21 @@ QString PacketList::allPacketComments() for (framenum = 1; framenum <= cap_file_->count ; framenum++) { fdata = frame_data_sequence_find(cap_file_->provider.frames, framenum); - char *pkt_comment = cf_get_packet_comment(cap_file_, fdata); + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); - if (pkt_comment) { - buf_str.append(QString(tr("Frame %1: %2\n\n")).arg(framenum).arg(pkt_comment)); - g_free(pkt_comment); - } - if (buf_str.length() > max_comments_to_fetch_) { - buf_str.append(QString(tr("[ Comment text exceeds %1. Stopping. ]")) - .arg(format_size(max_comments_to_fetch_, format_size_unit_bytes|format_size_prefix_si))); - return buf_str; + if (pkt_block) { + guint n_comments = wtap_block_count_option(pkt_block, OPT_COMMENT); + for (guint i = 0; i < n_comments; i++) { + char *comment_text; + if (WTAP_OPTTYPE_SUCCESS == wtap_block_get_nth_string_option_value(pkt_block, OPT_COMMENT, i, &comment_text)) { + buf_str.append(QString(tr("Frame %1: %2\n\n")).arg(framenum).arg(comment_text)); + if (buf_str.length() > max_comments_to_fetch_) { + buf_str.append(QString(tr("[ Comment text exceeds %1. Stopping. ]")) + .arg(format_size(max_comments_to_fetch_, format_size_unit_bytes|format_size_prefix_si))); + return buf_str; + } + } + } } } return buf_str; @@ -1447,16 +1482,24 @@ void PacketList::deleteAllPacketComments() guint32 framenum; frame_data *fdata; QString buf_str; + guint i; if (!cap_file_) return; for (framenum = 1; framenum <= cap_file_->count ; framenum++) { fdata = frame_data_sequence_find(cap_file_->provider.frames, framenum); + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + guint n_comments = wtap_block_count_option(pkt_block, OPT_COMMENT); - cf_set_user_packet_comment(cap_file_, fdata, NULL); + for (i = 0; i < n_comments; i++) { + wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, 0); + } + cf_set_user_packet_block(cap_file_, fdata, pkt_block); } + cap_file_->packet_comment_count = 0; + expert_update_comment_count(cap_file_->packet_comment_count); redrawVisiblePackets(); } diff --git a/ui/qt/packet_list.h b/ui/qt/packet_list.h index c3710c82e1..a9fc5bfb94 100644 --- a/ui/qt/packet_list.h +++ b/ui/qt/packet_list.h @@ -71,8 +71,9 @@ public: bool contextMenuActive(); QString getFilterFromRowAndColumn(QModelIndex idx); void resetColorized(); - QString packetComment(); - void setPacketComment(QString new_comment); + QString getPacketComment(guint c_number); + void addPacketComment(QString new_comment); + void setPacketComment(guint c_number, QString new_comment); QString allPacketComments(); void deleteAllPacketComments(); void setVerticalAutoScroll(bool enabled = true); diff --git a/ui/tap_export_pdu.c b/ui/tap_export_pdu.c index 850148ca55..1a2bf001a6 100644 --- a/ui/tap_export_pdu.c +++ b/ui/tap_export_pdu.c @@ -59,14 +59,14 @@ export_pdu_packet(void *tapdata, packet_info *pinfo, epan_dissect_t *edt, const rec.rec_header.packet_header.pkt_encap = exp_pdu_tap_data->pkt_encap; - /* rec.opt_comment is not modified by wtap_dump, but if for some reason the - * epan_get_user_comment() or pinfo->rec->opt_comment are invalidated, + /* rec.opt_block is not modified by wtap_dump, but if for some reason the + * epan_get_user_block() or pinfo->rec->block are invalidated, * copying it here does not hurt. (Can invalidation really happen?) */ - if (pinfo->fd->has_user_comment) { - rec.opt_comment = g_strdup(epan_get_user_comment(edt->session, pinfo->fd)); - rec.has_comment_changed = TRUE; - } else if (pinfo->fd->has_phdr_comment) { - rec.opt_comment = g_strdup(pinfo->rec->opt_comment); + if (pinfo->fd->has_user_block) { + rec.block = epan_get_user_block(edt->session, pinfo->fd); + rec.has_block_changed = TRUE; + } else if (pinfo->fd->has_phdr_block) { + rec.block = pinfo->rec->block; } /* XXX: should the rec.rec_header.packet_header.pseudo_header be set to the pinfo's pseudo-header? */ @@ -78,7 +78,6 @@ export_pdu_packet(void *tapdata, packet_info *pinfo, epan_dissect_t *edt, const } g_free(packet_buf); - g_free(rec.opt_comment); return status; } diff --git a/wiretap/erf.c b/wiretap/erf.c index 12147fc173..4ff5a01b98 100644 --- a/wiretap/erf.c +++ b/wiretap/erf.c @@ -1484,6 +1484,7 @@ static gboolean erf_write_anchor_meta_update_phdr(wtap_dumper *wdh, erf_dump_t * guint8 source_id = 0; gboolean ret = FALSE; guint64 implicit_host_id = dump_priv->implicit_host_id == ERF_META_HOST_ID_IMPLICIT ? 0 : dump_priv->implicit_host_id; + gchar *pkt_comment; /* @@ -1648,8 +1649,12 @@ static gboolean erf_write_anchor_meta_update_phdr(wtap_dumper *wdh, erf_dump_t * } /* Generate the metadata payload with the packet comment */ + /* XXX - can ERF have more than one comment? */ sections = g_ptr_array_new_with_free_func(erf_meta_section_free); - erf_comment_to_sections(wdh, ERF_META_SECTION_INFO, 0x8000 /*local to record*/, rec->opt_comment, sections); + if (WTAP_OPTTYPE_SUCCESS != wtap_block_get_nth_string_option_value(rec->block, OPT_COMMENT, 0, &pkt_comment)) { + pkt_comment = NULL; + } + erf_comment_to_sections(wdh, ERF_META_SECTION_INFO, 0x8000 /*local to record*/, pkt_comment, sections); /* Write the metadata record, but not the packet record as what we do depends * on the WTAP_ENCAP */ @@ -1977,11 +1982,11 @@ static gboolean erf_dump( * construct a new header with additional Host ID and Anchor ID * and insert a metadata record before that frame */ /*XXX: The user may have changed the comment to cleared! */ - if(rec->opt_comment || rec->has_comment_changed) { + if(rec->has_block_changed) { if (encap == WTAP_ENCAP_ERF) { /* XXX: What about ERF-in-pcapng with existing comment (that wasn't * modified)? */ - if(rec->has_comment_changed) { + if(rec->has_block_changed) { memmove(&other_phdr, pseudo_header, sizeof(union wtap_pseudo_header)); if(!erf_write_anchor_meta_update_phdr(wdh, dump_priv, rec, &other_phdr, err)) return FALSE; pseudo_header = &other_phdr; @@ -2251,16 +2256,8 @@ static int erf_update_anchors_from_header(erf_t *erf_priv, wtap_rec *rec, union if (comment) { /* Will be freed by either wtap_sequential_close (for rec = &wth->rec) or by * the caller of wtap_seek_read. See wtap_rec_cleanup. */ - g_free(rec->opt_comment); - rec->opt_comment = g_strdup(comment); - rec->presence_flags |= WTAP_HAS_COMMENTS; - } else { - /* WTAP_HAS_COMMENT has no visible effect? - * Need to set opt_comment to NULL to prevent other packets - * from displaying the same comment - */ - g_free(rec->opt_comment); - rec->opt_comment = NULL; + rec->block = wtap_block_create(WTAP_BLOCK_PACKET); + wtap_block_add_string_option(rec->block, OPT_COMMENT, comment, strlen(comment)); } return 0; diff --git a/wiretap/pcapng.c b/wiretap/pcapng.c index ad076724ab..df91b7a619 100644 --- a/wiretap/pcapng.c +++ b/wiretap/pcapng.c @@ -517,6 +517,13 @@ pcapng_process_string_option(wtapng_block_t *wblock, guint16 option_code, wtap_block_add_string_option(wblock->block, option_code, (const char *)option_content, option_length); } +static void +pcapng_process_bytes_option(wtapng_block_t *wblock, guint16 option_code, + guint16 option_length, const guint8 *option_content) +{ + wtap_block_add_bytes_option(wblock->block, option_code, (const char *)option_content, option_length); +} + static void pcapng_process_uint8_option(wtapng_block_t *wblock, guint16 option_code, guint16 option_length, @@ -589,29 +596,6 @@ pcapng_process_uint64_option(wtapng_block_t *wblock, } } -static gboolean -pcap_process_generic_custom_option(wtapng_block_t *wblock, - guint16 option_code, guint16 option_length, - guint32 pen, - const guint8 *option_content) -{ - wtap_option_t option; - - if (wblock->type == BLOCK_TYPE_EPB) { - if (wblock->rec->custom_options == NULL) { - wblock->rec->custom_options = g_array_new (FALSE, FALSE, sizeof(wtap_option_t)); - } - option.option_id = option_code; - option.value.custom_opt.pen = pen; - option.value.custom_opt.custom_data_len = option_length - 4; - option.value.custom_opt.custom_data = g_memdup2(option_content + 4, option_length - 4); - g_array_append_val(wblock->rec->custom_options, option); - } else { - wtap_block_add_custom_option(wblock->block, option_code, pen, option_content + 4, option_length - 4); - } - return TRUE; -} - static gboolean pcapng_process_custom_option(wtapng_block_t *wblock, const section_info_t *section_info, @@ -634,7 +618,7 @@ pcapng_process_custom_option(wtapng_block_t *wblock, switch (pen) { default: ws_debug("Custom option type 0x%04x with unknown pen %u with custom data of length %u", option_code, pen, option_length - 4); - if (!pcap_process_generic_custom_option(wblock, option_code, option_length, pen, option_content)) { + if (wtap_block_add_custom_option(wblock->block, option_code, pen, option_content + 4, option_length - 4) != WTAP_OPTTYPE_SUCCESS) { return FALSE; } break; @@ -1420,7 +1404,6 @@ pcapng_process_packet_block_option(wtapng_block_t *wblock, { guint32 tmp32; guint64 tmp64; - guint8 *option_content_copy; /* * Handle option content. @@ -1441,16 +1424,7 @@ pcapng_process_packet_block_option(wtapng_block_t *wblock, */ switch (option_code) { case(OPT_COMMENT): - if (option_length != 0) { - wblock->rec->presence_flags |= WTAP_HAS_COMMENTS; - g_free(wblock->rec->opt_comment); - wblock->rec->opt_comment = g_strndup((char *)option_content, option_length); - ws_debug("length %u opt_comment '%s'", - option_length, wblock->rec->opt_comment); - } else { - ws_debug("opt_comment length %u seems strange", - option_length); - } + pcapng_process_string_option(wblock, option_code, option_length, option_content); break; case(OPT_EPB_FLAGS): if (option_length != 4) { @@ -1543,7 +1517,7 @@ pcapng_process_packet_block_option(wtapng_block_t *wblock, switch (option_content[0]) { case(OPT_VERDICT_TYPE_HW): - option_content_copy = g_memdup2(option_content, option_length); + /* No byte swapping needed */ break; case(OPT_VERDICT_TYPE_TC): @@ -1554,11 +1528,10 @@ pcapng_process_packet_block_option(wtapng_block_t *wblock, /* XXX - free anything? */ return FALSE; } - option_content_copy = g_memdup2(option_content, option_length); if (section_info->byte_swapped) { - memcpy(&tmp64, option_content_copy + 1, sizeof(tmp64)); + memcpy(&tmp64, option_content + 1, sizeof(tmp64)); tmp64 = GUINT64_SWAP_LE_BE(tmp64); - memcpy(option_content_copy + 1, &tmp64, sizeof(tmp64)); + memcpy((void *)(option_content + 1), &tmp64, sizeof(tmp64)); } break; @@ -1570,11 +1543,10 @@ pcapng_process_packet_block_option(wtapng_block_t *wblock, /* XXX - free anything? */ return FALSE; } - option_content_copy = g_memdup2(option_content, option_length); if (section_info->byte_swapped) { - memcpy(&tmp64, option_content_copy + 1, sizeof(tmp64)); + memcpy(&tmp64, option_content + 1, sizeof(tmp64)); tmp64 = GUINT64_SWAP_LE_BE(tmp64); - memcpy(option_content_copy + 1, &tmp64, sizeof(tmp64)); + memcpy((void*)(option_content + 1), &tmp64, sizeof(tmp64)); } break; @@ -1582,16 +1554,7 @@ pcapng_process_packet_block_option(wtapng_block_t *wblock, /* Silently ignore unknown verdict types */ return TRUE; } - if (wblock->rec->packet_verdict == NULL) { - wblock->rec->presence_flags |= WTAP_HAS_VERDICT; - wblock->rec->packet_verdict = g_ptr_array_new_with_free_func((GDestroyNotify) g_bytes_unref); - } - - g_ptr_array_add(wblock->rec->packet_verdict, - g_bytes_new_with_free_func(option_content_copy, - option_length, - g_free, - option_content_copy)); + pcapng_process_bytes_option(wblock, option_code, option_length, (void*)option_content); ws_debug("verdict type %u, data len %u", option_content[0], option_length - 1); break; @@ -1619,8 +1582,8 @@ pcapng_read_packet_block(FILE_T fh, pcapng_block_header_t *bh, guint64 ts; int pseudo_header_len; int fcslen; - wtap_option_t option; - guint i; + + wblock->block = wtap_block_create(WTAP_BLOCK_PACKET); /* "(Enhanced) Packet Block" read fixed part */ if (enhanced) { @@ -1800,25 +1763,10 @@ pcapng_read_packet_block(FILE_T fh, pcapng_block_header_t *bh, } /* Option defaults */ - g_free(wblock->rec->opt_comment); /* Free memory from an earlier read. */ - wblock->rec->opt_comment = NULL; wblock->rec->rec_header.packet_header.drop_count = -1; wblock->rec->rec_header.packet_header.pack_flags = 0; wblock->rec->rec_header.packet_header.packet_id = 0; wblock->rec->rec_header.packet_header.interface_queue = 0; - if (wblock->rec->packet_verdict != NULL) { - g_ptr_array_free(wblock->rec->packet_verdict, TRUE); - wblock->rec->packet_verdict = NULL; - } - if (wblock->rec->custom_options != NULL) { - for (i = 0; i < wblock->rec->custom_options->len; i++) { - option = g_array_index(wblock->rec->custom_options, wtap_option_t, i); - g_free(option.value.custom_opt.custom_data); - option.value.custom_opt.custom_data = NULL; - } - g_array_free(wblock->rec->custom_options, TRUE); - wblock->rec->custom_options = NULL; - } /* FCS length default */ fcslen = iface_info.fcslen; @@ -1852,6 +1800,15 @@ pcapng_read_packet_block(FILE_T fh, pcapng_block_header_t *bh, */ wblock->internal = FALSE; + /* + * We want dissectors (particularly packet_frame) to be able to + * access packet comments and whatnot that are in the block. wblock->block + * will be unref'd by pcapng_seek_read(), so move the block to where + * dissectors can find it. + */ + wblock->rec->block = wblock->block; + wblock->block = NULL; + return TRUE; } @@ -1867,8 +1824,6 @@ pcapng_read_simple_packet_block(FILE_T fh, pcapng_block_header_t *bh, wtapng_simple_packet_t simple_packet; guint32 padding; int pseudo_header_len; - wtap_option_t option; - guint i; /* * Is this block long enough to be an SPB? @@ -1958,25 +1913,10 @@ pcapng_read_simple_packet_block(FILE_T fh, pcapng_block_header_t *bh, wblock->rec->ts.secs = 0; wblock->rec->ts.nsecs = 0; wblock->rec->rec_header.packet_header.interface_id = 0; - g_free(wblock->rec->opt_comment); /* Free memory from an earlier read. */ - wblock->rec->opt_comment = NULL; wblock->rec->rec_header.packet_header.drop_count = 0; wblock->rec->rec_header.packet_header.pack_flags = 0; wblock->rec->rec_header.packet_header.packet_id = 0; wblock->rec->rec_header.packet_header.interface_queue = 0; - if (wblock->rec->packet_verdict != NULL) { - g_ptr_array_free(wblock->rec->packet_verdict, TRUE); - wblock->rec->packet_verdict = NULL; - } - if (wblock->rec->custom_options != NULL) { - for (i = 0; i < wblock->rec->custom_options->len; i++) { - option = g_array_index(wblock->rec->custom_options, wtap_option_t, i); - g_free(option.value.custom_opt.custom_data); - option.value.custom_opt.custom_data = NULL; - } - g_array_free(wblock->rec->custom_options, TRUE); - wblock->rec->custom_options = NULL; - } memset((void *)&wblock->rec->rec_header.packet_header.pseudo_header, 0, sizeof(union wtap_pseudo_header)); pseudo_header_len = pcap_process_pseudo_header(fh, @@ -3136,14 +3076,14 @@ pcapng_open(wtap *wth, int *err, gchar **err_info) case PCAPNG_BLOCK_NOT_SHB: /* This doesn't look like an SHB, so this isn't a pcapng file. */ - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); *err = 0; g_free(*err_info); *err_info = NULL; return WTAP_OPEN_NOT_MINE; case PCAPNG_BLOCK_ERROR: - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); if (*err == WTAP_ERR_SHORT_READ) { /* * Short read. @@ -3165,7 +3105,7 @@ pcapng_open(wtap *wth, int *err, gchar **err_info) */ if (!pcapng_read_and_check_block_trailer(wth->fh, &bh, &first_section, err, err_info)) { /* Not readable or not valid. */ - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); return WTAP_OPEN_ERROR; } @@ -3178,7 +3118,7 @@ pcapng_open(wtap *wth, int *err, gchar **err_info) * SHBs for this file. */ wtap_block_copy(g_array_index(wth->shb_hdrs, wtap_block_t, 0), wblock.block); - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); wblock.block = NULL; wth->file_encap = WTAP_ENCAP_UNKNOWN; @@ -3264,7 +3204,7 @@ pcapng_open(wtap *wth, int *err, gchar **err_info) if (!pcapng_read_block(wth, wth->fh, pcapng, current_section, &new_section, &wblock, err, err_info)) { - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); if (*err == 0) { ws_debug("No more IDBs available..."); break; @@ -3274,7 +3214,7 @@ pcapng_open(wtap *wth, int *err, gchar **err_info) } } pcapng_process_idb(wth, current_section, &wblock); - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); ws_debug("Read IDB number_of_interfaces %u, wtap_encap %i", wth->interface_data->len, wth->file_encap); } @@ -3319,7 +3259,7 @@ pcapng_read(wtap *wth, wtap_rec *rec, Buffer *buf, int *err, &new_section, &wblock, err, err_info)) { ws_debug("data_offset is finally %" G_GINT64_MODIFIER "d", *data_offset); ws_debug("couldn't read packet block"); - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); return FALSE; } @@ -3360,7 +3300,7 @@ pcapng_read(wtap *wth, wtap_rec *rec, Buffer *buf, int *err, /* A new interface */ ws_debug("block type BLOCK_TYPE_IDB"); pcapng_process_idb(wth, current_section, &wblock); - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); break; case(BLOCK_TYPE_DSB): @@ -3425,7 +3365,7 @@ pcapng_read(wtap *wth, wtap_rec *rec, Buffer *buf, int *err, g_array_append_val(wtapng_if_descr_mand->interface_statistics, if_stats); wtapng_if_descr_mand->num_stat_entries++; } - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); break; default: @@ -3497,7 +3437,7 @@ pcapng_seek_read(wtap *wth, gint64 seek_off, if (!pcapng_read_block(wth, wth->random_fh, pcapng, section_info, &new_section, &wblock, err, err_info)) { ws_debug("couldn't read packet block (err=%d).", *err); - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); return FALSE; } @@ -3505,11 +3445,11 @@ pcapng_seek_read(wtap *wth, gint64 seek_off, if (wblock.internal) { ws_debug("block type 0x%08x is not one we return", wblock.type); - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); return FALSE; } - wtap_block_free(wblock.block); + wtap_block_unref(wblock.block); return TRUE; } @@ -3557,6 +3497,24 @@ static guint32 pcapng_compute_string_option_size(wtap_optval_t *optval) return size; } +#if 0 +static guint32 pcapng_compute_bytes_option_size(wtap_optval_t *optval) +{ + guint32 size = 0, pad; + + size = (guint32)g_bytes_get_size(optval->byteval) & 0xffff; + if ((size % 4)) { + pad = 4 - (size % 4); + } else { + pad = 0; + } + + size += pad; + + return size; +} +#endif + static guint32 pcapng_compute_if_filter_option_size(wtap_optval_t *optval) { if_filter_opt_t* filter = &optval->if_filterval; @@ -3741,6 +3699,40 @@ static gboolean pcapng_write_uint8_option(wtap_dumper *wdh, guint option_id, wta return TRUE; } +static gboolean pcapng_write_uint32_option(wtap_dumper *wdh, guint option_id, wtap_optval_t *optval, int *err) +{ + struct pcapng_option_header option_hdr; + + option_hdr.type = (guint16)option_id; + option_hdr.value_length = (guint16)4; + if (!wtap_dump_file_write(wdh, &option_hdr, 4, err)) + return FALSE; + wdh->bytes_dumped += 4; + + if (!wtap_dump_file_write(wdh, &optval->uint32val, 1, err)) + return FALSE; + wdh->bytes_dumped += 4; + + return TRUE; +} + +static gboolean pcapng_write_ipv4_option(wtap_dumper *wdh, guint option_id, wtap_optval_t *optval, int *err) +{ + struct pcapng_option_header option_hdr; + + option_hdr.type = (guint16)option_id; + option_hdr.value_length = (guint16)4; + if (!wtap_dump_file_write(wdh, &option_hdr, 4, err)) + return FALSE; + wdh->bytes_dumped += 4; + + if (!wtap_dump_file_write(wdh, &optval->ipv4val, 1, err)) + return FALSE; + wdh->bytes_dumped += 4; + + return TRUE; +} + static gboolean pcapng_write_uint64_option(wtap_dumper *wdh, guint option_id, wtap_optval_t *optval, int *err) { struct pcapng_option_header option_hdr; @@ -3758,6 +3750,23 @@ static gboolean pcapng_write_uint64_option(wtap_dumper *wdh, guint option_id, wt return TRUE; } +static gboolean pcapng_write_ipv6_option(wtap_dumper *wdh, guint option_id, wtap_optval_t *optval, int *err) +{ + struct pcapng_option_header option_hdr; + + option_hdr.type = (guint16)option_id; + option_hdr.value_length = (guint16)IPv6_ADDR_SIZE; + if (!wtap_dump_file_write(wdh, &option_hdr, 4, err)) + return FALSE; + wdh->bytes_dumped += 4; + + if (!wtap_dump_file_write(wdh, &optval->ipv6val.bytes, IPv6_ADDR_SIZE, err)) + return FALSE; + wdh->bytes_dumped += IPv6_ADDR_SIZE; + + return TRUE; +} + static gboolean pcapng_write_timestamp_option(wtap_dumper *wdh, guint option_id, wtap_optval_t *optval, int *err) { struct pcapng_option_header option_hdr; @@ -3828,6 +3837,53 @@ static gboolean pcapng_write_string_option(wtap_dumper *wdh, guint option_id, wt return TRUE; } +static gboolean pcapng_write_bytes_option(wtap_dumper *wdh, guint option_id, wtap_optval_t *optval, int *err) +{ + struct pcapng_option_header option_hdr; + size_t size = g_bytes_get_size(optval->byteval); + const guint32 zero_pad = 0; + guint32 pad; + + if (size == 0) + return TRUE; + if (size > 65535) { + /* + * Too big to fit in the option. + * Don't write anything. + * + * XXX - truncate it? Report an error? + */ + return TRUE; + } + + /* Bytes options don't consider pad bytes part of the length */ + option_hdr.type = (guint16)option_id; + option_hdr.value_length = (guint16)size; + if (!wtap_dump_file_write(wdh, &option_hdr, 4, err)) + return FALSE; + wdh->bytes_dumped += 4; + + if (!wtap_dump_file_write(wdh, optval->stringval, size, err)) + return FALSE; + wdh->bytes_dumped += size; + + if ((size % 4)) { + pad = 4 - (size % 4); + } else { + pad = 0; + } + + /* write padding (if any) */ + if (pad != 0) { + if (!wtap_dump_file_write(wdh, &zero_pad, pad, err)) + return FALSE; + + wdh->bytes_dumped += pad; + } + + return TRUE; +} + static gboolean pcapng_write_if_filter_option(wtap_dumper *wdh, guint option_id, wtap_optval_t *optval, int *err) { if_filter_opt_t* filter = &optval->if_filterval; @@ -4109,6 +4165,68 @@ pcapng_write_section_header_block(wtap_dumper *wdh, int *err) return TRUE; } +typedef struct pcapng_write_block_s +{ + wtap_dumper *wdh; + int *err; +} +pcapng_write_block_t; +/* Helper function used in pcapng_write_enhanced_packet_block(). + * Meant to be generic enough to be used elsewhere too, but currently, + * only WTAP_OPTTYPE_STRING and WTAP_OPTTYPE_BYTES are exercised (and thus tested). + * This is a wtap_block_foreach_func. + */ +static gboolean +pcapng_write_option_cb(wtap_block_t block _U_, guint option_id _U_, wtap_opttype_e option_type, wtap_optval_t *option, void *user_data) +{ + pcapng_write_block_t* write_block = (pcapng_write_block_t*)user_data; + gboolean ok = TRUE; + + /* The functions below write their option type and length fields, and any needed padding. */ + switch(option_type) + { + + case WTAP_OPTTYPE_UINT8: + ok = pcapng_write_uint8_option(write_block->wdh, option_id, option, write_block->err); + break; + + case WTAP_OPTTYPE_UINT32: + ok = pcapng_write_uint32_option(write_block->wdh, option_id, option, write_block->err); + break; + + case WTAP_OPTTYPE_UINT64: + ok = pcapng_write_uint64_option(write_block->wdh, option_id, option, write_block->err); + break; + + case WTAP_OPTTYPE_IPv4: + // wtap_opttypes.h implies this is stored in network byte order + ok = pcapng_write_ipv4_option(write_block->wdh, option_id, option, write_block->err); + break; + + case WTAP_OPTTYPE_IPv6: + ok = pcapng_write_ipv6_option(write_block->wdh, option_id, option, write_block->err); + break; + + case WTAP_OPTTYPE_STRING: + ok = pcapng_write_string_option(write_block->wdh, option_id, option, write_block->err); + break; + + case WTAP_OPTTYPE_BYTES: + ok = pcapng_write_bytes_option(write_block->wdh, option_id, option, write_block->err); + break; + + case WTAP_OPTTYPE_IF_FILTER: + ok = pcapng_write_if_filter_option(write_block->wdh, option_id, option, write_block->err); + break; + + case WTAP_OPTTYPE_CUSTOM: + ok = pcapng_write_custom_option(write_block->wdh, option_id, option, write_block->err); + break; + } + + return ok; +} + static gboolean pcapng_write_enhanced_packet_block(wtap_dumper *wdh, const wtap_rec *rec, const guint8 *pd, int *err, gchar **err_info) @@ -4123,9 +4241,13 @@ pcapng_write_enhanced_packet_block(wtap_dumper *wdh, const wtap_rec *rec, gboolean have_options = FALSE; guint32 options_total_length = 0; struct option option_hdr; - guint32 comment_len = 0, comment_pad_len = 0; + gsize options_len = 0; wtap_block_t int_data; wtapng_if_descr_mandatory_t *int_data_mand; + pcapng_write_block_t block_data; + + block_data.wdh = wdh; + block_data.err = err; /* Don't write anything we're not willing to read. */ if (rec->rec_header.packet_header.caplen > wtap_max_snaplen_for_encap(wdh->encap)) { @@ -4139,30 +4261,17 @@ pcapng_write_enhanced_packet_block(wtap_dumper *wdh, const wtap_rec *rec, } else { pad_len = 0; } - - /* Check if we should write comment option */ - if (rec->opt_comment) { - have_options = TRUE; - comment_len = (guint32)strlen(rec->opt_comment) & 0xffff; - if ((comment_len % 4)) { - comment_pad_len = 4 - (comment_len % 4); - } else { - comment_pad_len = 0; - } - options_total_length = options_total_length + comment_len + comment_pad_len + 4 /* comment options tag */ ; - } - if (rec->custom_options != NULL) { - have_options = TRUE; - for (guint i = 0; i < rec->custom_options->len; i++) { - wtap_option_t option; - - option = g_array_index(rec->custom_options, wtap_option_t, i); - if ((option.option_id == OPT_CUSTOM_STR_COPY) || - (option.option_id == OPT_CUSTOM_BIN_COPY)) { - options_total_length += pcapng_compute_custom_option_size(&option.value) + 4; - } + if (rec->block != NULL) { + // Current options expected to be here: comments, verdicts, custom. + // Remember to also add newly-supported option types to packet_block_options_supported + // below. + options_len = wtap_block_get_options_size_padded(rec->block); + if (options_len > 0) { + have_options = TRUE; + options_total_length += (guint32)options_len; } } + if (rec->presence_flags & WTAP_HAS_PACK_FLAGS) { have_options = TRUE; options_total_length = options_total_length + 8; @@ -4179,17 +4288,6 @@ pcapng_write_enhanced_packet_block(wtap_dumper *wdh, const wtap_rec *rec, have_options = TRUE; options_total_length = options_total_length + 8; } - if (rec->presence_flags & WTAP_HAS_VERDICT && rec->packet_verdict != NULL) { - - for(guint i = 0; i < rec->packet_verdict->len; i++) { - gsize len; - GBytes *verdict = (GBytes *) g_ptr_array_index(rec->packet_verdict, i); - - if (g_bytes_get_data(verdict, &len) && len != 0) - options_total_length += ROUND_TO_4BYTE(4 + len); - } - have_options = TRUE; - } if (have_options) { /* End-of options tag */ options_total_length += 4; @@ -4323,40 +4421,8 @@ pcapng_write_enhanced_packet_block(wtap_dumper *wdh, const wtap_rec *rec, * defined in the Linux pbf.h include). * opt_endofopt 0 0 It delimits the end of the optional fields. This block cannot be repeated within a given list of options. */ - if (rec->opt_comment) { - option_hdr.type = OPT_COMMENT; - option_hdr.value_length = comment_len; - if (!wtap_dump_file_write(wdh, &option_hdr, 4, err)) - return FALSE; - wdh->bytes_dumped += 4; - - /* Write the comments string */ - ws_debug("comment:'%s' comment_len %u comment_pad_len %u", - rec->opt_comment, comment_len, comment_pad_len); - if (!wtap_dump_file_write(wdh, rec->opt_comment, comment_len, err)) - return FALSE; - wdh->bytes_dumped += comment_len; - - /* write padding (if any) */ - if (comment_pad_len != 0) { - if (!wtap_dump_file_write(wdh, &zero_pad, comment_pad_len, err)) - return FALSE; - wdh->bytes_dumped += comment_pad_len; - } - - ws_debug("Wrote Options comments: comment_len %u, comment_pad_len %u", - comment_len, comment_pad_len); - } - if (rec->custom_options != NULL) { - for (guint i = 0; i < rec->custom_options->len; i++) { - wtap_option_t option; - - option = g_array_index(rec->custom_options, wtap_option_t, i); - if ((option.option_id == OPT_CUSTOM_STR_COPY) || - (option.option_id == OPT_CUSTOM_BIN_COPY)) { - pcapng_write_custom_option(wdh, option.option_id, &option.value, err); - } - } + if (!wtap_block_foreach_option(rec->block, pcapng_write_option_cb, &block_data)) { + return FALSE; } if (rec->presence_flags & WTAP_HAS_PACK_FLAGS) { option_hdr.type = OPT_EPB_FLAGS; @@ -4406,36 +4472,6 @@ pcapng_write_enhanced_packet_block(wtap_dumper *wdh, const wtap_rec *rec, ws_debug("Wrote Options queue: %u", rec->rec_header.packet_header.interface_queue); } - if (rec->presence_flags & WTAP_HAS_VERDICT && rec->packet_verdict != NULL) { - - for(guint i = 0; i < rec->packet_verdict->len; i++) { - gsize len; - GBytes *verdict = (GBytes *) g_ptr_array_index(rec->packet_verdict, i); - const guint8 *verdict_data = (const guint8 *) g_bytes_get_data(verdict, &len); - - if (verdict_data && len != 0) { - - option_hdr.type = OPT_EPB_VERDICT; - option_hdr.value_length = (guint16) len; - if (!wtap_dump_file_write(wdh, &option_hdr, 4, err)) - return FALSE; - wdh->bytes_dumped += 4; - if (!wtap_dump_file_write(wdh, verdict_data, len, err)) - return FALSE; - wdh->bytes_dumped += len; - - if (ROUND_TO_4BYTE(len) != len) { - size_t plen = ROUND_TO_4BYTE(len) - len; - - if (!wtap_dump_file_write(wdh, &zero_pad, plen, err)) - return FALSE; - wdh->bytes_dumped += plen; - } - ws_debug("Wrote Options verdict: %u", - verdict_data[0]); - } - } - } /* Write end of options if we have options */ if (have_options) { if (!pcapng_write_option_eofopt(wdh, err)) @@ -5668,8 +5704,8 @@ static const struct supported_option_type decryption_secrets_block_options_suppo /* Options for packet blocks. */ static const struct supported_option_type packet_block_options_supported[] = { - /* XXX - pending use of wtap_block_t's for packets */ { OPT_COMMENT, MULTIPLE_OPTIONS_SUPPORTED }, + { OPT_EPB_VERDICT, MULTIPLE_OPTIONS_SUPPORTED }, { OPT_CUSTOM_STR_COPY, MULTIPLE_OPTIONS_SUPPORTED }, { OPT_CUSTOM_BIN_COPY, MULTIPLE_OPTIONS_SUPPORTED }, { OPT_CUSTOM_STR_NO_COPY, MULTIPLE_OPTIONS_SUPPORTED }, diff --git a/wiretap/wtap.c b/wiretap/wtap.c index e77525c182..b69f1ac47e 100644 --- a/wiretap/wtap.c +++ b/wiretap/wtap.c @@ -1499,6 +1499,7 @@ wtapng_process_dsb(wtap *wth, wtap_block_t dsb) wth->add_new_secrets(dsb_mand->secrets_type, dsb_mand->secrets_data, dsb_mand->secrets_len); } +/* Perform per-packet initialization */ static void wtap_init_rec(wtap *wth, wtap_rec *rec) { @@ -1514,6 +1515,8 @@ wtap_init_rec(wtap *wth, wtap_rec *rec) */ rec->rec_header.packet_header.pkt_encap = wth->file_encap; rec->tsprec = wth->file_tsprec; + rec->block = NULL; + rec->has_block_changed = FALSE; } gboolean @@ -1654,35 +1657,26 @@ wtap_read_so_far(wtap *wth) return file_tell_raw(wth->fh); } +/* Perform global/initial initialization */ void wtap_rec_init(wtap_rec *rec) { memset(rec, 0, sizeof *rec); ws_buffer_init(&rec->options_buf, 0); + /* In the future, see if we can create rec->block here once + * and have it be reused like the rest of rec. + * Currently it's recreated for each packet. + */ } +/* clean up record metadata */ void wtap_rec_cleanup(wtap_rec *rec) { - wtap_option_t option; - guint i; - - g_free(rec->opt_comment); - rec->opt_comment = NULL; + wtap_block_unref(rec->block); + rec->block = NULL; + rec->has_block_changed = FALSE; ws_buffer_free(&rec->options_buf); - if (rec->packet_verdict != NULL) { - g_ptr_array_free(rec->packet_verdict, TRUE); - rec->packet_verdict = NULL; - } - if (rec->custom_options != NULL) { - for (i = 0; i < rec->custom_options->len; i++) { - option = g_array_index(rec->custom_options, wtap_option_t, i); - g_free(option.value.custom_opt.custom_data); - option.value.custom_opt.custom_data = NULL; - } - g_array_free(rec->custom_options, TRUE); - rec->custom_options = NULL; - } } gboolean diff --git a/wiretap/wtap.h b/wiretap/wtap.h index fd0e548ab8..0eff12953a 100644 --- a/wiretap/wtap.h +++ b/wiretap/wtap.h @@ -1342,17 +1342,10 @@ typedef struct { wtap_systemd_journal_export_header systemd_journal_export_header; wtap_custom_block_header custom_block_header; } rec_header; - /* - * XXX - this should become a full set of options. - */ - gchar *opt_comment; /* NULL if not available */ - gboolean has_comment_changed; /* TRUE if the comment has been changed. Currently only valid while dumping. */ - GPtrArray *packet_verdict; /* packet verdicts. It would have made more - sense to put this in packet_header above - but due to the way the current code is - reusing the wtap_rec structure, it's - impossible to nicely clean it up. */ - GArray *custom_options; /* Array of generic custom options of EPBs */ + + wtap_block_t block; /* packet block; holds comments and verdicts in its options */ + gboolean has_block_changed; /* TRUE if ANY aspect of the block has changed */ + /* * We use a Buffer so that we don't have to allocate and free * a buffer for the options for each record. @@ -1385,12 +1378,10 @@ typedef struct { #define WTAP_HAS_TS 0x00000001 /**< time stamp */ #define WTAP_HAS_CAP_LEN 0x00000002 /**< captured length separate from on-the-network length */ #define WTAP_HAS_INTERFACE_ID 0x00000004 /**< interface ID */ -#define WTAP_HAS_COMMENTS 0x00000008 /**< comments */ #define WTAP_HAS_DROP_COUNT 0x00000010 /**< drop count */ #define WTAP_HAS_PACK_FLAGS 0x00000020 /**< packet flags */ #define WTAP_HAS_PACKET_ID 0x00000040 /**< packet id */ #define WTAP_HAS_INT_QUEUE 0x00000080 /**< interface queue */ -#define WTAP_HAS_VERDICT 0x00000100 /**< packet verdict */ #ifndef MAXNAMELEN #define MAXNAMELEN 64 /* max name length (hostname and port name) */ diff --git a/wiretap/wtap_opttypes.c b/wiretap/wtap_opttypes.c index 307ae133bb..84b6c21ac5 100644 --- a/wiretap/wtap_opttypes.c +++ b/wiretap/wtap_opttypes.c @@ -18,6 +18,16 @@ #include #include +#include + +#if 0 +#define wtap_debug(...) g_warning(__VA_ARGS__) +#define DEBUG_COUNT_REFS +#else +#define wtap_debug(...) +#endif + +#define ROUND_TO_4BYTE(len) (((len) + 3) & ~3) /* * Structure describing a type of block. @@ -48,11 +58,36 @@ typedef struct { /* Flags */ #define WTAP_OPTTYPE_FLAG_MULTIPLE_ALLOWED 0x00000001 /* multiple instances allowed */ +/* Debugging reference counting */ +#ifdef DEBUG_COUNT_REFS +static guint block_count = 0; +static guint8 blocks_active[sizeof(guint)/8]; + +static void rc_set(guint refnum) +{ + guint cellno = refnum / 8; + guint bitno = refnum % 8; + blocks_active[cellno] |= (guint8)(1 << bitno); +} + +static void rc_clear(guint refnum) +{ + guint cellno = refnum / 8; + guint bitno = refnum % 8; + blocks_active[cellno] &= (guint8)~(1 << bitno); +} + +#endif /* DEBUG_COUNT_REFS */ + struct wtap_block { wtap_blocktype_t* info; void* mandatory_data; GArray* options; + gint ref_count; +#ifdef DEBUG_COUNT_REFS + guint id; +#endif }; /* Keep track of wtap_blocktype_t's via their id number */ @@ -182,6 +217,10 @@ wtap_block_get_option(wtap_block_t block, guint option_id) guint i; wtap_option_t *opt; + if (block == NULL) { + return NULL; + } + for (i = 0; i < block->options->len; i++) { opt = &g_array_index(block->options, wtap_option_t, i); if (opt->option_id == option_id) @@ -198,6 +237,10 @@ wtap_block_get_nth_option(wtap_block_t block, guint option_id, guint idx) wtap_option_t *opt; guint opt_idx; + if (block == NULL) { + return NULL; + } + opt_idx = 0; for (i = 0; i < block->options->len; i++) { opt = &g_array_index(block->options, wtap_option_t, i); @@ -222,6 +265,12 @@ wtap_block_t wtap_block_create(wtap_block_type_t block_type) block->info = blocktype_list[block_type]; block->options = g_array_new(FALSE, FALSE, sizeof(wtap_option_t)); block->info->create(block); + block->ref_count = 1; +#ifdef DEBUG_COUNT_REFS + block->id = block_count++; + rc_set(block->id); + wtap_debug("Created #%d %s", block->id, block->info->name); +#endif /* DEBUG_COUNT_REFS */ return block; } @@ -230,6 +279,10 @@ static void wtap_block_free_option(wtap_block_t block, wtap_option_t *opt) { const wtap_opttype_t *opttype; + if (block == NULL) { + return; + } + opttype = GET_OPTION_TYPE(block->info->options, opt->option_id); switch (opttype->data_type) { @@ -237,6 +290,10 @@ static void wtap_block_free_option(wtap_block_t block, wtap_option_t *opt) g_free(opt->value.stringval); break; + case WTAP_OPTTYPE_BYTES: + g_bytes_unref(opt->value.byteval); + break; + case WTAP_OPTTYPE_IF_FILTER: if_filter_free(&opt->value.if_filterval); break; @@ -255,23 +312,52 @@ static void wtap_block_free_options(wtap_block_t block) guint i; wtap_option_t *opt; + if (block == NULL || block->options == NULL) { + return; + } + for (i = 0; i < block->options->len; i++) { opt = &g_array_index(block->options, wtap_option_t, i); wtap_block_free_option(block, opt); } + g_array_remove_range(block->options, 0, block->options->len); } -void wtap_block_free(wtap_block_t block) +wtap_block_t wtap_block_ref(wtap_block_t block) +{ + if (block == NULL) { + return NULL; + } + + g_atomic_int_inc(&block->ref_count); +#ifdef DEBUG_COUNT_REFS + wtap_debug("Ref #%d %s", block->id, block->info->name); +#endif /* DEBUG_COUNT_REFS */ + return block; +} + +void wtap_block_unref(wtap_block_t block) { if (block != NULL) { - if (block->info->free_mand != NULL) - block->info->free_mand(block); + if (g_atomic_int_dec_and_test(&block->ref_count)) { +#ifdef DEBUG_COUNT_REFS + wtap_debug("Destroy #%d %s", block->id, block->info->name); + rc_clear(block->id); +#endif /* DEBUG_COUNT_REFS */ + if (block->info->free_mand != NULL) + block->info->free_mand(block); - g_free(block->mandatory_data); - wtap_block_free_options(block); - g_array_free(block->options, TRUE); - g_free(block); + g_free(block->mandatory_data); + wtap_block_free_options(block); + g_array_free(block->options, TRUE); + g_free(block); + } +#ifdef DEBUG_COUNT_REFS + else { + wtap_debug("Unref #%d %s", block->id, block->info->name); + } +#endif /* DEBUG_COUNT_REFS */ } } @@ -283,7 +369,7 @@ void wtap_block_array_free(GArray* block_array) return; for (block = 0; block < block_array->len; block++) { - wtap_block_free(g_array_index(block_array, wtap_block_t, block)); + wtap_block_unref(g_array_index(block_array, wtap_block_t, block)); } g_array_free(block_array, TRUE); } @@ -318,6 +404,10 @@ wtap_block_copy(wtap_block_t dest_block, wtap_block_t src_block) wtap_block_add_uint8_option(dest_block, src_opt->option_id, src_opt->value.uint8val); break; + case WTAP_OPTTYPE_UINT32: + wtap_block_add_uint32_option(dest_block, src_opt->option_id, src_opt->value.uint32val); + break; + case WTAP_OPTTYPE_UINT64: wtap_block_add_uint64_option(dest_block, src_opt->option_id, src_opt->value.uint64val); break; @@ -334,6 +424,10 @@ wtap_block_copy(wtap_block_t dest_block, wtap_block_t src_block) wtap_block_add_string_option(dest_block, src_opt->option_id, src_opt->value.stringval, strlen(src_opt->value.stringval)); break; + case WTAP_OPTTYPE_BYTES: + wtap_block_add_bytes_option_borrow(dest_block, src_opt->option_id, src_opt->value.byteval); + break; + case WTAP_OPTTYPE_IF_FILTER: wtap_block_add_if_filter_option(dest_block, src_opt->option_id, &src_opt->value.if_filterval); break; @@ -354,12 +448,129 @@ wtap_block_t wtap_block_make_copy(wtap_block_t block) return block_copy; } +/* + * Get the (un-padded) size of the given option value, by its type. + */ +gsize +wtap_block_option_get_value_size(wtap_opttype_e option_type, wtap_optval_t *option) +{ + gsize ret_val = 0; + + switch(option_type) { + + case WTAP_OPTTYPE_UINT8: + ret_val += 1; + break; + + case WTAP_OPTTYPE_UINT32: + ret_val += 4; + break; + + case WTAP_OPTTYPE_UINT64: + ret_val += 8; + break; + + case WTAP_OPTTYPE_IPv4: + ret_val += 4; + break; + + case WTAP_OPTTYPE_IPv6: + ret_val += IPv6_ADDR_SIZE; + break; + + case WTAP_OPTTYPE_STRING: + ret_val += strlen(option->stringval); + break; + + case WTAP_OPTTYPE_BYTES: + ret_val += g_bytes_get_size(option->byteval); + break; + + case WTAP_OPTTYPE_IF_FILTER: + switch(option->if_filterval.type) { + + case if_filter_pcap: + ret_val += 1 + strlen(option->if_filterval.data.filter_str); + break; + + case if_filter_bpf: + ret_val += 1 + (8 * option->if_filterval.data.bpf_prog.bpf_prog_len); + break; + } + break; + + case WTAP_OPTTYPE_CUSTOM: + ret_val += sizeof(guint32) + option->custom_opt.custom_data_len; + break; + } + return ret_val; +} + +/* + * Get the size of all options, padded to a 32-bit boundary. + */ +gsize +wtap_block_get_options_size_padded(wtap_block_t block) +{ + gsize ret_val = 0; + gsize opt_size = 0; + guint i; + wtap_option_t *opt; + const wtap_opttype_t *opttype; + + if (block == NULL) { + return 0; + } + + for (i = 0; i < block->options->len; i++) + { + + opt = &g_array_index(block->options, wtap_option_t, i); + opttype = GET_OPTION_TYPE(block->info->options, opt->option_id); + + opt_size = ROUND_TO_4BYTE(wtap_block_option_get_value_size(opttype->data_type, &opt->value)); + + /* pcapng.c silently skips over data that's too big to fit in an option. + * Reflect that when calculating the size of our options. + */ + if (opt_size <= 0xffff) { + ret_val += opt_size + 4; // 4 for size of type and length fields themselves + } + } + return ret_val; +} + +guint +wtap_block_count_option(wtap_block_t block, guint option_id) +{ + guint i; + guint ret_val = 0; + wtap_option_t *opt; + + if (block == NULL) { + return 0; + } + + for (i = 0; i < block->options->len; i++) { + opt = &g_array_index(block->options, wtap_option_t, i); + if (opt->option_id == option_id) + ret_val++; + } + + return ret_val; +} + + gboolean wtap_block_foreach_option(wtap_block_t block, wtap_block_foreach_func func, void* user_data) { guint i; wtap_option_t *opt; const wtap_opttype_t *opttype; + if (block == NULL) { + return TRUE; + } + for (i = 0; i < block->options->len; i++) { opt = &g_array_index(block->options, wtap_option_t, i); opttype = GET_OPTION_TYPE(block->info->options, opt->option_id); @@ -376,6 +587,10 @@ wtap_block_add_option_common(wtap_block_t block, guint option_id, wtap_opttype_e const wtap_opttype_t *opttype; guint i; + if (block == NULL) { + return WTAP_OPTTYPE_BAD_BLOCK; + } + opttype = GET_OPTION_TYPE(block->info->options, option_id); if (opttype == NULL) { /* There's no option for this block with that option ID */ @@ -424,6 +639,10 @@ wtap_block_get_option_common(wtap_block_t block, guint option_id, wtap_opttype_e const wtap_opttype_t *opttype; wtap_optval_t *optval; + if (block == NULL) { + return WTAP_OPTTYPE_BAD_BLOCK; + } + opttype = GET_OPTION_TYPE(block->info->options, option_id); if (opttype == NULL) { /* There's no option for this block with that option ID */ @@ -466,6 +685,10 @@ wtap_block_get_nth_option_common(wtap_block_t block, guint option_id, wtap_optty const wtap_opttype_t *opttype; wtap_optval_t *optval; + if (block == NULL) { + return WTAP_OPTTYPE_BAD_BLOCK; + } + opttype = GET_OPTION_TYPE(block->info->options, option_id); if (opttype == NULL) { /* There's no option for this block with that option ID */ @@ -541,6 +764,45 @@ wtap_block_get_uint8_option_value(wtap_block_t block, guint option_id, guint8* v return WTAP_OPTTYPE_SUCCESS; } +wtap_opttype_return_val +wtap_block_add_uint32_option(wtap_block_t block, guint option_id, guint32 value) +{ + wtap_opttype_return_val ret; + wtap_option_t *opt; + + ret = wtap_block_add_option_common(block, option_id, WTAP_OPTTYPE_UINT32, &opt); + if (ret != WTAP_OPTTYPE_SUCCESS) + return ret; + opt->value.uint32val = value; + return WTAP_OPTTYPE_SUCCESS; +} + +wtap_opttype_return_val +wtap_block_set_uint32_option_value(wtap_block_t block, guint option_id, guint32 value) +{ + wtap_opttype_return_val ret; + wtap_optval_t *optval; + + ret = wtap_block_get_option_common(block, option_id, WTAP_OPTTYPE_UINT32, &optval); + if (ret != WTAP_OPTTYPE_SUCCESS) + return ret; + optval->uint32val = value; + return WTAP_OPTTYPE_SUCCESS; +} + +wtap_opttype_return_val +wtap_block_get_uint32_option_value(wtap_block_t block, guint option_id, guint32* value) +{ + wtap_opttype_return_val ret; + wtap_optval_t *optval; + + ret = wtap_block_get_option_common(block, option_id, WTAP_OPTTYPE_UINT32, &optval); + if (ret != WTAP_OPTTYPE_SUCCESS) + return ret; + *value = optval->uint32val; + return WTAP_OPTTYPE_SUCCESS; +} + wtap_opttype_return_val wtap_block_add_uint64_option(wtap_block_t block, guint option_id, guint64 value) { @@ -809,6 +1071,95 @@ wtap_block_get_nth_string_option_value(wtap_block_t block, guint option_id, guin return WTAP_OPTTYPE_SUCCESS; } +wtap_opttype_return_val +wtap_block_add_bytes_option(wtap_block_t block, guint option_id, const guint8 *value, gsize value_length) +{ + wtap_opttype_return_val ret; + wtap_option_t *opt; + + ret = wtap_block_add_option_common(block, option_id, WTAP_OPTTYPE_BYTES, &opt); + if (ret != WTAP_OPTTYPE_SUCCESS) + return ret; + opt->value.byteval = g_bytes_new(value, value_length); + return WTAP_OPTTYPE_SUCCESS; +} + +wtap_opttype_return_val +wtap_block_add_bytes_option_borrow(wtap_block_t block, guint option_id, GBytes *value) +{ + wtap_opttype_return_val ret; + wtap_option_t *opt; + + ret = wtap_block_add_option_common(block, option_id, WTAP_OPTTYPE_BYTES, &opt); + if (ret != WTAP_OPTTYPE_SUCCESS) + return ret; + opt->value.byteval = g_bytes_ref(value); + return WTAP_OPTTYPE_SUCCESS; +} + +wtap_opttype_return_val +wtap_block_set_bytes_option_value(wtap_block_t block, guint option_id, const guint8 *value, size_t value_length) +{ + wtap_opttype_return_val ret; + wtap_optval_t *optval; + + ret = wtap_block_get_option_common(block, option_id, WTAP_OPTTYPE_BYTES, &optval); + if (ret != WTAP_OPTTYPE_SUCCESS) { + if (ret == WTAP_OPTTYPE_NOT_FOUND) { + /* + * There's no instance to set, so just try to create a new one + * with the value. + */ + return wtap_block_add_bytes_option(block, option_id, value, value_length); + } + /* Otherwise fail. */ + return ret; + } + g_bytes_unref(optval->byteval); + optval->byteval = g_bytes_new(value, value_length); + return WTAP_OPTTYPE_SUCCESS; +} + +wtap_opttype_return_val +wtap_block_set_nth_bytes_option_value(wtap_block_t block, guint option_id, guint idx, GBytes *value) +{ + wtap_opttype_return_val ret; + wtap_optval_t *optval; + + ret = wtap_block_get_nth_option_common(block, option_id, WTAP_OPTTYPE_BYTES, idx, &optval); + if (ret != WTAP_OPTTYPE_SUCCESS) + return ret; + g_bytes_unref(optval->byteval); + optval->byteval = g_bytes_ref(value); + return WTAP_OPTTYPE_SUCCESS; +} + +wtap_opttype_return_val +wtap_block_get_bytes_option_value(wtap_block_t block, guint option_id, GBytes** value) +{ + wtap_opttype_return_val ret; + wtap_optval_t *optval; + + ret = wtap_block_get_option_common(block, option_id, WTAP_OPTTYPE_BYTES, &optval); + if (ret != WTAP_OPTTYPE_SUCCESS) + return ret; + *value = optval->byteval; + return WTAP_OPTTYPE_SUCCESS; +} + +wtap_opttype_return_val +wtap_block_get_nth_bytes_option_value(wtap_block_t block, guint option_id, guint idx, GBytes** value) +{ + wtap_opttype_return_val ret; + wtap_optval_t *optval; + + ret = wtap_block_get_nth_option_common(block, option_id, WTAP_OPTTYPE_BYTES, idx, &optval); + if (ret != WTAP_OPTTYPE_SUCCESS) + return ret; + *value = optval->byteval; + return WTAP_OPTTYPE_SUCCESS; +} + wtap_opttype_return_val wtap_block_add_if_filter_option(wtap_block_t block, guint option_id, if_filter_opt_t* value) { @@ -875,6 +1226,10 @@ wtap_block_remove_option(wtap_block_t block, guint option_id) guint i; wtap_option_t *opt; + if (block == NULL) { + return WTAP_OPTTYPE_BAD_BLOCK; + } + opttype = GET_OPTION_TYPE(block->info->options, option_id); if (opttype == NULL) { /* There's no option for this block with that option ID */ @@ -915,6 +1270,10 @@ wtap_block_remove_nth_option_instance(wtap_block_t block, guint option_id, wtap_option_t *opt; guint opt_idx; + if (block == NULL) { + return WTAP_OPTTYPE_BAD_BLOCK; + } + opttype = GET_OPTION_TYPE(block->info->options, option_id); if (opttype == NULL) { /* There's no option for this block with that option ID */ @@ -992,7 +1351,7 @@ static void idb_free_mand(wtap_block_t block) for(j = 0; j < mand->num_stat_entries; j++) { if_stats = g_array_index(mand->interface_statistics, wtap_block_t, j); - wtap_block_free(if_stats); + wtap_block_unref(if_stats); } if (mand->interface_statistics) @@ -1044,6 +1403,17 @@ static void dsb_copy_mand(wtap_block_t dest_block, wtap_block_t src_block) dst->secrets_data = (guint8 *)g_memdup2(src->secrets_data, src->secrets_len); } +static void pkt_create(wtap_block_t block) +{ + /* Commented out for now, there's no mandatory data that isn't handled by + * Wireshark in other ways. + */ + //block->mandatory_data = g_new0(wtapng_packet_mandatory_t, 1); + + /* Ensure this is null, so when g_free is called on it, it simply returns */ + block->mandatory_data = NULL; +} + void wtap_opttypes_initialize(void) { static wtap_blocktype_t shb_block = { @@ -1222,6 +1592,56 @@ void wtap_opttypes_initialize(void) 0 }; + static wtap_blocktype_t pkt_block = { + WTAP_BLOCK_PACKET, /* block_type */ + "EPB/SPB/PB", /* name */ + "Packet Block", /* description */ + pkt_create, /* create */ + NULL, /* free_mand */ + NULL, /* copy_mand */ + NULL /* options */ + }; +#if 0 + // We handle these options via a different mechanism + static const wtap_opttype_t pkt_flags = { + "flags", + "Link-layer flags", + WTAP_OPTTYPE_UINT32, + 0 + }; + static const wtap_opttype_t pkt_hash = { + "hash", + "Hash of packet data", + WTAP_OPTTYPE_BYTES, // TODO: replace with a pkt_filter_opt_t + WTAP_OPTTYPE_FLAG_MULTIPLE_ALLOWED + }; + static const wtap_opttype_t pkt_dropcount = { + "dropcount", + "Packets Dropped since last packet", + WTAP_OPTTYPE_UINT64, + 0 + }; + static const wtap_opttype_t pkt_id = { + "packetid", + "Unique Packet Identifier", + WTAP_OPTTYPE_UINT64, + 0 + }; + static const wtap_opttype_t pkt_queue = { + "queue", + "Queue ID in which packet was received", + WTAP_OPTTYPE_UINT32, + 0 + }; +#endif + static const wtap_opttype_t pkt_verdict = { + "verdict", + "Packet Verdict", + WTAP_OPTTYPE_BYTES, // maybe replace with a pkt_verdict_opt_t + // (or maybe not, packet-frame.c reads the raw bytes) + WTAP_OPTTYPE_FLAG_MULTIPLE_ALLOWED + }; + /* * Register the SHB and the options that can appear in it. */ @@ -1267,11 +1687,38 @@ void wtap_opttypes_initialize(void) * Register the DSB, currently no options are defined. */ wtap_opttype_block_register(&dsb_block); + + /* + * Register EPB/SPB/PB and the options that can appear in it/them. + * NB: Simple Packet Blocks have no options. + * NB: obsolete Packet Blocks have dropcount as a mandatory member instead + * of an option. + */ + wtap_opttype_block_register(&pkt_block); +#if 0 + // We handle these options via a different mechanism + wtap_opttype_option_register(&pkt_block, OPT_PKT_FLAGS, &pkt_flags); + wtap_opttype_option_register(&pkt_block, OPT_PKT_HASH, &pkt_hash); + wtap_opttype_option_register(&pkt_block, OPT_PKT_DROPCOUNT, &pkt_dropcount); + wtap_opttype_option_register(&pkt_block, OPT_PKT_PACKETID, &pkt_id); + wtap_opttype_option_register(&pkt_block, OPT_PKT_QUEUE, &pkt_queue); +#endif + wtap_opttype_option_register(&pkt_block, OPT_PKT_VERDICT, &pkt_verdict); + +#ifdef DEBUG_COUNT_REFS + memset(blocks_active, 0, sizeof(blocks_active)); +#endif } void wtap_opttypes_cleanup(void) { guint block_type; +#ifdef DEBUG_COUNT_REFS + guint i; + guint cellno; + guint bitno; + guint8 mask; +#endif /* DEBUG_COUNT_REFS */ for (block_type = (guint)WTAP_BLOCK_SECTION; block_type < (guint)MAX_WTAP_BLOCK_TYPE_VALUE; block_type++) { @@ -1281,4 +1728,16 @@ void wtap_opttypes_cleanup(void) blocktype_list[block_type] = NULL; } } + +#ifdef DEBUG_COUNT_REFS + for (i = 0 ; i < block_count; i++) { + cellno = i / 8; + bitno = i % 8; + mask = 1 << bitno; + + if ((blocks_active[cellno] & mask) == mask) { + wtap_debug("wtap_opttypes_cleanup: orphaned block #%d", i); + } + } +#endif /* DEBUG_COUNT_REFS */ } diff --git a/wiretap/wtap_opttypes.h b/wiretap/wtap_opttypes.h index 6a797d2b9a..249f6482ab 100644 --- a/wiretap/wtap_opttypes.h +++ b/wiretap/wtap_opttypes.h @@ -12,6 +12,7 @@ #include "ws_symbol_export.h" +#include #include #ifdef __cplusplus @@ -128,6 +129,16 @@ extern "C" { #define OPT_ISB_OSDROP 7 #define OPT_ISB_USRDELIV 8 +/* + * These are the flags for an EPB, but we use them for all WTAP_BLOCK_PACKET + */ +#define OPT_PKT_FLAGS 2 +#define OPT_PKT_HASH 3 +#define OPT_PKT_DROPCOUNT 4 +#define OPT_PKT_PACKETID 5 +#define OPT_PKT_QUEUE 6 +#define OPT_PKT_VERDICT 7 + struct wtap_block; typedef struct wtap_block *wtap_block_t; @@ -235,6 +246,25 @@ typedef struct wtapng_dsb_mandatory_s { guint8 *secrets_data; /** Buffer of secrets (not NUL-terminated) */ } wtapng_dsb_mandatory_t; +/** + * Holds the required data from a WTAP_BLOCK_PACKET. + * This includes Enhanced Packet Block, Simple Packet Block, and the deprecated Packet Block. + * NB. I'm not including the packet data here since Wireshark handles it in other ways. + * If we were to add it we'd need to implement copy and free routines in wtap_opttypes.c + */ +#if 0 +/* Commented out for now, there's no mandatory data that isn't handled by + * Wireshark in other ways. + */ +typedef struct wtapng_packet_mandatory_s { + guint32 interface_id; + guint32 ts_high; + guint32 ts_low; + guint32 captured_len; + guint32 orig_len; +} wtapng_packet_mandatory_t; +#endif + /** * Holds the required data from a WTAP_BLOCK_FT_SPECIFIC_REPORT. */ @@ -245,8 +275,10 @@ typedef struct wtapng_ft_specific_mandatory_s { /* Currently supported option types */ typedef enum { WTAP_OPTTYPE_UINT8, + WTAP_OPTTYPE_UINT32, WTAP_OPTTYPE_UINT64, WTAP_OPTTYPE_STRING, + WTAP_OPTTYPE_BYTES, WTAP_OPTTYPE_IPv4, WTAP_OPTTYPE_IPv6, WTAP_OPTTYPE_IF_FILTER, @@ -259,7 +291,8 @@ typedef enum { WTAP_OPTTYPE_NOT_FOUND = -2, WTAP_OPTTYPE_TYPE_MISMATCH = -3, WTAP_OPTTYPE_NUMBER_MISMATCH = -4, - WTAP_OPTTYPE_ALREADY_EXISTS = -5 + WTAP_OPTTYPE_ALREADY_EXISTS = -5, + WTAP_OPTTYPE_BAD_BLOCK = -6, } wtap_opttype_return_val; /* Interface description data - if_filter option structure */ @@ -306,10 +339,12 @@ typedef struct custom_opt_s { */ typedef union { guint8 uint8val; + guint32 uint32val; guint64 uint64val; - guint32 ipv4val; /* network byte order */ + ws_in4_addr ipv4val; /* network byte order */ ws_in6_addr ipv6val; char *stringval; + GBytes *byteval; if_filter_opt_t if_filterval; custom_opt_t custom_opt; } wtap_optval_t; @@ -345,13 +380,22 @@ WS_DLL_PUBLIC void wtap_opttypes_initialize(void); */ WS_DLL_PUBLIC wtap_block_t wtap_block_create(wtap_block_type_t block_type); -/** Free a block +/** Increase reference count of a block * - * Needs to be called to clean up any allocated block + * Call when taking a copy of a block * - * @param[in] block Block to be freed + * @param[in] block Block add ref to + * @return The block */ -WS_DLL_PUBLIC void wtap_block_free(wtap_block_t block); +WS_DLL_PUBLIC wtap_block_t wtap_block_ref(wtap_block_t block); + +/** Decrease reference count of a block + * + * Needs to be called on any block once you're done with it + * + * @param[in] block Block to be deref'd + */ +WS_DLL_PUBLIC void wtap_block_unref(wtap_block_t block); /** Free an array of blocks * @@ -377,6 +421,15 @@ WS_DLL_PUBLIC wtap_block_type_t wtap_block_get_type(wtap_block_t block); */ WS_DLL_PUBLIC void* wtap_block_get_mandatory_data(wtap_block_t block); +/** Count the number of times the given option appears in the block + * + * @param[in] block Block to which to add the option + * @param[in] option_id Identifier value for option + * @return guint - the number of times the option was found + */ +WS_DLL_PUBLIC guint +wtap_block_count_option(wtap_block_t block, guint option_id); + /** Add UINT8 option value to a block * * @param[in] block Block to which to add the option @@ -410,6 +463,39 @@ wtap_block_set_uint8_option_value(wtap_block_t block, guint option_id, guint8 va WS_DLL_PUBLIC wtap_opttype_return_val wtap_block_get_uint8_option_value(wtap_block_t block, guint option_id, guint8* value) G_GNUC_WARN_UNUSED_RESULT; +/** Add UINT32 option value to a block + * + * @param[in] block Block to which to add the option + * @param[in] option_id Identifier value for option + * @param[in] value Value of option + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_add_uint32_option(wtap_block_t block, guint option_id, guint32 value); + +/** Set UINT32 option value in a block + * + * @param[in] block Block in which to set the option value + * @param[in] option_id Identifier value for option + * @param[in] value New value of option + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_set_uint32_option_value(wtap_block_t block, guint option_id, guint32 value); + +/** Get UINT32 option value from a block + * + * @param[in] block Block from which to get the option value + * @param[in] option_id Identifier value for option + * @param[out] value Returned value of option + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_get_uint32_option_value(wtap_block_t block, guint option_id, guint32* value) G_GNUC_WARN_UNUSED_RESULT; + /** Add UINT64 option value to a block * * @param[in] block Block to which to add the option @@ -609,6 +695,81 @@ wtap_block_get_string_option_value(wtap_block_t block, guint option_id, char** v WS_DLL_PUBLIC wtap_opttype_return_val wtap_block_get_nth_string_option_value(wtap_block_t block, guint option_id, guint idx, char** value) G_GNUC_WARN_UNUSED_RESULT; +/** Add a bytes option to a block + * + * @param[in] block Block to which to add the option + * @param[in] option_id Identifier value for option + * @param[in] value Value of option to copy + * @param[in] value_length Number of bytes to copy + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_add_bytes_option(wtap_block_t block, guint option_id, const guint8 *value, gsize value_length); + +/** Add a bytes option to a block, borrowing the value from a GBytes + * + * @param[in] block Block to which to add the option + * @param[in] option_id Identifier value for option + * @param[in] value Value of option as a GBytes + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_add_bytes_option_borrow(wtap_block_t block, guint option_id, GBytes *value); + +/** Set bytes option value in a block + * + * @param[in] block Block in which to set the option value + * @param[in] option_id Identifier value for option + * @param[in] value New value of option + * @param[in] value_length Number of bytes to copy. + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_set_bytes_option_value(wtap_block_t block, guint option_id, const guint8* value, gsize value_length); + +/** Set bytes option value for nth instance of a particular option in a block + * + * @param[in] block Block in which to set the option value + * @param[in] option_id Identifier value for option + * @param[in] idx Instance number of option with that ID + * @param[in] value New value of option + * @param[in] value_length Number of bytes to copy. + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_set_nth_bytes_option_value(wtap_block_t block, guint option_id, guint idx, GBytes* value); + +/** Get bytes option value from a block + * + * @param[in] block Block from which to get the option value + * @param[in] option_id Identifier value for option + * @param[out] value Returned value of option + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + * @note You should call g_bytes_ref() on value if you plan to keep it around + * (and then g_bytes_unref() when you're done with it). + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_get_bytes_option_value(wtap_block_t block, guint option_id, GBytes** value) G_GNUC_WARN_UNUSED_RESULT; + +/** Get bytes option value for nth instance of a particular option in a block + * + * @param[in] block Block from which to get the option value + * @param[in] option_id Identifier value for option + * @param[in] idx Instance number of option with that ID + * @param[out] value Returned value of option + * @return wtap_opttype_return_val - WTAP_OPTTYPE_SUCCESS if successful, + * error code otherwise + * @note You should call g_bytes_ref() on value if you plan to keep it around + * (and then g_bytes_unref() when you're done with it). + */ +WS_DLL_PUBLIC wtap_opttype_return_val +wtap_block_get_nth_bytes_option_value(wtap_block_t block, guint option_id, guint idx, GBytes** value) G_GNUC_WARN_UNUSED_RESULT; + /** Add an if_filter option value to a block * * @param[in] block Block to which to add the option @@ -677,6 +838,28 @@ WS_DLL_PUBLIC wtap_opttype_return_val wtap_block_remove_nth_option_instance(wtap_block_t block, guint option_id, guint idx); +/** + * Get the original (unpadded) length of an option's value + * + * @param[in] option_type The `wtap_opttype_e` for this option + * @param[in] option The option's value to measure + * @return gsize - the number of bytes occupied by the option's value + */ +WS_DLL_PUBLIC gsize +wtap_block_option_get_value_size(wtap_opttype_e option_type, wtap_optval_t *option); + +/** Get the padded length of all options in the block + * + * @param[in] block Block from which to remove the option instance + * @param[in] option_id Identifier value for option + * @param[in] idx Instance number of option with that ID + * @return gsize - size in bytes of all options, each padded to 32 bits + * @note The size of any options with values larger than can be held in an option + * is NOT included, because pcapng.c skips over such option values. + */ +WS_DLL_PUBLIC gsize +wtap_block_get_options_size_padded(wtap_block_t block); + /** Copy a block to another. * * Any options that are in the destination but not the source are not removed.