diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 8a1927a39ca..94057b39473 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,3 +3,5 @@ ### SDK Enhancements ### SDK Bugs +* `service/s3/s3crypto`: Fix client's temporary file buffer error on retry ([#3344](https://github.com/aws/aws-sdk-go/pull/3344)) + * Fixes the Crypto client's temporary file buffer cleanup returning an error when the request is retried. diff --git a/service/s3/s3crypto/helper.go b/service/s3/s3crypto/helper.go index be40fc90319..949286923a7 100644 --- a/service/s3/s3crypto/helper.go +++ b/service/s3/s3crypto/helper.go @@ -19,7 +19,7 @@ func getWriterStore(req *request.Request, path string, useTempFile bool) (io.Rea return nil, err } - req.Handlers.Send.PushBack(func(r *request.Request) { + req.Handlers.Complete.PushBack(func(r *request.Request) { // Close the temp file and cleanup f.Close() os.Remove(f.Name()) diff --git a/service/s3/s3crypto/helper_test.go b/service/s3/s3crypto/helper_test.go index 26993bcdc97..d9f07ca70d6 100644 --- a/service/s3/s3crypto/helper_test.go +++ b/service/s3/s3crypto/helper_test.go @@ -2,6 +2,7 @@ package s3crypto import ( "bytes" + "io/ioutil" "net/http" "os" "testing" @@ -118,6 +119,43 @@ func TestGetWriterStore_TempFile(t *testing.T) { } } +func TestGetWriterStore_TempFileWithRetry(t *testing.T) { + responses := []*http.Response{ + {StatusCode: 500, Header: http.Header{}, Body: ioutil.NopCloser(&bytes.Buffer{})}, + {StatusCode: 200, Header: http.Header{}, Body: ioutil.NopCloser(&bytes.Buffer{})}, + } + s := awstesting.NewClient(aws.NewConfig().WithMaxRetries(10)) + s.Handlers.Validate.Clear() + s.Handlers.Send.Clear() // mock sending + s.Handlers.Send.PushBack(func(r *request.Request) { + r.HTTPResponse = responses[0] + responses = responses[1:] + }) + type testData struct { + Data string + } + out := &testData{} + r := s.NewRequest(&request.Operation{Name: "Operation"}, nil, out) + f, err := getWriterStore(r, "", true) + if err != nil { + t.Fatalf("expected no error, but received %v", err) + } + tempFile, ok := f.(*os.File) + if !ok { + t.Fatal("io.ReadWriteSeeker expected to be *os.file") + } + err = r.Send() + if err != nil { + t.Errorf("expected no error, but received %v", err) + } + if _, err := os.Stat(tempFile.Name()); !os.IsNotExist(err) { + t.Errorf("expected temp file be deleted, but still exists %v", tempFile.Name()) + } + if v := len(responses); v != 0 { + t.Errorf("expect all retries to be used, have %v remaining", v) + } +} + func TestGetWriterStore_Memory(t *testing.T) { response := http.Response{StatusCode: 200} s := awstesting.NewClient(aws.NewConfig().WithMaxRetries(10))