service/s3/s3manager: Fix index out of range when a io.Reader returns -1

CHANGELOG_PENDING.md

@@ -10,7 +10,7 @@ Deprecations
 * Removes support for deprecated Go versions ([#393](https://github.com/aws/aws-sdk-go-v2/pull/393))
   * Removes support for Go version specific files from the SDK. Also removes irrelevant build tags, and updates the README.md file. 
   * Raises the minimum supported version to Go 1.11 for the SDK. Older versions may work, but are not actively supported
-
+  
 SDK Features
 ---
 
@@ -21,6 +21,8 @@ SDK Enhancements
   * Related to [aws/aws-sdk-go#2310](https://github.com/aws/aws-sdk-go/pull/2310)
   * Fixes [#251](https://github.com/aws/aws-sdk-go-v2/issues/251)
 * `aws/request` : Retryer is now a named field on Request. ([#393](https://github.com/aws/aws-sdk-go-v2/pull/393))
-  
+
 SDK Bugs
 ---
+* `service/s3/s3manager`: Fix index out of range when a streaming reader returns -1 ([#378](https://github.com/aws/aws-sdk-go-v2/pull/378))
+  * Fixes the S3 Upload Manager's handling of an unbounded streaming reader that returns negative bytes read.

service/s3/s3manager/upload.go

@@ -445,14 +445,22 @@ func (u *uploader) nextReader() (io.ReadSeeker, int, error) {
 	default:
 		part := make([]byte, u.cfg.PartSize)
 		n, err := readFillBuf(r, part)
+		if n < 0 {
+			if err == nil {
+				err = fmt.Errorf(
+					"unable to read part, got negative bytes read(%d) without error, %v",
+					n, err)
+			}
+			return nil, n, err
+		}
 		u.readerPos += int64(n)
 
 		return bytes.NewReader(part[0:n]), n, err
 	}
 }
 
 func readFillBuf(r io.Reader, b []byte) (offset int, err error) {
-	for offset < len(b) && err == nil {
+	for offset >= 0 && offset < len(b) && err == nil {
 		var n int
 		n, err = r.Read(b[offset:])
 		offset += n

service/s3/s3manager/upload_test.go

@@ -277,6 +277,35 @@ func TestUploadFailIfPartSizeTooSmall(t *testing.T) {
 	}
 }
 
+type negativeReader struct {
+}
+
+func (nr *negativeReader) Read(_ []byte) (int, error) {
+	return -1, io.ErrUnexpectedEOF
+}
+
+func TestUploadReaderReturnsNegative(t *testing.T) {
+	s, _, _ := loggingSvc(emptyList)
+	mgr := s3manager.NewUploaderWithClient(s, func(u *s3manager.Uploader) {
+		u.Concurrency = 1
+	})
+
+	// should not panic
+	_, err := mgr.Upload(&s3manager.UploadInput{
+		Bucket: aws.String("Bucket"),
+		Key:    aws.String("Key"),
+		Body:   &negativeReader{},
+	})
+	if err == nil {
+		t.Error("Expected error, but received none")
+	}
+
+	aerr := err.(awserr.Error)
+	if aerr.OrigErr() != io.ErrUnexpectedEOF {
+		t.Fatalf("expected %s. Got %s", io.ErrUnexpectedEOF, aerr.OrigErr())
+	}
+}
+
 func TestUploadOrderSingle(t *testing.T) {
 	s, ops, args := loggingSvc(emptyList)
 	mgr := s3manager.NewUploaderWithClient(s)