Don't use http.DefaultClient

This strips out http.DefaultClient everywhere I could immediately find
it. Too many things use it and then modify it in incompatible ways.

Fixes #700, I believe.

api/client.go

@@ -12,11 +12,7 @@ import (
 )
 
 var (
-	errRedirect            = errors.New("redirect")
-	defaultHTTPClientSetup sync.Once
-	defaultHTTPClient      = &http.Client{
-		Timeout: time.Second * 30,
-	}
+	errRedirect = errors.New("redirect")
 )
 
 // Config is used to configure the creation of the client.
@@ -30,6 +26,8 @@ type Config struct {
 	// HttpClient is the HTTP client to use, which will currently always have the
 	// same values as http.DefaultClient. This is used to control redirect behavior.
 	HttpClient *http.Client
+
+	redirectSetup sync.Once
 }
 
 // DefaultConfig returns a default configuration for the client. It is
@@ -39,8 +37,10 @@ type Config struct {
 // setting the `VAULT_ADDR` environment variable.
 func DefaultConfig() *Config {
 	config := &Config{
-		Address:    "https://127.0.0.1:8200",
-		HttpClient: defaultHTTPClient,
+		Address: "https://127.0.0.1:8200",
+		HttpClient: &http.Client{
+			Timeout: time.Second * 60,
+		},
 	}
 
 	if addr := os.Getenv("VAULT_ADDR"); addr != "" {
@@ -69,15 +69,22 @@ func NewClient(c *Config) (*Client, error) {
 		return nil, err
 	}
 
-	if c.HttpClient == defaultHTTPClient {
-		defaultHTTPClientSetup.Do(func() {
-			// Ensure redirects are not automatically followed
-			c.HttpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
-				return errRedirect
-			}
-		})
+	if c.HttpClient == nil {
+		c.HttpClient = DefaultConfig().HttpClient
 	}
 
+	redirFunc := func() {
+		// Ensure redirects are not automatically followed
+		// Note that this is sane for the API client as it has its own
+		// redirect handling logic (and thus also for command/meta),
+		// but in e.g. http_test actual redirect handling is necessary
+		c.HttpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+			return errRedirect
+		}
+	}
+
+	c.redirectSetup.Do(redirFunc)
+
 	client := &Client{
 		addr:   u,
 		config: c,

api/ssh_agent.go

@@ -61,15 +61,14 @@ type SSHAgentConfig struct {
 
 // TLSClient returns a HTTP client that uses TLS verification (TLS 1.2) for a given
 // certificate pool.
-func (c *SSHAgentConfig) TLSClient(certPool *x509.CertPool) *http.Client {
+func (c *SSHAgentConfig) SetTLSParameters(clientConfig *Config, certPool *x509.CertPool) {
 	tlsConfig := &tls.Config{
 		InsecureSkipVerify: c.TLSSkipVerify,
 		MinVersion:         tls.VersionTLS12,
 		RootCAs:            certPool,
 	}
 
-	client := *http.DefaultClient
-	client.Transport = &http.Transport{
+	clientConfig.HttpClient.Transport = &http.Transport{
 		Proxy: http.ProxyFromEnvironment,
 		Dial: (&net.Dialer{
 			Timeout:   30 * time.Second,
@@ -78,26 +77,6 @@ func (c *SSHAgentConfig) TLSClient(certPool *x509.CertPool) *http.Client {
 		TLSClientConfig:     tlsConfig,
 		TLSHandshakeTimeout: 10 * time.Second,
 	}
-
-	// From https://github.com/michiwend/gomusicbrainz/pull/4/files
-	defaultRedirectLimit := 30
-
-	client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
-		if len(via) > defaultRedirectLimit {
-			return fmt.Errorf("%d consecutive requests(redirects)", len(via))
-		}
-		if len(via) == 0 {
-			// No redirects
-			return nil
-		}
-		// mutate the subsequent redirect requests with the first Header
-		if token := via[0].Header.Get("X-Vault-Token"); len(token) != 0 {
-			req.Header.Set("X-Vault-Token", token)
-		}
-		return nil
-	}
-
-	return &client
 }
 
 // NewClient returns a new client for the configuration. This client will be used by the
@@ -124,8 +103,8 @@ func (c *SSHAgentConfig) NewClient() (*Client, error) {
 			return nil, err
 		}
 
-		// Change the configuration to have an HTTP client with TLS enabled.
-		clientConfig.HttpClient = c.TLSClient(certPool)
+		// Enable TLS on the HTTP client information
+		c.SetTLSParameters(clientConfig, certPool)
 	}
 
 	// Creating the client object for the given configuration

builtin/credential/github/backend.go

@@ -54,7 +54,7 @@ type backend struct {
 // Client returns the GitHub client to communicate to GitHub via the
 // configured settings.
 func (b *backend) Client(token string) (*github.Client, error) {
-	var tc *http.Client
+	tc := &http.Client{}
 	if token != "" {
 		tc = oauth2.NewClient(oauth2.NoContext, &tokenSource{Value: token})
 	}

builtin/credential/github/backend_test.go

@@ -26,7 +26,7 @@ func TestBackend_Config(t *testing.T) {
 
 	login_data := map[string]interface{}{
 		// This token has to be replaced with a working token for the test to work.
-		"token": "4fb5f4f0738c7aea5ee06dd595f15236ea78918b",
+		"token": os.Getenv("GITHUB_TOKEN"),
 	}
 	config_data1 := map[string]interface{}{
 		"organization": "hashicorp",

builtin/logical/aws/backend_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"net/http"
 	"os"
 	"testing"
 	"time"
@@ -97,6 +98,7 @@ func testAccStepReadUser(t *testing.T, name string) logicaltest.TestStep {
 			awsConfig := &aws.Config{
 				Credentials: creds,
 				Region:      aws.String("us-east-1"),
+				HTTPClient:  &http.Client{},
 			}
 			client := ec2.New(awsConfig)
 

builtin/logical/aws/client.go

@@ -2,6 +2,7 @@ package aws
 
 import (
 	"fmt"
+	"net/http"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/credentials"
@@ -29,6 +30,7 @@ func clientIAM(s logical.Storage) (*iam.IAM, error) {
 	awsConfig := &aws.Config{
 		Credentials: creds,
 		Region:      aws.String(config.Region),
+		HTTPClient:  &http.Client{},
 	}
 	return iam.New(awsConfig), nil
 }

command/meta.go

@@ -124,36 +124,19 @@ func (m *Meta) Client() (*api.Client, error) {
 			return nil, fmt.Errorf("Both client cert and client key must be provided")
 		}
 
-		client := *http.DefaultClient
-		client.Transport = &http.Transport{
-			Proxy: http.ProxyFromEnvironment,
-			Dial: (&net.Dialer{
-				Timeout:   30 * time.Second,
-				KeepAlive: 30 * time.Second,
-			}).Dial,
-			TLSClientConfig:     tlsConfig,
-			TLSHandshakeTimeout: 10 * time.Second,
+		client := &http.Client{
+			Transport: &http.Transport{
+				Proxy: http.ProxyFromEnvironment,
+				Dial: (&net.Dialer{
+					Timeout:   30 * time.Second,
+					KeepAlive: 30 * time.Second,
+				}).Dial,
+				TLSClientConfig:     tlsConfig,
+				TLSHandshakeTimeout: 10 * time.Second,
+			},
 		}
 
-		// From https://github.com/michiwend/gomusicbrainz/pull/4/files
-		defaultRedirectLimit := 30
-
-		client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
-			if len(via) > defaultRedirectLimit {
-				return fmt.Errorf("%d consecutive requests(redirects)", len(via))
-			}
-			if len(via) == 0 {
-				// No redirects
-				return nil
-			}
-			// mutate the subsequent redirect requests with the first Header
-			if token := via[0].Header.Get("X-Vault-Token"); len(token) != 0 {
-				req.Header.Set("X-Vault-Token", token)
-			}
-			return nil
-		}
-
-		config.HttpClient = &client
+		config.HttpClient = client
 	}
 
 	// Build the client

http/http_test.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"net/http"
 	"testing"
+	"time"
 )
 
 func testHttpGet(t *testing.T, token string, addr string) *http.Response {
@@ -46,7 +47,9 @@ func testHttpData(t *testing.T, method string, token string, addr string, body i
 		req.Header.Set("X-Vault-Token", token)
 	}
 
-	client := http.DefaultClient
+	client := &http.Client{
+		Timeout: 60 * time.Second,
+	}
 
 	// From https://github.com/michiwend/gomusicbrainz/pull/4/files
 	defaultRedirectLimit := 30

physical/consul.go

@@ -2,11 +2,11 @@ package physical
 
 import (
 	"fmt"
+	"io/ioutil"
+	"net/http"
 	"sort"
 	"strings"
 	"time"
-	"net/http"
-	"io/ioutil"
 
 	"crypto/tls"
 	"crypto/x509"
@@ -43,6 +43,7 @@ func newConsulBackend(conf map[string]string) (Backend, error) {
 
 	// Configure the client
 	consulConf := api.DefaultConfig()
+
 	if addr, ok := conf["address"]; ok {
 		consulConf.Address = addr
 	}
@@ -95,7 +96,7 @@ func setupTLSConfig(conf map[string]string) (*tls.Config, error) {
 	}
 
 	_, okCert := conf["tls_cert_file"]
-	_, okKey  := conf["tls_key_file"]
+	_, okKey := conf["tls_key_file"]
 
 	if okCert && okKey {
 		tlsCert, err := tls.LoadX509KeyPair(conf["tls_cert_file"], conf["tls_key_file"])