osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

Find: Switch search_pos to the start byte 

There's a lot of potentially confusing arithmetic from setting
search_pos to the last byte of a match. We can always test
search_len against zero to see if we actually had a match for
hightlighting purposes. (The ordinary byte searches won't find
zero length matches, but the regex search might, and not even
necessarily at the start of the bytes if there's a lookbehind term,
and currently they're handled incorrectly.)

We can't find fields with length zero based on an offset currently
anyway. (If we tried, would we match fields that contained the byte
before or after the zero length offset?)

Perhaps we shouldn't allow zero length regex matches for packet
byte searches at all; PCRE2 has an option to prevent such matches.
This commit is contained in:
John Thacker 2023-10-03 07:44:45 -04:00
parent 6de60e12be
commit c0885fe390
4 changed files with 40 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cfile.h
View File

@ -103,7 +103,7 @@ typedef struct _capture_file {
gboolean summary_data; /* TRUE if "String" search in "Packet list" (Info column) was last selected */
gboolean decode_data; /* TRUE if "String" search in "Packet details" was last selected */
gboolean packet_data; /* TRUE if "String" search in "Packet data" was last selected */
guint32 search_pos; /* Byte position of last byte found in a hex search */
guint32 search_pos; /* Byte position of first byte found in a hex search */
guint32 search_len; /* Length of bytes matching the search */
gboolean case_type; /* TRUE if case-insensitive text search */
ws_regex_t *regex; /* Set if regular expression search */

60
file.c
View File

@ -3610,10 +3610,9 @@ match_narrow_and_wide(capture_file *cf, frame_data *fdata,
c_match++;
if (c_match == textlen) {
result = MR_MATCHED;
cf->search_pos = i + (guint32)(pd - buf_start);
/* Save the position of the last character
for highlighting the field. */
cf->search_len = i + 1;
/* Save position and length for highlighting the field. */
cf->search_pos = (guint32)(pd - buf_start);
cf->search_len = (guint32)(textlen);
goto done;
}
} else {
@ -3629,10 +3628,9 @@ match_narrow_and_wide(capture_file *cf, frame_data *fdata,
c_match++;
if (c_match == textlen) {
result = MR_MATCHED;
cf->search_pos = i + (guint32)(pd - buf_start);
/* Save the position of the last character
for highlighting the field. */
cf->search_len = i + 1;
/* Save position and length for highlighting the field. */
cf->search_pos = (guint32)(pd - buf_start);
cf->search_len = (guint32)(textlen);
goto done;
}
i++;
@ -3686,10 +3684,9 @@ match_narrow_and_wide_case(capture_file *cf, frame_data *fdata,
c_match++;
if (c_match == textlen) {
result = MR_MATCHED;
cf->search_pos = i + (guint32)(pd - buf_start);
/* Save the position of the last character
for highlighting the field. */
cf->search_len = i + 1;
/* Save position and length for highlighting the field. */
cf->search_pos = (guint32)(pd - buf_start);
cf->search_len = (guint32)(textlen);
goto done;
}
} else {
@ -3705,10 +3702,9 @@ match_narrow_and_wide_case(capture_file *cf, frame_data *fdata,
c_match++;
if (c_match == textlen) {
result = MR_MATCHED;
cf->search_pos = i + (guint32)(pd - buf_start);
/* Save the position of the last character
for highlighting the field. */
cf->search_len = i + 1;
/* Save position and length for highlighting the field. */
cf->search_pos = (guint32)(pd - buf_start);
cf->search_len = (guint32)(textlen);
goto done;
}
i++;
@ -3760,11 +3756,10 @@ match_narrow_case(capture_file *cf, frame_data *fdata,
if (c_char == ascii_text[c_match]) {
c_match++;
if (c_match == textlen) {
/* Save position and length for highlighting the field. */
result = MR_MATCHED;
cf->search_pos = i + (guint32)(pd - buf_start);
/* Save the position of the last character
for highlighting the field. */
cf->search_len = i + 1;
cf->search_pos = (guint32)(pd - buf_start);
cf->search_len = (guint32)(textlen);
goto done;
}
} else {
@ -3811,10 +3806,9 @@ match_wide(capture_file *cf, frame_data *fdata,
c_match++;
if (c_match == textlen) {
result = MR_MATCHED;
cf->search_pos = i + (guint32)(pd - buf_start);
/* Save the position of the last character
for highlighting the field. */
cf->search_len = i + 1;
/* Save position and length for highlighting the field. */
cf->search_pos = (guint32)(pd - buf_start);
cf->search_len = (guint32)(textlen);
goto done;
}
i++;
@ -3867,10 +3861,9 @@ match_wide_case(capture_file *cf, frame_data *fdata,
c_match++;
if (c_match == textlen) {
result = MR_MATCHED;
cf->search_pos = i + (guint32)(pd - buf_start);
/* Save the position of the last character
for highlighting the field. */
cf->search_len = i + 1;
/* Save position and length for highlighting the field. */
cf->search_pos = (guint32)(pd - buf_start);
cf->search_len = (guint32)(textlen);
goto done;
}
i++;
@ -3905,9 +3898,8 @@ match_binary(capture_file *cf, frame_data *fdata,
pd = ws_memmem(buf_start, fdata->cap_len, info->data, datalen);
if (pd != NULL) {
result = MR_MATCHED;
cf->search_pos = (uint32_t)(pd - buf_start + datalen - 1);
/* Save the position of the last character
for highlighting the field. */
/* Save position and length for highlighting the field. */
cf->search_pos = (uint32_t)(pd - buf_start);
cf->search_len = (uint32_t)datalen;
}
@ -3931,8 +3923,12 @@ match_regex(capture_file *cf, frame_data *fdata,
(const gchar *)ws_buffer_start_ptr(buf),
fdata->cap_len,
result_pos)) {
//TODO: A chosen regex can match the empty string (zero length)
// which doesn't make a lot of sense for searching the packet bytes.
// Should we search with the PCRE2_NOTEMPTY option?
//TODO: Fix cast.
cf->search_pos = (guint32)(result_pos[1] - 1); /* last byte = end position - 1 */
/* Save position and length for highlighting the field. */
cf->search_pos = (guint32)(result_pos[0]);
cf->search_len = (guint32)(result_pos[1] - result_pos[0]);
result = MR_MATCHED;
}

2
ui/qt/byte_view_tab.cpp
View File

@ -307,7 +307,7 @@ void ByteViewTab::selectedFieldChanged(FieldInformation *selected)
if (cap_file_->search_in_progress && (cap_file_->hex || (cap_file_->string && cap_file_->packet_data))) {
// In the hex view, only highlight the target bytes or string. The entire
// field can then be displayed by clicking on any of the bytes in the field.
f_start = cap_file_->search_pos - cap_file_->search_len + 1;
f_start = (int)cap_file_->search_pos;
f_length = (int) cap_file_->search_len;
} else {
f_start = selected->position().start;

12
ui/qt/packet_list.cpp
View File

@ -586,9 +586,17 @@ void PacketList::selectionChanged (const QItemSelection & selected, const QItemS
// The tree where the target string matched one of the labels was discarded in
// match_protocol_tree() so we have to search again in the latest tree.
fi = cf_find_string_protocol_tree(cap_file_, cap_file_->edt->tree);
} else if (cap_file_->search_pos != 0) {
} else if (cap_file_->search_len != 0) {
// Find the finfo that corresponds to our byte.
fi = proto_find_field_from_offset(cap_file_->edt->tree, cap_file_->search_pos,
// The match can span multiple fields (and a single byte can
// match more than one field.) Our behavior is to find the last
// field in the tree (so hopefully spanning fewer bytes) that
// matches the last byte in the search match.
// (regex search can find a zero length match not at the
// start of the frame if lookbehind is used, but
// proto_find_field_from_offset doesn't match such a field
// and it's not clear which field we would want to match.)
fi = proto_find_field_from_offset(cap_file_->edt->tree, cap_file_->search_pos + cap_file_->search_len - 1,
cap_file_->edt->tvb);
}