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 <aaron.lehmann@docker.com>
Upstream-commit: 8691a77e441996fef96019b94f299a11b7244080
Component: engine

components/engine/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}

components/engine/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)
+}

components/engine/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
 			}