fix(rislive)!: error on unparsed message bodies; next-hop consistency fixes - #340
Merged
Conversation
RisMessage::msg is a flattened Option<RisMessageEnum>, so a body-level deserialisation failure arrives as None: indistinguishable from a frame without a body, and parse_ris_live_message returned Ok(vec![]) for it. That is how the comma-joined next-hop bug (#339) dropped a frame's routes without a trace. parse_ris_live_message now re-reads the body when msg is None and, for a frame that declares a message type this crate decodes, returns ParserRisliveError::UnparsedMessageBody with the declared type and the serde reason. A message type this crate does not decode (a future RIS Live addition) still returns no elems, since nothing was lost there. parse_ris_live_message_raw is unaffected: it reads the `raw` bytes. Also: - parse_link_state_nlri resolves RFC 2545 pairs with NextHopAddress::global_addr() instead of the positional addr(), matching the MRT/raw and RIS Live elem paths. - BgpModelsError and ParserRisliveError are #[non_exhaustive]; both grew a variant this cycle, and future variants will not break downstream exhaustive matches. - add tests/fixtures/rislive/README.md (source, size, SHA-256, what the tests require of the fixture) and un-ignore *.md in that directory, as the RIPE and PacketLife fixture directories already are.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused changes are consistent with existing APIs and include appropriate regression coverage.
Pull request overview
This PR prevents silent RIS Live route loss and standardizes RFC 2545 next-hop handling.
Changes:
- Reports deserialization failures for recognized RIS Live message bodies.
- Resolves BGP-LS next-hop pairs by address scope.
- Documents breaking changes and captured fixtures.
File summaries
| File | Description |
|---|---|
src/parser/rislive/mod.rs |
Detects unparsed bodies and adds regression tests. |
src/parser/rislive/error.rs |
Adds the new error and marks the enum non-exhaustive. |
src/parser/bgp/attributes/attr_29_linkstate.rs |
Selects global next hops by scope. |
src/models/err.rs |
Marks model errors non-exhaustive. |
tests/fixtures/rislive/README.md |
Documents fixture provenance and regeneration. |
.gitignore |
Allows RIS Live fixture documentation. |
CHANGELOG.md |
Records breaking changes and fixes. |
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #340 +/- ##
==========================================
+ Coverage 91.41% 91.50% +0.08%
==========================================
Files 103 103
Lines 26412 26483 +71
==========================================
+ Hits 24145 24232 +87
+ Misses 2267 2251 -16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The envelope type check cannot fail from the `RisMessage` arm, and a body that re-parses on its own is never reported: report the error only when the re-parse reproduces it. The test asserts the variant with `matches!`, so it has no unreachable failure arm. Removes the three lines codecov flagged as uncovered (patch coverage 96% -> 100%).
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.
Wrap-up of the notes from #339's review.
Unparsed bodies are errors, not empty output
RisMessage::msgis a flattenedOption<RisMessageEnum>, so any body-level deserialisation failure arrives asNone, indistinguishable from a frame without a body.parse_ris_live_messagereturnedOk(vec![])for those frames: that is how the comma-joined next-hop bug in #339 dropped a frame's routes without a trace.parse_ris_live_messagenow re-reads the body whenmsgisNoneand, for a frame that declares a message type this crate decodes, returnsParserRisliveError::UnparsedMessageBodycarrying the declared type and the underlying serde reason. A message type this crate does not decode (a future RIS Live addition) still returns no elems, because nothing was lost there.parse_ris_live_message_rawis unaffected: it reads therawbytes rather than the JSON projection.The set of decoded types lives in
DECODED_MESSAGE_TYPESnext to the parse function and has to be kept in sync withRisMessageEnum; a missing entry costs the loud failure for that type, never a wrong result.Next-hop consistency
parse_link_state_nlritook the first address of an RFC 2545 pair positionally, so a reversed pair put the link-local address in the BGP-LS NLRI. It now usesNextHopAddress::global_addr(), matching the MRT/raw and RIS Live elem paths fixed in #339. This was the last position-basedNextHopAddress -> IpAddrconversion in the tree.Breaking changes (documented under
## Unreleased)BgpModelsErrorandParserRisliveErrorare now#[non_exhaustive]. Both grew a variant this cycle (NextHopParsingError,UnparsedMessageBody), and future variants will no longer break exhaustive matches downstream.parse_ris_live_messagereturns an error where it previously returnedOk(vec![])for a frame whose body did not deserialize; see above.Fixture documentation
tests/fixtures/rislive/README.mdrecords the frame fixture's origin, size, and SHA-256, whattests/rislive_frames.rsrequires of it, and how to regenerate it..gitignoreun-ignores*.mdunder that directory, as it already does for the RIPE and PacketLife fixture directories (without it the README would have been silently dropped from the commit).