Skip to content

Commit

Permalink
service/s3/s3manager: Fix index out of range when a io.Reader returns…
Browse files Browse the repository at this point in the history
… -1 (#378)

Fixes the S3 Upload Manager's handling of an unbounded streaming reader that returns negative bytes read.
  • Loading branch information
dgrr authored and jasdel committed Oct 1, 2019
1 parent 57d74d6 commit 60a96ac
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 3 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
---

Expand All @@ -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.
10 changes: 9 additions & 1 deletion service/s3/s3manager/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions service/s3/s3manager/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 60a96ac

Please sign in to comment.