Skip to content
Snippets Groups Projects

epan: Register columns fields and make them filterable (dynamic version)

Merged John Thacker requested to merge johnthacker/wireshark:column_filter_dynamic into master

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).

Edited by John Thacker

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • John Thacker changed the description

    changed the description

  • John Thacker added 1 commit

    added 1 commit

    • f364ffdf - epan: Register columns fields and make them filterable

    Compare with previous version

  • John Thacker added 1 commit

    added 1 commit

    • 5ec9d21e - epan: Register columns fields and make them filterable

    Compare with previous version

  • John Thacker added 1 commit

    added 1 commit

    • f263650a - epan: Register columns fields and make them filterable

    Compare with previous version

  • 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ørlykke
  • For columns that "doesn't work the way most people expect", I will suggest we don't add. I would rather explain why they don't have a field name, than explain why it sometimes doesn't work as expected.

  • Author Developer

    Yes, 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.

  • Author Developer

    I think you're right, not adding those fields is better. (There's already some difficulty around color filtering based on those fields, and color filters based on the color filter fields.)

    Thanks for the feedback

  • 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.

  • Author Developer

    "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.)

  • Author Developer

    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 Thacker
  • John Thacker marked this merge request as draft

    marked this merge request as draft

  • John Thacker changed title from Draft: epan: Register columns fields and make them filterable to Draft: epan: Register columns fields and make them filterable (dynamic version)

    changed title from Draft: epan: Register columns fields and make them filterable to Draft: epan: Register columns fields and make them filterable (dynamic version)

  • John Thacker added 62 commits

    added 62 commits

    • f263650a...118815ca - 61 commits from branch wireshark:master
    • f30160a0 - epan: Register dynamic column fields and make them filterable

    Compare with previous version

  • Author Developer

    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.

  • John Thacker added 73 commits

    added 73 commits

    • f30160a0...32e17503 - 72 commits from branch wireshark:master
    • 78abcc02 - epan: Register dynamic column fields and make them filterable

    Compare with previous version

  • John Thacker resolved all threads

    resolved all threads

  • Author Developer

    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.

  • John Thacker added 232 commits

    added 232 commits

    • 78abcc02...a82d5b56 - 231 commits from branch wireshark:master
    • f6ee05c6 - epan: Register dynamic column fields and make them filterable

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading