Refactor to resolve with path.Parser

Right now path.Resolve() takes UNIX style paths. In order to add Windows
pathanme support, one approach would be to have path.ResolveUNIX() and
path.ResolveWindows(). But this is not sufficient, because during a
single resolution pass we could encounter both kinds of paths.

We solve this by wrapping pathname strings in a path.Parser. This allows
path.Resolve() to remain a generic resolution algorithm that is
oblivious of the path that is being resolved.

pkg/filesystem/path/BUILD.bazel

@@ -9,10 +9,12 @@ go_library(
         "component_walker.go",
         "components_list.go",
         "loop_detecting_scope_walker.go",
+        "parser.go",
         "relative_scope_walker.go",
         "resolve.go",
         "scope_walker.go",
         "trace.go",
+        "unix_parser.go",
         "virtual_root_scope_walker_factory.go",
         "void_component_walker.go",
         "void_scope_walker.go",

pkg/filesystem/path/absolute_scope_walker_test.go

@@ -19,7 +19,7 @@ func TestAbsoluteScopeWalker(t *testing.T) {
 		componentWalker := mock.NewMockComponentWalker(ctrl)
 		componentWalker.EXPECT().OnTerminal(path.MustNewComponent("hello"))
 
-		require.NoError(t, path.Resolve("/hello", path.NewAbsoluteScopeWalker(componentWalker)))
+		require.NoError(t, path.Resolve(path.MustNewUNIXParser("/hello"), path.NewAbsoluteScopeWalker(componentWalker)))
 	})
 
 	t.Run("Relative", func(t *testing.T) {
@@ -28,6 +28,6 @@ func TestAbsoluteScopeWalker(t *testing.T) {
 		require.Equal(
 			t,
 			status.Error(codes.InvalidArgument, "Path is relative, while an absolute path was expected"),
-			path.Resolve("hello", path.NewAbsoluteScopeWalker(componentWalker)))
+			path.Resolve(path.MustNewUNIXParser("hello"), path.NewAbsoluteScopeWalker(componentWalker)))
 	})
 }

pkg/filesystem/path/builder_test.go

@@ -32,7 +32,7 @@ func TestBuilder(t *testing.T) {
 		} {
 			t.Run(p, func(t *testing.T) {
 				builder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
-				require.NoError(t, path.Resolve(p, scopeWalker))
+				require.NoError(t, path.Resolve(path.MustNewUNIXParser(p), scopeWalker))
 				require.Equal(t, p, builder.String())
 			})
 		}
@@ -57,7 +57,7 @@ func TestBuilder(t *testing.T) {
 		} {
 			t.Run(from, func(t *testing.T) {
 				builder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
-				require.NoError(t, path.Resolve(from, scopeWalker))
+				require.NoError(t, path.Resolve(path.MustNewUNIXParser(from), scopeWalker))
 				require.Equal(t, to, builder.String())
 			})
 		}
@@ -75,7 +75,7 @@ func TestBuilder(t *testing.T) {
 		} {
 			t.Run(from, func(t *testing.T) {
 				builder, scopeWalker := path.RootBuilder.Join(path.VoidScopeWalker)
-				require.NoError(t, path.Resolve(from, scopeWalker))
+				require.NoError(t, path.Resolve(path.MustNewUNIXParser(from), scopeWalker))
 				require.Equal(t, to, builder.String())
 			})
 		}
@@ -97,7 +97,7 @@ func TestBuilder(t *testing.T) {
 		componentWalker2.EXPECT().OnUp().Return(componentWalker3, nil)
 
 		builder, s := path.EmptyBuilder.Join(scopeWalker)
-		require.NoError(t, path.Resolve("hello/..", s))
+		require.NoError(t, path.Resolve(path.MustNewUNIXParser("hello/.."), s))
 		require.Equal(t, ".", builder.String())
 	})
 
@@ -116,7 +116,7 @@ func TestBuilder(t *testing.T) {
 		componentWalker3.EXPECT().OnUp().Return(componentWalker4, nil)
 
 		builder, s := path.EmptyBuilder.Join(scopeWalker)
-		require.NoError(t, path.Resolve("../hello/..", s))
+		require.NoError(t, path.Resolve(path.MustNewUNIXParser("../hello/.."), s))
 		require.Equal(t, "..", builder.String())
 	})
 
@@ -138,7 +138,7 @@ func TestBuilder(t *testing.T) {
 		componentWalker3.EXPECT().OnUp().Return(componentWalker4, nil)
 
 		builder, s := path.EmptyBuilder.Join(scopeWalker)
-		require.NoError(t, path.Resolve("/hello/world/..", s))
+		require.NoError(t, path.Resolve(path.MustNewUNIXParser("/hello/world/.."), s))
 		require.Equal(t, "/hello/", builder.String())
 	})
 }

pkg/filesystem/path/component_walker.go

@@ -56,7 +56,7 @@ type ComponentWalker interface {
 	// If the pathname component refers to a symbolic link, this
 	// function will return a GotSymlink containing a ScopeWalker, which
 	// can be used to perform expansion of the symbolic link. The
-	// Resolve() function will call into OnAbsolute() or OnRealtive() to
+	// Resolve() function will call into OnAbsolute() or OnRelative() to
 	// signal whether resolution should continue at the root directory
 	// or at the directory that contained the symbolic link.
 	OnDirectory(name Component) (GotDirectoryOrSymlink, error)

pkg/filesystem/path/loop_detecting_scope_walker_test.go

@@ -28,7 +28,7 @@ func TestLoopDetectingScopeWalker(t *testing.T) {
 		require.Equal(
 			t,
 			status.Error(codes.InvalidArgument, "Maximum number of symbolic link redirections reached"),
-			path.Resolve("foo", path.NewLoopDetectingScopeWalker(scopeWalker)))
+			path.Resolve(path.MustNewUNIXParser("foo"), path.NewLoopDetectingScopeWalker(scopeWalker)))
 	})
 
 	t.Run("Success", func(t *testing.T) {
@@ -49,6 +49,6 @@ func TestLoopDetectingScopeWalker(t *testing.T) {
 
 		require.NoError(
 			t,
-			path.Resolve("/tmp", path.NewLoopDetectingScopeWalker(scopeWalker1)))
+			path.Resolve(path.MustNewUNIXParser("/tmp"), path.NewLoopDetectingScopeWalker(scopeWalker1)))
 	})
 }

pkg/filesystem/path/parser.go

@@ -0,0 +1,16 @@
+package path
+
+// Parser is used by Resolve to parse paths in the resolution. Implementations
+// of ParseScope() should return a new copy of Parser and leave the current
+// instance unmodified. It is permitted to call ParseScope() multiple times.
+type Parser interface {
+	ParseScope(scopeWalker ScopeWalker) (next ComponentWalker, remainder RelativeParser, err error)
+}
+
+// RelativeParser is used by Resolve to parse relative paths in the resolution.
+// Implementations of ParseFirstComponent() should return a new copy of Parser
+// and leave the current instance unmodified. It is permitted to call
+// ParseFirstComponent() multiple times.
+type RelativeParser interface {
+	ParseFirstComponent(componentWalker ComponentWalker, mustBeDirectory bool) (next GotDirectoryOrSymlink, remainder RelativeParser, err error)
+}

pkg/filesystem/path/relative_scope_walker_test.go

@@ -19,7 +19,7 @@ func TestRelativeScopeWalker(t *testing.T) {
 		componentWalker := mock.NewMockComponentWalker(ctrl)
 		componentWalker.EXPECT().OnTerminal(path.MustNewComponent("hello"))
 
-		require.NoError(t, path.Resolve("hello", path.NewRelativeScopeWalker(componentWalker)))
+		require.NoError(t, path.Resolve(path.MustNewUNIXParser("hello"), path.NewRelativeScopeWalker(componentWalker)))
 	})
 
 	t.Run("Absolute", func(t *testing.T) {
@@ -28,6 +28,6 @@ func TestRelativeScopeWalker(t *testing.T) {
 		require.Equal(
 			t,
 			status.Error(codes.InvalidArgument, "Path is absolute, while a relative path was expected"),
-			path.Resolve("/hello", path.NewRelativeScopeWalker(componentWalker)))
+			path.Resolve(path.MustNewUNIXParser("/hello"), path.NewRelativeScopeWalker(componentWalker)))
 	})
 }

pkg/filesystem/path/resolve.go

@@ -1,153 +1,82 @@
 package path
 
-import (
-	"strings"
-
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
-)
-
-func stripOneOrMoreSlashes(p string) string {
-	for {
-		p = p[1:]
-		if p == "" || p[0] != '/' {
-			return p
-		}
-	}
-}
-
 type resolverState struct {
-	stack           []string
+	stack           []RelativeParser
 	componentWalker ComponentWalker
 }
 
-// Push a new path string onto the stack of paths that need to be
+// Push a new path onto the stack of paths that need to be
 // processed. This happens once when the resolution process starts, and
 // will happen for every symlink encountered.
-func (rs *resolverState) push(scopeWalker ScopeWalker, path string) error {
-	// Unix-style paths are generally passed to system calls that
-	// accept C strings. There is no way these can accept null
-	// bytes.
-	if strings.ContainsRune(path, '\x00') {
-		return status.Error(codes.InvalidArgument, "Path contains a null byte")
-	}
-
-	// Determine whether the path is absolute.
-	absolute := false
-	if path != "" && path[0] == '/' {
-		path = stripOneOrMoreSlashes(path)
-		absolute = true
-	}
-
+func (rs *resolverState) push(scopeWalker ScopeWalker, parser Parser) error {
 	// Push the path without any leading slashes onto the stack, so
-	// that its components may be processed. Apply  them against the
+	// that its components may be processed. Apply them against the
 	// right directory.
-	var componentWalker ComponentWalker
-	var err error
-	if absolute {
-		componentWalker, err = scopeWalker.OnAbsolute()
-	} else {
-		componentWalker, err = scopeWalker.OnRelative()
-	}
+	componentWalker, remainder, err := parser.ParseScope(scopeWalker)
 	if err != nil {
 		return err
 	}
-	rs.stack = append(rs.stack, path)
+	rs.stack = append(rs.stack, remainder)
 	rs.componentWalker = componentWalker
 	return nil
 }
 
 // Pop a single filename from the stack of paths that need to be
 // processed. This filename is the first one that should be processed.
-func (rs *resolverState) pop() string {
-	p := &rs.stack[len(rs.stack)-1]
-	slash := strings.IndexByte(*p, '/')
-	if slash == -1 {
-		// Path no longer contains a slash. Consume it entirely.
-		name := *p
+func (rs *resolverState) pop() (GotDirectoryOrSymlink, error) {
+	p := rs.stack[len(rs.stack)-1]
+	node, remainder, err := p.ParseFirstComponent(rs.componentWalker, len(rs.stack) > 1)
+	if err != nil {
+		return nil, err
+	}
+	if remainder == nil {
 		rs.stack = rs.stack[:len(rs.stack)-1]
-		return name
+	} else {
+		rs.stack[len(rs.stack)-1] = remainder
 	}
-
-	// Consume the next component and as many slashes as possible.
-	name := (*p)[:slash]
-	*p = stripOneOrMoreSlashes((*p)[slash:])
-	return name
+	return node, err
 }
 
 func (rs *resolverState) resolve() error {
 	for len(rs.stack) > 0 {
-		switch name := rs.pop(); name {
-		case "", ".":
-			// An explicit "." entry, or an empty component.
-			// Empty components can occur if paths end with
-			// one or more slashes. Treat "foo/" identical
-			// to "foo/."
-		case "..":
-			// Traverse to the parent directory.
-			componentWalker, err := rs.componentWalker.OnUp()
+		r, err := rs.pop()
+		if err != nil {
+			return err
+		}
+		switch rv := r.(type) {
+		case GotDirectory:
+			rs.componentWalker = rv.Child
+		case GotSymlink:
+			target, err := NewUNIXParser(rv.Target)
 			if err != nil {
 				return err
 			}
-			rs.componentWalker = componentWalker
-		default:
-			if len(rs.stack) > 0 {
-				// A filename that was followed by a
-				// slash, or we are symlink expanding
-				// one or more paths that are followed
-				// by a slash. This component must yield
-				// a directory or symlink.
-				r, err := rs.componentWalker.OnDirectory(Component{
-					name: name,
-				})
-				if err != nil {
-					return err
-				}
-				switch rv := r.(type) {
-				case GotDirectory:
-					rs.componentWalker = rv.Child
-				case GotSymlink:
-					if err := rs.push(rv.Parent, rv.Target); err != nil {
-						return err
-					}
-				default:
-					panic("Missing result")
-				}
-			} else {
-				// This component may be any kind of file.
-				r, err := rs.componentWalker.OnTerminal(Component{
-					name: name,
-				})
-				if err != nil || r == nil {
-					// Path resolution ended with
-					// any file other than a symlink.
-					return err
-				}
-				// Observed a symlink at the end of a
-				// path. We should continue to run.
-				if err := rs.push(r.Parent, r.Target); err != nil {
-					return err
-				}
+			if err := rs.push(rv.Parent, target); err != nil {
+				return err
 			}
+		default:
+			panic("Missing result")
+		case nil: // Do nothing
 		}
 	}
 
 	// Path resolution ended in a directory.
 	return nil
 }
 
-// Resolve a Unix-style pathname string, similar to how the namei()
-// function would work in the kernel. For every productive component in
-// the pathname, a call against a ScopeWalker or ComponentWalker object
-// is made. This object is responsible for registering the path
-// traversal and returning symbolic link contents.
+// Resolve a pathname string, similar to how the namei() function would
+// work in the kernel. For every productive component in the pathname, a
+// call against a ScopeWalker or ComponentWalker object is made. This
+// object is responsible for registering the path traversal and
+// returning symbolic link contents. Unix-style paths can be created
+// with NewUNIXParser.
 //
 // This function only implements the core algorithm for path resolution.
 // Features like symlink loop detection, chrooting, etc. should all be
 // implemented as decorators for ScopeWalker and ComponentWalker.
-func Resolve(path string, scopeWalker ScopeWalker) error {
+func Resolve(parser Parser, scopeWalker ScopeWalker) error {
 	state := resolverState{}
-	if err := state.push(scopeWalker, path); err != nil {
+	if err := state.push(scopeWalker, parser); err != nil {
 		return err
 	}
 	return state.resolve()