copier.Put: check for is-not-a-directory using lstat, not stat

When checking if something that we want to overwrite with a directory is already a directory or not, use lstat instead of stat. If it's a symbolic link, it's not a directory. This is a subtle behavior change, but it's in line with docker build. Signed-off-by: Nalin Dahyabhai <[email protected]>
containers · Jan 24, 2022 · 9091023 · 9091023
1 parent 867c1bc
commit 9091023
Show file tree

Hide file tree

Showing 7 changed files with 14 additions and 2 deletions.
diff --git a/copier/copier.go b/copier/copier.go
@@ -1660,7 +1660,7 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
 				// only check the length if there wasn't an error, which we'll
 				// check along with errors for other types of entries
 				if err == nil && written != hdr.Size {
-					return errors.Errorf("copier: put: error creating %q: incorrect length (%d != %d)", path, written, hdr.Size)
+					return errors.Errorf("copier: put: error creating regular file %q: incorrect length (%d != %d)", path, written, hdr.Size)
 				}
 			case tar.TypeLink:
 				var linkTarget string
@@ -1733,7 +1733,7 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
 			case tar.TypeDir:
 				if err = os.Mkdir(path, 0700); err != nil && os.IsExist(err) {
 					var st os.FileInfo
-					if st, err = os.Stat(path); err == nil && !st.IsDir() {
+					if st, err = os.Lstat(path); err == nil && !st.IsDir() {
 						// it's not a directory, so remove it and mkdir
 						if err = os.Remove(path); err == nil {
 							err = os.Mkdir(path, 0700)

diff --git a/copier/copier_test.go b/copier/copier_test.go
@@ -528,6 +528,9 @@ func testPut(t *testing.T) {
 		for _, typeFlag := range []byte{tar.TypeReg, tar.TypeLink, tar.TypeSymlink, tar.TypeChar, tar.TypeBlock, tar.TypeFifo} {
 			t.Run(fmt.Sprintf("overwrite=%v,type=%c", overwrite, typeFlag), func(t *testing.T) {
 				archive := makeArchiveSlice([]tar.Header{
+					{Name: "target", Typeflag: tar.TypeSymlink, Mode: 0755, Linkname: "target", ModTime: testDate},
+					{Name: "target", Typeflag: tar.TypeDir, Mode: 0755, ModTime: testDate},
+					{Name: "target", Typeflag: tar.TypeSymlink, Mode: 0755, Linkname: "target", ModTime: testDate},
 					{Name: "target", Typeflag: tar.TypeReg, Size: 123, Mode: 0755, ModTime: testDate},
 					{Name: "test", Typeflag: tar.TypeDir, Size: 0, Mode: 0755, ModTime: testDate},
 					{Name: "test", Typeflag: typeFlag, Size: 0, Mode: 0755, Linkname: "target", ModTime: testDate},

diff --git a/tests/conformance/conformance_test.go b/tests/conformance/conformance_test.go
@@ -2870,4 +2870,9 @@ var internalTestCases = []testCase{
 		contextDir: "dockerignore/exceptions-skip",
 		fsSkip:     []string{"(dir):volume:mtime"},
 	},
+
+	{
+		name:       "replace-symlink-with-directory",
+		contextDir: "replace/symlink-with-directory",
+	},
 }
diff --git a/tests/conformance/testdata/replace/symlink-with-directory/Dockerfile b/tests/conformance/testdata/replace/symlink-with-directory/Dockerfile
@@ -0,0 +1,3 @@
+FROM scratch
+COPY tree1/ /
+COPY tree2/ /
diff --git a/tests/conformance/testdata/replace/symlink-with-directory/tree1/directory/file-in-directory b/tests/conformance/testdata/replace/symlink-with-directory/tree1/directory/file-in-directory
diff --git a/tests/conformance/testdata/replace/symlink-with-directory/tree1/maybe-directory b/tests/conformance/testdata/replace/symlink-with-directory/tree1/maybe-directory
@@ -0,0 +1 @@
+directory
diff --git a/...nce/testdata/replace/symlink-with-directory/tree2/maybe-directory/file-in-maybe-directory b/...nce/testdata/replace/symlink-with-directory/tree2/maybe-directory/file-in-maybe-directory