Fix inclusive parseUntil condition in the main packet parse loop.
#1964
Conversation
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
- The `parseUntil` is now fully inclusive and parses until it exhausts a sequence of target protocol types. This is to allow parsing until recursive layers that are followed by a layer of the same type. - Readability improvements.
Dimi1010
changed the title
parseUntil condition the main packet parse loop.
Dimi1010
changed the title
parseUntil condition the main packet parse loop.parseUntil condition in the main packet parse loop.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #1964 +/- ##
==========================================
- Coverage 83.51% 83.50% -0.01%
==========================================
Files 310 310
Lines 54884 54881 -3
Branches 12220 11896 -324
==========================================
- Hits 45834 45827 -7
- Misses 7786 7802 +16
+ Partials 1264 1252 -12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Dimi1010
commented
Sep 12, 2025
| m_LastLayer->m_NextLayer = nullptr; | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
The old code is kept mostly so it does not interfere with the diff.
It would probably be better to be removed prior to merge.
seladb
reviewed
Sep 19, 2025
| // As the stop conditions are inclusive, the parse must go one layer further and then roll back if needed | ||
| bool rollbackLastLayer = false; | ||
| bool foundTargetProtocol = false; | ||
| for (Layer* curLayer = m_FirstLayer; curLayer != nullptr; curLayer = curLayer->getNextLayer()) |
There was a problem hiding this comment.
nit: we can use auto* instead of Layer*
There was a problem hiding this comment.
Can we add tests to prove correct parsing when there is chaining of the same protocol type (both with parseUntil and parseUntilLayer)?
There was a problem hiding this comment.
Sure, BGP will probably be easiest to set up.
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.
The
parseUntilis now fully inclusive and parses until it exhausts a sequence of target protocol types. This allows specifying target protocols that allow chaining of the same protocol types.Simplified the rollback mechanism.