Merge pull request #10865 from ahmetalpbalkan/win-cli/tar-path

pkg/archive: Canonicalize stored paths

pkg/archive/archive.go

@@ -172,6 +172,21 @@ type tarAppender struct {
 	SeenFiles map[uint64]string
 }
 
+// canonicalTarName provides a platform-independent and consistent posix-style
+//path for files and directories to be archived regardless of the platform.
+func canonicalTarName(name string, isDir bool) (string, error) {
+	name, err := canonicalTarNameForPath(name)
+	if err != nil {
+		return "", err
+	}
+
+	// suffix with '/' for directories
+	if isDir && !strings.HasSuffix(name, "/") {
+		name += "/"
+	}
+	return name, nil
+}
+
 func (ta *tarAppender) addTarFile(path, name string) error {
 	fi, err := os.Lstat(path)
 	if err != nil {
@@ -190,10 +205,10 @@ func (ta *tarAppender) addTarFile(path, name string) error {
 		return err
 	}
 
-	if fi.IsDir() && !strings.HasSuffix(name, "/") {
-		name = name + "/"
+	name, err = canonicalTarName(name, fi.IsDir())
+	if err != nil {
+		return fmt.Errorf("tar: cannot canonicalize path: %v", err)
 	}
-
 	hdr.Name = name
 
 	nlink, inode, err := setHeaderForSpecialDevice(hdr, ta, name, fi.Sys())

pkg/archive/archive_unix.go

@@ -9,6 +9,13 @@ import (
 	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
 )
 
+// canonicalTarNameForPath returns platform-specific filepath
+// to canonical posix-style path for tar archival. p is relative
+// path.
+func canonicalTarNameForPath(p string) (string, error) {
+	return p, nil // already unix-style
+}
+
 func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) {
 	s, ok := stat.(*syscall.Stat_t)
 

pkg/archive/archive_unix_test.go

@@ -0,0 +1,42 @@
+// +build !windows
+
+package archive
+
+import (
+	"testing"
+)
+
+func TestCanonicalTarNameForPath(t *testing.T) {
+	cases := []struct{ in, expected string }{
+		{"foo", "foo"},
+		{"foo/bar", "foo/bar"},
+		{"foo/dir/", "foo/dir/"},
+	}
+	for _, v := range cases {
+		if out, err := canonicalTarNameForPath(v.in); err != nil {
+			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
+		} else if out != v.expected {
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
+		}
+	}
+}
+
+func TestCanonicalTarName(t *testing.T) {
+	cases := []struct {
+		in       string
+		isDir    bool
+		expected string
+	}{
+		{"foo", false, "foo"},
+		{"foo", true, "foo/"},
+		{"foo/bar", false, "foo/bar"},
+		{"foo/bar", true, "foo/bar/"},
+	}
+	for _, v := range cases {
+		if out, err := canonicalTarName(v.in, v.isDir); err != nil {
+			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
+		} else if out != v.expected {
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
+		}
+	}
+}

pkg/archive/archive_windows.go

@@ -3,9 +3,26 @@
 package archive
 
 import (
+	"fmt"
+	"strings"
+
 	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
 )
 
+// canonicalTarNameForPath returns platform-specific filepath
+// to canonical posix-style path for tar archival. p is relative
+// path.
+func canonicalTarNameForPath(p string) (string, error) {
+	// windows: convert windows style relative path with backslashes
+	// into forward slashes. since windows does not allow '/' or '\'
+	// in file names, it is mostly safe to replace however we must
+	// check just in case
+	if strings.Contains(p, "/") {
+		return "", fmt.Errorf("windows path contains forward slash: %s", p)
+	}
+	return strings.Replace(p, "\\", "/", -1), nil
+}
+
 func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) {
 	// do nothing. no notion of Rdev, Inode, Nlink in stat on Windows
 	return

pkg/archive/archive_windows_test.go

@@ -0,0 +1,48 @@
+// +build windows
+
+package archive
+
+import (
+	"testing"
+)
+
+func TestCanonicalTarNameForPath(t *testing.T) {
+	cases := []struct {
+		in, expected string
+		shouldFail   bool
+	}{
+		{"foo", "foo", false},
+		{"foo/bar", "___", true}, // unix-styled windows path must fail
+		{`foo\bar`, "foo/bar", false},
+		{`foo\bar`, "foo/bar/", false},
+	}
+	for _, v := range cases {
+		if out, err := canonicalTarNameForPath(v.in); err != nil && !v.shouldFail {
+			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
+		} else if v.shouldFail && err == nil {
+			t.Fatalf("canonical path call should have pailed with error. in=%s out=%s", v.in, out)
+		} else if !v.shouldFail && out != v.expected {
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
+		}
+	}
+}
+
+func TestCanonicalTarName(t *testing.T) {
+	cases := []struct {
+		in       string
+		isDir    bool
+		expected string
+	}{
+		{"foo", false, "foo"},
+		{"foo", true, "foo/"},
+		{`foo\bar`, false, "foo/bar"},
+		{`foo\bar`, true, "foo/bar/"},
+	}
+	for _, v := range cases {
+		if out, err := canonicalTarName(v.in, v.isDir); err != nil {
+			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
+		} else if out != v.expected {
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
+		}
+	}
+}