Browse Source

Merge pull request #31993 from dccurtis/issue-31816

Fixing ‘docker cp’ to allow new target file name in a host symlinked directory
Yong Tang 7 years ago
parent
commit
149f3aceb2
3 changed files with 85 additions and 26 deletions
  1. 56 0
      pkg/archive/archive_unix_test.go
  2. 28 25
      pkg/archive/changes_test.go
  3. 1 1
      pkg/archive/copy.go

+ 56 - 0
pkg/archive/archive_unix_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"syscall"
 	"testing"
 
@@ -261,3 +262,58 @@ func TestTarUntarWithXattr(t *testing.T) {
 		}
 	}
 }
+
+func TestCopyInfoDestinationPathSymlink(t *testing.T) {
+	tmpDir, _ := getTestTempDirs(t)
+	defer removeAllPaths(tmpDir)
+
+	root := strings.TrimRight(tmpDir, "/") + "/"
+
+	type FileTestData struct {
+		resource FileData
+		file     string
+		expected CopyInfo
+	}
+
+	testData := []FileTestData{
+		//Create a directory: /tmp/archive-copy-test*/dir1
+		//Test will "copy" file1 to dir1
+		{resource: FileData{filetype: Dir, path: "dir1", permissions: 0740}, file: "file1", expected: CopyInfo{Path: root + "dir1/file1", Exists: false, IsDir: false}},
+
+		//Create a symlink directory to dir1: /tmp/archive-copy-test*/dirSymlink -> dir1
+		//Test will "copy" file2 to dirSymlink
+		{resource: FileData{filetype: Symlink, path: "dirSymlink", contents: root + "dir1", permissions: 0600}, file: "file2", expected: CopyInfo{Path: root + "dirSymlink/file2", Exists: false, IsDir: false}},
+
+		//Create a file in tmp directory: /tmp/archive-copy-test*/file1
+		//Test to cover when the full file path already exists.
+		{resource: FileData{filetype: Regular, path: "file1", permissions: 0600}, file: "", expected: CopyInfo{Path: root + "file1", Exists: true}},
+
+		//Create a directory: /tmp/archive-copy*/dir2
+		//Test to cover when the full directory path already exists
+		{resource: FileData{filetype: Dir, path: "dir2", permissions: 0740}, file: "", expected: CopyInfo{Path: root + "dir2", Exists: true, IsDir: true}},
+
+		//Create a symlink to a non-existent target: /tmp/archive-copy*/symlink1 -> noSuchTarget
+		//Negative test to cover symlinking to a target that does not exit
+		{resource: FileData{filetype: Symlink, path: "symlink1", contents: "noSuchTarget", permissions: 0600}, file: "", expected: CopyInfo{Path: root + "noSuchTarget", Exists: false}},
+
+		//Create a file in tmp directory for next test: /tmp/existingfile
+		{resource: FileData{filetype: Regular, path: "existingfile", permissions: 0600}, file: "", expected: CopyInfo{Path: root + "existingfile", Exists: true}},
+
+		//Create a symlink to an existing file: /tmp/archive-copy*/symlink2 -> /tmp/existingfile
+		//Test to cover when the parent directory of a new file is a symlink
+		{resource: FileData{filetype: Symlink, path: "symlink2", contents: "existingfile", permissions: 0600}, file: "", expected: CopyInfo{Path: root + "existingfile", Exists: true}},
+	}
+
+	var dirs []FileData
+	for _, data := range testData {
+		dirs = append(dirs, data.resource)
+	}
+	provisionSampleDir(t, tmpDir, dirs)
+
+	for _, info := range testData {
+		p := filepath.Join(tmpDir, info.resource.path, info.file)
+		ci, err := CopyInfoDestinationPath(p)
+		assert.NoError(t, err)
+		assert.Equal(t, info.expected, ci)
+	}
+}

+ 28 - 25
pkg/archive/changes_test.go

@@ -50,32 +50,35 @@ type FileData struct {
 
 func createSampleDir(t *testing.T, root string) {
 	files := []FileData{
-		{Regular, "file1", "file1\n", 0600},
-		{Regular, "file2", "file2\n", 0666},
-		{Regular, "file3", "file3\n", 0404},
-		{Regular, "file4", "file4\n", 0600},
-		{Regular, "file5", "file5\n", 0600},
-		{Regular, "file6", "file6\n", 0600},
-		{Regular, "file7", "file7\n", 0600},
-		{Dir, "dir1", "", 0740},
-		{Regular, "dir1/file1-1", "file1-1\n", 01444},
-		{Regular, "dir1/file1-2", "file1-2\n", 0666},
-		{Dir, "dir2", "", 0700},
-		{Regular, "dir2/file2-1", "file2-1\n", 0666},
-		{Regular, "dir2/file2-2", "file2-2\n", 0666},
-		{Dir, "dir3", "", 0700},
-		{Regular, "dir3/file3-1", "file3-1\n", 0666},
-		{Regular, "dir3/file3-2", "file3-2\n", 0666},
-		{Dir, "dir4", "", 0700},
-		{Regular, "dir4/file3-1", "file4-1\n", 0666},
-		{Regular, "dir4/file3-2", "file4-2\n", 0666},
-		{Symlink, "symlink1", "target1", 0666},
-		{Symlink, "symlink2", "target2", 0666},
-		{Symlink, "symlink3", root + "/file1", 0666},
-		{Symlink, "symlink4", root + "/symlink3", 0666},
-		{Symlink, "dirSymlink", root + "/dir1", 0740},
-	}
+		{filetype: Regular, path: "file1", contents: "file1\n", permissions: 0600},
+		{filetype: Regular, path: "file2", contents: "file2\n", permissions: 0666},
+		{filetype: Regular, path: "file3", contents: "file3\n", permissions: 0404},
+		{filetype: Regular, path: "file4", contents: "file4\n", permissions: 0600},
+		{filetype: Regular, path: "file5", contents: "file5\n", permissions: 0600},
+		{filetype: Regular, path: "file6", contents: "file6\n", permissions: 0600},
+		{filetype: Regular, path: "file7", contents: "file7\n", permissions: 0600},
+		{filetype: Dir, path: "dir1", contents: "", permissions: 0740},
+		{filetype: Regular, path: "dir1/file1-1", contents: "file1-1\n", permissions: 01444},
+		{filetype: Regular, path: "dir1/file1-2", contents: "file1-2\n", permissions: 0666},
+		{filetype: Dir, path: "dir2", contents: "", permissions: 0700},
+		{filetype: Regular, path: "dir2/file2-1", contents: "file2-1\n", permissions: 0666},
+		{filetype: Regular, path: "dir2/file2-2", contents: "file2-2\n", permissions: 0666},
+		{filetype: Dir, path: "dir3", contents: "", permissions: 0700},
+		{filetype: Regular, path: "dir3/file3-1", contents: "file3-1\n", permissions: 0666},
+		{filetype: Regular, path: "dir3/file3-2", contents: "file3-2\n", permissions: 0666},
+		{filetype: Dir, path: "dir4", contents: "", permissions: 0700},
+		{filetype: Regular, path: "dir4/file3-1", contents: "file4-1\n", permissions: 0666},
+		{filetype: Regular, path: "dir4/file3-2", contents: "file4-2\n", permissions: 0666},
+		{filetype: Symlink, path: "symlink1", contents: "target1", permissions: 0666},
+		{filetype: Symlink, path: "symlink2", contents: "target2", permissions: 0666},
+		{filetype: Symlink, path: "symlink3", contents: root + "/file1", permissions: 0666},
+		{filetype: Symlink, path: "symlink4", contents: root + "/symlink3", permissions: 0666},
+		{filetype: Symlink, path: "dirSymlink", contents: root + "/dir1", permissions: 0740},
+	}
+	provisionSampleDir(t, root, files)
+}
 
+func provisionSampleDir(t *testing.T, root string, files []FileData) {
 	now := time.Now()
 	for _, info := range files {
 		p := path.Join(root, info.path)

+ 1 - 1
pkg/archive/copy.go

@@ -223,7 +223,7 @@ func CopyInfoDestinationPath(path string) (info CopyInfo, err error) {
 		// Ensure destination parent dir exists.
 		dstParent, _ := SplitPathDirEntry(path)
 
-		parentDirStat, err := os.Lstat(dstParent)
+		parentDirStat, err := os.Stat(dstParent)
 		if err != nil {
 			return CopyInfo{}, err
 		}