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 moby#21715

Signed-off-by: Aaron Lehmann <[email protected]>

Author: aaronlehmann

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}

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)
+}

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
 			}