diff --git a/client/client.go b/client/client.go index df3698adc6..f8f2fc6ad5 100644 --- a/client/client.go +++ b/client/client.go @@ -46,6 +46,7 @@ For example, to list running containers (the equivalent of "docker ps"): package client import ( + "errors" "fmt" "net/http" "net/url" @@ -58,6 +59,9 @@ import ( "github.com/docker/go-connections/tlsconfig" ) +// ErrRedirect is the error returned by checkRedirect when the request is non-GET. +var ErrRedirect = errors.New("unexpected redirect in response") + // Client is the API client that performs all operations // against a docker server. type Client struct { @@ -81,6 +85,23 @@ type Client struct { manualOverride bool } +// CheckRedirect specifies the policy for dealing with redirect responses: +// If the request is non-GET return `ErrRedirect`. Otherwise use the last response. +// +// Go 1.8 changes behavior for HTTP redirects (specificlaly 301, 307, and 308) in the client . +// The Docker client (and by extension docker API client) can be made to to send a request +// like POST /containers//start where what would normally be in the name section of the URL is empty. +// This triggers an HTTP 301 from the daemon. +// In go 1.8 this 301 will be converted to a GET request, and ends up getting a 404 from the daemon. +// This behavior change manifests in the client in that before the 301 was not followed and +// the client did not generate an error, but now results in a message like Error response from daemon: page not found. +func CheckRedirect(req *http.Request, via []*http.Request) error { + if via[0].Method == http.MethodGet { + return http.ErrUseLastResponse + } + return ErrRedirect +} + // NewEnvClient initializes a new API client based on environment variables. // Use DOCKER_HOST to set the url to the docker server. // Use DOCKER_API_VERSION to set the version of the API to reach, leave empty for latest. @@ -104,6 +125,7 @@ func NewEnvClient() (*Client, error) { Transport: &http.Transport{ TLSClientConfig: tlsc, }, + CheckRedirect: CheckRedirect, } } @@ -147,7 +169,8 @@ func NewClient(host string, version string, client *http.Client, httpHeaders map transport := new(http.Transport) sockets.ConfigureTransport(transport, proto, addr) client = &http.Client{ - Transport: transport, + Transport: transport, + CheckRedirect: CheckRedirect, } } diff --git a/client/client_test.go b/client/client_test.go index 0816d8a9bf..77214bc53c 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -13,6 +13,7 @@ import ( "github.com/docker/docker/api" "github.com/docker/docker/api/types" + "github.com/stretchr/testify/assert" "golang.org/x/net/context" ) @@ -282,3 +283,52 @@ func TestNewEnvClientSetsDefaultVersion(t *testing.T) { os.Setenv(key, envVarValues[key]) } } + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (rtf roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return rtf(req) +} + +type bytesBufferClose struct { + *bytes.Buffer +} + +func (bbc bytesBufferClose) Close() error { + return nil +} + +func TestClientRedirect(t *testing.T) { + client := &http.Client{ + CheckRedirect: CheckRedirect, + Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.String() == "/bla" { + return &http.Response{StatusCode: 404}, nil + } + return &http.Response{ + StatusCode: 301, + Header: map[string][]string{"Location": {"/bla"}}, + Body: bytesBufferClose{bytes.NewBuffer(nil)}, + }, nil + }), + } + + cases := []struct { + httpMethod string + expectedErr error + statusCode int + }{ + {http.MethodGet, nil, 301}, + {http.MethodPost, &url.Error{Op: "Post", URL: "/bla", Err: ErrRedirect}, 301}, + {http.MethodPut, &url.Error{Op: "Put", URL: "/bla", Err: ErrRedirect}, 301}, + {http.MethodDelete, &url.Error{Op: "Delete", URL: "/bla", Err: ErrRedirect}, 301}, + } + + for _, tc := range cases { + req, err := http.NewRequest(tc.httpMethod, "/redirectme", nil) + assert.NoError(t, err) + resp, err := client.Do(req) + assert.Equal(t, tc.expectedErr, err) + assert.Equal(t, tc.statusCode, resp.StatusCode) + } +}