Fix frame reader #436
Merged
Fix frame reader #436
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
In the frame reader, we were storing the frame size including the header (frame size + flags which is 6 bytes). Also, after reading the frame size from the header, we were incrementing the bytes_read. That approach works for small frames, that we can read with one go, but was not working for frames that are large. For the same frame, the next time we call read_frame, the length of the reader (the data we haven't read from the buffer) was equal to frame size, but the variable frame_size was equal to `frame_size + HEADER_SIZE`. So, we were not reading the data that was actually there due to some faulty length check. We were able to read the data that is in the buffer after some long time, when the response for the client ping request comes as it was incrementing the length of the reader enough to pass the length check. To fix that, now, the frame_size field holds the frame size excluding the header size.
Codecov Report
@@ Coverage Diff @@
## master #436 +/- ##
==========================================
- Coverage 86.61% 86.42% -0.19%
==========================================
Files 336 336
Lines 17398 17398
==========================================
- Hits 15070 15037 -33
- Misses 2328 2361 +33
Continue to review full report at Codecov.
|
|
Thanks for the review Yüce |
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.
In the frame reader, we were storing the frame size including the
header (frame size + flags which is 6 bytes). Also, after reading
the frame size from the header, we were incrementing the bytes_read.
That approach works for small frames, that we can read with one go,
but was not working for frames that are large. For the same frame,
the next time we call read_frame, the length of the reader (the data
we haven't read from the buffer) was equal to frame size, but the
variable frame_size was equal to
frame_size + HEADER_SIZE. So,we were not reading the data that was actually there due to some
faulty length check.
We were able to read the data that is in the buffer after some long
time, when the response for the client ping request comes as it
was incrementing the length of the reader enough to pass the length
check.
To fix that, now, the frame_size field holds the frame size excluding
the header size.
closes #434