Browse Source

Add '-L' option for `cp`

Fixes #16555

Original docker `cp` always copy symbol link itself instead of target,
now we provide '-L' option to allow docker to follow symbol link to real
target.

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
Zhang Wei 9 năm trước cách đây
mục cha
commit
92600bdec1

+ 51 - 15
api/client/cp.go

@@ -26,22 +26,26 @@ const (
 	acrossContainers = fromContainer | toContainer
 )
 
+type cpConfig struct {
+	followLink bool
+}
+
 // CmdCp copies files/folders to or from a path in a container.
 //
-// When copying from a container, if LOCALPATH is '-' the data is written as a
+// When copying from a container, if DEST_PATH is '-' the data is written as a
 // tar archive file to STDOUT.
 //
-// When copying to a container, if LOCALPATH is '-' the data is read as a tar
-// archive file from STDIN, and the destination CONTAINER:PATH, must specify
+// When copying to a container, if SRC_PATH is '-' the data is read as a tar
+// archive file from STDIN, and the destination CONTAINER:DEST_PATH, must specify
 // a directory.
 //
 // Usage:
-// 	docker cp CONTAINER:PATH LOCALPATH|-
-// 	docker cp LOCALPATH|- CONTAINER:PATH
+// 	docker cp CONTAINER:SRC_PATH DEST_PATH|-
+// 	docker cp SRC_PATH|- CONTAINER:DEST_PATH
 func (cli *DockerCli) CmdCp(args ...string) error {
 	cmd := Cli.Subcmd(
 		"cp",
-		[]string{"CONTAINER:PATH LOCALPATH|-", "LOCALPATH|- CONTAINER:PATH"},
+		[]string{"CONTAINER:SRC_PATH DEST_PATH|-", "SRC_PATH|- CONTAINER:DEST_PATH"},
 		strings.Join([]string{
 			Cli.DockerCommands["cp"].Description,
 			"\nUse '-' as the source to read a tar archive from stdin\n",
@@ -52,6 +56,8 @@ func (cli *DockerCli) CmdCp(args ...string) error {
 		true,
 	)
 
+	followLink := cmd.Bool([]string{"L", "-follow-link"}, false, "Always follow symbol link in SRC_PATH")
+
 	cmd.Require(flag.Exact, 2)
 	cmd.ParseFlags(args, true)
 
@@ -73,11 +79,15 @@ func (cli *DockerCli) CmdCp(args ...string) error {
 		direction |= toContainer
 	}
 
+	cpParam := &cpConfig{
+		followLink: *followLink,
+	}
+
 	switch direction {
 	case fromContainer:
-		return cli.copyFromContainer(srcContainer, srcPath, dstPath)
+		return cli.copyFromContainer(srcContainer, srcPath, dstPath, cpParam)
 	case toContainer:
-		return cli.copyToContainer(srcPath, dstContainer, dstPath)
+		return cli.copyToContainer(srcPath, dstContainer, dstPath, cpParam)
 	case acrossContainers:
 		// Copying between containers isn't supported.
 		return fmt.Errorf("copying between containers is not supported")
@@ -161,7 +171,7 @@ func resolveLocalPath(localPath string) (absPath string, err error) {
 	return archive.PreserveTrailingDotOrSeparator(absPath, localPath), nil
 }
 
-func (cli *DockerCli) copyFromContainer(srcContainer, srcPath, dstPath string) (err error) {
+func (cli *DockerCli) copyFromContainer(srcContainer, srcPath, dstPath string, cpParam *cpConfig) (err error) {
 	if dstPath != "-" {
 		// Get an absolute destination path.
 		dstPath, err = resolveLocalPath(dstPath)
@@ -170,6 +180,26 @@ func (cli *DockerCli) copyFromContainer(srcContainer, srcPath, dstPath string) (
 		}
 	}
 
+	// if client requests to follow symbol link, then must decide target file to be copied
+	var rebaseName string
+	if cpParam.followLink {
+		srcStat, err := cli.statContainerPath(srcContainer, srcPath)
+
+		// If the destination is a symbolic link, we should follow it.
+		if err == nil && srcStat.Mode&os.ModeSymlink != 0 {
+			linkTarget := srcStat.LinkTarget
+			if !system.IsAbs(linkTarget) {
+				// Join with the parent directory.
+				srcParent, _ := archive.SplitPathDirEntry(srcPath)
+				linkTarget = filepath.Join(srcParent, linkTarget)
+			}
+
+			linkTarget, rebaseName = archive.GetRebaseName(srcPath, linkTarget)
+			srcPath = linkTarget
+		}
+
+	}
+
 	query := make(url.Values, 1)
 	query.Set("path", filepath.ToSlash(srcPath)) // Normalize the paths used in the API.
 
@@ -205,18 +235,24 @@ func (cli *DockerCli) copyFromContainer(srcContainer, srcPath, dstPath string) (
 
 	// Prepare source copy info.
 	srcInfo := archive.CopyInfo{
-		Path:   srcPath,
-		Exists: true,
-		IsDir:  stat.Mode.IsDir(),
+		Path:       srcPath,
+		Exists:     true,
+		IsDir:      stat.Mode.IsDir(),
+		RebaseName: rebaseName,
 	}
 
+	preArchive := response.body
+	if len(srcInfo.RebaseName) != 0 {
+		_, srcBase := archive.SplitPathDirEntry(srcInfo.Path)
+		preArchive = archive.RebaseArchiveEntries(response.body, srcBase, srcInfo.RebaseName)
+	}
 	// See comments in the implementation of `archive.CopyTo` for exactly what
 	// goes into deciding how and whether the source archive needs to be
 	// altered for the correct copy behavior.
-	return archive.CopyTo(response.body, srcInfo, dstPath)
+	return archive.CopyTo(preArchive, srcInfo, dstPath)
 }
 
-func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string) (err error) {
+func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string, cpParam *cpConfig) (err error) {
 	if srcPath != "-" {
 		// Get an absolute source path.
 		srcPath, err = resolveLocalPath(srcPath)
@@ -271,7 +307,7 @@ func (cli *DockerCli) copyToContainer(srcPath, dstContainer, dstPath string) (er
 		}
 	} else {
 		// Prepare source copy info.
-		srcInfo, err := archive.CopyInfoSourcePath(srcPath)
+		srcInfo, err := archive.CopyInfoSourcePath(srcPath, cpParam.followLink)
 		if err != nil {
 			return err
 		}

+ 48 - 50
docs/reference/commandline/cp.md

@@ -10,81 +10,79 @@ parent = "smn_cli"
 
 # cp
 
-    Usage: docker cp [OPTIONS] CONTAINER:PATH LOCALPATH|-
-           docker cp [OPTIONS] LOCALPATH|- CONTAINER:PATH
+    Usage: docker cp [OPTIONS] CONTAINER:SRC_PATH DEST_PATH | -
+           docker cp [OPTIONS] SRC_PATH | - CONTAINER:DEST_PATH
 
     Copy files/folders between a container and the local filesystem
 
-      --help=false        Print usage
-
-In the first synopsis form, the `docker cp` utility copies the contents of
-`PATH` from the filesystem of `CONTAINER` to the `LOCALPATH` (or stream as
-a tar archive to `STDOUT` if `-` is specified).
-
-In the second synopsis form, the contents of `LOCALPATH` (or a tar archive
-streamed from `STDIN` if `-` is specified) are copied from the local machine to
-`PATH` in the filesystem of `CONTAINER`.
-
-You can copy to or from either a running or stopped container. The `PATH` can
-be a file or directory. The `docker cp` command assumes all `CONTAINER:PATH`
-values are relative to the `/` (root) directory of the container. This means
-supplying the initial forward slash is optional; The command sees
-`compassionate_darwin:/tmp/foo/myfile.txt` and
-`compassionate_darwin:tmp/foo/myfile.txt` as identical. If a `LOCALPATH` value
-is not absolute, is it considered relative to the current working directory.
-
-Behavior is similar to the common Unix utility `cp -a` in that directories are
+      -L, --follow-link=false    Always follow symbol link in SRC_PATH
+      --help=false               Print usage
+
+The `docker cp` utility copies the contents of `SRC_PATH` to the `DEST_PATH`.
+You can copy from the container's file system to the local machine or the
+reverse, from the local filesystem to the container. If `-` is specified for
+either the `SRC_PATH` or `DEST_PATH`, you can also stream a tar archive from
+`STDIN` or to `STDOUT`. The `CONTAINER` can be a running or stopped container.
+The `SRC_PATH` or `DEST_PATH` be a file or directory.
+
+The `docker cp` command assumes container paths are relative to the container's 
+`/` (root) directory. This means supplying the initial forward slash is optional;
+The command sees `compassionate_darwin:/tmp/foo/myfile.txt` and
+`compassionate_darwin:tmp/foo/myfile.txt` as identical. Local machine paths can
+be an absolute or relative value. The command interprets a local machine's
+relative paths as relative to the current working directory where `docker cp` is
+run.
+
+The `cp` command behaves like the Unix `cp -a` command in that directories are
 copied recursively with permissions preserved if possible. Ownership is set to
-the user and primary group on the receiving end of the transfer. For example,
-files copied to a container will be created with `UID:GID` of the root user.
-Files copied to the local machine will be created with the `UID:GID` of the
-user which invoked the `docker cp` command.
+the user and primary group at the destination. For example, files copied to a
+container are created with `UID:GID` of the root user. Files copied to the local
+machine are created with the `UID:GID` of the user which invoked the `docker cp`
+command.  If you specify the `-L` option, `docker cp` follows any symbolic link
+in the `SRC_PATH`.
 
 Assuming a path separator of `/`, a first argument of `SRC_PATH` and second
-argument of `DST_PATH`, the behavior is as follows:
+argument of `DEST_PATH`, the behavior is as follows:
 
 - `SRC_PATH` specifies a file
-    - `DST_PATH` does not exist
-        - the file is saved to a file created at `DST_PATH`
-    - `DST_PATH` does not exist and ends with `/`
+    - `DEST_PATH` does not exist
+        - the file is saved to a file created at `DEST_PATH`
+    - `DEST_PATH` does not exist and ends with `/`
         - Error condition: the destination directory must exist.
-    - `DST_PATH` exists and is a file
-        - the destination is overwritten with the contents of the source file
-    - `DST_PATH` exists and is a directory
+    - `DEST_PATH` exists and is a file
+        - the destination is overwritten with the source file's contents
+    - `DEST_PATH` exists and is a directory
         - the file is copied into this directory using the basename from
           `SRC_PATH`
 - `SRC_PATH` specifies a directory
-    - `DST_PATH` does not exist
-        - `DST_PATH` is created as a directory and the *contents* of the source
+    - `DEST_PATH` does not exist
+        - `DEST_PATH` is created as a directory and the *contents* of the source
            directory are copied into this directory
-    - `DST_PATH` exists and is a file
+    - `DEST_PATH` exists and is a file
         - Error condition: cannot copy a directory to a file
-    - `DST_PATH` exists and is a directory
+    - `DEST_PATH` exists and is a directory
         - `SRC_PATH` does not end with `/.`
             - the source directory is copied into this directory
         - `SRC_PATH` does end with `/.`
             - the *content* of the source directory is copied into this
               directory
 
-The command requires `SRC_PATH` and `DST_PATH` to exist according to the above
+The command requires `SRC_PATH` and `DEST_PATH` to exist according to the above
 rules. If `SRC_PATH` is local and is a symbolic link, the symbolic link, not
-the target, is copied.
+the target, is copied by default. To copy the link target and not the link, specify 
+the `-L` option.
 
-A colon (`:`) is used as a delimiter between `CONTAINER` and `PATH`, but `:`
-could also be in a valid `LOCALPATH`, like `file:name.txt`. This ambiguity is
-resolved by requiring a `LOCALPATH` with a `:` to be made explicit with a
-relative or absolute path, for example:
+A colon (`:`) is used as a delimiter between `CONTAINER` and its path. You can
+also use `:` when specifying paths to a `SRC_PATH` or `DEST_PATH` on a local
+machine, for example  `file:name.txt`. If you use a `:` in a local machine path,
+you must be explicit with a relative or absolute path, for example:
 
     `/path/to/file:name.txt` or `./file:name.txt`
 
 It is not possible to copy certain system files such as resources under
 `/proc`, `/sys`, `/dev`, and mounts created by the user in the container.
 
-Using `-` as the first argument in place of a `LOCALPATH` will stream the
-contents of `STDIN` as a tar archive which will be extracted to the `PATH` in
-the filesystem of the destination container. In this case, `PATH` must specify
-a directory.
-
-Using `-` as the second argument in place of a `LOCALPATH` will stream the
-contents of the resource from the source container as a tar archive to
-`STDOUT`.
+Using `-` as the `SRC_PATH` streams the contents of `STDIN` as a tar archive.
+The command extracts the content of the tar to the `DEST_PATH` in container's
+filesystem. In this case, `DEST_PATH` must specify a directory. Using `-` as
+`DEST_PATH` streams the contents of the resource as a tar archive to `STDOUT`.

+ 54 - 0
integration-cli/docker_cli_cp_test.go

@@ -609,3 +609,57 @@ func (s *DockerSuite) TestCopyCreatedContainer(c *check.C) {
 	defer os.RemoveAll(tmpDir)
 	dockerCmd(c, "cp", "test_cp:/bin/sh", tmpDir)
 }
+
+// test copy with option `-L`: following symbol link
+// Check that symlinks to a file behave as expected when copying one from
+// a container to host following symbol link
+func (s *DockerSuite) TestCpSymlinkFromConToHostFollowSymlink(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+	out, exitCode := dockerCmd(c, "run", "-d", "busybox", "/bin/sh", "-c", "mkdir -p '"+cpTestPath+"' && echo -n '"+cpContainerContents+"' > "+cpFullPath+" && ln -s "+cpFullPath+" /dir_link")
+	if exitCode != 0 {
+		c.Fatal("failed to create a container", out)
+	}
+
+	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-symlink-container-to-host-follow-symlink")
+	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", "-L", cleanedContainerID+":"+"/dir_link", testDir)
+
+	expectedPath := filepath.Join(testDir, "dir_link")
+
+	expected := []byte(cpContainerContents)
+	actual, err := ioutil.ReadFile(expectedPath)
+
+	if !bytes.Equal(actual, expected) {
+		c.Fatalf("Expected copied file to be duplicate of the container symbol link target")
+	}
+	os.Remove(expectedPath)
+
+	// now test copy symbol link to an non-existing file in host
+	expectedPath = filepath.Join(testDir, "somefile_host")
+	// expectedPath shouldn't exist, if exists, remove it
+	if _, err := os.Lstat(expectedPath); err == nil {
+		os.Remove(expectedPath)
+	}
+
+	dockerCmd(c, "cp", "-L", cleanedContainerID+":"+"/dir_link", expectedPath)
+
+	actual, err = ioutil.ReadFile(expectedPath)
+
+	if !bytes.Equal(actual, expected) {
+		c.Fatalf("Expected copied file to be duplicate of the container symbol link target")
+	}
+	defer os.Remove(expectedPath)
+}

+ 64 - 53
man/docker-cp.1.md

@@ -7,87 +7,87 @@ docker-cp - Copy files/folders between a container and the local filesystem.
 # SYNOPSIS
 **docker cp**
 [**--help**]
-CONTAINER:PATH LOCALPATH|-
+CONTAINER:SRC_PATH DEST_PATH|-
 
 **docker cp**
 [**--help**]
-LOCALPATH|- CONTAINER:PATH
+SRC_PATH|- CONTAINER:DEST_PATH
 
 # DESCRIPTION
 
-In the first synopsis form, the `docker cp` utility copies the contents of
-`PATH` from the filesystem of `CONTAINER` to the `LOCALPATH` (or stream as
-a tar archive to `STDOUT` if `-` is specified).
-
-In the second synopsis form, the contents of `LOCALPATH` (or a tar archive
-streamed from `STDIN` if `-` is specified) are copied from the local machine to
-`PATH` in the filesystem of `CONTAINER`.
-
-You can copy to or from either a running or stopped container. The `PATH` can
-be a file or directory. The `docker cp` command assumes all `CONTAINER:PATH`
-values are relative to the `/` (root) directory of the container. This means
-supplying the initial forward slash is optional; The command sees
-`compassionate_darwin:/tmp/foo/myfile.txt` and
-`compassionate_darwin:tmp/foo/myfile.txt` as identical. If a `LOCALPATH` value
-is not absolute, is it considered relative to the current working directory.
-
-Behavior is similar to the common Unix utility `cp -a` in that directories are
+The `docker cp` utility copies the contents of `SRC_PATH` to the `DEST_PATH`.
+You can copy from the container's file system to the local machine or the
+reverse, from the local filesystem to the container. If `-` is specified for
+either the `SRC_PATH` or `DEST_PATH`, you can also stream a tar archive from
+`STDIN` or to `STDOUT`. The `CONTAINER` can be a running or stopped container.
+The `SRC_PATH` or `DEST_PATH` be a file or directory.
+
+The `docker cp` command assumes container paths are relative to the container's 
+`/` (root) directory. This means supplying the initial forward slash is optional; 
+The command sees `compassionate_darwin:/tmp/foo/myfile.txt` and
+`compassionate_darwin:tmp/foo/myfile.txt` as identical. Local machine paths can
+be an absolute or relative value. The command interprets a local machine's
+relative paths as relative to the current working directory where `docker cp` is
+run.
+
+The `cp` command behaves like the Unix `cp -a` command in that directories are
 copied recursively with permissions preserved if possible. Ownership is set to
-the user and primary group on the receiving end of the transfer. For example,
-files copied to a container will be created with `UID:GID` of the root user.
-Files copied to the local machine will be created with the `UID:GID` of the
-user which invoked the `docker cp` command.
+the user and primary group at the destination. For example, files copied to a
+container are created with `UID:GID` of the root user. Files copied to the local
+machine are created with the `UID:GID` of the user which invoked the `docker cp`
+command.  If you specify the `-L` option, `docker cp` follows any symbolic link
+in the `SRC_PATH`.
 
 Assuming a path separator of `/`, a first argument of `SRC_PATH` and second
-argument of `DST_PATH`, the behavior is as follows:
+argument of `DEST_PATH`, the behavior is as follows:
 
 - `SRC_PATH` specifies a file
-    - `DST_PATH` does not exist
-        - the file is saved to a file created at `DST_PATH`
-    - `DST_PATH` does not exist and ends with `/`
+    - `DEST_PATH` does not exist
+        - the file is saved to a file created at `DEST_PATH`
+    - `DEST_PATH` does not exist and ends with `/`
         - Error condition: the destination directory must exist.
-    - `DST_PATH` exists and is a file
-        - the destination is overwritten with the contents of the source file
-    - `DST_PATH` exists and is a directory
+    - `DEST_PATH` exists and is a file
+        - the destination is overwritten with the source file's contents
+    - `DEST_PATH` exists and is a directory
         - the file is copied into this directory using the basename from
           `SRC_PATH`
 - `SRC_PATH` specifies a directory
-    - `DST_PATH` does not exist
-        - `DST_PATH` is created as a directory and the *contents* of the source
+    - `DEST_PATH` does not exist
+        - `DEST_PATH` is created as a directory and the *contents* of the source
            directory are copied into this directory
-    - `DST_PATH` exists and is a file
+    - `DEST_PATH` exists and is a file
         - Error condition: cannot copy a directory to a file
-    - `DST_PATH` exists and is a directory
+    - `DEST_PATH` exists and is a directory
         - `SRC_PATH` does not end with `/.`
             - the source directory is copied into this directory
         - `SRC_PATH` does end with `/.`
             - the *content* of the source directory is copied into this
               directory
 
-The command requires `SRC_PATH` and `DST_PATH` to exist according to the above
+The command requires `SRC_PATH` and `DEST_PATH` to exist according to the above
 rules. If `SRC_PATH` is local and is a symbolic link, the symbolic link, not
-the target, is copied.
+the target, is copied by default. To copy the link target and not the link, 
+specify the `-L` option.
 
-A colon (`:`) is used as a delimiter between `CONTAINER` and `PATH`, but `:`
-could also be in a valid `LOCALPATH`, like `file:name.txt`. This ambiguity is
-resolved by requiring a `LOCALPATH` with a `:` to be made explicit with a
-relative or absolute path, for example:
+A colon (`:`) is used as a delimiter between `CONTAINER` and its path. You can
+also use `:` when specifying paths to a `SRC_PATH` or `DEST_PATH` on a local
+machine, for example  `file:name.txt`. If you use a `:` in a local machine path,
+you must be explicit with a relative or absolute path, for example:
 
     `/path/to/file:name.txt` or `./file:name.txt`
 
 It is not possible to copy certain system files such as resources under
 `/proc`, `/sys`, `/dev`, and mounts created by the user in the container.
 
-Using `-` as the first argument in place of a `LOCALPATH` will stream the
-contents of `STDIN` as a tar archive which will be extracted to the `PATH` in
-the filesystem of the destination container. In this case, `PATH` must specify
-a directory.
-
-Using `-` as the second argument in place of a `LOCALPATH` will stream the
-contents of the resource from the source container as a tar archive to
-`STDOUT`.
+Using `-` as the `SRC_PATH` streams the contents of `STDIN` as a tar archive.
+The command extracts the content of the tar to the `DEST_PATH` in container's
+filesystem. In this case, `DEST_PATH` must specify a directory. Using `-` as
+`DEST_PATH` streams the contents of the resource as a tar archive to `STDOUT`.
 
 # OPTIONS
+**-L**, **--follow-link**=*true*|*false*
+  Follow symbol link in SRC_PATH
+
 **--help**
   Print usage statement
 
@@ -102,13 +102,13 @@ If you want to copy the `/tmp/foo` directory from a container to the
 existing `/tmp` directory on your host. If you run `docker cp` in your `~`
 (home) directory on the local host:
 
-		$ docker cp compassionate_darwin:tmp/foo /tmp
+    $ docker cp compassionate_darwin:tmp/foo /tmp
 
 Docker creates a `/tmp/foo` directory on your host. Alternatively, you can omit
 the leading slash in the command. If you execute this command from your home
 directory:
 
-		$ docker cp compassionate_darwin:tmp/foo tmp
+    $ docker cp compassionate_darwin:tmp/foo tmp
 
 If `~/tmp` does not exist, Docker will create it and copy the contents of
 `/tmp/foo` from the container into this new directory. If `~/tmp` already
@@ -120,7 +120,7 @@ will either overwrite the contents of `LOCALPATH` if it is a file or place it
 into `LOCALPATH` if it is a directory, overwriting an existing file of the same
 name if one exists. For example, this command:
 
-		$ docker cp sharp_ptolemy:/tmp/foo/myfile.txt /test
+    $ docker cp sharp_ptolemy:/tmp/foo/myfile.txt /test
 
 If `/test` does not exist on the local machine, it will be created as a file
 with the contents of `/tmp/foo/myfile.txt` from the container. If `/test`
@@ -137,16 +137,27 @@ If you have a file, `config.yml`, in the current directory on your local host
 and wish to copy it to an existing directory at `/etc/my-app.d` in a container,
 this command can be used:
 
-		$ docker cp config.yml myappcontainer:/etc/my-app.d
+    $ docker cp config.yml myappcontainer:/etc/my-app.d
 
 If you have several files in a local directory `/config` which you need to copy
 to a directory `/etc/my-app.d` in a container:
 
-		$ docker cp /config/. myappcontainer:/etc/my-app.d
+    $ docker cp /config/. myappcontainer:/etc/my-app.d
 
 The above command will copy the contents of the local `/config` directory into
 the directory `/etc/my-app.d` in the container.
 
+Finally, if you want to copy a symbolic link into a container, you typically
+want to  copy the linked target and not the link itself. To copy the target, use
+the `-L` option, for example:
+
+    $ ln -s /tmp/somefile /tmp/somefile.ln
+    $ docker cp -L /tmp/somefile.ln myappcontainer:/tmp/
+
+This command copies content of the local `/tmp/somefile` into the file
+`/tmp/somefile.ln` in the container. Without `-L` option, the `/tmp/somefile.ln`
+preserves its symbolic link but not its content.
+
 # HISTORY
 April 2014, Originally compiled by William Henry (whenry at redhat dot com)
 based on docker.com source material and internal work.

+ 3 - 0
pkg/archive/changes_test.go

@@ -63,6 +63,9 @@ func createSampleDir(t *testing.T, root string) {
 		{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},
 	}
 
 	now := time.Now()

+ 76 - 25
pkg/archive/copy.go

@@ -135,30 +135,17 @@ type CopyInfo struct {
 // 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.
+func CopyInfoSourcePath(path string, followLink bool) (CopyInfo, error) {
+	// normalize the file path and then evaluate the symbol link
+	// we will use the target file instead of the symbol link if
+	// followLink is set
 	path = normalizePath(path)
-	dirPath, basePath := filepath.Split(path)
 
-	resolvedDirPath, err := filepath.EvalSymlinks(dirPath)
+	resolvedPath, rebaseName, err := ResolveHostSourcePath(path, followLink)
 	if err != nil {
 		return CopyInfo{}, 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
@@ -279,7 +266,10 @@ func PrepareArchiveCopy(srcContent Reader, srcInfo, dstInfo CopyInfo) (dstDir st
 		// The destination exists as some type of file and the source content
 		// is also a file. The source content entry will have to be renamed to
 		// have a basename which matches the destination path's basename.
-		return dstDir, rebaseArchiveEntries(srcContent, srcBase, dstBase), nil
+		if len(srcInfo.RebaseName) != 0 {
+			srcBase = srcInfo.RebaseName
+		}
+		return dstDir, RebaseArchiveEntries(srcContent, srcBase, dstBase), nil
 	case srcInfo.IsDir:
 		// The destination does not exist and the source content is an archive
 		// of a directory. The archive should be extracted to the parent of
@@ -287,7 +277,10 @@ func PrepareArchiveCopy(srcContent Reader, srcInfo, dstInfo CopyInfo) (dstDir st
 		// created as a result should take the name of the destination path.
 		// The source content entries will have to be renamed to have a
 		// basename which matches the destination path's basename.
-		return dstDir, rebaseArchiveEntries(srcContent, srcBase, dstBase), nil
+		if len(srcInfo.RebaseName) != 0 {
+			srcBase = srcInfo.RebaseName
+		}
+		return dstDir, RebaseArchiveEntries(srcContent, srcBase, dstBase), nil
 	case assertsDirectory(dstInfo.Path):
 		// The destination does not exist and is asserted to be created as a
 		// directory, but the source content is not a directory. This is an
@@ -301,14 +294,17 @@ func PrepareArchiveCopy(srcContent Reader, srcInfo, dstInfo CopyInfo) (dstDir st
 		// to be created when the archive is extracted and the source content
 		// entry will have to be renamed to have a basename which matches the
 		// destination path's basename.
-		return dstDir, rebaseArchiveEntries(srcContent, srcBase, dstBase), nil
+		if len(srcInfo.RebaseName) != 0 {
+			srcBase = srcInfo.RebaseName
+		}
+		return dstDir, RebaseArchiveEntries(srcContent, srcBase, dstBase), nil
 	}
 
 }
 
-// rebaseArchiveEntries rewrites the given srcContent archive replacing
+// RebaseArchiveEntries rewrites the given srcContent archive replacing
 // an occurrence of oldBase with newBase at the beginning of entry names.
-func rebaseArchiveEntries(srcContent Reader, oldBase, newBase string) Archive {
+func RebaseArchiveEntries(srcContent Reader, oldBase, newBase string) Archive {
 	if oldBase == string(os.PathSeparator) {
 		// If oldBase specifies the root directory, use an empty string as
 		// oldBase instead so that newBase doesn't replace the path separator
@@ -355,7 +351,7 @@ func rebaseArchiveEntries(srcContent Reader, oldBase, newBase string) Archive {
 // CopyResource performs an archive copy from the given source path to the
 // given destination path. The source path MUST exist and the destination
 // path's parent directory must exist.
-func CopyResource(srcPath, dstPath string) error {
+func CopyResource(srcPath, dstPath string, followLink bool) error {
 	var (
 		srcInfo CopyInfo
 		err     error
@@ -369,7 +365,7 @@ func CopyResource(srcPath, dstPath string) error {
 	srcPath = PreserveTrailingDotOrSeparator(filepath.Clean(srcPath), srcPath)
 	dstPath = PreserveTrailingDotOrSeparator(filepath.Clean(dstPath), dstPath)
 
-	if srcInfo, err = CopyInfoSourcePath(srcPath); err != nil {
+	if srcInfo, err = CopyInfoSourcePath(srcPath, followLink); err != nil {
 		return err
 	}
 
@@ -405,3 +401,58 @@ func CopyTo(content Reader, srcInfo CopyInfo, dstPath string) error {
 
 	return Untar(copyArchive, dstDir, options)
 }
+
+// ResolveHostSourcePath decides real path need to be copied with parameters such as
+// whether to follow symbol link or not, if followLink is true, resolvedPath will return
+// link target of any symbol link file, else it will only resolve symlink of directory
+// but return symbol link file itself without resolving.
+func ResolveHostSourcePath(path string, followLink bool) (resolvedPath, rebaseName string, err error) {
+	if followLink {
+		resolvedPath, err = filepath.EvalSymlinks(path)
+		if err != nil {
+			return
+		}
+
+		resolvedPath, rebaseName = GetRebaseName(path, resolvedPath)
+	} else {
+		dirPath, basePath := filepath.Split(path)
+
+		// if not follow symbol link, then resolve symbol link of parent dir
+		var resolvedDirPath string
+		resolvedDirPath, err = filepath.EvalSymlinks(dirPath)
+		if err != nil {
+			return
+		}
+		// 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
+		if hasTrailingPathSeparator(path) && filepath.Base(path) != filepath.Base(resolvedPath) {
+			rebaseName = filepath.Base(path)
+		}
+	}
+	return resolvedPath, rebaseName, nil
+}
+
+// GetRebaseName normalizes and compares path and resolvedPath,
+// return completed resolved path and rebased file name
+func GetRebaseName(path, resolvedPath string) (string, string) {
+	// linkTarget will have been cleaned (no trailing path separators and dot) so
+	// we can manually join it with them
+	var rebaseName string
+	if specifiesCurrentDir(path) && !specifiesCurrentDir(resolvedPath) {
+		resolvedPath += string(filepath.Separator) + "."
+	}
+
+	if hasTrailingPathSeparator(path) && !hasTrailingPathSeparator(resolvedPath) {
+		resolvedPath += string(filepath.Separator)
+	}
+
+	if 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)
+	}
+	return resolvedPath, rebaseName
+}

+ 353 - 4
pkg/archive/copy_test.go

@@ -120,9 +120,15 @@ func logDirContents(t *testing.T, dirPath string) {
 }
 
 func testCopyHelper(t *testing.T, srcPath, dstPath string) (err error) {
-	t.Logf("copying from %q to %q", srcPath, dstPath)
+	t.Logf("copying from %q to %q (not follow symbol link)", srcPath, dstPath)
 
-	return CopyResource(srcPath, dstPath)
+	return CopyResource(srcPath, dstPath, false)
+}
+
+func testCopyHelperFSym(t *testing.T, srcPath, dstPath string) (err error) {
+	t.Logf("copying from %q to %q (follow symbol link)", srcPath, dstPath)
+
+	return CopyResource(srcPath, dstPath, true)
 }
 
 // Basic assumptions about SRC and DST:
@@ -138,7 +144,7 @@ func TestCopyErrSrcNotExists(t *testing.T) {
 	tmpDirA, tmpDirB := getTestTempDirs(t)
 	defer removeAllPaths(tmpDirA, tmpDirB)
 
-	if _, err := CopyInfoSourcePath(filepath.Join(tmpDirA, "file1")); !os.IsNotExist(err) {
+	if _, err := CopyInfoSourcePath(filepath.Join(tmpDirA, "file1"), false); !os.IsNotExist(err) {
 		t.Fatalf("expected IsNotExist error, but got %T: %s", err, err)
 	}
 }
@@ -152,7 +158,7 @@ func TestCopyErrSrcNotDir(t *testing.T) {
 	// Load A with some sample files and directories.
 	createSampleDir(t, tmpDirA)
 
-	if _, err := CopyInfoSourcePath(joinTrailingSep(tmpDirA, "file1")); !isNotDir(err) {
+	if _, err := CopyInfoSourcePath(joinTrailingSep(tmpDirA, "file1"), false); !isNotDir(err) {
 		t.Fatalf("expected IsNotDir error, but got %T: %s", err, err)
 	}
 }
@@ -286,6 +292,27 @@ func TestCopyCaseA(t *testing.T) {
 	if err = fileContentsEqual(t, srcPath, dstPath); err != nil {
 		t.Fatal(err)
 	}
+	os.Remove(dstPath)
+
+	symlinkPath := filepath.Join(tmpDirA, "symlink3")
+	symlinkPath1 := filepath.Join(tmpDirA, "symlink4")
+	linkTarget := filepath.Join(tmpDirA, "file1")
+
+	if err = testCopyHelperFSym(t, symlinkPath, dstPath); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = fileContentsEqual(t, linkTarget, dstPath); err != nil {
+		t.Fatal(err)
+	}
+	os.Remove(dstPath)
+	if err = testCopyHelperFSym(t, symlinkPath1, dstPath); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = fileContentsEqual(t, linkTarget, dstPath); err != nil {
+		t.Fatal(err)
+	}
 }
 
 // B. SRC specifies a file and DST (with trailing path separator) doesn't
@@ -310,6 +337,16 @@ func TestCopyCaseB(t *testing.T) {
 	if err != ErrDirNotExists {
 		t.Fatalf("expected ErrDirNotExists error, but got %T: %s", err, err)
 	}
+
+	symlinkPath := filepath.Join(tmpDirA, "symlink3")
+
+	if err = testCopyHelperFSym(t, symlinkPath, dstDir); err == nil {
+		t.Fatal("expected ErrDirNotExists error, but got nil instead")
+	}
+	if err != ErrDirNotExists {
+		t.Fatalf("expected ErrDirNotExists error, but got %T: %s", err, err)
+	}
+
 }
 
 // C. SRC specifies a file and DST exists as a file. This should overwrite
@@ -341,6 +378,44 @@ func TestCopyCaseC(t *testing.T) {
 	}
 }
 
+// C. Symbol link following version:
+//    SRC specifies a file and DST exists as a file. This should overwrite
+//    the file at DST with the contents of the source file.
+func TestCopyCaseCFSym(t *testing.T) {
+	tmpDirA, tmpDirB := getTestTempDirs(t)
+	defer removeAllPaths(tmpDirA, tmpDirB)
+
+	// Load A and B with some sample files and directories.
+	createSampleDir(t, tmpDirA)
+	createSampleDir(t, tmpDirB)
+
+	symlinkPathBad := filepath.Join(tmpDirA, "symlink1")
+	symlinkPath := filepath.Join(tmpDirA, "symlink3")
+	linkTarget := filepath.Join(tmpDirA, "file1")
+	dstPath := filepath.Join(tmpDirB, "file2")
+
+	var err error
+
+	// first to test broken link
+	if err = testCopyHelperFSym(t, symlinkPathBad, dstPath); err == nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// test symbol link -> symbol link -> target
+	// Ensure they start out different.
+	if err = fileContentsEqual(t, linkTarget, dstPath); err == nil {
+		t.Fatal("expected different file contents")
+	}
+
+	if err = testCopyHelperFSym(t, symlinkPath, dstPath); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = fileContentsEqual(t, linkTarget, dstPath); err != nil {
+		t.Fatal(err)
+	}
+}
+
 // D. SRC specifies a file and DST exists as a directory. This should place
 //    a copy of the source file inside it using the basename from SRC. Ensure
 //    this works whether DST has a trailing path separator or not.
@@ -392,6 +467,59 @@ func TestCopyCaseD(t *testing.T) {
 	}
 }
 
+// D. Symbol link following version:
+//    SRC specifies a file and DST exists as a directory. This should place
+//    a copy of the source file inside it using the basename from SRC. Ensure
+//    this works whether DST has a trailing path separator or not.
+func TestCopyCaseDFSym(t *testing.T) {
+	tmpDirA, tmpDirB := getTestTempDirs(t)
+	defer removeAllPaths(tmpDirA, tmpDirB)
+
+	// Load A and B with some sample files and directories.
+	createSampleDir(t, tmpDirA)
+	createSampleDir(t, tmpDirB)
+
+	srcPath := filepath.Join(tmpDirA, "symlink4")
+	linkTarget := filepath.Join(tmpDirA, "file1")
+	dstDir := filepath.Join(tmpDirB, "dir1")
+	dstPath := filepath.Join(dstDir, "symlink4")
+
+	var err error
+
+	// Ensure that dstPath doesn't exist.
+	if _, err = os.Stat(dstPath); !os.IsNotExist(err) {
+		t.Fatalf("did not expect dstPath %q to exist", dstPath)
+	}
+
+	if err = testCopyHelperFSym(t, srcPath, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = fileContentsEqual(t, linkTarget, dstPath); err != nil {
+		t.Fatal(err)
+	}
+
+	// Now try again but using a trailing path separator for dstDir.
+
+	if err = os.RemoveAll(dstDir); err != nil {
+		t.Fatalf("unable to remove dstDir: %s", err)
+	}
+
+	if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil {
+		t.Fatalf("unable to make dstDir: %s", err)
+	}
+
+	dstDir = joinTrailingSep(tmpDirB, "dir1")
+
+	if err = testCopyHelperFSym(t, srcPath, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = fileContentsEqual(t, linkTarget, dstPath); err != nil {
+		t.Fatal(err)
+	}
+}
+
 // E. SRC specifies a directory and DST does not exist. This should create a
 //    directory at DST and copy the contents of the SRC directory into the DST
 //    directory. Ensure this works whether DST has a trailing path separator or
@@ -436,6 +564,52 @@ func TestCopyCaseE(t *testing.T) {
 	}
 }
 
+// E. Symbol link following version:
+//    SRC specifies a directory and DST does not exist. This should create a
+//    directory at DST and copy the contents of the SRC directory into the DST
+//    directory. Ensure this works whether DST has a trailing path separator or
+//    not.
+func TestCopyCaseEFSym(t *testing.T) {
+	tmpDirA, tmpDirB := getTestTempDirs(t)
+	defer removeAllPaths(tmpDirA, tmpDirB)
+
+	// Load A with some sample files and directories.
+	createSampleDir(t, tmpDirA)
+
+	srcDir := filepath.Join(tmpDirA, "dirSymlink")
+	linkTarget := filepath.Join(tmpDirA, "dir1")
+	dstDir := filepath.Join(tmpDirB, "testDir")
+
+	var err error
+
+	if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = dirContentsEqual(t, dstDir, linkTarget); err != nil {
+		t.Log("dir contents not equal")
+		logDirContents(t, tmpDirA)
+		logDirContents(t, tmpDirB)
+		t.Fatal(err)
+	}
+
+	// Now try again but using a trailing path separator for dstDir.
+
+	if err = os.RemoveAll(dstDir); err != nil {
+		t.Fatalf("unable to remove dstDir: %s", err)
+	}
+
+	dstDir = joinTrailingSep(tmpDirB, "testDir")
+
+	if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = dirContentsEqual(t, dstDir, linkTarget); err != nil {
+		t.Fatal(err)
+	}
+}
+
 // F. SRC specifies a directory and DST exists as a file. This should cause an
 //    error as it is not possible to overwrite a file with a directory.
 func TestCopyCaseF(t *testing.T) {
@@ -447,6 +621,7 @@ func TestCopyCaseF(t *testing.T) {
 	createSampleDir(t, tmpDirB)
 
 	srcDir := filepath.Join(tmpDirA, "dir1")
+	symSrcDir := filepath.Join(tmpDirA, "dirSymlink")
 	dstFile := filepath.Join(tmpDirB, "file1")
 
 	var err error
@@ -458,6 +633,15 @@ func TestCopyCaseF(t *testing.T) {
 	if err != ErrCannotCopyDir {
 		t.Fatalf("expected ErrCannotCopyDir error, but got %T: %s", err, err)
 	}
+
+	// now test with symbol link
+	if err = testCopyHelperFSym(t, symSrcDir, dstFile); err == nil {
+		t.Fatal("expected ErrCannotCopyDir error, but got nil instead")
+	}
+
+	if err != ErrCannotCopyDir {
+		t.Fatalf("expected ErrCannotCopyDir error, but got %T: %s", err, err)
+	}
 }
 
 // G. SRC specifies a directory and DST exists as a directory. This should copy
@@ -506,6 +690,54 @@ func TestCopyCaseG(t *testing.T) {
 	}
 }
 
+// G. Symbol link version:
+//    SRC specifies a directory and DST exists as a directory. This should copy
+//    the SRC directory and all its contents to the DST directory. Ensure this
+//    works whether DST has a trailing path separator or not.
+func TestCopyCaseGFSym(t *testing.T) {
+	tmpDirA, tmpDirB := getTestTempDirs(t)
+	defer removeAllPaths(tmpDirA, tmpDirB)
+
+	// Load A and B with some sample files and directories.
+	createSampleDir(t, tmpDirA)
+	createSampleDir(t, tmpDirB)
+
+	srcDir := filepath.Join(tmpDirA, "dirSymlink")
+	linkTarget := filepath.Join(tmpDirA, "dir1")
+	dstDir := filepath.Join(tmpDirB, "dir2")
+	resultDir := filepath.Join(dstDir, "dirSymlink")
+
+	var err error
+
+	if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = dirContentsEqual(t, resultDir, linkTarget); err != nil {
+		t.Fatal(err)
+	}
+
+	// Now try again but using a trailing path separator for dstDir.
+
+	if err = os.RemoveAll(dstDir); err != nil {
+		t.Fatalf("unable to remove dstDir: %s", err)
+	}
+
+	if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil {
+		t.Fatalf("unable to make dstDir: %s", err)
+	}
+
+	dstDir = joinTrailingSep(tmpDirB, "dir2")
+
+	if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = dirContentsEqual(t, resultDir, linkTarget); err != nil {
+		t.Fatal(err)
+	}
+}
+
 // H. SRC specifies a directory's contents only and DST does not exist. This
 //    should create a directory at DST and copy the contents of the SRC
 //    directory (but not the directory itself) into the DST directory. Ensure
@@ -553,6 +785,55 @@ func TestCopyCaseH(t *testing.T) {
 	}
 }
 
+// H. Symbol link following version:
+//    SRC specifies a directory's contents only and DST does not exist. This
+//    should create a directory at DST and copy the contents of the SRC
+//    directory (but not the directory itself) into the DST directory. Ensure
+//    this works whether DST has a trailing path separator or not.
+func TestCopyCaseHFSym(t *testing.T) {
+	tmpDirA, tmpDirB := getTestTempDirs(t)
+	defer removeAllPaths(tmpDirA, tmpDirB)
+
+	// Load A with some sample files and directories.
+	createSampleDir(t, tmpDirA)
+
+	srcDir := joinTrailingSep(tmpDirA, "dirSymlink") + "."
+	linkTarget := filepath.Join(tmpDirA, "dir1")
+	dstDir := filepath.Join(tmpDirB, "testDir")
+
+	var err error
+
+	if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = dirContentsEqual(t, dstDir, linkTarget); err != nil {
+		t.Log("dir contents not equal")
+		logDirContents(t, tmpDirA)
+		logDirContents(t, tmpDirB)
+		t.Fatal(err)
+	}
+
+	// Now try again but using a trailing path separator for dstDir.
+
+	if err = os.RemoveAll(dstDir); err != nil {
+		t.Fatalf("unable to remove dstDir: %s", err)
+	}
+
+	dstDir = joinTrailingSep(tmpDirB, "testDir")
+
+	if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = dirContentsEqual(t, dstDir, linkTarget); err != nil {
+		t.Log("dir contents not equal")
+		logDirContents(t, tmpDirA)
+		logDirContents(t, tmpDirB)
+		t.Fatal(err)
+	}
+}
+
 // I. SRC specifies a directory's contents only and DST exists as a file. This
 //    should cause an error as it is not possible to overwrite a file with a
 //    directory.
@@ -565,6 +846,7 @@ func TestCopyCaseI(t *testing.T) {
 	createSampleDir(t, tmpDirB)
 
 	srcDir := joinTrailingSep(tmpDirA, "dir1") + "."
+	symSrcDir := filepath.Join(tmpDirB, "dirSymlink")
 	dstFile := filepath.Join(tmpDirB, "file1")
 
 	var err error
@@ -576,6 +858,15 @@ func TestCopyCaseI(t *testing.T) {
 	if err != ErrCannotCopyDir {
 		t.Fatalf("expected ErrCannotCopyDir error, but got %T: %s", err, err)
 	}
+
+	// now try with symbol link of dir
+	if err = testCopyHelperFSym(t, symSrcDir, dstFile); err == nil {
+		t.Fatal("expected ErrCannotCopyDir error, but got nil instead")
+	}
+
+	if err != ErrCannotCopyDir {
+		t.Fatalf("expected ErrCannotCopyDir error, but got %T: %s", err, err)
+	}
 }
 
 // J. SRC specifies a directory's contents only and DST exists as a directory.
@@ -595,6 +886,11 @@ func TestCopyCaseJ(t *testing.T) {
 
 	var err error
 
+	// first to create an empty dir
+	if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil {
+		t.Fatalf("unable to make dstDir: %s", err)
+	}
+
 	if err = testCopyHelper(t, srcDir, dstDir); err != nil {
 		t.Fatalf("unexpected error %T: %s", err, err)
 	}
@@ -623,3 +919,56 @@ func TestCopyCaseJ(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+// J. Symbol link following version:
+//    SRC specifies a directory's contents only and DST exists as a directory.
+//    This should copy the contents of the SRC directory (but not the directory
+//    itself) into the DST directory. Ensure this works whether DST has a
+//    trailing path separator or not.
+func TestCopyCaseJFSym(t *testing.T) {
+	tmpDirA, tmpDirB := getTestTempDirs(t)
+	defer removeAllPaths(tmpDirA, tmpDirB)
+
+	// Load A and B with some sample files and directories.
+	createSampleDir(t, tmpDirA)
+	createSampleDir(t, tmpDirB)
+
+	srcDir := joinTrailingSep(tmpDirA, "dirSymlink") + "."
+	linkTarget := filepath.Join(tmpDirA, "dir1")
+	dstDir := filepath.Join(tmpDirB, "dir5")
+
+	var err error
+
+	// first to create an empty dir
+	if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil {
+		t.Fatalf("unable to make dstDir: %s", err)
+	}
+
+	if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = dirContentsEqual(t, dstDir, linkTarget); err != nil {
+		t.Fatal(err)
+	}
+
+	// Now try again but using a trailing path separator for dstDir.
+
+	if err = os.RemoveAll(dstDir); err != nil {
+		t.Fatalf("unable to remove dstDir: %s", err)
+	}
+
+	if err = os.MkdirAll(dstDir, os.FileMode(0755)); err != nil {
+		t.Fatalf("unable to make dstDir: %s", err)
+	}
+
+	dstDir = joinTrailingSep(tmpDirB, "dir5")
+
+	if err = testCopyHelperFSym(t, srcDir, dstDir); err != nil {
+		t.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	if err = dirContentsEqual(t, dstDir, linkTarget); err != nil {
+		t.Fatal(err)
+	}
+}