From 9091023db213edee1451f5dc70d155574d99e5fa Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 30 Nov 2021 10:10:42 -0500 Subject: [PATCH] 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 --- copier/copier.go | 4 ++-- copier/copier_test.go | 3 +++ tests/conformance/conformance_test.go | 5 +++++ .../testdata/replace/symlink-with-directory/Dockerfile | 3 +++ .../symlink-with-directory/tree1/directory/file-in-directory | 0 .../replace/symlink-with-directory/tree1/maybe-directory | 1 + .../tree2/maybe-directory/file-in-maybe-directory | 0 7 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 tests/conformance/testdata/replace/symlink-with-directory/Dockerfile create mode 100644 tests/conformance/testdata/replace/symlink-with-directory/tree1/directory/file-in-directory create mode 120000 tests/conformance/testdata/replace/symlink-with-directory/tree1/maybe-directory create mode 100644 tests/conformance/testdata/replace/symlink-with-directory/tree2/maybe-directory/file-in-maybe-directory diff --git a/copier/copier.go b/copier/copier.go index 1823e523840..96cb9230038 100644 --- 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 index 7771e0ba878..566d8fc8444 100644 --- 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 index f3ac3b14e05..fdd71f06c3b 100644 --- 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 new file mode 100644 index 00000000000..11234bdd577 --- /dev/null +++ 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 new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/conformance/testdata/replace/symlink-with-directory/tree1/maybe-directory b/tests/conformance/testdata/replace/symlink-with-directory/tree1/maybe-directory new file mode 120000 index 00000000000..6d0450cc242 --- /dev/null +++ b/tests/conformance/testdata/replace/symlink-with-directory/tree1/maybe-directory @@ -0,0 +1 @@ +directory \ No newline at end of file diff --git a/tests/conformance/testdata/replace/symlink-with-directory/tree2/maybe-directory/file-in-maybe-directory b/tests/conformance/testdata/replace/symlink-with-directory/tree2/maybe-directory/file-in-maybe-directory new file mode 100644 index 00000000000..e69de29bb2d