CESNET / ipfixcol

IPFIXcol is an implementation of an IPFIX (RFC 7011) collector
Other
64 stars 37 forks source link

Invalid handling of unspecified IEs #32

Closed ghost closed 9 years ago

ghost commented 9 years ago

In case an IE in a template has not been specified in ipfix-elements.xml, the function get_type_from_xml (fastbit_element.cpp) would return NULL. As such, the switch-statement in fastbit_table.cpp (template_table::parse_template) would not trigger the UNKNOWN or default cases, resulting in undefined behavior.

mikeek commented 9 years ago

get_type_from_xml returns value of type enum store_type so NULL is never returned. It is converted to the default enum value, which is zero -> UINT is triggered. This is not the right behavior anyway. There are 2 possibilities how to fix this:

1) Make UNKNOWN the first (and default) value of enum store_type - but it may have some unexpected consequences (needs testing) 2) Do it your way

I personaly prefer the first option to keep code efficiency and minimize map accesses.

ghost commented 9 years ago

I fully agree with you that map access should be minimized and that the first option would be a logical consequence. However, such implicit knowledge can easily be overseen in the future, resulting in bugs; the fact that values are not present in a map which, in another function in another file, triggers some behavior, is something that is easily overlooked. Although I don't have a strong preference for 2), it may be the least error-prone approach. Do you agree?

mikeek commented 9 years ago

I agree, present state is not sufficient due to unexpected behavior (after triggering UINT). So I'll merge your request until we find a better solution.

Anyway, thanks for the report.