epan: Register columns fields and make them filterable (dynamic version)
Make the text of each column a FT_STRING field that can be filtered, prefixed with _ws.col - these work in display filters, filters in taps, coloring rules, Wireshark read filters, and in the -Y, -R, and -e options to tshark. Use them as the default "Apply as Filter" value for the columns that aren't handled by anything else currently.
Because only the columns formats that actually correspond to columns get filled in (invisible columns work), register and deregister the fields when the columns change.
Currently the field name is based on the column title - but column titles can have characters not allowed in fields, including spaces and a trailing period (the default No.), and all illegal characters are replaced with _. That doesn't work so well if some has non-English column names.
This adds a number of conditions to "when are the columns needed", including when the main display filter or any filter on a tap is using one of these fields. For custom columns, the tree then has to be further primed with any fields used by the custom columns as well.
Thus for custom columns, or for a number of built-in columns, you're better off for performance reasons matching the ordinary field; it takes extra time to generate the columns and many of them are numeric types. (Note that you can always convert a non-string field to a string field if you want regex matching, consult the wireshark-filter(4) man page.) It does save a bit on typing (especially for a multifield custom column) and remembering the column title might be easier in some cases.
The columns are set before the color filters, which means that you can have a color filter that depends on a built-in column like Info or Protocol. Conversely, you can't filter on the color filter fields with a _ws.col.Custom field, but you can filter on frame.coloring_rule.name directly, so that doesn't matter.
The output format for -T fields remains the same; all that special handling is removed (except for remembering if someone asked for a column field to know that columns should be constructed.)
They're also set before the postdissectors, so postdissectors can have access.
Anything that depends on "the last packet displayed" (COL_DELTA_TIME_DIS or COL_CUMULATIVE_BYTES) doesn't work the way most people expect, for different reasons. The same is already true of color filters that use those (along with color filters that use the color filter fields.)
Fix #16576 (closed). Fix #17971 (closed). Fix #4684 (closed). Fix #13491 (closed). Fix #13941 (closed).
Merge request reports
Activity
added 1 commit
- f364ffdf - epan: Register columns fields and make them filterable
added 1 commit
- 5ec9d21e - epan: Register columns fields and make them filterable
added 1 commit
- f263650a - epan: Register columns fields and make them filterable
Is it possible to use a lower case version the
COL_
name instead of the title?- This will make the field name equal for everyone, regardless of local changes.
- This will avoid having replacement characters for UTF-8 symbols.
- The title may be changed by the user, giving a different field name.
- The columns can be registered once, and never deregistered.
- Multiple columns may have the same title, which will give the same field name.
- The field name can be shown in the column header tooltip, like we do for custom columns.
- The field name can also be shown in the "Fields" column in Preferences -> Columns, just not editable.
- The field name can also be shown in the "Fields:" part of "Edit column", just not editable.
- I can probably think of more benefits after posting this comment.
Edited by Stig BjørlykkeYes, that's the other option I'm considering. I have gone back and forth on it It has all the advantages you mentioned. The disadvantages I can think of:
- It's not how the current
-e fields
option of tshark works - It's not clear how to handle custom columns - is using one field for all of them ok? Maybe don't register fields for them at all, but it seems useful to be able to specify a particular custom column by name (but bad for performance)
Maybe others.
- It's not how the current
Do we need to handle custom columns? They already have a field with the proper type, which should give better filters. I see a use case in custom columns with multiple fields, but also here I think it's better to use the individual fields instead of a common string-only field. "Apply as Filter" already work pretty good in this cases.
- Resolved by John Thacker
Some COL_INFO strings have a trailing space (probably because of some generic append code), and this is impossible to spot in the column. This will not give expected result when using _ws.col.info == "Visible String". I'm not sure how to handle this.
"I would rather explain why they don't have a field name, than explain why it sometimes doesn't work as expected."
Ah, I remembered another downside of statically registering all the field types. Any types that don't have columns (hidden or not) don't get their information filled and don't get added to the tree, so if for some reason someone didn't have a COL_INFO column (or something not default, say COL_EXPERT), the filter field would exist but the filter would never return anything. That would probably end up having to be explained to some people too (unless any fields for columns not present were deregistered.)
See !10522 (closed) for the static version of this, which might be preferable. That uses all lower case names based off of the COL_ names.
Edited by John Thackeradded 62 commits
-
f263650a...118815ca - 61 commits from branch
wireshark:master
- f30160a0 - epan: Register dynamic column fields and make them filterable
-
f263650a...118815ca - 61 commits from branch
I updated this to be close to !10522 (closed), except that fields for columns that aren't in the prefs aren't registered (and are deregistered.) Validation then makes it impossible to create a custom column that won't work or filter on a column that isn't in the prefs. If you create a custom column based on, e.g., _ws.col.expert and then delete the built in expert column, there will be an invalid column with no text still.
added 73 commits
-
f30160a0...32e17503 - 72 commits from branch
wireshark:master
- 78abcc02 - epan: Register dynamic column fields and make them filterable
-
f30160a0...32e17503 - 72 commits from branch
After looking at your comments, I think I like this version better, where the field names aren't registered if the columns don't exist, instead of the column internal field names always existing but giving empty strings if the columns don't exist. That lessens the chance of user confusion.
added 232 commits
-
78abcc02...a82d5b56 - 231 commits from branch
wireshark:master
- f6ee05c6 - epan: Register dynamic column fields and make them filterable
-
78abcc02...a82d5b56 - 231 commits from branch