Repository navigation
fix: route each chained message to its own scope in AutoScopedParser - #329
Merged
Merged
Conversation
parse_from_source derived the scope once, from the first message's header, then handed the entire buffer to that single child parser. A call carrying several chained IPFIX messages with different Observation Domain IDs therefore cached every domain's templates against the first domain. That is silent data corruption, not just a miscount. When two domains reuse a Template ID with different field layouts, the later definition overwrites the earlier one in the wrong cache and data records decode to the wrong fields with no error reported: a sourceIPv4Address of 192.0.2.1 decodes as packetDeltaCount 3221225985, its own bytes read as a counter. Re-derive the scope from the head of the remaining buffer on every iteration so each message reaches its own (source_addr, observation_domain_id) parser, as RFC 7011 section 8 requires. Error handling mirrors NetflowParser::parse_bytes: parsing stops at the first failure and already-parsed packets are retained. Only IPFIX was affected in practice. NetFlow v9 has no packet-length field, so a chained v9 buffer already fails to parse rather than misrouting, and RouterScopedParser is keyed entirely by the caller's key and never derives scope from packet contents. iter_packets_from_source still borrows one child parser for the iterator's whole lifetime and keeps this limitation, which is now documented on the method. Lifting it needs an iterator able to own several child parsers. Reproducer from Costa Tsaousis (#325), extended here with a regression test that pins the decoded field rather than the source count, and a test that chained messages sharing one domain still share a parser.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
parse_from_source derived the scope once, from the first message's header, then handed the entire buffer to that single child parser. A call carrying several chained IPFIX messages with different Observation Domain IDs therefore cached every domain's templates against the first domain.
That is silent data corruption, not just a miscount. When two domains reuse a Template ID with different field layouts, the later definition overwrites the earlier one in the wrong cache and data records decode to the wrong fields with no error reported: a sourceIPv4Address of 192.0.2.1 decodes as packetDeltaCount 3221225985, its own bytes read as a counter.
Re-derive the scope from the head of the remaining buffer on every iteration so each message reaches its own (source_addr, observation_domain_id) parser, as RFC 7011 section 8 requires. Error handling mirrors NetflowParser::parse_bytes: parsing stops at the first failure and already-parsed packets are retained.
Only IPFIX was affected in practice. NetFlow v9 has no packet-length field, so a chained v9 buffer already fails to parse rather than misrouting, and RouterScopedParser is keyed entirely by the caller's key and never derives scope from packet contents.
iter_packets_from_source still borrows one child parser for the iterator's whole lifetime and keeps this limitation, which is now documented on the method. Lifting it needs an iterator able to own several child parsers.
Reproducer from Costa Tsaousis (#325), extended here with a regression test that pins the decoded field rather than the source count, and a test that chained messages sharing one domain still share a parser.