From 8691a77e441996fef96019b94f299a11b7244080 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Fri, 1 Apr 2016 10:49:04 -0700 Subject: [PATCH] Fix build cache false positives when build context tar contains unnormalized paths If a build context tar has path names of the form 'x/./y', they will be stored in this unnormalized form internally by tarsum. When the builder walks the untarred directory tree and queries hashes for each relative path, it will query paths of the form 'x/y', and they will not be found. To correct this, have tarsum normalize path names by calling Clean. Add a test to detect this caching false positive. Fixes #21715 Signed-off-by: Aaron Lehmann --- builder/tarsum.go | 2 +- integration-cli/docker_api_build_test.go | 59 ++++++++++++++++++++++++ pkg/tarsum/tarsum.go | 3 +- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/builder/tarsum.go b/builder/tarsum.go index 201e3ef7d9..48372cb01c 100644 --- a/builder/tarsum.go +++ b/builder/tarsum.go @@ -138,7 +138,7 @@ func (c *tarSumContext) Walk(root string, walkFn WalkFunc) error { } sum := rel - if tsInfo := c.sums.GetFile(rel); tsInfo != nil { + if tsInfo := c.sums.GetFile(filepath.ToSlash(rel)); tsInfo != nil { sum = tsInfo.Sum() } fi := &HashedFileInfo{PathFileInfo{FileInfo: info, FilePath: fullpath}, sum} diff --git a/integration-cli/docker_api_build_test.go b/integration-cli/docker_api_build_test.go index 49de71c9af..805ec74097 100644 --- a/integration-cli/docker_api_build_test.go +++ b/integration-cli/docker_api_build_test.go @@ -4,6 +4,8 @@ import ( "archive/tar" "bytes" "net/http" + "regexp" + "strings" "github.com/docker/docker/pkg/integration/checker" "github.com/go-check/check" @@ -255,3 +257,60 @@ func (s *DockerSuite) TestBuildApiDockerfileSymlink(c *check.C) { // a nonexistent file. c.Assert(string(out), checker.Contains, "Cannot locate specified Dockerfile: Dockerfile", check.Commentf("Didn't complain about leaving build context")) } + +func (s *DockerSuite) TestBuildApiUnnormalizedTarPaths(c *check.C) { + // Make sure that build context tars with entries of the form + // x/./y don't cause caching false positives. + + buildFromTarContext := func(fileContents []byte) string { + buffer := new(bytes.Buffer) + tw := tar.NewWriter(buffer) + defer tw.Close() + + dockerfile := []byte(`FROM busybox + COPY dir /dir/`) + err := tw.WriteHeader(&tar.Header{ + Name: "Dockerfile", + Size: int64(len(dockerfile)), + }) + //failed to write tar file header + c.Assert(err, checker.IsNil) + + _, err = tw.Write(dockerfile) + // failed to write Dockerfile in tar file content + c.Assert(err, checker.IsNil) + + err = tw.WriteHeader(&tar.Header{ + Name: "dir/./file", + Size: int64(len(fileContents)), + }) + //failed to write tar file header + c.Assert(err, checker.IsNil) + + _, err = tw.Write(fileContents) + // failed to write file contents in tar file content + c.Assert(err, checker.IsNil) + + // failed to close tar archive + c.Assert(tw.Close(), checker.IsNil) + + res, body, err := sockRequestRaw("POST", "/build", buffer, "application/x-tar") + c.Assert(err, checker.IsNil) + c.Assert(res.StatusCode, checker.Equals, http.StatusOK) + + out, err := readBody(body) + c.Assert(err, checker.IsNil) + lines := strings.Split(string(out), "\n") + c.Assert(len(lines), checker.GreaterThan, 1) + c.Assert(lines[len(lines)-2], checker.Matches, ".*Successfully built [0-9a-f]{12}.*") + + re := regexp.MustCompile("Successfully built ([0-9a-f]{12})") + matches := re.FindStringSubmatch(lines[len(lines)-2]) + return matches[1] + } + + imageA := buildFromTarContext([]byte("abc")) + imageB := buildFromTarContext([]byte("def")) + + c.Assert(imageA, checker.Not(checker.Equals), imageB) +} diff --git a/pkg/tarsum/tarsum.go b/pkg/tarsum/tarsum.go index 4dc89bd415..154788db82 100644 --- a/pkg/tarsum/tarsum.go +++ b/pkg/tarsum/tarsum.go @@ -28,6 +28,7 @@ import ( "fmt" "hash" "io" + "path" "strings" ) @@ -235,7 +236,7 @@ func (ts *tarSum) Read(buf []byte) (int, error) { } return n, err } - ts.currentFile = strings.TrimSuffix(strings.TrimPrefix(currentHeader.Name, "./"), "/") + ts.currentFile = path.Clean(currentHeader.Name) if err := ts.encodeHeader(currentHeader); err != nil { return 0, err }