Wireshark-dev: Re: [Wireshark-dev] Enabling/disabling ANY heuristic dissector
From: Hadriel Kaplan <[email protected]>
Date: Sun, 5 Jul 2015 23:11:06 -0400
> On Jul 5, 2015, at 7:02 PM, [email protected] wrote:
> 
> I uploaded a patch to Gerrit that allows enabling/disabling of any heuristic dissector (https://code.wireshark.org/review/9508/).
>  
> Some comments about the patch (others are welcome to add more):
> 1. Not sure how to best express the relationship between the "name" of the heuristic dissector and its "parent"/table name.  For example, the AD-win Config protocol has a heuristic dissector that goes on top of TCP and UDP.  Each instance (TCP or UDP) can be enabled/disabled separately.  Requiring an individual name for each heuristic dissector sounds like a bit too much to ask.  Right now they are slash (/) delimited in the GUI and comma delimited in the "disabled_heuristics" file.

I think the slash delimiter in the GUI is ok - not great, but I can’t think of anything more clear. Perhaps have the Column title also be slash delimited, like “Heuristic Protocol/Underlying Protocol”? (or some better word for “Underlying”)


> 2. These "preferences" are read right after the enable/disable protocols are read/applied.  This seemed like a logical place for it (since I added support for enable/disable heuristics in disabled_protos.c), but I'm not sure how that effects the "general" protocol preferences.  Both manipulate the same heuristic dissector list, so "last one would win”.

I think all the individual preferences to enable/disable a heuristic should be removed. Are you talking about some other “general" protocol preference?

One note: this would mean we can’t ship the Qt-based GUI until the Enabled Protocols dialog box is available in it.


> 3. I'd like to remove any individual dissector preferences that enable/disable a heuristic dissector (future patch) using the same logic/justification as Decode As. Not sure how to handle heuristic dissectors that should be off by default.  heur_dissector_set_enabled(..., FALSE) can be used, but that won't be "overridden" (enabled) by the fact that the heuristic dissector ISN'T in the disabled heuristics file.

I think we should change all the heur_dissector_add() function calls to take a boolean for whether it is enable by default or not; that way developers of future heuristic functions have to think about it and can’t forget. The heur_dissector_set_enabled() should be removed.


> 4. I understand the "feature" of enabling/disabling a (heuristic dissector) preference from the context menu, and that could be justification/argument for keeping it (the preference).  Maybe just "appending" the context menu preferences for any protocol that has a heuristic dissector would be a good compromise?

You can currently disable a protocol itself with the context menu, without them having a preference for doing so, right?  So I suggest we just follow that same behavior for heuristics, whatever that is. :)

-hadriel