beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

client: doRequest: make sure we return a connection-error

Browse source 
This function has various errors that are returned when failing to make a
connection (due to permission issues, TLS mis-configuration, or failing to
resolve the TCP address).

The errConnectionFailed error is currently used as a special case when
processing Ping responses. The current code did not consistently treat
connection errors, and because of that could either absorb the error,
or process the empty response.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2024-02-23 15:13:22 +01:00
parent 901b90593d
commit 913478b428
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
2 changed files with 20 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
client/errors.go
View file

@ -11,15 +11,16 @@ import (
// errConnectionFailed implements an error returned when connection failed. // errConnectionFailed implements an error returned when connection failed.
type errConnectionFailed struct { type errConnectionFailed struct {
host string error
} }
// Error returns a string representation of an errConnectionFailed // Error returns a string representation of an errConnectionFailed
func (err errConnectionFailed) Error() string { func (e errConnectionFailed) Error() string {
if err.host == "" { return e.error.Error()
return "Cannot connect to the Docker daemon. Is the docker daemon running on this host?" }
}
return fmt.Sprintf("Cannot connect to the Docker daemon at %s. Is the docker daemon running?", err.host) func (e errConnectionFailed) Unwrap() error {
return e.error
} }
// IsErrConnectionFailed returns true if the error is caused by connection failed. // IsErrConnectionFailed returns true if the error is caused by connection failed.
@ -29,7 +30,13 @@ func IsErrConnectionFailed(err error) bool {
// ErrorConnectionFailed returns an error with host in the error message when connection to docker daemon failed. // ErrorConnectionFailed returns an error with host in the error message when connection to docker daemon failed.
func ErrorConnectionFailed(host string) error { func ErrorConnectionFailed(host string) error {
return errConnectionFailed{host: host} var err error
if host == "" {
err = fmt.Errorf("Cannot connect to the Docker daemon. Is the docker daemon running on this host?")
} else {
err = fmt.Errorf("Cannot connect to the Docker daemon at %s. Is the docker daemon running?", host)
}
return errConnectionFailed{error: err}
} }
// IsErrNotFound returns true if the error is a NotFound error, which is returned // IsErrNotFound returns true if the error is a NotFound error, which is returned

10
client/request.go
View file

@ -134,17 +134,18 @@ func (cli *Client) sendRequest(ctx context.Context, method, path string, query u
return resp, errdefs.FromStatusCode(err, resp.statusCode) return resp, errdefs.FromStatusCode(err, resp.statusCode)
} }
// FIXME(thaJeztah): Should this actually return a serverResp when a connection error occurred?
func (cli *Client) doRequest(req *http.Request) (serverResponse, error) { func (cli *Client) doRequest(req *http.Request) (serverResponse, error) {
serverResp := serverResponse{statusCode: -1, reqURL: req.URL} serverResp := serverResponse{statusCode: -1, reqURL: req.URL}
resp, err := cli.client.Do(req) resp, err := cli.client.Do(req)
if err != nil { if err != nil {
if cli.scheme != "https" && strings.Contains(err.Error(), "malformed HTTP response") { if cli.scheme != "https" && strings.Contains(err.Error(), "malformed HTTP response") {
return serverResp, fmt.Errorf("%v.\n* Are you trying to connect to a TLS-enabled daemon without TLS?", err) return serverResp, errConnectionFailed{fmt.Errorf("%v.\n* Are you trying to connect to a TLS-enabled daemon without TLS?", err)}
} }
if cli.scheme == "https" && strings.Contains(err.Error(), "bad certificate") { if cli.scheme == "https" && strings.Contains(err.Error(), "bad certificate") {
return serverResp, errors.Wrap(err, "the server probably has client authentication (--tlsverify) enabled; check your TLS client certification settings") return serverResp, errConnectionFailed{errors.Wrap(err, "the server probably has client authentication (--tlsverify) enabled; check your TLS client certification settings")}
} }
// Don't decorate context sentinel errors; users may be comparing to // Don't decorate context sentinel errors; users may be comparing to
@ -156,12 +157,13 @@ func (cli *Client) doRequest(req *http.Request) (serverResponse, error) {
if uErr, ok := err.(*url.Error); ok { if uErr, ok := err.(*url.Error); ok {
if nErr, ok := uErr.Err.(*net.OpError); ok { if nErr, ok := uErr.Err.(*net.OpError); ok {
if os.IsPermission(nErr.Err) { if os.IsPermission(nErr.Err) {
return serverResp, errors.Wrapf(err, "permission denied while trying to connect to the Docker daemon socket at %v", cli.host) return serverResp, errConnectionFailed{errors.Wrapf(err, "permission denied while trying to connect to the Docker daemon socket at %v", cli.host)}
} }
} }
} }
if nErr, ok := err.(net.Error); ok { if nErr, ok := err.(net.Error); ok {
// FIXME(thaJeztah): any net.Error should be considered a connection error (but we should include the original error)?
if nErr.Timeout() { if nErr.Timeout() {
return serverResp, ErrorConnectionFailed(cli.host) return serverResp, ErrorConnectionFailed(cli.host)
} }
@ -190,7 +192,7 @@ func (cli *Client) doRequest(req *http.Request) (serverResponse, error) {
} }
} }
return serverResp, errors.Wrap(err, "error during connect") return serverResp, errConnectionFailed{errors.Wrap(err, "error during connect")}
} }
if resp != nil { if resp != nil {