aws/request: Fix for Go 1.8 request incorrectly sent with body

aws/request/request.go

@@ -245,7 +245,79 @@ func (r *Request) ResetBody() {
 	}
 
 	r.safeBody = newOffsetReader(r.Body, r.BodyStart)
-	r.HTTPRequest.Body = r.safeBody
+
+	// Go 1.8 tightened and clarified the rules code needs to use when building
+	// requests with the http package. Go 1.8 removed the automatic detection
+	// of if the Request.Body was empty, or actually had bytes in it. The SDK
+	// always sets the Request.Body even if it is empty and should not actually
+	// be sent. This is incorrect.
+	//
+	// Go 1.8 did add a http.NoBody value that the SDK can use to tell the http
+	// client that the request really should be sent without a body. The
+	// Request.Body cannot be set to nil, which is preferable, because the
+	// field is exported and could introduce nil pointer dereferences for users
+	// of the SDK if they used that field.
+	//
+	// Related golang/go#18257
+	l, err := computeBodyLength(r.Body)
+	if err != nil {
+		r.Error = awserr.New("SerializationError", "failed to compute request body size", err)
+		return
+	}
+
+	if l == 0 {
+		r.HTTPRequest.Body = noBodyReader
+	} else if l > 0 {
+		r.HTTPRequest.Body = r.safeBody
+	} else {
+		// Hack to prevent sending bodies for methods where the body
+		// should be ignored by the server. Sending bodies on these
+		// methods without an associated ContentLength will cause the
+		// request to socket timeout because the server does not handle
+		// Transfer-Encoding: chunked bodies for these methods.
+		switch r.Operation.HTTPMethod {
+		case "GET", "HEAD", "DELETE":
+			r.HTTPRequest.Body = noBodyReader
+		default:
+			r.HTTPRequest.Body = r.safeBody
+		}
+	}
+}
+
+// Attempts to compute the length of the body of the reader using the
+// io.Seeker interface. If the value is not seekable because of being
+// a ReaderSeekerCloser without an unerlying Seeker -1 will be returned.
+// If no error occurs the length of the body will be returned.
+func computeBodyLength(r io.ReadSeeker) (int64, error) {
+	seekable := true
+	// Determine if the seeker is actually seekable. ReaderSeekerCloser
+	// hides the fact that a io.Readers might not actually be seekable.
+	switch v := r.(type) {
+	case aws.ReaderSeekerCloser:
+		seekable = v.IsSeeker()
+	case *aws.ReaderSeekerCloser:
+		seekable = v.IsSeeker()
+	}
+	if !seekable {
+		return -1, nil
+	}
+
+	curOffset, err := r.Seek(0, 1)
+	if err != nil {
+		return 0, err
+	}
+
+	endOffset, err := r.Seek(0, 2)
+	if err != nil {
+		return 0, err
+	}
+
+	_, err = r.Seek(curOffset, 0)
+	if err != nil {
+		return 0, err
+	}
+
+	return endOffset - curOffset, nil
 }
 
 // GetBody will return an io.ReadSeeker of the Request's underlying

aws/request/request_1_7.go

@@ -0,0 +1,21 @@
+// +build !go1.8
+
+package request
+
+import "io"
+
+// NoBody is an io.ReadCloser with no bytes. Read always returns EOF
+// and Close always returns nil. It can be used in an outgoing client
+// request to explicitly signal that a request has zero bytes.
+// An alternative, however, is to simply set Request.Body to nil.
+//
+// Copy of Go 1.8 NoBody type from net/http/http.go
+type noBody struct{}
+
+func (noBody) Read([]byte) (int, error)         { return 0, io.EOF }
+func (noBody) Close() error                     { return nil }
+func (noBody) WriteTo(io.Writer) (int64, error) { return 0, nil }
+
+// Is an empty reader that will trigger the Go HTTP client to not include
+// and body in the HTTP request.
+var noBodyReader = noBody{}

aws/request/request_1_7_test.go

@@ -0,0 +1,24 @@
+// +build !go1.8
+
+package request
+
+import (
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func TestResetBody_WithEmptyBody(t *testing.T) {
+	r := Request{
+		HTTPRequest: &http.Request{},
+	}
+
+	reader := strings.NewReader("")
+	r.Body = reader
+
+	r.ResetBody()
+
+	if a, e := r.HTTPRequest.Body, (noBody{}); a != e {
+		t.Errorf("expected request body to be set to reader, got %#v", r.HTTPRequest.Body)
+	}
+}

aws/request/request_1_8.go

@@ -0,0 +1,9 @@
+// +build go1.8
+
+package request
+
+import "net/http"
+
+// Is a http.NoBody reader instructing Go HTTP client to not include
+// and body in the HTTP request.
+var noBodyReader = http.NoBody

aws/request/request_1_8_test.go

@@ -0,0 +1,25 @@
+// +build go1.8
+
+package request
+
+import (
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func TestResetBody_WithEmptyBody(t *testing.T) {
+	r := Request{
+		HTTPRequest: &http.Request{},
+	}
+
+	reader := strings.NewReader("")
+	r.Body = reader
+
+	r.ResetBody()
+
+	if a, e := r.HTTPRequest.Body, http.NoBody; a != e {
+		t.Errorf("expected request body to be set to reader, got %#v",
+			r.HTTPRequest.Body)
+	}
+}

aws/request/request_resetbody_test.go

@@ -0,0 +1,23 @@
+package request
+
+import (
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func TestResetBody_WithBodyContents(t *testing.T) {
+	r := Request{
+		HTTPRequest: &http.Request{},
+	}
+
+	reader := strings.NewReader("abc")
+	r.Body = reader
+
+	r.ResetBody()
+
+	if v, ok := r.HTTPRequest.Body.(*offsetReader); !ok || v == nil {
+		t.Errorf("expected request body to be set to reader, got %#v",
+			r.HTTPRequest.Body)
+	}
+}

aws/types.go

@@ -6,6 +6,8 @@ import (
 )
 
 // ReadSeekCloser wraps a io.Reader returning a ReaderSeekerCloser
+//
+// Deprecated: Masking reader without valid seeker hides errors. If using for S3 PutObject use s3manager.Uploader instead.
 func ReadSeekCloser(r io.Reader) ReaderSeekerCloser {
 	return ReaderSeekerCloser{r}
 }
@@ -44,6 +46,12 @@ func (r ReaderSeekerCloser) Seek(offset int64, whence int) (int64, error) {
 	return int64(0), nil
 }
 
+// IsSeeker returns if the underlying reader is also a seeker.
+func (r ReaderSeekerCloser) IsSeeker() bool {
+	_, ok := r.r.(io.Seeker)
+	return ok
+}
+
 // Close closes the ReaderSeekerCloser.
 //
 // If the ReaderSeekerCloser is not an io.Closer nothing will be done.

private/model/api/param_filler.go

@@ -55,7 +55,7 @@ func (f paramFiller) paramsStructAny(value interface{}, shape *Shape) string {
 	case "blob":
 		v := reflect.Indirect(reflect.ValueOf(value))
 		if v.IsValid() && shape.Streaming {
-			return fmt.Sprintf("aws.ReadSeekCloser(bytes.NewBufferString(%#v))", v.Interface())
+			return fmt.Sprintf("bytes.NewReader([]byte(%#v))", v.Interface())
 		} else if v.IsValid() {
 			return fmt.Sprintf("[]byte(%#v)", v.Interface())
 		}

private/protocol/rest/unmarshal.go

@@ -1,6 +1,7 @@
 package rest
 
 import (
+	"bytes"
 	"encoding/base64"
 	"fmt"
 	"io"
@@ -11,7 +12,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/aws/request"
 )
@@ -70,10 +70,16 @@ func unmarshalBody(r *request.Request, v reflect.Value) {
 						}
 					default:
 						switch payload.Type().String() {
-						case "io.ReadSeeker":
-							payload.Set(reflect.ValueOf(aws.ReadSeekCloser(r.HTTPResponse.Body)))
-						case "aws.ReadSeekCloser", "io.ReadCloser":
+						case "io.ReadCloser":
 							payload.Set(reflect.ValueOf(r.HTTPResponse.Body))
+						case "io.ReadSeeker":
+							b, err := ioutil.ReadAll(r.HTTPResponse.Body)
+							if err != nil {
+								r.Error = awserr.New("SerializationError",
+									"failed to read response body", err)
+								return
+							}
+							payload.Set(reflect.ValueOf(ioutil.NopCloser(bytes.NewReader(b))))
 						default:
 							io.Copy(ioutil.Discard, r.HTTPResponse.Body)
 							defer r.HTTPResponse.Body.Close()

private/protocol/restjson/build_test.go

@@ -3590,7 +3590,7 @@ func TestInputService9ProtocolTestURIParameterQuerystringParamsHeadersAndJSONBod
 func TestInputService10ProtocolTestStreamingPayloadCase1(t *testing.T) {
 	svc := NewInputService10ProtocolTest(unit.Session, &aws.Config{Endpoint: aws.String("https://test")})
 	input := &InputService10TestShapeInputService10TestCaseOperation1Input{
-		Body:      aws.ReadSeekCloser(bytes.NewBufferString("contents")),
+		Body:      bytes.NewReader([]byte("contents")),
 		Checksum:  aws.String("foo"),
 		VaultName: aws.String("name"),
 	}

service/s3/statusok_error.go

@@ -5,7 +5,6 @@ import (
 	"io/ioutil"
 	"net/http"
 
-	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/aws/request"
 )
@@ -17,8 +16,8 @@ func copyMultipartStatusOKUnmarhsalError(r *request.Request) {
 		return
 	}
 	body := bytes.NewReader(b)
-	r.HTTPResponse.Body = aws.ReadSeekCloser(body)
-	defer r.HTTPResponse.Body.(aws.ReaderSeekerCloser).Seek(0, 0)
+	r.HTTPResponse.Body = ioutil.NopCloser(body)
+	defer body.Seek(0, 0)
 
 	if body.Len() == 0 {
 		// If there is no body don't attempt to parse the body.