osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

Lift restriction on upper case protocol display filter names 

Unlike other header fields in filter expressions protocol names
cannot contain upper-case letters. Remove that restriction. This
should make start-up slightly faster as it remove an extra loop
for each protocol filter name.

This was added in 9ead15a6eb but
I don't see a reason to have different rules for protocols and
fields, it seems the README.developer was just being vague and
conflating PROTOABBREV with PROTOFILTERNAME.

The recommendation for lower case is a style recommendation,
and it's a good one, but it should be applied uniformly. As
long as we are not enforcing this for all field filter values
there is no point in enforcing it just for protocol names and
actually it is detrimental, e.g:

hi2operations
HI2Operations.IRIsContent
HI2Operations.UUS1_Content_element
HI2Operations.iRIContent
HI2Operations.iRISequence
HI2Operations.IRIContent
HI2Operations.iRI_Begin_record_element
HI2Operations.iRI_End_record_element
HI2Operations.iRI_Continue_record_element
HI2Operations.iRI_Report_record_element
(...)

It's weird and unexpected to have this difference and there is
no technical reason to require it. What we should probably do
is not include the protocol name in the FIELDFILTERNAME and
have the registration mechanism append it to the PROTOFILTERNAME.

Also disallow leading '-' everywhere in filter names, not just
protocol filter names. It's a universal requirement.
This commit is contained in:
João Valverde 2021-11-01 00:00:19 +00:00 committed by Wireshark GitLab Utility
parent e63857aa4e
commit 070aeddf76
6 changed files with 41 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
doc/README.dissector
View File

@ -94,15 +94,27 @@ PROTOSHORTNAME An abbreviated name for the protocol; this is displayed
any preferences, in the dialog box of enabled protocols,
and in the dialog box for filter fields when constructing
a filter expression.
PROTOABBREV A name for the protocol for use in filter expressions;
it may contain only lower-case letters, digits, and hyphens,
underscores, and periods.
PROTOFILTERNAME A name for the protocol for use in filter expressions;
it may contain only letters, digits, hyphens, underscores and
periods. Lower-case letters are the preferred style.
PROTOABBREV An abbreviation for the protocol; this is used in code and
must be a valid C identifier. Additionally it should follow
any applicable C style guidelines. It is usually the same as
PROTOFILTERNAME with all lower-case letters and
non-alphanumerics replaced with underscores.
LICENSE The license this dissector is under. Please use a SPDX License
identifier.
YEARS The years the above license is valid for.
FIELDNAME The displayed name for the header field.
FIELDABBREV The abbreviated name for the header field; it may contain
only letters, digits, hyphens, underscores, and periods.
FIELDFILTERNAME A name for the header field for use in filter expressions;
it may contain only letters, digits, hyphens, underscores and
periods. Lower-case letters are the preferred style and it
must start with PROTOFILTERNAME followed by a dot.
FIELDABBREV An abbreviation for the header field; this is used in code and
must be a valid C identifier. Additionally it should follow
any applicable C style guidelines. It is usually the same as
FIELDFILTERNAME with all lower-case letters and
non-alphanumerics replaced with underscores.
FIELDTYPE FT_NONE, FT_BOOLEAN, FT_CHAR, FT_UINT8, FT_UINT16, FT_UINT24,
FT_UINT32, FT_UINT40, FT_UINT48, FT_UINT56, FT_UINT64,
FT_INT8, FT_INT16, FT_INT24, FT_INT32, FT_INT40, FT_INT48,
@ -195,7 +207,7 @@ BITMASK Used to mask a field not 8-bit aligned or with a size other
FIELDDESCR A brief description of the field, or NULL. [Please do not use ""].
If, for example, PROTONAME is "Internet Bogosity Discovery Protocol",
PROTOSHORTNAME would be "IBDP", and PROTOABBREV would be "ibdp". Try to
PROTOSHORTNAME would be "IBDP", and PROTOFILTERNAME would be "ibdp". Try to
conform with IANA names.
1.2.1 Automatic substitution in code skeleton
@ -578,9 +590,9 @@ anyway, so it's best to use 'col_add_str' rather than 'col_set_str' in
that case.
For example, to set the "Protocol" column
to "PROTOABBREV":
to "PROTOFILTERNAME":
col_set_str(pinfo->cinfo, COL_PROTOCOL, "PROTOABBREV");
col_set_str(pinfo->cinfo, COL_PROTOCOL, "PROTOFILTERNAME");
1.4.2 The col_add_str function.
@ -828,18 +840,16 @@ A string representing the name of the field. This is the name
that will appear in the graphical protocol tree. It must be a non-empty
string.
abbrev (FIELDABBREV)
abbrev (FIELDFILTERNAME)
--------------------
A string with an abbreviation of the field. The abbreviation should start
with the abbreviation of the parent protocol followed by a period as a
A string with a filter name for the field. The name should start
with the filter name of the parent protocol followed by a period as a
separator. For example, the "src" field in an IP packet would have "ip.src"
as an abbreviation. It is acceptable to have multiple levels of periods if,
as a filter name. It is acceptable to have multiple levels of periods if,
for example, you have fields in your protocol that are then subdivided into
subfields. For example, TRMAC has multiple error fields, so the abbreviations
subfields. For example, TRMAC has multiple error fields, so the names
follow this pattern: "trmac.errors.iso", "trmac.errors.noniso", etc.
The abbreviation is the identifier used in a display filter. As such it
cannot be an empty string.
It must be a non-empty string.
type (FIELDTYPE)
----------------

12
doc/packet-PROTOABBREV.c
View File

@ -44,7 +44,7 @@ void proto_register_PROTOABBREV(void);
/* Initialize the protocol and registered fields */
static int proto_PROTOABBREV = -1;
static int hf_PROTOABBREV_FIELDABBREV = -1;
static int hf_FIELDABBREV = -1;
static expert_field ei_PROTOABBREV_EXPERTABBREV = EI_INIT;
/* Global sample preference ("controls" display of numbers) */
@ -166,13 +166,13 @@ dissect_PROTOABBREV(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
/* Add an item to the subtree, see section 1.5 of README.dissector for more
* information. */
expert_ti = proto_tree_add_item(PROTOABBREV_tree, hf_PROTOABBREV_FIELDABBREV, tvb,
expert_ti = proto_tree_add_item(PROTOABBREV_tree, hf_FIELDABBREV, tvb,
offset, len, ENC_xxx);
offset += len;
/* Some fields or situations may require "expert" analysis that can be
* specifically highlighted. */
if ( TEST_EXPERT_condition )
/* value of hf_PROTOABBREV_FIELDABBREV isn't what's expected */
/* value of hf_FIELDABBREV isn't what's expected */
expert_add_info(pinfo, expert_ti, &ei_PROTOABBREV_EXPERTABBREV);
/* Continue adding tree items to process the packet here... */
@ -199,8 +199,8 @@ proto_register_PROTOABBREV(void)
/* Setup list of header fields See Section 1.5 of README.dissector for
* details. */
static hf_register_info hf[] = {
{ &hf_PROTOABBREV_FIELDABBREV,
{ "FIELDNAME", "PROTOABBREV.FIELDABBREV",
{ &hf_FIELDABBREV,
{ "FIELDNAME", "FIELDFILTERNAME",
FT_FIELDTYPE, FIELDDISPLAY, FIELDCONVERT, BITMASK,
"FIELDDESCR", HFILL }
}
@ -221,7 +221,7 @@ proto_register_PROTOABBREV(void)
/* Register the protocol name and description */
proto_PROTOABBREV = proto_register_protocol("PROTONAME",
"PROTOSHORTNAME", "PROTOABBREV");
"PROTOSHORTNAME", "PROTOFILTERNAME");
/* Required function calls to register the header fields and subtrees */
proto_register_field_array(proto_PROTOABBREV, hf, array_length(hf));

2
epan/dissectors/asn1/HI2Operations/packet-HI2Operations-template.c
View File

@ -22,7 +22,7 @@
#define PNAME "HI2Operations"
#define PSNAME "HI2OPERATIONS"
#define PFNAME "hi2operations"
#define PFNAME "HI2operations"
void proto_register_HI2Operations(void);
void proto_reg_handoff_HI2Operations(void);

2
epan/dissectors/packet-HI2Operations.c
View File

@ -30,7 +30,7 @@
#define PNAME "HI2Operations"
#define PSNAME "HI2OPERATIONS"
#define PFNAME "hi2operations"
#define PFNAME "HI2operations"
void proto_register_HI2Operations(void);
void proto_reg_handoff_HI2Operations(void);

26
epan/proto.c
View File

@ -7363,21 +7363,9 @@ proto_tree_set_appendix(proto_tree *tree, tvbuff_t *tvb, gint start,
static void
check_valid_filter_name_or_fail(const char *filter_name)
{
gboolean found_invalid = proto_check_field_name(filter_name);
/* Additionally forbid upper case characters. */
if (!found_invalid) {
for (guint i = 0; filter_name[i]; i++) {
if (g_ascii_isupper(filter_name[i])) {
found_invalid = TRUE;
break;
}
}
}
if (found_invalid) {
if (proto_check_field_name(filter_name) != '\0') {
ws_error("Protocol filter name \"%s\" has one or more invalid characters."
" Allowed are lower characters, digits, '-', '_' and non-repeating '.'."
" Allowed are letters, digits, '-', '_' and non-repeating '.'."
" This might be caused by an inappropriate plugin or a development error.", filter_name);
}
@ -7386,12 +7374,6 @@ check_valid_filter_name_or_fail(const char *filter_name)
ws_error("Protocol filter name \"%s\" is invalid because it is a reserved keyword."
" This might be caused by an inappropriate plugin or a development error.", filter_name);
}
/* First character cannot be '-'. */
if (filter_name[0] == '-') {
ws_error("Protocol filter name \"%s\" cannot begin with '-'."
" This might be caused by an inappropriate plugin or a development error.", filter_name);
}
}
int
@ -13168,6 +13150,10 @@ proto_check_field_name(const gchar *field_name)
const char *p = field_name;
guchar c = '.', lastc;
/* First character cannot be '-'. */
if (field_name[0] == '-')
return '-';
do {
lastc = c;
c = *(p++);

2
epan/proto.h
View File

@ -735,7 +735,7 @@ typedef struct _header_field_info header_field_info;
struct _header_field_info {
/* ---------- set by dissector --------- */
const char *name; /**< [FIELDNAME] full name of this field */
const char *abbrev; /**< [FIELDABBREV] abbreviated name of this field */
const char *abbrev; /**< [FIELDFILTERNAME] filter name of this field */
enum ftenum type; /**< [FIELDTYPE] field type, one of FT_ (from ftypes.h) */
int display; /**< [FIELDDISPLAY] one of BASE_, or field bit-width if FT_BOOLEAN and non-zero bitmask */
const void *strings; /**< [FIELDCONVERT] value_string, val64_string, range_string or true_false_string,