From abed0e1f97e4ba0186d9321f476d48cad54285d2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 16 Oct 2024 17:16:55 +0200 Subject: [PATCH] client: ContainerResize, ContainerExecResize: don't overflow width/height Mostly theoretical, but let's be correct here. It's worth noting that the API (backend) accepts uint32, but container.ResizeOptions uses uint (uint64). We could decide to add checks for this on the client side, or to change the type (but that would be a breaking change). Signed-off-by: Sebastiaan van Stijn --- client/container_resize.go | 5 +- client/container_resize_test.go | 115 +++++++++++++++++++++++--------- 2 files changed, 86 insertions(+), 34 deletions(-) diff --git a/client/container_resize.go b/client/container_resize.go index 5cfd01d4798e3..6f1a8f5605cb7 100644 --- a/client/container_resize.go +++ b/client/container_resize.go @@ -19,9 +19,10 @@ func (cli *Client) ContainerExecResize(ctx context.Context, execID string, optio } func (cli *Client) resize(ctx context.Context, basePath string, height, width uint) error { + // FIXME(thaJeztah): the API / backend accepts uint32, but container.ResizeOptions uses uint. query := url.Values{} - query.Set("h", strconv.Itoa(int(height))) - query.Set("w", strconv.Itoa(int(width))) + query.Set("h", strconv.FormatUint(uint64(height), 10)) + query.Set("w", strconv.FormatUint(uint64(width), 10)) resp, err := cli.post(ctx, basePath+"/resize", query, nil, nil) ensureReaderClosed(resp) diff --git a/client/container_resize_test.go b/client/container_resize_test.go index 76559ef928283..00960c17741f9 100644 --- a/client/container_resize_test.go +++ b/client/container_resize_test.go @@ -3,10 +3,9 @@ package client // import "github.com/docker/docker/client" import ( "bytes" "context" - "fmt" "io" + "math" "net/http" - "strings" "testing" "github.com/docker/docker/api/types/container" @@ -32,47 +31,99 @@ func TestContainerExecResizeError(t *testing.T) { } func TestContainerResize(t *testing.T) { - client := &Client{ - client: newMockClient(resizeTransport("/containers/container_id/resize")), - } + const expectedURL = "/containers/container_id/resize" - err := client.ContainerResize(context.Background(), "container_id", container.ResizeOptions{ - Height: 500, - Width: 600, - }) - if err != nil { - t.Fatal(err) + tests := []struct { + doc string + opts container.ResizeOptions + expectedHeight, expectedWidth string + }{ + { + doc: "zero width height", // valid, but not very useful + opts: container.ResizeOptions{}, + expectedWidth: "0", + expectedHeight: "0", + }, + { + doc: "valid resize", + opts: container.ResizeOptions{ + Height: 500, + Width: 600, + }, + expectedHeight: "500", + expectedWidth: "600", + }, + { + doc: "larger than maxint64", + opts: container.ResizeOptions{ + Height: math.MaxInt64 + 1, + Width: math.MaxInt64 + 2, + }, + expectedHeight: "9223372036854775808", + expectedWidth: "9223372036854775809", + }, + } + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + client := &Client{ + client: newMockClient(resizeTransport(t, expectedURL, tc.expectedHeight, tc.expectedWidth)), + } + err := client.ContainerResize(context.Background(), "container_id", tc.opts) + assert.Check(t, err) + }) } } func TestContainerExecResize(t *testing.T) { - client := &Client{ - client: newMockClient(resizeTransport("/exec/exec_id/resize")), + const expectedURL = "/exec/exec_id/resize" + tests := []struct { + doc string + opts container.ResizeOptions + expectedHeight, expectedWidth string + }{ + { + doc: "zero width height", // valid, but not very useful + opts: container.ResizeOptions{}, + expectedWidth: "0", + expectedHeight: "0", + }, + { + doc: "valid resize", + opts: container.ResizeOptions{ + Height: 500, + Width: 600, + }, + expectedHeight: "500", + expectedWidth: "600", + }, + { + doc: "larger than maxint64", + opts: container.ResizeOptions{ + Height: math.MaxInt64 + 1, + Width: math.MaxInt64 + 2, + }, + expectedHeight: "9223372036854775808", + expectedWidth: "9223372036854775809", + }, } - - err := client.ContainerExecResize(context.Background(), "exec_id", container.ResizeOptions{ - Height: 500, - Width: 600, - }) - if err != nil { - t.Fatal(err) + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + client := &Client{ + client: newMockClient(resizeTransport(t, expectedURL, tc.expectedHeight, tc.expectedWidth)), + } + err := client.ContainerExecResize(context.Background(), "exec_id", tc.opts) + assert.Check(t, err) + }) } } -func resizeTransport(expectedURL string) func(req *http.Request) (*http.Response, error) { +func resizeTransport(t *testing.T, expectedURL, expectedHeight, expectedWidth string) func(req *http.Request) (*http.Response, error) { return func(req *http.Request) (*http.Response, error) { - if !strings.HasPrefix(req.URL.Path, expectedURL) { - return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) - } + assert.Check(t, is.Equal(req.URL.Path, expectedURL)) + query := req.URL.Query() - h := query.Get("h") - if h != "500" { - return nil, fmt.Errorf("h not set in URL query properly. Expected '500', got %s", h) - } - w := query.Get("w") - if w != "600" { - return nil, fmt.Errorf("w not set in URL query properly. Expected '600', got %s", w) - } + assert.Check(t, is.Equal(query.Get("h"), expectedHeight)) + assert.Check(t, is.Equal(query.Get("w"), expectedWidth)) return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader([]byte(""))),