From af3670124694483aaeead644c4a8d603f2a8373c Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Fri, 21 Sep 2018 14:37:21 +0800 Subject: [PATCH] bugfix: use same memory in ringbuffer ContainerIO' backend will read data into ringbuffer. But the data blocks use the same memory so that the comming data will override the previous one. Need to copy data before push into ringbuffer. Signed-off-by: Wei Fu --- daemon/containerio/container_io.go | 11 ++++- test/api_container_exec_start_test.go | 59 +++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/daemon/containerio/container_io.go b/daemon/containerio/container_io.go index b67899dcd..7c37fdd95 100644 --- a/daemon/containerio/container_io.go +++ b/daemon/containerio/container_io.go @@ -356,7 +356,16 @@ func (cio *ContainerIO) converge(name, id string, in io.Reader) { break } - cover, err := cio.ring.Push(data[:n]) + // FIXME(fuwei): In case that the data slice is reused by the writer, + // we should copy the data before we push it into the ringbuffer. + // The previous data shares the same address with the coming data. + // If we don't copy the data and the previous data isn't consumed by + // ringbuf pop action, the incoming data will override the previous data + // in the ringbuf. + copyData := make([]byte, n) + copy(copyData, data[:n]) + + cover, err := cio.ring.Push(copyData) if err != nil { break } diff --git a/test/api_container_exec_start_test.go b/test/api_container_exec_start_test.go index 334757812..2ff7e3464 100644 --- a/test/api_container_exec_start_test.go +++ b/test/api_container_exec_start_test.go @@ -3,10 +3,13 @@ package main import ( "bufio" "bytes" + "crypto/rand" + "encoding/hex" "io" "net" "strings" + "github.com/alibaba/pouch/apis/types" "github.com/alibaba/pouch/test/environment" "github.com/alibaba/pouch/test/request" @@ -46,6 +49,62 @@ func checkEchoSuccess(c *check.C, tty bool, conn net.Conn, br *bufio.Reader, exp c.Assert(strings.TrimSpace(buf.String()), check.Equals, exp, check.Commentf("Expected %s, got %s", exp, buf.String())) } +func (suite *APIContainerExecStartSuite) TestContainerExecWithLongInput(c *check.C) { + cname := "TestContainerExecWithLongInput" + + CreateBusyboxContainerOk(c, cname) + defer DelContainerForceMultyTime(c, cname) + StartContainerOk(c, cname) + + var execID string + + // create exec + { + // echo the input + obj := map[string]interface{}{ + "Cmd": []string{"cat"}, + "AttachStderr": true, + "AttachStdout": true, + "AttachStdin": true, + "Tty": true, // avoid to use docker stdcopy + } + + body := request.WithJSONBody(obj) + resp, err := request.Post("/containers/"+cname+"/exec", body) + c.Assert(err, check.IsNil) + CheckRespStatus(c, resp, 201) + + var got types.ExecCreateResp + c.Assert(request.DecodeBody(&got, resp.Body), check.IsNil) + execID = got.ID + } + + // start exec + { + obj := map[string]interface{}{ + "Tty": true, // avoid to use docker stdcopy + } + resp, conn, br, err := request.Hijack("/exec/"+execID+"/start", request.WithJSONBody(obj)) + c.Assert(err, check.IsNil) + CheckRespStatus(c, resp, 200) + defer conn.Close() + + // input 256 chars + input := make([]byte, 256) + _, err = rand.Read(input) + c.Assert(err, check.IsNil) + content := hex.EncodeToString(input) + + _, err = conn.Write([]byte(content + "\n")) + c.Assert(err, check.IsNil) + + reader := bufio.NewReader(br) + got, _, err := reader.ReadLine() + c.Assert(err, check.IsNil) + c.Assert(string(got), check.Equals, content) + } +} + // TestContainerExecStartWithoutUpgrade tests start exec without upgrade which will return 200 OK. func (suite *APIContainerExecStartSuite) TestContainerExecStartWithoutUpgrade(c *check.C) { cname := "TestContainerCreateExecStartWithoutUpgrade"