Skip to content

Commit

Permalink
client: use io.LimitedReader for reading HTTP error
Browse files Browse the repository at this point in the history
client.checkResponseErr() was hanging and consuming infinite memory
when the serverResp.Body io.Reader returns infinite stream.

This commit prohibits reading more than 1MiB.

Signed-off-by: Akihiro Suda <[email protected]>
  • Loading branch information
AkihiroSuda committed Oct 10, 2018
1 parent 82a4797 commit 1db4be0
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
10 changes: 9 additions & 1 deletion client/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,18 @@ func (cli *Client) checkResponseErr(serverResp serverResponse) error {
return nil
}

body, err := ioutil.ReadAll(serverResp.body)
bodyMax := 1 * 1024 * 1024 // 1 MiB
bodyR := &io.LimitedReader{
R: serverResp.body,
N: int64(bodyMax),
}
body, err := ioutil.ReadAll(bodyR)
if err != nil {
return err
}
if bodyR.N == 0 {
return fmt.Errorf("request returned %s with a message (> %d bytes) for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), bodyMax, serverResp.reqURL)
}
if len(body) == 0 {
return fmt.Errorf("request returned %s for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), serverResp.reqURL)
}
Expand Down
17 changes: 17 additions & 0 deletions client/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import (
"context"
"fmt"
"io/ioutil"
"math/rand"
"net/http"
"strings"
"testing"

"github.com/docker/docker/api/types"
"gotest.tools/assert"
is "gotest.tools/assert/cmp"
)

// TestSetHostHeader should set fake host for local communications, set real host
Expand Down Expand Up @@ -87,3 +89,18 @@ func TestPlainTextError(t *testing.T) {
t.Fatalf("expected a Server Error, got %v", err)
}
}

func TestInfiniteError(t *testing.T) {
infinitR := rand.New(rand.NewSource(42))
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
resp := &http.Response{StatusCode: http.StatusInternalServerError}
resp.Header = http.Header{}
resp.Body = ioutil.NopCloser(infinitR)
return resp, nil
}),
}

_, err := client.Ping(context.Background())
assert.Check(t, is.ErrorContains(err, "request returned Internal Server Error"))
}

0 comments on commit 1db4be0

Please sign in to comment.