-
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
api: PutObjectSignV2 should support uploading 0byte objects. #762
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.
Can you add a small test here ?
2a174da
to
d7a13a5
Compare
api-put-object-multipart.go
Outdated
@@ -141,7 +141,7 @@ func (c Client) putObjectMultipartNoStream(bucketName, objectName string, reader | |||
} | |||
|
|||
// Verify if we uploaded all the data. | |||
if size > 0 { | |||
if size >= 0 { |
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.
putObjectNoChecksum
is called when size
is zero. So, this check is not needed.
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 reason this is done is multipart should also work for size == 0 independent of its upper layer usage.
api-put-object-common.go
Outdated
// object size is '0' set default parts. | ||
if objectSize == 0 { | ||
return 1, 0, 0, nil | ||
} |
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.
putObjectNoChecksum
is called when size
is zero. So, this check is not needed.
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.
This code should work accordingly regardless of how its stitched together
@@ -3596,6 +3596,64 @@ func testUserMetadataCopyingV2() { | |||
testUserMetadataCopyingWrapper(c) | |||
} | |||
|
|||
// Test put object with 0 byte object. |
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.
We should have a test for V4 too.
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.
V4 we already have PutObjectStreaming uploads a 0byte file.
@vadmeste can you take a look PR is updated with unit test as requested. |
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.
LGTM and tested
Fixes #761