Merge pull request #22353 from Microsoft/jjh/dockercp

Windows: docker cp platform semantically consistent paths

container/archive.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 
 	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/system"
 	"github.com/docker/engine-api/types"
 )
 
@@ -13,6 +14,12 @@ import (
 // the absolute path to the resource relative to the container's rootfs, and
 // an error if the path points to outside the container's rootfs.
 func (container *Container) ResolvePath(path string) (resolvedPath, absPath string, err error) {
+	// Check if a drive letter supplied, it must be the system drive. No-op except on Windows
+	path, err = system.CheckSystemDriveAndRemoveDriveLetter(path)
+	if err != nil {
+		return "", "", err
+	}
+
 	// Consider the given path as an absolute path in the container.
 	absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join(string(filepath.Separator), path), path)
 

daemon/archive.go

@@ -13,6 +13,7 @@ import (
 	"github.com/docker/docker/pkg/chrootarchive"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/ioutils"
+	"github.com/docker/docker/pkg/system"
 	"github.com/docker/engine-api/types"
 )
 
@@ -188,6 +189,12 @@ func (daemon *Daemon) containerExtractToDir(container *container.Container, path
 		return err
 	}
 
+	// Check if a drive letter supplied, it must be the system drive. No-op except on Windows
+	path, err = system.CheckSystemDriveAndRemoveDriveLetter(path)
+	if err != nil {
+		return err
+	}
+
 	// 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

daemon/archive_unix.go

@@ -3,9 +3,10 @@
 package daemon
 
 import (
-	"github.com/docker/docker/container"
 	"os"
 	"path/filepath"
+
+	"github.com/docker/docker/container"
 )
 
 // checkIfPathIsInAVolume checks if the path is in a volume. If it is, it

pkg/system/path_unix.go

@@ -6,3 +6,9 @@ package system
 // executables. Each directory is separated from the next by a colon
 // ':' character .
 const DefaultPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+
+// CheckSystemDriveAndRemoveDriveLetter verifies that a path, if it includes a drive letter,
+// is the system drive. This is a no-op on Linux.
+func CheckSystemDriveAndRemoveDriveLetter(path string) (string, error) {
+	return path, nil
+}

pkg/system/path_windows.go

@@ -2,6 +2,36 @@
 
 package system
 
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+)
+
 // DefaultPathEnv is deliberately empty on Windows as the default path will be set by
 // the container. Docker has no context of what the default path should be.
 const DefaultPathEnv = ""
+
+// CheckSystemDriveAndRemoveDriveLetter verifies and manipulates a Windows path.
+// This is used, for example, when validating a user provided path in docker cp.
+// If a drive letter is supplied, it must be the system drive. The drive letter
+// is always removed. Also, it translates it to OS semantics (IOW / to \). We
+// need the path in this syntax so that it can ultimately be contatenated with
+// a Windows long-path which doesn't support drive-letters. Examples:
+// C:			--> Fail
+// C:\			--> \
+// a			--> a
+// /a			--> \a
+// d:\			--> Fail
+func CheckSystemDriveAndRemoveDriveLetter(path string) (string, error) {
+	if len(path) == 2 && string(path[1]) == ":" {
+		return "", fmt.Errorf("No relative path specified in %q", path)
+	}
+	if !filepath.IsAbs(path) || len(path) < 2 {
+		return filepath.FromSlash(path), nil
+	}
+	if string(path[1]) == ":" && !strings.EqualFold(string(path[0]), "c") {
+		return "", fmt.Errorf("The specified path is not on the system drive (C:)")
+	}
+	return filepath.FromSlash(path[2:]), nil
+}

pkg/system/path_windows_test.go

@@ -0,0 +1,78 @@
+// +build windows
+
+package system
+
+import "testing"
+
+// TestCheckSystemDriveAndRemoveDriveLetter tests CheckSystemDriveAndRemoveDriveLetter
+func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
+	// Fails if not C drive.
+	path, err := CheckSystemDriveAndRemoveDriveLetter(`d:\`)
+	if err == nil || (err != nil && err.Error() != "The specified path is not on the system drive (C:)") {
+		t.Fatalf("Expected error for d:")
+	}
+
+	// Single character is unchanged
+	if path, err = CheckSystemDriveAndRemoveDriveLetter("z"); err != nil {
+		t.Fatalf("Single character should pass")
+	}
+	if path != "z" {
+		t.Fatalf("Single character should be unchanged")
+	}
+
+	// Two characters without colon is unchanged
+	if path, err = CheckSystemDriveAndRemoveDriveLetter("AB"); err != nil {
+		t.Fatalf("2 characters without colon should pass")
+	}
+	if path != "AB" {
+		t.Fatalf("2 characters without colon should be unchanged")
+	}
+
+	// Abs path without drive letter
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`\l`); err != nil {
+		t.Fatalf("abs path no drive letter should pass")
+	}
+	if path != `\l` {
+		t.Fatalf("abs path without drive letter should be unchanged")
+	}
+
+	// Abs path without drive letter, linux style
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`/l`); err != nil {
+		t.Fatalf("abs path no drive letter linux style should pass")
+	}
+	if path != `\l` {
+		t.Fatalf("abs path without drive letter linux failed %s", path)
+	}
+
+	// Drive-colon should be stripped
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:\`); err != nil {
+		t.Fatalf("An absolute path should pass")
+	}
+	if path != `\` {
+		t.Fatalf(`An absolute path should have been shortened to \ %s`, path)
+	}
+
+	// Verify with a linux-style path
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:/`); err != nil {
+		t.Fatalf("An absolute path should pass")
+	}
+	if path != `\` {
+		t.Fatalf(`A linux style absolute path should have been shortened to \ %s`, path)
+	}
+
+	// Failure on c:
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:`); err == nil {
+		t.Fatalf("c: should fail")
+	}
+	if err.Error() != `No relative path specified in "c:"` {
+		t.Fatalf(path, err)
+	}
+
+	// Failure on d:
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`d:`); err == nil {
+		t.Fatalf("c: should fail")
+	}
+	if err.Error() != `No relative path specified in "d:"` {
+		t.Fatalf(path, err)
+	}
+}