search

rusticata / pcap-parser

PCAP/PCAPNG file format parser written in pure Rust. Fast, zero-copy, safe.
Other
103 stars 24 forks source link

to_vec ignores if_tsresol field of InterfaceDescriptionBlock #24

Closed lf- closed 7 months ago

lf- commented 1 year ago

Hi!

Thanks so much for writing this library, it's been invaluable in writing the tooling I'm currently writing. I am investigating using it for serialization and I found what I appear to be a bug: when serializing InterfaceDescriptionBlock, the if_tsresol field is ignored. It appears that when it is deserialized, it is synthesized from the options, but when it is serialized, it is disregarded. This means that InterfaceDescriptionBlock values do not round-trip.

https://github.com/rusticata/pcap-parser/blob/444ccc6ccb43b30694cc352fa3b2fea9c9fb2baa/src/serialize.rs#L123-L129

It would definitely be gilding the lily to do this, but this kind of bug could be caught with property-based testing with the proptest crate or similar, generating blocks and verifying that serializing then deserializing results in the same value.

chifflier commented 1 year ago

Hi, Indeed, the if_tsresol and offset are hardcoded when serializing blocks. I will have a look ASAP

chifflier commented 1 year ago

Unfortunately, due to the current design, fixing this requires a breaking change. PcapNGOption can only borrow data, which makes adding a dynamic value impossible since the block cannot own it.

Scheduling this for next major release. The option will probably become a Cow on underlying bytes so it can store new options.