-
Notifications
You must be signed in to change notification settings - Fork 657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
get: Fix reading an object if its size is unknown #694
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the commit message mention when the content-length is missing happens and also remove the case where GCS is specially handled for its Content-Length in the same PR.
api-get-object.go
Outdated
return 0, io.EOF | ||
} | ||
o.currOffset += offset | ||
case 2: | ||
// If we don't the object size, immediately return an error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"If we don't know the object size return an error for io.SeekEnd"
All comments addressed. |
api-stat.go
Outdated
size, err := strconv.ParseInt(resp.Header.Get("Content-Length"), 10, 64) | ||
if err != nil { | ||
// Content-Length is not valid, do not verify. | ||
size = -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question "Why are we ignoring ParseInt error" arises in mind, previous code was better. Or you can do:
@@ -126,11 +124,12 @@ func (c Client) statObject(bucketName, objectName string, reqHeaders RequestHead
md5sum := strings.TrimPrefix(resp.Header.Get("ETag"), "\"")
md5sum = strings.TrimSuffix(md5sum, "\"")
- // Content-Length is not valid for Google Cloud Storage, do not verify.
+ // If server does not send Content-Length then we set size to -1.
var size int64 = -1
- if !s3utils.IsGoogleEndpoint(c.endpointURL) {
+ lenStr := resp.Header.Get("Content-Length")
+ if lenStr != "" {
// Parse content length.
- size, err = strconv.ParseInt(resp.Header.Get("Content-Length"), 10, 64)
+ size, err = strconv.ParseInt(lenStr, 10, 64)
if err != nil {
return ObjectInfo{}, ErrorResponse{
Code: "InternalError",
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Getting an object and reading it doesn't work as expected when S3 server doesn't return the object content length, which happens sometimes with Google Cloud Service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested waiting on @krishnasrinivas review.
Getting an object and reading it doesn't work as expected when S3 server
doesn't return the object content length. (though very rare)
Fixes #693