[fix] respect content-range header in resumable upload - #2056
Open
JornEngelbart wants to merge 1 commit into
Open
[fix] respect content-range header in resumable upload#2056JornEngelbart wants to merge 1 commit into
JornEngelbart wants to merge 1 commit into
Conversation
Owner
|
hey @JornEngelbart, apologies for the delayed review. do you still want to get this in? If so, can you update your branch/fix the conflicts? Change looks good, so I think this is ready to ship once CI passes! |
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.
Fix Content-Range header handling in resumable uploads
Problem
The resumable upload implementation was not correctly handling the
Content-Rangeheader when uploading chunks. The code was simply appending all chunks sequentially (obj.Content = append(obj.Content, content...)), which caused incorrect behavior when:This resulted in:
Solution
The fix properly parses and respects the
Content-Rangeheader to place content at the exact byte positions specified:Key Changes
Parse Content-Range before appending: The code now parses the
Content-Rangeheader first to determine where content should be placedPlace content at specified positions: Instead of appending, content is copied to the exact byte range specified:
Handle overlapping ranges: When the same range is uploaded multiple times, the content correctly overwrites the previous content at those positions
Extend buffer as needed: The buffer is automatically extended with zero-filled bytes when a range requires a larger buffer size
Validate content length: Added validation to ensure the actual content length matches the Content-Range header specification
Example Scenario
Previously, if a client sent:
Content-Range: bytes 0-262143/377464(262144 bytes)Content-Range: bytes 1-262144/377464(262144 bytes, overlapping)Content-Range: bytes 1-377463/377464(377463 bytes, overlapping)The old code would append all chunks sequentially, resulting in an incorrect file.
The new code correctly:
Testing
Added comprehensive test coverage in
TestResumableUploadWithContentRangethat verifies:Impact
This fix ensures that fake-gcs-server correctly handles resumable uploads with Content-Range headers, making it compatible with clients that:
The fix is backward compatible and maintains the existing behavior for uploads without Content-Range headers (falls back to append behavior).