From 726642632da1593b37cb690259fb57c02fec04d8 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte Date: Mon, 18 Feb 2019 09:33:55 -0800 Subject: [PATCH] service/s3/s3manager: Update S3 Upload Multipart location (#2453) Updates the Location returned value of S3 Upload's Multipart UploadOutput type to be consistent with single part upload URL. This update also brings the multipart upload Location inline with the S3 object URLs created by the SDK Fix #1385 --- service/s3/s3manager/upload.go | 13 ++++++++++++- service/s3/s3manager/upload_test.go | 12 ++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/service/s3/s3manager/upload.go b/service/s3/s3manager/upload.go index a9a005f0ac1..da1838c9db8 100644 --- a/service/s3/s3manager/upload.go +++ b/service/s3/s3manager/upload.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/awsutil" "github.com/aws/aws-sdk-go/aws/client" + "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/service/s3" "github.com/aws/aws-sdk-go/service/s3/s3iface" @@ -593,8 +594,18 @@ func (u *multiuploader) upload(firstBuf io.ReadSeeker, firstPart []byte) (*Uploa uploadID: u.uploadID, } } + + // Create a presigned URL of the S3 Get Object in order to have parity with + // single part upload. + getReq, _ := u.cfg.S3.GetObjectRequest(&s3.GetObjectInput{ + Bucket: u.in.Bucket, + Key: u.in.Key, + }) + getReq.Config.Credentials = credentials.AnonymousCredentials + uploadLocation, _, _ := getReq.PresignRequest(1) + return &UploadOutput{ - Location: aws.StringValue(complete.Location), + Location: uploadLocation, VersionID: complete.VersionId, UploadID: u.uploadID, }, nil diff --git a/service/s3/s3manager/upload_test.go b/service/s3/s3manager/upload_test.go index c058082364b..0848a31a41c 100644 --- a/service/s3/s3manager/upload_test.go +++ b/service/s3/s3manager/upload_test.go @@ -104,7 +104,7 @@ func TestUploadOrderMulti(t *testing.T) { resp, err := u.Upload(&s3manager.UploadInput{ Bucket: aws.String("Bucket"), - Key: aws.String("Key"), + Key: aws.String("Key - value"), Body: bytes.NewReader(buf12MB), ServerSideEncryption: aws.String("aws:kms"), SSEKMSKeyId: aws.String("KmsId"), @@ -120,8 +120,8 @@ func TestUploadOrderMulti(t *testing.T) { t.Errorf("Expected %v, but received %v", expected, *ops) } - if "https://location" != resp.Location { - t.Errorf("Expected %q, but received %q", "https://location", resp.Location) + if e, a := `https://s3.mock-region.amazonaws.com/Bucket/Key%20-%20value`, resp.Location; e != a { + t.Errorf("Expected %q, but received %q", e, a) } if "UPLOAD-ID" != resp.UploadID { @@ -282,7 +282,7 @@ func TestUploadOrderSingle(t *testing.T) { mgr := s3manager.NewUploaderWithClient(s) resp, err := mgr.Upload(&s3manager.UploadInput{ Bucket: aws.String("Bucket"), - Key: aws.String("Key"), + Key: aws.String("Key - value"), Body: bytes.NewReader(buf2MB), ServerSideEncryption: aws.String("aws:kms"), SSEKMSKeyId: aws.String("KmsId"), @@ -297,8 +297,8 @@ func TestUploadOrderSingle(t *testing.T) { t.Errorf("Expected %v, but received %v", vals, *ops) } - if len(resp.Location) == 0 { - t.Error("Expected Location to not be empty") + if e, a := `https://s3.mock-region.amazonaws.com/Bucket/Key%20-%20value`, resp.Location; e != a { + t.Errorf("Expected %q, but received %q", e, a) } if e := "VERSION-ID"; e != *resp.VersionID {