Merge pull request #113 from darstahl/ResolveRoot

dmcgowan · web-flow · commit d3c23511c1bf · 2018-05-24T14:06:03.000-07:00
Resolve context root on context.Walk when root is symlink
diff --git a/context.go b/context.go
@@ -572,8 +572,16 @@ func (c *context) Apply(resource Resource) error {
 // the context. Otherwise identical to filepath.Walk, the path argument is
 // corrected to be contained within the context.
 func (c *context) Walk(fn filepath.WalkFunc) error {
-	return c.pathDriver.Walk(c.root, func(p string, fi os.FileInfo, err error) error {
-		contained, err := c.contain(p)
+	root := c.root
+	fi, err := c.driver.Lstat(c.root)
+	if err == nil && fi.Mode()&os.ModeSymlink != 0 {
+		root, err = c.driver.Readlink(c.root)
+		if err != nil {
+			return err
+		}
+	}
+	return c.pathDriver.Walk(root, func(p string, fi os.FileInfo, err error) error {
+		contained, err := c.containWithRoot(p, root)
 		return fn(contained, fi, err)
 	})
 }
@@ -592,7 +600,15 @@ func (c *context) fullpath(p string) (string, error) {
 // contain cleans and santizes the filesystem path p to be an absolute path,
 // effectively relative to the context root.
 func (c *context) contain(p string) (string, error) {
-	sanitized, err := c.pathDriver.Rel(c.root, p)
+	return c.containWithRoot(p, c.root)
+}
+
+// containWithRoot cleans and santizes the filesystem path p to be an absolute path,
+// effectively relative to the passed root. Extra care should be used when calling this
+// instead of contain. This is needed for Walk, as if context root is a symlink,
+// it must be evaluated prior to the Walk
+func (c *context) containWithRoot(p string, root string) (string, error) {
+	sanitized, err := c.pathDriver.Rel(root, p)
 	if err != nil {
 		return "", err
 	}
diff --git a/driver/driver.go b/driver/driver.go
@@ -122,10 +122,6 @@ func (d *driver) Lstat(p string) (os.FileInfo, error) {
 	return os.Lstat(p)
 }
 
-func (d *driver) Readlink(p string) (string, error) {
-	return os.Readlink(p)
-}
-
 func (d *driver) Mkdir(p string, mode os.FileMode) error {
 	return os.Mkdir(p, mode)
 }
diff --git a/driver/driver_unix.go b/driver/driver_unix.go
@@ -120,3 +120,8 @@ func (d *driver) LSetxattr(path string, attrMap map[string][]byte) error {
 func (d *driver) DeviceInfo(fi os.FileInfo) (maj uint64, min uint64, err error) {
 	return devices.DeviceInfo(fi)
 }
+
+// Readlink was forked on Windows to fix a Golang bug, use the "os" package here
+func (d *driver) Readlink(p string) (string, error) {
+	return os.Readlink(p)
+}
diff --git a/driver/driver_windows.go b/driver/driver_windows.go
@@ -3,6 +3,7 @@ package driver
 import (
 	"os"
 
+	"github.com/containerd/continuity/sysx"
 	"github.com/pkg/errors"
 )
 
@@ -19,3 +20,9 @@ func (d *driver) Lchmod(path string, mode os.FileMode) (err error) {
 	// TODO: Use Window's equivalent
 	return os.Chmod(path, mode)
 }
+
+// Readlink is forked in order to support Volume paths which are used
+// in container layers.
+func (d *driver) Readlink(p string) (string, error) {
+	return sysx.Readlink(p)
+}
diff --git a/fs/fstest/compare.go b/fs/fstest/compare.go
@@ -33,7 +33,15 @@ func CheckDirectoryEqual(d1, d2 string) error {
 
 	diff := diffResourceList(m1.Resources, m2.Resources)
 	if diff.HasDiff() {
-		return errors.Errorf("directory diff between %s and %s\n%s", d1, d2, diff.String())
+		if len(diff.Deletions) != 0 {
+			return errors.Errorf("directory diff between %s and %s\n%s", d1, d2, diff.String())
+		}
+		// TODO: Also skip Recycle Bin contents in Windows layers which is used to store deleted files in some cases
+		for _, add := range diff.Additions {
+			if ok, _ := metadataFiles[add.Path()]; !ok {
+				return errors.Errorf("directory diff between %s and %s\n%s", d1, d2, diff.String())
+			}
+		}
 	}
 
 	return nil
diff --git a/fs/fstest/compare_unix.go b/fs/fstest/compare_unix.go
@@ -0,0 +1,5 @@
+// +build !windows
+
+package fstest
+
+var metadataFiles map[string]bool
diff --git a/fs/fstest/compare_windows.go b/fs/fstest/compare_windows.go
@@ -0,0 +1,7 @@
+package fstest
+
+// TODO: Any more metadata files generated by Windows layers?
+var metadataFiles = map[string]bool{
+	"\\System Volume Information": true,
+	"\\WcSandboxState":            true,
+}
diff --git a/fs/fstest/file_unix.go b/fs/fstest/file_unix.go
@@ -27,3 +27,10 @@ func Lchtimes(name string, atime, mtime time.Time) Applier {
 		return unix.UtimesNanoAt(unix.AT_FDCWD, path, utimes[0:], unix.AT_SYMLINK_NOFOLLOW)
 	})
 }
+
+func Base() Applier {
+	return applyFn(func(root string) error {
+		// do nothing, as the base is not special
+		return nil
+	})
+}
diff --git a/fs/fstest/file_windows.go b/fs/fstest/file_windows.go
@@ -12,3 +12,18 @@ func Lchtimes(name string, atime, mtime time.Time) Applier {
 		return errors.New("Not implemented")
 	})
 }
+
+// Base applies the files required to make a valid Windows container layer
+// that the filter will mount. It is used for testing the snapshotter
+func Base() Applier {
+	return Apply(
+		CreateDir("Windows", 0755),
+		CreateDir("Windows/System32", 0755),
+		CreateDir("Windows/System32/Config", 0755),
+		CreateFile("Windows/System32/Config/SYSTEM", []byte("foo\n"), 0777),
+		CreateFile("Windows/System32/Config/SOFTWARE", []byte("foo\n"), 0777),
+		CreateFile("Windows/System32/Config/SAM", []byte("foo\n"), 0777),
+		CreateFile("Windows/System32/Config/SECURITY", []byte("foo\n"), 0777),
+		CreateFile("Windows/System32/Config/DEFAULT", []byte("foo\n"), 0777),
+	)
+}
diff --git a/manifest.go b/manifest.go
@@ -66,7 +66,7 @@ func BuildManifest(ctx Context) (*Manifest, error) {
 			return fmt.Errorf("error walking %s: %v", p, err)
 		}
 
-		if p == "/" {
+		if p == string(os.PathSeparator) {
 			// skip root
 			return nil
 		}
diff --git a/syscallx/syscall_unix.go b/syscallx/syscall_unix.go
@@ -0,0 +1,10 @@
+// +build !windows
+
+package syscallx
+
+import "syscall"
+
+// Readlink returns the destination of the named symbolic link.
+func Readlink(path string, buf []byte) (n int, err error) {
+	return syscall.Readlink(path, buf)
+}
diff --git a/syscallx/syscall_windows.go b/syscallx/syscall_windows.go
@@ -0,0 +1,96 @@
+package syscallx
+
+import (
+	"syscall"
+	"unsafe"
+)
+
+type reparseDataBuffer struct {
+	ReparseTag        uint32
+	ReparseDataLength uint16
+	Reserved          uint16
+
+	// GenericReparseBuffer
+	reparseBuffer byte
+}
+
+type mountPointReparseBuffer struct {
+	SubstituteNameOffset uint16
+	SubstituteNameLength uint16
+	PrintNameOffset      uint16
+	PrintNameLength      uint16
+	PathBuffer           [1]uint16
+}
+
+type symbolicLinkReparseBuffer struct {
+	SubstituteNameOffset uint16
+	SubstituteNameLength uint16
+	PrintNameOffset      uint16
+	PrintNameLength      uint16
+	Flags                uint32
+	PathBuffer           [1]uint16
+}
+
+const (
+	_IO_REPARSE_TAG_MOUNT_POINT = 0xA0000003
+	_SYMLINK_FLAG_RELATIVE      = 1
+)
+
+// Readlink returns the destination of the named symbolic link.
+func Readlink(path string, buf []byte) (n int, err error) {
+	fd, err := syscall.CreateFile(syscall.StringToUTF16Ptr(path), syscall.GENERIC_READ, 0, nil, syscall.OPEN_EXISTING,
+		syscall.FILE_FLAG_OPEN_REPARSE_POINT|syscall.FILE_FLAG_BACKUP_SEMANTICS, 0)
+	if err != nil {
+		return -1, err
+	}
+	defer syscall.CloseHandle(fd)
+
+	rdbbuf := make([]byte, syscall.MAXIMUM_REPARSE_DATA_BUFFER_SIZE)
+	var bytesReturned uint32
+	err = syscall.DeviceIoControl(fd, syscall.FSCTL_GET_REPARSE_POINT, nil, 0, &rdbbuf[0], uint32(len(rdbbuf)), &bytesReturned, nil)
+	if err != nil {
+		return -1, err
+	}
+
+	rdb := (*reparseDataBuffer)(unsafe.Pointer(&rdbbuf[0]))
+	var s string
+	switch rdb.ReparseTag {
+	case syscall.IO_REPARSE_TAG_SYMLINK:
+		data := (*symbolicLinkReparseBuffer)(unsafe.Pointer(&rdb.reparseBuffer))
+		p := (*[0xffff]uint16)(unsafe.Pointer(&data.PathBuffer[0]))
+		s = syscall.UTF16ToString(p[data.SubstituteNameOffset/2 : (data.SubstituteNameOffset+data.SubstituteNameLength)/2])
+		if data.Flags&_SYMLINK_FLAG_RELATIVE == 0 {
+			if len(s) >= 4 && s[:4] == `\??\` {
+				s = s[4:]
+				switch {
+				case len(s) >= 2 && s[1] == ':': // \??\C:\foo\bar
+					// do nothing
+				case len(s) >= 4 && s[:4] == `UNC\`: // \??\UNC\foo\bar
+					s = `\\` + s[4:]
+				default:
+					// unexpected; do nothing
+				}
+			} else {
+				// unexpected; do nothing
+			}
+		}
+	case _IO_REPARSE_TAG_MOUNT_POINT:
+		data := (*mountPointReparseBuffer)(unsafe.Pointer(&rdb.reparseBuffer))
+		p := (*[0xffff]uint16)(unsafe.Pointer(&data.PathBuffer[0]))
+		s = syscall.UTF16ToString(p[data.SubstituteNameOffset/2 : (data.SubstituteNameOffset+data.SubstituteNameLength)/2])
+		if len(s) >= 4 && s[:4] == `\??\` { // \??\C:\foo\bar
+			if len(s) < 48 || s[:11] != `\??\Volume{` {
+				s = s[4:]
+			}
+		} else {
+			// unexpected; do nothing
+		}
+	default:
+		// the path is not a symlink or junction but another type of reparse
+		// point
+		return -1, syscall.ENOENT
+	}
+	n = copy(buf, []byte(s))
+
+	return n, nil
+}
diff --git a/sysx/file_posix.go b/sysx/file_posix.go
@@ -0,0 +1,112 @@
+package sysx
+
+import (
+	"os"
+	"path/filepath"
+
+	"github.com/containerd/continuity/syscallx"
+)
+
+// Readlink returns the destination of the named symbolic link.
+// If there is an error, it will be of type *PathError.
+func Readlink(name string) (string, error) {
+	for len := 128; ; len *= 2 {
+		b := make([]byte, len)
+		n, e := fixCount(syscallx.Readlink(fixLongPath(name), b))
+		if e != nil {
+			return "", &os.PathError{Op: "readlink", Path: name, Err: e}
+		}
+		if n < len {
+			return string(b[0:n]), nil
+		}
+	}
+}
+
+// Many functions in package syscall return a count of -1 instead of 0.
+// Using fixCount(call()) instead of call() corrects the count.
+func fixCount(n int, err error) (int, error) {
+	if n < 0 {
+		n = 0
+	}
+	return n, err
+}
+
+// fixLongPath returns the extended-length (\\?\-prefixed) form of
+// path when needed, in order to avoid the default 260 character file
+// path limit imposed by Windows. If path is not easily converted to
+// the extended-length form (for example, if path is a relative path
+// or contains .. elements), or is short enough, fixLongPath returns
+// path unmodified.
+//
+// See https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath
+func fixLongPath(path string) string {
+	// Do nothing (and don't allocate) if the path is "short".
+	// Empirically (at least on the Windows Server 2013 builder),
+	// the kernel is arbitrarily okay with < 248 bytes. That
+	// matches what the docs above say:
+	// "When using an API to create a directory, the specified
+	// path cannot be so long that you cannot append an 8.3 file
+	// name (that is, the directory name cannot exceed MAX_PATH
+	// minus 12)." Since MAX_PATH is 260, 260 - 12 = 248.
+	//
+	// The MSDN docs appear to say that a normal path that is 248 bytes long
+	// will work; empirically the path must be less then 248 bytes long.
+	if len(path) < 248 {
+		// Don't fix. (This is how Go 1.7 and earlier worked,
+		// not automatically generating the \\?\ form)
+		return path
+	}
+
+	// The extended form begins with \\?\, as in
+	// \\?\c:\windows\foo.txt or \\?\UNC\server\share\foo.txt.
+	// The extended form disables evaluation of . and .. path
+	// elements and disables the interpretation of / as equivalent
+	// to \. The conversion here rewrites / to \ and elides
+	// . elements as well as trailing or duplicate separators. For
+	// simplicity it avoids the conversion entirely for relative
+	// paths or paths containing .. elements. For now,
+	// \\server\share paths are not converted to
+	// \\?\UNC\server\share paths because the rules for doing so
+	// are less well-specified.
+	if len(path) >= 2 && path[:2] == `\\` {
+		// Don't canonicalize UNC paths.
+		return path
+	}
+	if !filepath.IsAbs(path) {
+		// Relative path
+		return path
+	}
+
+	const prefix = `\\?`
+
+	pathbuf := make([]byte, len(prefix)+len(path)+len(`\`))
+	copy(pathbuf, prefix)
+	n := len(path)
+	r, w := 0, len(prefix)
+	for r < n {
+		switch {
+		case os.IsPathSeparator(path[r]):
+			// empty block
+			r++
+		case path[r] == '.' && (r+1 == n || os.IsPathSeparator(path[r+1])):
+			// /./
+			r++
+		case r+1 < n && path[r] == '.' && path[r+1] == '.' && (r+2 == n || os.IsPathSeparator(path[r+2])):
+			// /../ is currently unhandled
+			return path
+		default:
+			pathbuf[w] = '\\'
+			w++
+			for ; r < n && !os.IsPathSeparator(path[r]); r++ {
+				pathbuf[w] = path[r]
+				w++
+			}
+		}
+	}
+	// A drive's root directory needs a trailing \
+	if w == len(`\\?\c:`) {
+		pathbuf[w] = '\\'
+		w++
+	}
+	return string(pathbuf[:w])
+}

Original file line number	Diff line number	Diff line change
`@@ -122,10 +122,6 @@ func (d *driver) Lstat(p string) (os.FileInfo, error) {`
`122`	`122`	`return os.Lstat(p)`
`123`	`123`	`}`
`124`	`124`
`125`		`-func (d *driver) Readlink(p string) (string, error) {`
`126`		`- return os.Readlink(p)`
`127`		`-}`
`128`		`-`
`129`	`125`	`func (d *driver) Mkdir(p string, mode os.FileMode) error {`
`130`	`126`	`return os.Mkdir(p, mode)`
`131`	`127`	`}`
Original file line number	Diff line number	Diff line change
`@@ -120,3 +120,8 @@ func (d *driver) LSetxattr(path string, attrMap map[string][]byte) error {`
`120`	`120`	`func (d *driver) DeviceInfo(fi os.FileInfo) (maj uint64, min uint64, err error) {`
`121`	`121`	`return devices.DeviceInfo(fi)`
`122`	`122`	`}`
	`123`	`+`
	`124`	`+// Readlink was forked on Windows to fix a Golang bug, use the "os" package here`
	`125`	`+func (d *driver) Readlink(p string) (string, error) {`
	`126`	`+ return os.Readlink(p)`
	`127`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ func BuildManifest(ctx Context) (*Manifest, error) {`
`66`	`66`	`return fmt.Errorf("error walking %s: %v", p, err)`
`67`	`67`	`}`
`68`	`68`
`69`		`- if p == "/" {`
	`69`	`+ if p == string(os.PathSeparator) {`
`70`	`70`	`// skip root`
`71`	`71`	`return nil`
`72`	`72`	`}`