From a5aed699cfaa4d84b1b134033fb468b3a7a874f0 Mon Sep 17 00:00:00 2001 From: John Howard Date: Wed, 27 Jun 2018 12:07:17 -0700 Subject: [PATCH] LCOW: lazycontext: Use correct lstat, fix archive check Signed-off-by: John Howard --- builder/remotecontext/lazycontext.go | 2 +- pkg/archive/archive.go | 15 ++++----------- pkg/archive/archive_unix.go | 4 ++-- pkg/archive/archive_unix_test.go | 12 ++++-------- pkg/archive/archive_windows.go | 14 ++------------ pkg/archive/archive_windows_test.go | 21 +++++++-------------- 6 files changed, 20 insertions(+), 48 deletions(-) diff --git a/builder/remotecontext/lazycontext.go b/builder/remotecontext/lazycontext.go index 442cecad85..96435f2585 100644 --- a/builder/remotecontext/lazycontext.go +++ b/builder/remotecontext/lazycontext.go @@ -45,7 +45,7 @@ func (c *lazySource) Hash(path string) (string, error) { return "", errors.WithStack(convertPathError(err, cleanPath)) } - fi, err := os.Lstat(fullPath) + fi, err := c.root.Lstat(fullPath) if err != nil { // Backwards compatibility: a missing file returns a path as hash. // This is reached in the case of a broken symlink. diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index daddebded4..86922ddcac 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -367,11 +367,7 @@ func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, erro hdr.AccessTime = time.Time{} hdr.ChangeTime = time.Time{} hdr.Mode = fillGo18FileTypeBits(int64(chmodTarEntry(os.FileMode(hdr.Mode))), fi) - name, err = canonicalTarName(name, fi.IsDir()) - if err != nil { - return nil, fmt.Errorf("tar: cannot canonicalize path: %v", err) - } - hdr.Name = name + hdr.Name = canonicalTarName(name, fi.IsDir()) if err := setHeaderForSpecialDevice(hdr, name, fi.Sys()); err != nil { return nil, err } @@ -447,17 +443,14 @@ func newTarAppender(idMapping *idtools.IDMappings, writer io.Writer, chownOpts * // 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 - } +func canonicalTarName(name string, isDir bool) string { + name = CanonicalTarNameForPath(name) // suffix with '/' for directories if isDir && !strings.HasSuffix(name, "/") { name += "/" } - return name, nil + return name } // addTarFile adds to the tar archive a file from `path` as `name` diff --git a/pkg/archive/archive_unix.go b/pkg/archive/archive_unix.go index e81076c170..fda3f50c6a 100644 --- a/pkg/archive/archive_unix.go +++ b/pkg/archive/archive_unix.go @@ -32,8 +32,8 @@ func getWalkRoot(srcPath string, include string) string { // 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 CanonicalTarNameForPath(p string) string { + return p // already unix-style } // chmodTarEntry is used to adjust the file permissions used in tar header based diff --git a/pkg/archive/archive_unix_test.go b/pkg/archive/archive_unix_test.go index 808878d094..83deab0840 100644 --- a/pkg/archive/archive_unix_test.go +++ b/pkg/archive/archive_unix_test.go @@ -26,10 +26,8 @@ func TestCanonicalTarNameForPath(t *testing.T) { {"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) + if CanonicalTarNameForPath(v.in) != v.expected { + t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, CanonicalTarNameForPath(v.in)) } } } @@ -46,10 +44,8 @@ func TestCanonicalTarName(t *testing.T) { {"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) + if canonicalTarName(v.in, v.isDir) != v.expected { + t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, canonicalTarName(v.in, v.isDir)) } } } diff --git a/pkg/archive/archive_windows.go b/pkg/archive/archive_windows.go index 69aadd823c..3160c5a8fd 100644 --- a/pkg/archive/archive_windows.go +++ b/pkg/archive/archive_windows.go @@ -2,10 +2,8 @@ package archive // import "github.com/docker/docker/pkg/archive" import ( "archive/tar" - "fmt" "os" "path/filepath" - "strings" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/longpath" @@ -26,16 +24,8 @@ func getWalkRoot(srcPath string, include string) string { // 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, string(os.PathSeparator), "/", -1), nil - +func CanonicalTarNameForPath(p string) string { + return filepath.ToSlash(p) } // chmodTarEntry is used to adjust the file permissions used in tar header based diff --git a/pkg/archive/archive_windows_test.go b/pkg/archive/archive_windows_test.go index b3dbb32754..6f0e25ccac 100644 --- a/pkg/archive/archive_windows_test.go +++ b/pkg/archive/archive_windows_test.go @@ -36,19 +36,14 @@ func TestCopyFileWithInvalidDest(t *testing.T) { 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", "foo"}, + {"foo/bar", "foo/bar"}, + {`foo\bar`, "foo/bar"}, } 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 failed 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) + if CanonicalTarNameForPath(v.in) != v.expected { + t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, CanonicalTarNameForPath(v.in)) } } } @@ -65,10 +60,8 @@ func TestCanonicalTarName(t *testing.T) { {`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) + if canonicalTarName(v.in, v.isDir) != v.expected { + t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, canonicalTarName(v.in, v.isDir)) } } }