don't attempt range request without object size #227
Merged
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
The range logic was being erroneously applied when there was no Content-Length returned by the HEAD call. Clone the http header when copying them to new requests, so that changes are not inadvertently reflected in the caller. The GetFile method still has the unfortunate behavior of not verifying the destination file for the range request (which it currently can't, because there is no metadata to compare). This means that a call to replace an existing file when the server supports byte-range, will either succeed if the new length is <= the previous, or corrupt the file by appending new bytes. Silent corruption can be avoided by added a `checksum` parameter to the go-getter request, but will still result in failure to download. Update the HttpGetter.GetFile method docs to reflect this behavior. In most cases however the caller is not looking at the HttpGetter itself, as go-getter is attempting to abstract this away. This means there needs to be global support in the Client to make this a viable option. How to add this can be decided separate from this fix.
jbardin
force-pushed
the
jbardin/head-accept-range
branch
from
d345d1a to
0d90093
Compare
apparentlymart
approved these changes
Dec 21, 2019
azr
approved these changes
Jan 6, 2020
Merged
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 range logic was being erroneously applied when there was no
Content-Length returned by the HEAD call. This frequently causes
download requests to report success without downloading anything,
which results in the next process in the stream (the decompressor in
the tar.gz case) to return an EOF error because there is no file content.
Clone the http header when copying them to new requests, so that changes
are not inadvertently reflected in the caller.
The GetFile method still has the unfortunate behavior of not verifying
the destination file for the range request (which it currently can't,
because there is no metadata to compare). This means that a call to
replace an existing file when the server supports byte-range, will
either succeed if the new length is <= the previous, or corrupt the file
by appending new bytes. Silent corruption can be avoided by added a
checksumparameter to the go-getter request, but will still result infailure to download.
Update the HttpGetter.GetFile method docs to reflect this behavior. In most
cases however the caller is not looking at the HttpGetter itself, as
go-getter is attempting to abstract this away. This means there needs to
be global support in the Client to make this a viable option. How to add
this can be decided separate from this fix.