1. docker
  2. Commit 28842d3f093e69cf62a090a532aacba1e6ff6d1c
Browse code

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 <ahmetalpbalkan@gmail.com>

Ahmet Alp Balkan authored on 2015/02/18 05:27:07
Showing 5 changed files
pkg/archive/archive.go
History View file @ 28842d3
... ... 
@@ -172,6 +172,21 @@ type tarAppender struct {
172 172 
 	SeenFiles map[uint64]string
173 173 
 }
174 174
175 
+// canonicalTarName provides a platform-independent and consistent posix-style
176 
+//path for files and directories to be archived regardless of the platform.
177 
+func canonicalTarName(name string, isDir bool) (string, error) {
178 
+	name, err := canonicalTarNameForPath(name)
179 
+	if err != nil {
180 
+		return "", err
181 
+	}
182 
+
183 
+	// suffix with '/' for directories
184 
+	if isDir && !strings.HasSuffix(name, "/") {
185 
+		name += "/"
186 
+	}
187 
+	return name, nil
188 
+}
189 
+
175 190 
 func (ta *tarAppender) addTarFile(path, name string) error {
176 191 
 	fi, err := os.Lstat(path)
177 192 
 	if err != nil {
... ... 
@@ -190,10 +205,10 @@ func (ta *tarAppender) addTarFile(path, name string) error {
190 190 
 		return err
191 191 
 	}
192 192
193 
-	if fi.IsDir() && !strings.HasSuffix(name, "/") {
194 
-		name = name + "/"
193 
+	name, err = canonicalTarName(name, fi.IsDir())
194 
+	if err != nil {
195 
+		return fmt.Errorf("tar: cannot canonicalize path: %v", err)
195 196 
 	}
196 
-
197 197 
 	hdr.Name = name
198 198
199 199 
 	nlink, inode, err := setHeaderForSpecialDevice(hdr, ta, name, fi.Sys())
pkg/archive/archive_unix.go
History View file @ 28842d3
... ... 
@@ -9,6 +9,13 @@ import (
9 9 
 	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
10 10 
 )
11 11
12 
+// canonicalTarNameForPath returns platform-specific filepath
13 
+// to canonical posix-style path for tar archival. p is relative
14 
+// path.
15 
+func canonicalTarNameForPath(p string) (string, error) {
16 
+	return p, nil // already unix-style
17 
+}
18 
+
12 19 
 func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) {
13 20 
 	s, ok := stat.(*syscall.Stat_t)
14 21
pkg/archive/archive_unix_test.go
History View file @ 28842d3
15 22 
new file mode 100644
... ... 
@@ -0,0 +1,42 @@
0 
+// +build !windows
1 
+
2 
+package archive
3 
+
4 
+import (
5 
+	"testing"
6 
+)
7 
+
8 
+func TestCanonicalTarNameForPath(t *testing.T) {
9 
+	cases := []struct{ in, expected string }{
10 
+		{"foo", "foo"},
11 
+		{"foo/bar", "foo/bar"},
12 
+		{"foo/dir/", "foo/dir/"},
13 
+	}
14 
+	for _, v := range cases {
15 
+		if out, err := canonicalTarNameForPath(v.in); err != nil {
16 
+			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
17 
+		} else if out != v.expected {
18 
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
19 
+		}
20 
+	}
21 
+}
22 
+
23 
+func TestCanonicalTarName(t *testing.T) {
24 
+	cases := []struct {
25 
+		in       string
26 
+		isDir    bool
27 
+		expected string
28 
+	}{
29 
+		{"foo", false, "foo"},
30 
+		{"foo", true, "foo/"},
31 
+		{"foo/bar", false, "foo/bar"},
32 
+		{"foo/bar", true, "foo/bar/"},
33 
+	}
34 
+	for _, v := range cases {
35 
+		if out, err := canonicalTarName(v.in, v.isDir); err != nil {
36 
+			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
37 
+		} else if out != v.expected {
38 
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
39 
+		}
40 
+	}
41 
+}
pkg/archive/archive_windows.go
History View file @ 28842d3
... ... 
@@ -3,9 +3,26 @@
3 3 
 package archive
4 4
5 5 
 import (
6 
+	"fmt"
7 
+	"strings"
8 
+
6 9 
 	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
7 10 
 )
8 11
12 
+// canonicalTarNameForPath returns platform-specific filepath
13 
+// to canonical posix-style path for tar archival. p is relative
14 
+// path.
15 
+func canonicalTarNameForPath(p string) (string, error) {
16 
+	// windows: convert windows style relative path with backslashes
17 
+	// into forward slashes. since windows does not allow '/' or '\'
18 
+	// in file names, it is mostly safe to replace however we must
19 
+	// check just in case
20 
+	if strings.Contains(p, "/") {
21 
+		return "", fmt.Errorf("windows path contains forward slash: %s", p)
22 
+	}
23 
+	return strings.Replace(p, "\\", "/", -1), nil
24 
+}
25 
+
9 26 
 func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) {
10 27 
 	// do nothing. no notion of Rdev, Inode, Nlink in stat on Windows
11 28 
 	return
pkg/archive/archive_windows_test.go
History View file @ 28842d3
12 29 
new file mode 100644
... ... 
@@ -0,0 +1,48 @@
0 
+// +build windows
1 
+
2 
+package archive
3 
+
4 
+import (
5 
+	"testing"
6 
+)
7 
+
8 
+func TestCanonicalTarNameForPath(t *testing.T) {
9 
+	cases := []struct {
10 
+		in, expected string
11 
+		shouldFail   bool
12 
+	}{
13 
+		{"foo", "foo", false},
14 
+		{"foo/bar", "___", true}, // unix-styled windows path must fail
15 
+		{`foo\bar`, "foo/bar", false},
16 
+		{`foo\bar`, "foo/bar/", false},
17 
+	}
18 
+	for _, v := range cases {
19 
+		if out, err := canonicalTarNameForPath(v.in); err != nil && !v.shouldFail {
20 
+			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
21 
+		} else if v.shouldFail && err == nil {
22 
+			t.Fatalf("canonical path call should have pailed with error. in=%s out=%s", v.in, out)
23 
+		} else if !v.shouldFail && out != v.expected {
24 
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
25 
+		}
26 
+	}
27 
+}
28 
+
29 
+func TestCanonicalTarName(t *testing.T) {
30 
+	cases := []struct {
31 
+		in       string
32 
+		isDir    bool
33 
+		expected string
34 
+	}{
35 
+		{"foo", false, "foo"},
36 
+		{"foo", true, "foo/"},
37 
+		{`foo\bar`, false, "foo/bar"},
38 
+		{`foo\bar`, true, "foo/bar/"},
39 
+	}
40 
+	for _, v := range cases {
41 
+		if out, err := canonicalTarName(v.in, v.isDir); err != nil {
42 
+			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
43 
+		} else if out != v.expected {
44 
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
45 
+		}
46 
+	}
47 
+}