service/s3/s3manager: Update S3 Upload Multipart location

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 aws#1385
jasdel · Feb 15, 2019 · 1b879b0 · 1b879b0
1 parent c083ef3
commit 1b879b0
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 7 deletions.
diff --git 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
@@ -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 {