Producer: Avoid drop bytes on errors & TCP/UDP Sockets: Move flush to load #2244
Conversation
This file contains 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
Producer: * Use internal buffers for the parsed items to reuse the memory avoiding allocating new memory on each call. * Removed stream interface to avoid getting into multiple mutable reference for producer and its buffer at the same time. * Fix all unit tests ensuring all of them are passing without any change in their core functionality. * Remove index counter because it's not used anywhere. All Parsers: * Parse all provided bytes returning all available items until hitting the first error. * Adjust unit tests on SomeIP ensuring the length of the return items is correct. * Provide new test for string tokenizer to ensure bytes are parsed on the first call. Chipmunk-CLI: * Integrate changes in producer * Ensure all external dependencies are used. Indexer-CLI: * Integrate changes in producer. Benchmarks: * Fix parser multiple time to return an iterator instead vector to avoid allocating memory.
Flushing the internal buffer on load is more reliable to avoid potential bugs when having an incomplete parser result on last packet before having the internal buffer almost filled.
* Change handling parse error from dropping all loaded bytes to skipping them one by one and reapplying parse on the remaining to avoid data loss. * Finish the session on initial parse errors to avoid calling parse repeatedly on each byte drop of the source in case the input isn't suitable for the selected parser.
Extend unit tests for producer loop with mocks to cover possible scenarios with parse errors
This was referenced Feb 26, 2025
* Fix mapping error type from SomeIP to parser in case more data are needed since this error type is handheld differently in producer loop. * Matching adjustments on unit, snapshot and integration tests.
AmmarAbouZor
force-pushed
the
fix_parse_error
branch
from
9783300 to
28527af
Compare
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.
This PR extends #2233 and replaces #2240.
It marked is draft until #2233 is merged
It provides the following changes:
TCP/UDP Sources:
Flushing the internal buffer is moved to
load()method to tie freeing bytes in internal buffer and loading it with more data, making it more reliable to avoid potential bugs when having an incomplete parser result on last packet before having the internal buffer almost filled.Producer Loop:
suitable for the selected parser.
SomeIP parser:
Fix converting errors from SomeIP crate into parser, ensuring
Incompleteerror type is delivered correctly in producer loop since this error will be handled different than parse errors