diff --git a/api/client/cp.go b/api/client/cp.go index 99278adfc1..a36212a79a 100644 --- a/api/client/cp.go +++ b/api/client/cp.go @@ -232,6 +232,20 @@ func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string) (er // Prepare destination copy info by stat-ing the container path. dstInfo := archive.CopyInfo{Path: dstPath} dstStat, err := cli.statContainerPath(dstContainer, dstPath) + + // If the destination is a symbolic link, we should evaluate it. + if err == nil && dstStat.Mode&os.ModeSymlink != 0 { + linkTarget := dstStat.LinkTarget + if !filepath.IsAbs(linkTarget) { + // Join with the parent directory. + dstParent, _ := archive.SplitPathDirEntry(dstPath) + linkTarget = filepath.Join(dstParent, linkTarget) + } + + dstInfo.Path = linkTarget + dstStat, err = cli.statContainerPath(dstContainer, linkTarget) + } + // Ignore any error and assume that the parent directory of the destination // path exists, in which case the copy may still succeed. If there is any // type of conflict (e.g., non-directory overwriting an existing directory @@ -242,15 +256,26 @@ func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string) (er dstInfo.Exists, dstInfo.IsDir = true, dstStat.Mode.IsDir() } - var content io.Reader + var ( + content io.Reader + resolvedDstPath string + ) + if srcPath == "-" { // Use STDIN. content = os.Stdin + resolvedDstPath = dstInfo.Path if !dstInfo.IsDir { return fmt.Errorf("destination %q must be a directory", fmt.Sprintf("%s:%s", dstContainer, dstPath)) } } else { - srcArchive, err := archive.TarResource(srcPath) + // Prepare source copy info. + srcInfo, err := archive.CopyInfoSourcePath(srcPath) + if err != nil { + return err + } + + srcArchive, err := archive.TarResource(srcInfo) if err != nil { return err } @@ -262,12 +287,6 @@ func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string) (er // it to the specified directory in the container we get the disired // copy behavior. - // Prepare source copy info. - srcInfo, err := archive.CopyInfoStatPath(srcPath, true) - if err != nil { - return err - } - // See comments in the implementation of `archive.PrepareArchiveCopy` // for exactly what goes into deciding how and whether the source // archive needs to be altered for the correct copy behavior when it is @@ -280,12 +299,12 @@ func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string) (er } defer preparedArchive.Close() - dstPath = dstDir + resolvedDstPath = dstDir content = preparedArchive } query := make(url.Values, 2) - query.Set("path", filepath.ToSlash(dstPath)) // Normalize the paths used in the API. + query.Set("path", filepath.ToSlash(resolvedDstPath)) // Normalize the paths used in the API. // Do not allow for an existing directory to be overwritten by a non-directory and vice versa. query.Set("noOverwriteDirNonDir", "true") diff --git a/api/types/types.go b/api/types/types.go index c99e5a9a45..844ddd0f24 100644 --- a/api/types/types.go +++ b/api/types/types.go @@ -130,14 +130,13 @@ type CopyConfig struct { // ContainerPathStat is used to encode the header from // GET /containers/{name:.*}/archive -// "name" is the file or directory name. -// "path" is the absolute path to the resource in the container. +// "name" is basename of the resource. type ContainerPathStat struct { - Name string `json:"name"` - Path string `json:"path"` - Size int64 `json:"size"` - Mode os.FileMode `json:"mode"` - Mtime time.Time `json:"mtime"` + Name string `json:"name"` + Size int64 `json:"size"` + Mode os.FileMode `json:"mode"` + Mtime time.Time `json:"mtime"` + LinkTarget string `json:"linkTarget"` } // GET "/containers/{name:.*}/top" diff --git a/daemon/archive.go b/daemon/archive.go index 272112ced6..c2e47187f0 100644 --- a/daemon/archive.go +++ b/daemon/archive.go @@ -70,6 +70,66 @@ func (daemon *Daemon) ContainerExtractToDir(name, path string, noOverwriteDirNon return container.ExtractToDir(path, noOverwriteDirNonDir, content) } +// resolvePath resolves the given path in the container to a resource on the +// host. Returns a resolved path (absolute path to the resource on the host), +// the absolute path to the resource relative to the container's rootfs, and +// a error if the path points to outside the container's rootfs. +func (container *Container) resolvePath(path string) (resolvedPath, absPath string, err error) { + // Consider the given path as an absolute path in the container. + absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join(string(filepath.Separator), path), path) + + // Split the absPath into its Directory and Base components. We will + // resolve the dir in the scope of the container then append the base. + dirPath, basePath := filepath.Split(absPath) + + resolvedDirPath, err := container.GetResourcePath(dirPath) + if err != nil { + return "", "", err + } + + // resolvedDirPath will have been cleaned (no trailing path separators) so + // we can manually join it with the base path element. + resolvedPath = resolvedDirPath + string(filepath.Separator) + basePath + + return resolvedPath, absPath, nil +} + +// statPath is the unexported version of StatPath. Locks and mounts should +// be aquired before calling this method and the given path should be fully +// resolved to a path on the host corresponding to the given absolute path +// inside the container. +func (container *Container) statPath(resolvedPath, absPath string) (stat *types.ContainerPathStat, err error) { + lstat, err := os.Lstat(resolvedPath) + if err != nil { + return nil, err + } + + var linkTarget string + if lstat.Mode()&os.ModeSymlink != 0 { + // Fully evaluate the symlink in the scope of the container rootfs. + hostPath, err := container.GetResourcePath(absPath) + if err != nil { + return nil, err + } + + linkTarget, err = filepath.Rel(container.basefs, hostPath) + if err != nil { + return nil, err + } + + // Make it an absolute path. + linkTarget = filepath.Join(string(filepath.Separator), linkTarget) + } + + return &types.ContainerPathStat{ + Name: filepath.Base(absPath), + Size: lstat.Size(), + Mode: lstat.Mode(), + Mtime: lstat.ModTime(), + LinkTarget: linkTarget, + }, nil +} + // StatPath stats the filesystem resource at the specified path in this // container. Returns stat info about the resource. func (container *Container) StatPath(path string) (stat *types.ContainerPathStat, err error) { @@ -87,39 +147,12 @@ func (container *Container) StatPath(path string) (stat *types.ContainerPathStat return nil, err } - // Consider the given path as an absolute path in the container. - absPath := path - if !filepath.IsAbs(absPath) { - absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join(string(os.PathSeparator), path), path) - } - - resolvedPath, err := container.GetResourcePath(absPath) + resolvedPath, absPath, err := container.resolvePath(path) if err != nil { return nil, err } - // A trailing "." or separator has important meaning. For example, if - // `"foo"` is a symlink to some directory `"dir"`, then `os.Lstat("foo")` - // will stat the link itself, while `os.Lstat("foo/")` will stat the link - // target. If the basename of the path is ".", it means to archive the - // contents of the directory with "." as the first path component rather - // than the name of the directory. This would cause extraction of the - // archive to *not* make another directory, but instead use the current - // directory. - resolvedPath = archive.PreserveTrailingDotOrSeparator(resolvedPath, absPath) - - lstat, err := os.Lstat(resolvedPath) - if err != nil { - return nil, err - } - - return &types.ContainerPathStat{ - Name: lstat.Name(), - Path: absPath, - Size: lstat.Size(), - Mode: lstat.Mode(), - Mtime: lstat.ModTime(), - }, nil + return container.statPath(resolvedPath, absPath) } // ArchivePath creates an archive of the filesystem resource at the specified @@ -154,41 +187,25 @@ func (container *Container) ArchivePath(path string) (content io.ReadCloser, sta return nil, nil, err } - // Consider the given path as an absolute path in the container. - absPath := path - if !filepath.IsAbs(absPath) { - absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join(string(os.PathSeparator), path), path) - } - - resolvedPath, err := container.GetResourcePath(absPath) + resolvedPath, absPath, err := container.resolvePath(path) if err != nil { return nil, nil, err } - // A trailing "." or separator has important meaning. For example, if - // `"foo"` is a symlink to some directory `"dir"`, then `os.Lstat("foo")` - // will stat the link itself, while `os.Lstat("foo/")` will stat the link - // target. If the basename of the path is ".", it means to archive the - // contents of the directory with "." as the first path component rather - // than the name of the directory. This would cause extraction of the - // archive to *not* make another directory, but instead use the current - // directory. - resolvedPath = archive.PreserveTrailingDotOrSeparator(resolvedPath, absPath) - - lstat, err := os.Lstat(resolvedPath) + stat, err = container.statPath(resolvedPath, absPath) if err != nil { return nil, nil, err } - stat = &types.ContainerPathStat{ - Name: lstat.Name(), - Path: absPath, - Size: lstat.Size(), - Mode: lstat.Mode(), - Mtime: lstat.ModTime(), - } - - data, err := archive.TarResource(resolvedPath) + // We need to rebase the archive entries if the last element of the + // resolved path was a symlink that was evaluated and is now different + // than the requested path. For example, if the given path was "/foo/bar/", + // but it resolved to "/var/lib/docker/containers/{id}/foo/baz/", we want + // to ensure that the archive entries start with "bar" and not "baz". This + // also catches the case when the root directory of the container is + // requested: we want the archive entries to start with "/" and not the + // container ID. + data, err := archive.TarResourceRebase(resolvedPath, filepath.Base(absPath)) if err != nil { return nil, nil, err } @@ -227,27 +244,21 @@ func (container *Container) ExtractToDir(path string, noOverwriteDirNonDir bool, return err } - // Consider the given path as an absolute path in the container. - absPath := path - if !filepath.IsAbs(absPath) { - absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join(string(os.PathSeparator), path), path) - } + // The destination path needs to be resolved to a host path, with all + // symbolic links followed in the scope of the container's rootfs. Note + // that we do not use `container.resolvePath(path)` here because we need + // to also evaluate the last path element if it is a symlink. This is so + // that you can extract an archive to a symlink that points to a directory. + // Consider the given path as an absolute path in the container. + absPath := archive.PreserveTrailingDotOrSeparator(filepath.Join(string(filepath.Separator), path), path) + + // This will evaluate the last path element if it is a symlink. resolvedPath, err := container.GetResourcePath(absPath) if err != nil { return err } - // A trailing "." or separator has important meaning. For example, if - // `"foo"` is a symlink to some directory `"dir"`, then `os.Lstat("foo")` - // will stat the link itself, while `os.Lstat("foo/")` will stat the link - // target. If the basename of the path is ".", it means to archive the - // contents of the directory with "." as the first path component rather - // than the name of the directory. This would cause extraction of the - // archive to *not* make another directory, but instead use the current - // directory. - resolvedPath = archive.PreserveTrailingDotOrSeparator(resolvedPath, absPath) - stat, err := os.Lstat(resolvedPath) if err != nil { return err @@ -257,15 +268,20 @@ func (container *Container) ExtractToDir(path string, noOverwriteDirNonDir bool, return ErrExtractPointNotDirectory } + // Need to check if the path is in a volume. If it is, it cannot be in a + // read-only volume. If it is not in a volume, the container cannot be + // configured with a read-only rootfs. + + // Use the resolved path relative to the container rootfs as the new + // absPath. This way we fully follow any symlinks in a volume that may + // lead back outside the volume. baseRel, err := filepath.Rel(container.basefs, resolvedPath) if err != nil { return err } - absPath = filepath.Join(string(os.PathSeparator), baseRel) + // Make it an absolute path. + absPath = filepath.Join(string(filepath.Separator), baseRel) - // Need to check if the path is in a volume. If it is, it cannot be in a - // read-only volume. If it is not in a volume, the container cannot be - // configured with a read-only rootfs. toVolume, err := checkIfPathIsInAVolume(container, absPath) if err != nil { return err diff --git a/docs/reference/api/docker_remote_api_v1.20.md b/docs/reference/api/docker_remote_api_v1.20.md index 49934708a0..40c58af0e7 100644 --- a/docs/reference/api/docker_remote_api_v1.20.md +++ b/docs/reference/api/docker_remote_api_v1.20.md @@ -1109,7 +1109,7 @@ Query Parameters: HTTP/1.1 200 OK Content-Type: application/x-tar - X-Docker-Container-Path-Stat: eyJuYW1lIjoicm9vdCIsInBhdGgiOiIvcm9vdCIsInNpemUiOjQwOTYsIm1vZGUiOjIxNDc0ODQwOTYsIm10aW1lIjoiMjAxNC0wMi0yN1QyMDo1MToyM1oifQ== + X-Docker-Container-Path-Stat: eyJuYW1lIjoicm9vdCIsInNpemUiOjQwOTYsIm1vZGUiOjIxNDc0ODQwOTYsIm10aW1lIjoiMjAxNC0wMi0yN1QyMDo1MToyM1oiLCJsaW5rVGFyZ2V0IjoiIn0= {{ TAR STREAM }} @@ -1120,10 +1120,10 @@ JSON object (whitespace added for readability): { "name": "root", - "path": "/root", "size": 4096, "mode": 2147484096, - "mtime": "2014-02-27T20:51:23Z" + "mtime": "2014-02-27T20:51:23Z", + "linkTarget": "" } A `HEAD` request can also be made to this endpoint if only this information is diff --git a/docs/reference/api/docker_remote_api_v1.21.md b/docs/reference/api/docker_remote_api_v1.21.md index 802708265f..05e802367f 100644 --- a/docs/reference/api/docker_remote_api_v1.21.md +++ b/docs/reference/api/docker_remote_api_v1.21.md @@ -1109,7 +1109,7 @@ Query Parameters: HTTP/1.1 200 OK Content-Type: application/x-tar - X-Docker-Container-Path-Stat: eyJuYW1lIjoicm9vdCIsInBhdGgiOiIvcm9vdCIsInNpemUiOjQwOTYsIm1vZGUiOjIxNDc0ODQwOTYsIm10aW1lIjoiMjAxNC0wMi0yN1QyMDo1MToyM1oifQ== + X-Docker-Container-Path-Stat: eyJuYW1lIjoicm9vdCIsInNpemUiOjQwOTYsIm1vZGUiOjIxNDc0ODQwOTYsIm10aW1lIjoiMjAxNC0wMi0yN1QyMDo1MToyM1oiLCJsaW5rVGFyZ2V0IjoiIn0= {{ TAR STREAM }} @@ -1120,10 +1120,10 @@ JSON object (whitespace added for readability): { "name": "root", - "path": "/root", "size": 4096, "mode": 2147484096, - "mtime": "2014-02-27T20:51:23Z" + "mtime": "2014-02-27T20:51:23Z", + "linkTarget": "" } A `HEAD` request can also be made to this endpoint if only this information is diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 07c5abf987..44c172c0ea 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -4,9 +4,11 @@ import ( "archive/tar" "bytes" "encoding/json" + "fmt" "io" "net/http" "net/http/httputil" + "net/url" "os" "strconv" "strings" @@ -1697,3 +1699,35 @@ func (s *DockerSuite) TestContainersApiCreateNoHostConfig118(c *check.C) { c.Assert(err, check.IsNil) c.Assert(status, check.Equals, http.StatusCreated) } + +// Ensure an error occurs when you have a container read-only rootfs but you +// extract an archive to a symlink in a writable volume which points to a +// directory outside of the volume. +func (s *DockerSuite) TestPutContainerArchiveErrSymlinkInVolumeToReadOnlyRootfs(c *check.C) { + testRequires(c, SameHostDaemon) // Requires local volume mount bind. + + testVol := getTestDir(c, "test-put-container-archive-err-symlink-in-volume-to-read-only-rootfs-") + defer os.RemoveAll(testVol) + + makeTestContentInDir(c, testVol) + + cID := makeTestContainer(c, testContainerOptions{ + readOnly: true, + volumes: defaultVolumes(testVol), // Our bind mount is at /vol2 + }) + defer deleteContainer(cID) + + // Attempt to extract to a symlink in the volume which points to a + // directory outside the volume. This should cause an error because the + // rootfs is read-only. + query := make(url.Values, 1) + query.Set("path", "/vol2/symlinkToAbsDir") + urlPath := fmt.Sprintf("/v1.20/containers/%s/archive?%s", cID, query.Encode()) + + statusCode, body, err := sockRequest("PUT", urlPath, nil) + c.Assert(err, check.IsNil) + + if !isCpCannotCopyReadOnly(fmt.Errorf(string(body))) { + c.Fatalf("expected ErrContainerRootfsReadonly error, but got %d: %s", statusCode, string(body)) + } +} diff --git a/integration-cli/docker_cli_cp_from_container_test.go b/integration-cli/docker_cli_cp_from_container_test.go index 3dc8bab195..ddcab51c51 100644 --- a/integration-cli/docker_cli_cp_from_container_test.go +++ b/integration-cli/docker_cli_cp_from_container_test.go @@ -130,6 +130,114 @@ func (s *DockerSuite) TestCpFromErrDstNotDir(c *check.C) { } } +// Check that copying from a container to a local symlink copies to the symlink +// target and does not overwrite the local symlink itself. +func (s *DockerSuite) TestCpFromSymlinkDestination(c *check.C) { + cID := makeTestContainer(c, testContainerOptions{addContent: true}) + defer deleteContainer(cID) + + tmpDir := getTestDir(c, "test-cp-from-err-dst-not-dir") + defer os.RemoveAll(tmpDir) + + makeTestContentInDir(c, tmpDir) + + // First, copy a file from the container to a symlink to a file. This + // should overwrite the symlink target contents with the source contents. + srcPath := containerCpPath(cID, "/file2") + dstPath := cpPath(tmpDir, "symlinkToFile1") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, dstPath, "file1"); err != nil { + c.Fatal(err) + } + + // The file should have the contents of "file2" now. + if err := fileContentEquals(c, cpPath(tmpDir, "file1"), "file2\n"); err != nil { + c.Fatal(err) + } + + // Next, copy a file from the container to a symlink to a directory. This + // should copy the file into the symlink target directory. + dstPath = cpPath(tmpDir, "symlinkToDir1") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, dstPath, "dir1"); err != nil { + c.Fatal(err) + } + + // The file should have the contents of "file2" now. + if err := fileContentEquals(c, cpPath(tmpDir, "file2"), "file2\n"); err != nil { + c.Fatal(err) + } + + // Next, copy a file from the container to a symlink to a file that does + // not exist (a broken symlink). This should create the target file with + // the contents of the source file. + dstPath = cpPath(tmpDir, "brokenSymlinkToFileX") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, dstPath, "fileX"); err != nil { + c.Fatal(err) + } + + // The file should have the contents of "file2" now. + if err := fileContentEquals(c, cpPath(tmpDir, "fileX"), "file2\n"); err != nil { + c.Fatal(err) + } + + // Next, copy a directory from the container to a symlink to a local + // directory. This should copy the directory into the symlink target + // directory and not modify the symlink. + srcPath = containerCpPath(cID, "/dir2") + dstPath = cpPath(tmpDir, "symlinkToDir1") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, dstPath, "dir1"); err != nil { + c.Fatal(err) + } + + // The directory should now contain a copy of "dir2". + if err := fileContentEquals(c, cpPath(tmpDir, "dir1/dir2/file2-1"), "file2-1\n"); err != nil { + c.Fatal(err) + } + + // Next, copy a directory from the container to a symlink to a local + // directory that does not exist (a broken symlink). This should create + // the target as a directory with the contents of the source directory. It + // should not modify the symlink. + dstPath = cpPath(tmpDir, "brokenSymlinkToDirX") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, dstPath, "dirX"); err != nil { + c.Fatal(err) + } + + // The "dirX" directory should now be a copy of "dir2". + if err := fileContentEquals(c, cpPath(tmpDir, "dirX/file2-1"), "file2-1\n"); err != nil { + c.Fatal(err) + } +} + // Possibilities are reduced to the remaining 10 cases: // // case | srcIsDir | onlyDirContents | dstExists | dstIsDir | dstTrSep | action diff --git a/integration-cli/docker_cli_cp_test.go b/integration-cli/docker_cli_cp_test.go index 03c0a4a63e..64ae0b5d82 100644 --- a/integration-cli/docker_cli_cp_test.go +++ b/integration-cli/docker_cli_cp_test.go @@ -250,29 +250,185 @@ func (s *DockerSuite) TestCpAbsoluteSymlink(c *check.C) { c.Fatal(err) } - tmpname := filepath.Join(tmpdir, cpTestName) + tmpname := filepath.Join(tmpdir, "container_path") defer os.RemoveAll(tmpdir) path := path.Join("/", "container_path") dockerCmd(c, "cp", cleanedContainerID+":"+path, tmpdir) - file, _ := os.Open(tmpname) - defer file.Close() - - test, err := ioutil.ReadAll(file) + // We should have copied a symlink *NOT* the file itself! + linkTarget, err := os.Readlink(tmpname) if err != nil { c.Fatal(err) } - if string(test) == cpHostContents { - c.Errorf("output matched host file -- absolute symlink can escape container rootfs") + if linkTarget != filepath.FromSlash(cpFullPath) { + c.Errorf("symlink target was %q, but expected: %q", linkTarget, cpFullPath) + } +} + +// Check that symlinks to a directory behave as expected when copying one from +// a container. +func (s *DockerSuite) TestCpFromSymlinkToDirectory(c *check.C) { + out, exitCode := dockerCmd(c, "run", "-d", "busybox", "/bin/sh", "-c", "mkdir -p '"+cpTestPath+"' && echo -n '"+cpContainerContents+"' > "+cpFullPath+" && ln -s "+cpTestPathParent+" /dir_link") + if exitCode != 0 { + c.Fatal("failed to create a container", out) } - if string(test) != cpContainerContents { - c.Errorf("output doesn't match the input for absolute symlink") + cleanedContainerID := strings.TrimSpace(out) + + out, _ = dockerCmd(c, "wait", cleanedContainerID) + if strings.TrimSpace(out) != "0" { + c.Fatal("failed to set up container", out) } + testDir, err := ioutil.TempDir("", "test-cp-from-symlink-to-dir-") + if err != nil { + c.Fatal(err) + } + defer os.RemoveAll(testDir) + + // This copy command should copy the symlink, not the target, into the + // temporary directory. + dockerCmd(c, "cp", cleanedContainerID+":"+"/dir_link", testDir) + + expectedPath := filepath.Join(testDir, "dir_link") + linkTarget, err := os.Readlink(expectedPath) + if err != nil { + c.Fatalf("unable to read symlink at %q: %v", expectedPath, err) + } + + if linkTarget != filepath.FromSlash(cpTestPathParent) { + c.Errorf("symlink target was %q, but expected: %q", linkTarget, cpTestPathParent) + } + + os.Remove(expectedPath) + + // This copy command should resolve the symlink (note the trailing + // seperator), copying the target into the temporary directory. + dockerCmd(c, "cp", cleanedContainerID+":"+"/dir_link/", testDir) + + // It *should not* have copied the directory using the target's name, but + // used the given name instead. + unexpectedPath := filepath.Join(testDir, cpTestPathParent) + if stat, err := os.Lstat(unexpectedPath); err == nil { + c.Fatalf("target name was copied: %q - %q", stat.Mode(), stat.Name()) + } + + // It *should* have copied the directory using the asked name "dir_link". + stat, err := os.Lstat(expectedPath) + if err != nil { + c.Fatalf("unable to stat resource at %q: %v", expectedPath, err) + } + + if !stat.IsDir() { + c.Errorf("should have copied a directory but got %q instead", stat.Mode()) + } +} + +// Check that symlinks to a directory behave as expected when copying one to a +// container. +func (s *DockerSuite) TestCpToSymlinkToDirectory(c *check.C) { + testRequires(c, SameHostDaemon) // Requires local volume mount bind. + + testVol, err := ioutil.TempDir("", "test-cp-to-symlink-to-dir-") + if err != nil { + c.Fatal(err) + } + defer os.RemoveAll(testVol) + + // Create a test container with a local volume. We will test by copying + // to the volume path in the container which we can then verify locally. + out, exitCode := dockerCmd(c, "create", "-v", testVol+":/testVol", "busybox") + if exitCode != 0 { + c.Fatal("failed to create a container", out) + } + + cleanedContainerID := strings.TrimSpace(out) + + // Create a temp directory to hold a test file nested in a direcotry. + testDir, err := ioutil.TempDir("", "test-cp-to-symlink-to-dir-") + if err != nil { + c.Fatal(err) + } + defer os.RemoveAll(testDir) + + // This file will be at "/testDir/some/path/test" and will be copied into + // the test volume later. + hostTestFilename := filepath.Join(testDir, cpFullPath) + if err := os.MkdirAll(filepath.Dir(hostTestFilename), os.FileMode(0700)); err != nil { + c.Fatal(err) + } + if err := ioutil.WriteFile(hostTestFilename, []byte(cpHostContents), os.FileMode(0600)); err != nil { + c.Fatal(err) + } + + // Now create another temp directory to hold a symlink to the + // "/testDir/some" directory. + linkDir, err := ioutil.TempDir("", "test-cp-to-symlink-to-dir-") + if err != nil { + c.Fatal(err) + } + defer os.RemoveAll(linkDir) + + // Then symlink "/linkDir/dir_link" to "/testdir/some". + linkTarget := filepath.Join(testDir, cpTestPathParent) + localLink := filepath.Join(linkDir, "dir_link") + if err := os.Symlink(linkTarget, localLink); err != nil { + c.Fatal(err) + } + + // Now copy that symlink into the test volume in the container. + dockerCmd(c, "cp", localLink, cleanedContainerID+":/testVol") + + // This copy command should have copied the symlink *not* the target. + expectedPath := filepath.Join(testVol, "dir_link") + actualLinkTarget, err := os.Readlink(expectedPath) + if err != nil { + c.Fatalf("unable to read symlink at %q: %v", expectedPath, err) + } + + if actualLinkTarget != linkTarget { + c.Errorf("symlink target was %q, but expected: %q", actualLinkTarget, linkTarget) + } + + // Good, now remove that copied link for the next test. + os.Remove(expectedPath) + + // This copy command should resolve the symlink (note the trailing + // seperator), copying the target into the test volume directory in the + // container. + dockerCmd(c, "cp", localLink+"/", cleanedContainerID+":/testVol") + + // It *should not* have copied the directory using the target's name, but + // used the given name instead. + unexpectedPath := filepath.Join(testVol, cpTestPathParent) + if stat, err := os.Lstat(unexpectedPath); err == nil { + c.Fatalf("target name was copied: %q - %q", stat.Mode(), stat.Name()) + } + + // It *should* have copied the directory using the asked name "dir_link". + stat, err := os.Lstat(expectedPath) + if err != nil { + c.Fatalf("unable to stat resource at %q: %v", expectedPath, err) + } + + if !stat.IsDir() { + c.Errorf("should have copied a directory but got %q instead", stat.Mode()) + } + + // And this directory should contain the file copied from the host at the + // expected location: "/testVol/dir_link/path/test" + expectedFilepath := filepath.Join(testVol, "dir_link/path/test") + fileContents, err := ioutil.ReadFile(expectedFilepath) + if err != nil { + c.Fatal(err) + } + + if string(fileContents) != cpHostContents { + c.Fatalf("file contains %q but expected %q", string(fileContents), cpHostContents) + } } // Test for #5619 diff --git a/integration-cli/docker_cli_cp_to_container_test.go b/integration-cli/docker_cli_cp_to_container_test.go index a924c56144..9cb613f5a4 100644 --- a/integration-cli/docker_cli_cp_to_container_test.go +++ b/integration-cli/docker_cli_cp_to_container_test.go @@ -146,6 +146,118 @@ func (s *DockerSuite) TestCpToErrDstNotDir(c *check.C) { } } +// Check that copying from a local path to a symlink in a container copies to +// the symlink target and does not overwrite the container symlink itself. +func (s *DockerSuite) TestCpToSymlinkDestination(c *check.C) { + testRequires(c, SameHostDaemon) // Requires local volume mount bind. + + testVol := getTestDir(c, "test-cp-to-symlink-destination-") + defer os.RemoveAll(testVol) + + makeTestContentInDir(c, testVol) + + cID := makeTestContainer(c, testContainerOptions{ + volumes: defaultVolumes(testVol), // Our bind mount is at /vol2 + }) + defer deleteContainer(cID) + + // First, copy a local file to a symlink to a file in the container. This + // should overwrite the symlink target contents with the source contents. + srcPath := cpPath(testVol, "file2") + dstPath := containerCpPath(cID, "/vol2/symlinkToFile1") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, cpPath(testVol, "symlinkToFile1"), "file1"); err != nil { + c.Fatal(err) + } + + // The file should have the contents of "file2" now. + if err := fileContentEquals(c, cpPath(testVol, "file1"), "file2\n"); err != nil { + c.Fatal(err) + } + + // Next, copy a local file to a symlink to a directory in the container. + // This should copy the file into the symlink target directory. + dstPath = containerCpPath(cID, "/vol2/symlinkToDir1") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, cpPath(testVol, "symlinkToDir1"), "dir1"); err != nil { + c.Fatal(err) + } + + // The file should have the contents of "file2" now. + if err := fileContentEquals(c, cpPath(testVol, "file2"), "file2\n"); err != nil { + c.Fatal(err) + } + + // Next, copy a file to a symlink to a file that does not exist (a broken + // symlink) in the container. This should create the target file with the + // contents of the source file. + dstPath = containerCpPath(cID, "/vol2/brokenSymlinkToFileX") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, cpPath(testVol, "brokenSymlinkToFileX"), "fileX"); err != nil { + c.Fatal(err) + } + + // The file should have the contents of "file2" now. + if err := fileContentEquals(c, cpPath(testVol, "fileX"), "file2\n"); err != nil { + c.Fatal(err) + } + + // Next, copy a local directory to a symlink to a directory in the + // container. This should copy the directory into the symlink target + // directory and not modify the symlink. + srcPath = cpPath(testVol, "/dir2") + dstPath = containerCpPath(cID, "/vol2/symlinkToDir1") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, cpPath(testVol, "symlinkToDir1"), "dir1"); err != nil { + c.Fatal(err) + } + + // The directory should now contain a copy of "dir2". + if err := fileContentEquals(c, cpPath(testVol, "dir1/dir2/file2-1"), "file2-1\n"); err != nil { + c.Fatal(err) + } + + // Next, copy a local directory to a symlink to a local directory that does + // not exist (a broken symlink) in the container. This should create the + // target as a directory with the contents of the source directory. It + // should not modify the symlink. + dstPath = containerCpPath(cID, "/vol2/brokenSymlinkToDirX") + + if err := runDockerCp(c, srcPath, dstPath); err != nil { + c.Fatalf("unexpected error %T: %s", err, err) + } + + // The symlink should not have been modified. + if err := symlinkTargetEquals(c, cpPath(testVol, "brokenSymlinkToDirX"), "dirX"); err != nil { + c.Fatal(err) + } + + // The "dirX" directory should now be a copy of "dir2". + if err := fileContentEquals(c, cpPath(testVol, "dirX/file2-1"), "file2-1\n"); err != nil { + c.Fatal(err) + } +} + // Possibilities are reduced to the remaining 10 cases: // // case | srcIsDir | onlyDirContents | dstExists | dstIsDir | dstTrSep | action diff --git a/integration-cli/docker_cli_cp_utils.go b/integration-cli/docker_cli_cp_utils.go index 8c637391b3..4642922e00 100644 --- a/integration-cli/docker_cli_cp_utils.go +++ b/integration-cli/docker_cli_cp_utils.go @@ -74,8 +74,11 @@ var defaultFileData = []fileData{ {ftRegular, "dir4/file3-1", "file4-1"}, {ftRegular, "dir4/file3-2", "file4-2"}, {ftDir, "dir5", ""}, - {ftSymlink, "symlink1", "target1"}, - {ftSymlink, "symlink2", "target2"}, + {ftSymlink, "symlinkToFile1", "file1"}, + {ftSymlink, "symlinkToDir1", "dir1"}, + {ftSymlink, "brokenSymlinkToFileX", "fileX"}, + {ftSymlink, "brokenSymlinkToDirX", "dirX"}, + {ftSymlink, "symlinkToAbsDir", "/root"}, } func defaultMkContentCommand() string { @@ -268,6 +271,21 @@ func fileContentEquals(c *check.C, filename, contents string) (err error) { return } +func symlinkTargetEquals(c *check.C, symlink, expectedTarget string) (err error) { + c.Logf("checking that the symlink %q points to %q\n", symlink, expectedTarget) + + actualTarget, err := os.Readlink(symlink) + if err != nil { + return err + } + + if actualTarget != expectedTarget { + return fmt.Errorf("symlink target points to %q not %q", actualTarget, expectedTarget) + } + + return nil +} + func containerStartOutputEquals(c *check.C, cID, contents string) (err error) { c.Logf("checking that container %q start output contains %q\n", cID, contents) diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index 11a707d20e..3f3c819ac1 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -37,11 +37,13 @@ type ( Compression Compression NoLchown bool ChownOpts *TarChownOptions - Name string IncludeSourceDir bool // When unpacking, specifies whether overwriting a directory with a // non-directory is allowed and vice versa. NoOverwriteDirNonDir bool + // For each include when creating an archive, the included name will be + // replaced with the matching name from this map. + RebaseNames map[string]string } // Archiver allows the reuse of most utility functions of this package @@ -454,8 +456,9 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) seen := make(map[string]bool) - var renamedRelFilePath string // For when tar.Options.Name is set for _, include := range options.IncludeFiles { + rebaseName := options.RebaseNames[include] + // We can't use filepath.Join(srcPath, include) because this will // clean away a trailing "." or "/" which may be important. walkRoot := strings.Join([]string{srcPath, include}, string(filepath.Separator)) @@ -503,14 +506,17 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) } seen[relFilePath] = true - // TODO Windows: Verify if this needs to be os.Pathseparator - // Rename the base resource - if options.Name != "" && filePath == srcPath+"/"+filepath.Base(relFilePath) { - renamedRelFilePath = relFilePath - } - // Set this to make sure the items underneath also get renamed - if options.Name != "" { - relFilePath = strings.Replace(relFilePath, renamedRelFilePath, options.Name, 1) + // Rename the base resource. + if rebaseName != "" { + var replacement string + if rebaseName != string(filepath.Separator) { + // Special case the root directory to replace with an + // empty string instead so that we don't end up with + // double slashes in the paths. + replacement = rebaseName + } + + relFilePath = strings.Replace(relFilePath, include, replacement, 1) } if err := ta.addTarFile(filePath, relFilePath); err != nil { diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go index b93c76cdad..b9bfc23906 100644 --- a/pkg/archive/archive_test.go +++ b/pkg/archive/archive_test.go @@ -695,7 +695,7 @@ func TestTarWithOptions(t *testing.T) { {&TarOptions{ExcludePatterns: []string{"2"}}, 1}, {&TarOptions{ExcludePatterns: []string{"1", "folder*"}}, 2}, {&TarOptions{IncludeFiles: []string{"1", "1"}}, 2}, - {&TarOptions{Name: "test", IncludeFiles: []string{"1"}}, 4}, + {&TarOptions{IncludeFiles: []string{"1"}, RebaseNames: map[string]string{"1": "test"}}, 4}, } for _, testCase := range cases { changes, err := tarUntar(t, origin, testCase.opts) diff --git a/pkg/archive/copy.go b/pkg/archive/copy.go index 93c81e8465..90b3e81134 100644 --- a/pkg/archive/copy.go +++ b/pkg/archive/copy.go @@ -6,7 +6,6 @@ import ( "io" "io/ioutil" "os" - "path" "path/filepath" "strings" @@ -64,34 +63,33 @@ func SpecifiesCurrentDir(path string) bool { return filepath.Base(path) == "." } -// SplitPathDirEntry splits the given path between its -// parent directory and its basename in that directory. -func SplitPathDirEntry(localizedPath string) (dir, base string) { - normalizedPath := filepath.ToSlash(localizedPath) - vol := filepath.VolumeName(normalizedPath) - normalizedPath = normalizedPath[len(vol):] +// SplitPathDirEntry splits the given path between its directory name and its +// basename by first cleaning the path but preserves a trailing "." if the +// original path specified the current directory. +func SplitPathDirEntry(path string) (dir, base string) { + cleanedPath := filepath.Clean(path) - if normalizedPath == "/" { - // Specifies the root path. - return filepath.FromSlash(vol + normalizedPath), "." + if SpecifiesCurrentDir(path) { + cleanedPath += string(filepath.Separator) + "." } - trimmedPath := vol + strings.TrimRight(normalizedPath, "/") - - dir = filepath.FromSlash(path.Dir(trimmedPath)) - base = filepath.FromSlash(path.Base(trimmedPath)) - - return dir, base + return filepath.Dir(cleanedPath), filepath.Base(cleanedPath) } -// TarResource archives the resource at the given sourcePath into a Tar +// TarResource archives the resource described by the given CopyInfo to a Tar // archive. A non-nil error is returned if sourcePath does not exist or is // asserted to be a directory but exists as another type of file. // // This function acts as a convenient wrapper around TarWithOptions, which // requires a directory as the source path. TarResource accepts either a // directory or a file path and correctly sets the Tar options. -func TarResource(sourcePath string) (content Archive, err error) { +func TarResource(sourceInfo CopyInfo) (content Archive, err error) { + return TarResourceRebase(sourceInfo.Path, sourceInfo.RebaseName) +} + +// TarResourceRebase is like TarResource but renames the first path element of +// items in the resulting tar archive to match the given rebaseName if not "". +func TarResourceRebase(sourcePath, rebaseName string) (content Archive, err error) { if _, err = os.Lstat(sourcePath); err != nil { // Catches the case where the source does not exist or is not a // directory if asserted to be a directory, as this also causes an @@ -99,22 +97,6 @@ func TarResource(sourcePath string) (content Archive, err error) { return } - if len(sourcePath) > 1 && HasTrailingPathSeparator(sourcePath) { - // In the case where the source path is a symbolic link AND it ends - // with a path separator, we will want to evaluate the symbolic link. - trimmedPath := sourcePath[:len(sourcePath)-1] - stat, err := os.Lstat(trimmedPath) - if err != nil { - return nil, err - } - - if stat.Mode()&os.ModeSymlink != 0 { - if sourcePath, err = filepath.EvalSymlinks(trimmedPath); err != nil { - return nil, err - } - } - } - // Separate the source path between it's directory and // the entry in that directory which we are archiving. sourceDir, sourceBase := SplitPathDirEntry(sourcePath) @@ -127,32 +109,137 @@ func TarResource(sourcePath string) (content Archive, err error) { Compression: Uncompressed, IncludeFiles: filter, IncludeSourceDir: true, + RebaseNames: map[string]string{ + sourceBase: rebaseName, + }, }) } // CopyInfo holds basic info about the source // or destination path of a copy operation. type CopyInfo struct { - Path string - Exists bool - IsDir bool + Path string + Exists bool + IsDir bool + RebaseName string } -// CopyInfoStatPath stats the given path to create a CopyInfo -// struct representing that resource. If mustExist is true, then -// it is an error if there is no file or directory at the given path. -func CopyInfoStatPath(path string, mustExist bool) (CopyInfo, error) { - pathInfo := CopyInfo{Path: path} +// CopyInfoSourcePath stats the given path to create a CopyInfo +// struct representing that resource for the source of an archive copy +// operation. The given path should be an absolute local path. A source path +// has all symlinks evaluated that appear before the last path separator ("/" +// on Unix). As it is to be a copy source, the path must exist. +func CopyInfoSourcePath(path string) (CopyInfo, error) { + // Split the given path into its Directory and Base components. We will + // evaluate symlinks in the directory component then append the base. + dirPath, basePath := filepath.Split(path) - fileInfo, err := os.Lstat(path) - - if err == nil { - pathInfo.Exists, pathInfo.IsDir = true, fileInfo.IsDir() - } else if os.IsNotExist(err) && !mustExist { - err = nil + resolvedDirPath, err := filepath.EvalSymlinks(dirPath) + if err != nil { + return CopyInfo{}, err } - return pathInfo, err + // resolvedDirPath will have been cleaned (no trailing path separators) so + // we can manually join it with the base path element. + resolvedPath := resolvedDirPath + string(filepath.Separator) + basePath + + var rebaseName string + if HasTrailingPathSeparator(path) && filepath.Base(path) != filepath.Base(resolvedPath) { + // In the case where the path had a trailing separator and a symlink + // evaluation has changed the last path component, we will need to + // rebase the name in the archive that is being copied to match the + // originally requested name. + rebaseName = filepath.Base(path) + } + + stat, err := os.Lstat(resolvedPath) + if err != nil { + return CopyInfo{}, err + } + + return CopyInfo{ + Path: resolvedPath, + Exists: true, + IsDir: stat.IsDir(), + RebaseName: rebaseName, + }, nil +} + +// CopyInfoDestinationPath stats the given path to create a CopyInfo +// struct representing that resource for the destination of an archive copy +// operation. The given path should be an absolute local path. +func CopyInfoDestinationPath(path string) (info CopyInfo, err error) { + maxSymlinkIter := 10 // filepath.EvalSymlinks uses 255, but 10 already seems like a lot. + originalPath := path + + stat, err := os.Lstat(path) + + if err == nil && stat.Mode()&os.ModeSymlink == 0 { + // The path exists and is not a symlink. + return CopyInfo{ + Path: path, + Exists: true, + IsDir: stat.IsDir(), + }, nil + } + + // While the path is a symlink. + for n := 0; err == nil && stat.Mode()&os.ModeSymlink != 0; n++ { + if n > maxSymlinkIter { + // Don't follow symlinks more than this arbitrary number of times. + return CopyInfo{}, errors.New("too many symlinks in " + originalPath) + } + + // The path is a symbolic link. We need to evaluate it so that the + // destination of the copy operation is the link target and not the + // link itself. This is notably different than CopyInfoSourcePath which + // only evaluates symlinks before the last appearing path separator. + // Also note that it is okay if the last path element is a broken + // symlink as the copy operation should create the target. + var linkTarget string + + linkTarget, err = os.Readlink(path) + if err != nil { + return CopyInfo{}, err + } + + if !filepath.IsAbs(linkTarget) { + // Join with the parent directory. + dstParent, _ := SplitPathDirEntry(path) + linkTarget = filepath.Join(dstParent, linkTarget) + } + + path = linkTarget + stat, err = os.Lstat(path) + } + + if err != nil { + // It's okay if the destination path doesn't exist. We can still + // continue the copy operation if the parent directory exists. + if !os.IsNotExist(err) { + return CopyInfo{}, err + } + + // Ensure destination parent dir exists. + dstParent, _ := SplitPathDirEntry(path) + + parentDirStat, err := os.Lstat(dstParent) + if err != nil { + return CopyInfo{}, err + } + if !parentDirStat.IsDir() { + return CopyInfo{}, ErrNotDirectory + } + + return CopyInfo{Path: path}, nil + } + + // The path exists after resolving symlinks. + return CopyInfo{ + Path: path, + Exists: true, + IsDir: stat.IsDir(), + }, nil } // PrepareArchiveCopy prepares the given srcContent archive, which should @@ -210,6 +297,13 @@ func PrepareArchiveCopy(srcContent ArchiveReader, srcInfo, dstInfo CopyInfo) (ds // rebaseArchiveEntries rewrites the given srcContent archive replacing // an occurance of oldBase with newBase at the beginning of entry names. func rebaseArchiveEntries(srcContent ArchiveReader, oldBase, newBase string) Archive { + if oldBase == "/" { + // If oldBase specifies the root directory, use an empty string as + // oldBase instead so that newBase doesn't replace the path separator + // that all paths will start with. + oldBase = "" + } + rebased, w := io.Pipe() go func() { @@ -259,11 +353,11 @@ func CopyResource(srcPath, dstPath string) error { srcPath = PreserveTrailingDotOrSeparator(filepath.Clean(srcPath), srcPath) dstPath = PreserveTrailingDotOrSeparator(filepath.Clean(dstPath), dstPath) - if srcInfo, err = CopyInfoStatPath(srcPath, true); err != nil { + if srcInfo, err = CopyInfoSourcePath(srcPath); err != nil { return err } - content, err := TarResource(srcPath) + content, err := TarResource(srcInfo) if err != nil { return err } @@ -275,24 +369,13 @@ func CopyResource(srcPath, dstPath string) error { // CopyTo handles extracting the given content whose // entries should be sourced from srcInfo to dstPath. func CopyTo(content ArchiveReader, srcInfo CopyInfo, dstPath string) error { - dstInfo, err := CopyInfoStatPath(dstPath, false) + // The destination path need not exist, but CopyInfoDestinationPath will + // ensure that at least the parent directory exists. + dstInfo, err := CopyInfoDestinationPath(dstPath) if err != nil { return err } - if !dstInfo.Exists { - // Ensure destination parent dir exists. - dstParent, _ := SplitPathDirEntry(dstPath) - - dstStat, err := os.Lstat(dstParent) - if err != nil { - return err - } - if !dstStat.IsDir() { - return ErrNotDirectory - } - } - dstDir, copyArchive, err := PrepareArchiveCopy(content, srcInfo, dstInfo) if err != nil { return err diff --git a/pkg/archive/copy_test.go b/pkg/archive/copy_test.go index dd0b323626..25f1811a96 100644 --- a/pkg/archive/copy_test.go +++ b/pkg/archive/copy_test.go @@ -138,13 +138,7 @@ func TestCopyErrSrcNotExists(t *testing.T) { tmpDirA, tmpDirB := getTestTempDirs(t) defer removeAllPaths(tmpDirA, tmpDirB) - content, err := TarResource(filepath.Join(tmpDirA, "file1")) - if err == nil { - content.Close() - t.Fatal("expected IsNotExist error, but got nil instead") - } - - if !os.IsNotExist(err) { + if _, err := CopyInfoSourcePath(filepath.Join(tmpDirA, "file1")); !os.IsNotExist(err) { t.Fatalf("expected IsNotExist error, but got %T: %s", err, err) } } @@ -158,13 +152,7 @@ func TestCopyErrSrcNotDir(t *testing.T) { // Load A with some sample files and directories. createSampleDir(t, tmpDirA) - content, err := TarResource(joinTrailingSep(tmpDirA, "file1")) - if err == nil { - content.Close() - t.Fatal("expected IsNotDir error, but got nil instead") - } - - if !isNotDir(err) { + if _, err := CopyInfoSourcePath(joinTrailingSep(tmpDirA, "file1")); !isNotDir(err) { t.Fatalf("expected IsNotDir error, but got %T: %s", err, err) } } @@ -181,7 +169,7 @@ func TestCopyErrDstParentNotExists(t *testing.T) { srcInfo := CopyInfo{Path: filepath.Join(tmpDirA, "file1"), Exists: true, IsDir: false} // Try with a file source. - content, err := TarResource(srcInfo.Path) + content, err := TarResource(srcInfo) if err != nil { t.Fatalf("unexpected error %T: %s", err, err) } @@ -199,7 +187,7 @@ func TestCopyErrDstParentNotExists(t *testing.T) { // Try with a directory source. srcInfo = CopyInfo{Path: filepath.Join(tmpDirA, "dir1"), Exists: true, IsDir: true} - content, err = TarResource(srcInfo.Path) + content, err = TarResource(srcInfo) if err != nil { t.Fatalf("unexpected error %T: %s", err, err) } @@ -228,7 +216,7 @@ func TestCopyErrDstNotDir(t *testing.T) { // Try with a file source. srcInfo := CopyInfo{Path: filepath.Join(tmpDirA, "file1"), Exists: true, IsDir: false} - content, err := TarResource(srcInfo.Path) + content, err := TarResource(srcInfo) if err != nil { t.Fatalf("unexpected error %T: %s", err, err) } @@ -245,7 +233,7 @@ func TestCopyErrDstNotDir(t *testing.T) { // Try with a directory source. srcInfo = CopyInfo{Path: filepath.Join(tmpDirA, "dir1"), Exists: true, IsDir: true} - content, err = TarResource(srcInfo.Path) + content, err = TarResource(srcInfo) if err != nil { t.Fatalf("unexpected error %T: %s", err, err) }