Fix range requests returning excess data in file_stream()#3150
Open
ChihweiLHBird wants to merge 2 commits into
Open
Fix range requests returning excess data in file_stream()#3150ChihweiLHBird wants to merge 2 commits into
ChihweiLHBird wants to merge 2 commits into
Conversation
Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an over-read bug in Sanic’s file_stream() range-response streaming loop that could send more bytes than requested when the range length is not aligned with chunk_size, and adds a regression test to lock in the correct behavior.
Changes:
- Correct range streaming to read
min(to_send, chunk_size)per iteration (instead of a constant range size). - Add a regression test asserting a multi-chunk range response returns the exact requested byte length.
- Add a test helper to compute file sizes via
stat()rather than reading whole files.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
sanic/response/convenience.py |
Fix range streaming loop to prevent sending excess bytes on multi-chunk range reads. |
tests/test_response.py |
Add regression test for correct range body length and introduce get_file_size() helper used by range tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3150 +/- ##
=============================================
- Coverage 87.793% 87.731% -0.062%
=============================================
Files 105 105
Lines 8143 8143
Branches 1290 1290
=============================================
- Hits 7149 7144 -5
- Misses 687 692 +5
Partials 307 307 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
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.
Summary
file_stream()sending more bytes than requested for range requests when the range size is not aligned tochunk_size_range.size(a constant) instead ofto_send(which decreases each iteration), causing over-reads on subsequent chunksget_file_size()test helper usingstat()instead of reading entire file contentsFixes #2507
Test plan
test_file_stream_response_rangetests passtest_file_stream_response_range_correct_lengthtest verifies a 100-byte range with 32-byte chunks returns exactly 100 bytes