aws: Add default HTTP client instead of http.DefaultClient/Transport (#315)

Adds a new BuildableHTTPClient type to the SDK's aws package. The type
uses the builder pattern with immutable changes. Modifications to the
buildable client create copies of the client.

Adds a HTTPClient interface to the aws package that the SDK will use as
an abstraction over the specific HTTP client implementation. The SDK
will default to the BuildableHTTPClient, but a *http.Client can be also
provided for custom configuration.

When the SDK's aws.Config.HTTPClient value is a BuildableHTTPClient the
SDK will be able to use API client specific request timeout options.

Fix #279
Fix #269

Author: jasdel

aws/client.go

@@ -8,7 +8,7 @@ import (
 type Metadata struct {
 	ServiceName string
 	ServiceID   string
-	EndpointsID  string
+	EndpointsID string
 	APIVersion  string
 
 	Endpoint      string
@@ -36,15 +36,15 @@ type Client struct {
 	LogLevel LogLevel
 	Logger   Logger
 
-	HTTPClient *http.Client
+	HTTPClient HTTPClient
 }
 
 // NewClient will return a pointer to a new initialized service client.
 func NewClient(cfg Config, metadata Metadata) *Client {
 	svc := &Client{
 		Metadata: metadata,
 
-		// TODO remove config when request reqfactored
+		// TODO remove config when request refactored
 		Config: cfg,
 
 		Region:           cfg.Region,
@@ -57,6 +57,10 @@ func NewClient(cfg Config, metadata Metadata) *Client {
 		Logger:   cfg.Logger,
 	}
 
+	if c, ok := svc.Config.HTTPClient.(*http.Client); ok {
+		svc.Config.HTTPClient = wrapWithoutRedirect(c)
+	}
+
 	retryer := cfg.Retryer
 	if retryer == nil {
 		// TODO need better way of specifing default num retries
@@ -85,3 +89,60 @@ func (c *Client) AddDebugHandlers() {
 	c.Handlers.Send.PushFrontNamed(NamedHandler{Name: "awssdk.client.LogRequest", Fn: logRequest})
 	c.Handlers.Send.PushBackNamed(NamedHandler{Name: "awssdk.client.LogResponse", Fn: logResponse})
 }
+
+func wrapWithoutRedirect(c *http.Client) *http.Client {
+	tr := c.Transport
+	if tr == nil {
+		tr = http.DefaultTransport
+	}
+
+	cc := *c
+	cc.CheckRedirect = limitedRedirect
+	cc.Transport = stubBadHTTPRedirectTransport{
+		tr: tr,
+	}
+
+	return &cc
+}
+
+func limitedRedirect(r *http.Request, via []*http.Request) error {
+	// Request.Response, in CheckRedirect is the response that is triggering
+	// the redirect.
+	resp := r.Response
+	if r.URL.String() == stubBadHTTPRedirectLocation {
+		resp.Header.Del(stubBadHTTPRedirectLocation)
+		return http.ErrUseLastResponse
+	}
+
+	switch resp.StatusCode {
+	case 307, 308:
+		// Only allow 307 and 308 redirects as they preserve the method.
+		return nil
+	}
+
+	return http.ErrUseLastResponse
+}
+
+type stubBadHTTPRedirectTransport struct {
+	tr http.RoundTripper
+}
+
+const stubBadHTTPRedirectLocation = `https://amazonaws.com/badhttpredirectlocation`
+
+func (t stubBadHTTPRedirectTransport) RoundTrip(r *http.Request) (*http.Response, error) {
+	resp, err := t.tr.RoundTrip(r)
+	if err != nil {
+		return resp, err
+	}
+
+	// TODO S3 is the only known service to return 301 without location header.
+	// consider moving this to a S3 customization.
+	switch resp.StatusCode {
+	case 301, 302:
+		if v := resp.Header.Get("Location"); len(v) == 0 {
+			resp.Header.Set("Location", stubBadHTTPRedirectLocation)
+		}
+	}
+
+	return resp, err
+}

aws/config.go

@@ -1,9 +1,5 @@
 package aws
 
-import (
-	"net/http"
-)
-
 // A Config provides service configuration for service clients.
 type Config struct {
 	// The region to send requests to. This parameter is required and must
@@ -24,9 +20,13 @@ type Config struct {
 	// to use based on region.
 	EndpointResolver EndpointResolver
 
-	// The HTTP client to use when sending requests. Defaults to
-	// `http.DefaultClient`.
-	HTTPClient *http.Client
+	// The HTTP Client the SDK's API clients will use to invoke HTTP requests.
+	// The SDK defaults to a BuildableHTTPClient allowing API clients to create
+	// copies of the HTTP Client for service specific customizations.
+	//
+	// Use a (*http.Client) for custom behavior. Using a custom http.Client
+	// will prevent the SDK from modifying the HTTP client.
+	HTTPClient HTTPClient
 
 	// TODO document
 	Handlers Handlers

aws/defaults/defaults.go

@@ -9,9 +9,7 @@ package defaults
 
 import (
 	"log"
-	"net/http"
 	"os"
-	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/aws/endpoints"
@@ -55,17 +53,8 @@ func Config() aws.Config {
 // HTTPClient will return a new HTTP Client configured for the SDK.
 //
 // Does not use http.DefaultClient nor http.DefaultTransport.
-func HTTPClient() *http.Client {
-	return &http.Client{
-		Transport: &http.Transport{
-			Proxy:                 http.ProxyFromEnvironment,
-			MaxIdleConns:          100,
-			MaxIdleConnsPerHost:   10,
-			IdleConnTimeout:       30 * time.Second,
-			TLSHandshakeTimeout:   10 * time.Second,
-			ExpectContinueTimeout: 5 * time.Second,
-		},
-	}
+func HTTPClient() aws.HTTPClient {
+	return aws.NewBuildableHTTPClient()
 }
 
 // Handlers returns the default request handlers.

aws/defaults/handlers.go

@@ -91,11 +91,9 @@ var ValidateReqSigHandler = aws.NamedHandler{
 var SendHandler = aws.NamedHandler{
 	Name: "core.SendHandler",
 	Fn: func(r *aws.Request) {
-		sender := sendFollowRedirects
-		if r.DisableFollowRedirects {
-			sender = sendWithoutFollowRedirects
-		}
 
+		// TODO remove this complexity the SDK's built http.Request should
+		// set Request.Body to nil, if there is no body to send. #318
 		if aws.NoBody == r.HTTPRequest.Body {
 			// Strip off the request body if the NoBody reader was used as a
 			// place holder for a request body. This prevents the SDK from
@@ -113,26 +111,13 @@ var SendHandler = aws.NamedHandler{
 		}
 
 		var err error
-		r.HTTPResponse, err = sender(r)
+		r.HTTPResponse, err = r.Config.HTTPClient.Do(r.HTTPRequest)
 		if err != nil {
 			handleSendError(r, err)
 		}
 	},
 }
 
-func sendFollowRedirects(r *aws.Request) (*http.Response, error) {
-	return r.Config.HTTPClient.Do(r.HTTPRequest)
-}
-
-func sendWithoutFollowRedirects(r *aws.Request) (*http.Response, error) {
-	transport := r.Config.HTTPClient.Transport
-	if transport == nil {
-		transport = http.DefaultTransport
-	}
-
-	return transport.RoundTrip(r.HTTPRequest)
-}
-
 func handleSendError(r *aws.Request, err error) {
 	// Prevent leaking if an HTTPResponse was returned. Clean up
 	// the body.

aws/defaults/handlers_test.go

@@ -252,7 +252,6 @@ func TestSendWithoutFollowRedirects(t *testing.T) {
 		Name:     "Operation",
 		HTTPPath: "/original",
 	}, nil, nil)
-	r.DisableFollowRedirects = true
 
 	err := r.Send()
 	if err != nil {

aws/ec2metadata/api_client.go

@@ -11,9 +11,11 @@ import (
 	"bytes"
 	"errors"
 	"io"
+	"net"
 	"net/http"
 	"os"
 	"strings"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/aws/awserr"
@@ -34,13 +36,26 @@ type Client struct {
 //     // Create a Client client from just a config.
 //     svc := ec2metadata.New(cfg)
 func New(config aws.Config) *Client {
+	if c, ok := config.HTTPClient.(*aws.BuildableHTTPClient); ok {
+		// TODO consider moving this to a client configuration via client builder
+		// instead automatically being set.
+
+		// Use a custom Dial timeout for the EC2 Metadata service to account
+		// for the possibility the application might not be running in an
+		// environment with the service present. The client should fail fast in
+		// this case.
+		config.HTTPClient = c.WithDialerOptions(func(d *net.Dialer) {
+			d.Timeout = 5 * time.Second
+		})
+	}
+
 	svc := &Client{
 		Client: aws.NewClient(
 			config,
 			aws.Metadata{
 				ServiceName: "EC2 Instance Metadata",
 				ServiceID:   "EC2InstanceMetadata",
-				EndpointsID:  "ec2metadata",
+				EndpointsID: "ec2metadata",
 				APIVersion:  "latest",
 			},
 		),
@@ -72,10 +87,6 @@ func New(config aws.Config) *Client {
 	return svc
 }
 
-func httpClientZero(c *http.Client) bool {
-	return c == nil || (c.Transport == nil && c.CheckRedirect == nil && c.Jar == nil && c.Timeout == 0)
-}
-
 type metadataOutput struct {
 	Content string
 }