From 588f95dca07f6aae693d786224e6c74e53552d71 Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Tue, 17 Feb 2015 12:27:07 -0800 Subject: [PATCH] pkg/archive: Canonicalize stored paths Currently pkg/archive stores nested windows files with backslashes (e.g. `dir\`, `dir\file.txt`) and this causes tar not being correctly extracted on Linux daemon. This change assures we canonicalize all paths to unix paths and add them to tar with that name independent of platform. Fixes the following test cases for Windows CI: - TestBuildAddFileWithWhitespace - TestBuildCopyFileWithWhitespace - TestBuildAddDirContentToRoot - TestBuildAddDirContentToExistingDir - TestBuildCopyDirContentToRoot - TestBuildCopyDirContentToExistDir - TestBuildDockerignore - TestBuildEnvUsage - TestBuildEnvUsage2 Signed-off-by: Ahmet Alp Balkan --- archive/archive.go | 21 ++++++++++++--- archive/archive_unix.go | 7 +++++ archive/archive_unix_test.go | 42 +++++++++++++++++++++++++++++ archive/archive_windows.go | 17 ++++++++++++ archive/archive_windows_test.go | 48 +++++++++++++++++++++++++++++++++ 5 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 archive/archive_unix_test.go create mode 100644 archive/archive_windows_test.go diff --git a/archive/archive.go b/archive/archive.go index d9fcead..201cd46 100644 --- a/archive/archive.go +++ b/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()) diff --git a/archive/archive_unix.go b/archive/archive_unix.go index c0e8aee..19590ec 100644 --- a/archive/archive_unix.go +++ b/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) diff --git a/archive/archive_unix_test.go b/archive/archive_unix_test.go new file mode 100644 index 0000000..27cfd77 --- /dev/null +++ b/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) + } + } +} diff --git a/archive/archive_windows.go b/archive/archive_windows.go index 3cc2493..b763844 100644 --- a/archive/archive_windows.go +++ b/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 diff --git a/archive/archive_windows_test.go b/archive/archive_windows_test.go new file mode 100644 index 0000000..d79f333 --- /dev/null +++ b/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) + } + } +}