Implement Windows drive letter support

Author: Nils Wireklint

pkg/filesystem/local_directory_windows.go

@@ -109,11 +109,7 @@ func newLocalDirectory(absPath string, openReparsePoint bool) (DirectoryCloser,
 }
 
 func NewLocalDirectory(path string) (DirectoryCloser, error) {
-	absPath, err := filepath.Abs(path)
-	if err != nil {
-		return nil, err
-	}
-	absPath = "\\??\\" + absPath
+	absPath := "\\??\\" + path
 	return newLocalDirectory(absPath, true)
 }
 

pkg/filesystem/path/BUILD.bazel

@@ -21,6 +21,7 @@ go_library(
         "virtual_root_scope_walker_factory.go",
         "void_component_walker.go",
         "void_scope_walker.go",
+        "windows_parser.go",
     ],
     importpath = "github.com/buildbarn/bb-storage/pkg/filesystem/path",
     visibility = ["//visibility:public"],

pkg/filesystem/path/absolute_scope_walker.go

@@ -24,3 +24,7 @@ func (pw *absoluteScopeWalker) OnRelative() (ComponentWalker, error) {
 func (pw *absoluteScopeWalker) OnAbsolute() (ComponentWalker, error) {
 	return pw.componentWalker, nil
 }
+
+func (pw *absoluteScopeWalker) OnDriveLetter(drive rune) (ComponentWalker, error) {
+	return pw.componentWalker, nil
+}

pkg/filesystem/path/builder.go

@@ -2,6 +2,9 @@ package path
 
 import (
 	"strings"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 // Builder for normalized pathname strings.
@@ -20,6 +23,7 @@ import (
 // system.
 type Builder struct {
 	absolute             bool
+	driveLetter          rune
 	components           []string
 	firstReversibleIndex int
 	suffix               string
@@ -41,8 +45,17 @@ var EmptyBuilder = Builder{
 // Builder that use this path as their starting point can be created by
 // calling RootBuilder.Join().
 var RootBuilder = Builder{
-	absolute: true,
-	suffix:   "/",
+	absolute:    true,
+	suffix:      "/",
+}
+
+// NewDriveLetterBuilder returns a builder rooted at a Windows drive.
+func NewDriveLetterBuilder(drive rune) Builder {
+	return Builder{
+		absolute:    false,
+		driveLetter: drive,
+		suffix:      "/",
+	}
 }
 
 // GetUNIXString returns a string representation of the path for use on
@@ -66,10 +79,63 @@ func (b *Builder) GetUNIXString() string {
 	return out.String()
 }
 
+// GetWindowsString returns a string representation of the path for use on
+// Windows.
+func (b *Builder) GetWindowsString() (string, error) {
+	// Emit pathname components.
+	var out strings.Builder
+	prefix := ""
+
+	if b.driveLetter != rune(0) {
+		out.WriteString(string(b.driveLetter))
+		out.WriteString(":")
+		prefix = "\\"
+	} else if b.absolute {
+		prefix = "\\"
+	}
+
+	for _, component := range b.components {
+		for _, c := range component {
+			if byte(c) < 31 {
+				return "", status.Errorf(codes.InvalidArgument, "Path component contains a control character %#v.", component)
+			}
+			for _, forbidden := range []rune {
+				'<',
+				'>',
+				':',
+				'"',
+				'/',
+				'\\',
+				'|',
+				'?',
+				'*',
+			} {
+				if c == forbidden {
+					return "", status.Errorf(codes.InvalidArgument, "Path component contains a forbidden character %#v %#v", component, forbidden)
+				}
+			}
+		}
+		out.WriteString(prefix)
+		out.WriteString(component)
+		prefix = "\\"
+	}
+
+	// Emit trailing slash in case the path refers to a directory, or a dot or
+	// slash if the path is empty. The suffix may have been constructed by
+	// platform-independent code that uses forward slashes. To construct a
+	// Windows path we must use a backslash.
+	suffix := b.suffix
+	if suffix == "/" {
+		suffix = "\\"
+	}
+	out.WriteString(suffix)
+	return out.String(), nil
+}
+
 func (b *Builder) addTrailingSlash() {
 	if len(b.components) == 0 {
 		// An empty path. Ensure we either emit a "/" or ".",
-		// depending on whether the path is absolute.
+		// depending on whether the path is absolute/drive letter.
 		if b.absolute {
 			b.suffix = "/"
 		} else {
@@ -104,7 +170,9 @@ func (b *Builder) getComponentWalker(base ComponentWalker) ComponentWalker {
 // directly to Resolve(). This can be used to replay resolution of a
 // previously constructed path.
 func (b *Builder) ParseScope(scopeWalker ScopeWalker) (next ComponentWalker, remainder RelativeParser, err error) {
-	if b.absolute {
+	if b.driveLetter != rune(0) {
+		next, err = scopeWalker.OnDriveLetter(b.driveLetter)
+	} else if b.absolute {
 		next, err = scopeWalker.OnAbsolute()
 	} else {
 		next, err = scopeWalker.OnRelative()
@@ -154,6 +222,20 @@ func (w *buildingScopeWalker) OnAbsolute() (ComponentWalker, error) {
 	return w.b.getComponentWalker(componentWalker), nil
 }
 
+func (w *buildingScopeWalker) OnDriveLetter(drive rune) (ComponentWalker, error) {
+	componentWalker, err := w.base.OnDriveLetter(drive)
+	if err != nil {
+		return nil, err
+	}
+	*w.b = Builder{
+		absolute:    true,
+		driveLetter: drive,
+		components:  w.b.components[:0],
+		suffix:      "/",
+	}
+	return w.b.getComponentWalker(componentWalker), nil
+}
+
 func (w *buildingScopeWalker) OnRelative() (ComponentWalker, error) {
 	componentWalker, err := w.base.OnRelative()
 	if err != nil {

pkg/filesystem/path/builder_test.go

@@ -11,12 +11,17 @@ import (
 
 func TestBuilder(t *testing.T) {
 	ctrl := gomock.NewController(t)
+	getWindowsString := func(t *testing.T, builder *path.Builder) string {
+		s, err := builder.GetWindowsString()
+		require.NoError(t, err)
+		return s
+	}
 
 	// The following paths should remain completely identical when
 	// resolved without making any assumptions about the layout of
 	// the underlying file system. ".." elements should not be
 	// removed from paths.
-	t.Run("Identity", func(t *testing.T) {
+	t.Run("UNIXIdentity", func(t *testing.T) {
 		for _, p := range []string{
 			".",
 			"..",
@@ -31,6 +36,7 @@ func TestBuilder(t *testing.T) {
 			"/hello/../world/foo",
 		} {
 			t.Run(p, func(t *testing.T) {
+				// Unix Parser
 				builder1, scopewalker1 := path.EmptyBuilder.Join(path.VoidScopeWalker)
 				require.NoError(t, path.Resolve(path.MustNewUNIXParser(p), scopewalker1))
 				require.Equal(t, p, builder1.GetUNIXString())
@@ -42,6 +48,81 @@ func TestBuilder(t *testing.T) {
 		}
 	})
 
+	t.Run("WindowsParseUNIXPaths", func(t *testing.T) {
+		for _, data := range [][]string{
+			{".",                   "."},
+			{"..",                  ".."},
+			{"/",                   "\\"},
+			{"hello",               "hello"},
+			{"hello/",              "hello\\"},
+			{"hello/..",            "hello\\.."},
+			{"/hello/",             "\\hello\\"},
+			{"/hello/..",           "\\hello\\.."},
+			{"/hello/../world",     "\\hello\\..\\world"},
+			{"/hello/../world/",    "\\hello\\..\\world\\"},
+			{"/hello/../world/foo", "\\hello\\..\\world\\foo"},
+		} {
+			p := data[0]
+			expected := data[1]
+			t.Run(p, func(t *testing.T) {
+				// Windows Parser, compare Windows and UNIX string identity.
+				builder1, scopewalker1 := path.EmptyBuilder.Join(path.VoidScopeWalker)
+				require.NoError(t, path.Resolve(path.MustNewWindowsParser(p), scopewalker1))
+				require.Equal(t, expected, getWindowsString(t, builder1))
+				require.Equal(t, p, builder1.GetUNIXString())
+
+				builder2, scopewalker2 := path.EmptyBuilder.Join(path.VoidScopeWalker)
+				require.NoError(t, path.Resolve(builder1, scopewalker2))
+				require.Equal(t, expected, getWindowsString(t, builder2))
+				require.Equal(t, p, builder2.GetUNIXString())
+			})
+		}
+	})
+
+	t.Run("WindowsIdentity", func(t *testing.T) {
+		for _, p := range []string{
+			"C:\\",
+			"C:\\hello\\",
+			"C:\\hello\\..",
+			"C:\\hello\\..\\world",
+			"C:\\hello\\..\\world\\",
+			"C:\\hello\\..\\world\\foo",
+		} {
+			t.Run(p, func(t *testing.T) {
+				builder1, scopewalker1 := path.EmptyBuilder.Join(path.VoidScopeWalker)
+				require.NoError(t, path.Resolve(path.MustNewWindowsParser(p), scopewalker1))
+				require.Equal(t, p, getWindowsString(t, builder1))
+
+				builder2, scopewalker2 := path.EmptyBuilder.Join(path.VoidScopeWalker)
+				require.NoError(t, path.Resolve(builder1, scopewalker2))
+				require.Equal(t, p, getWindowsString(t, builder2))
+			})
+		}
+	})
+
+	t.Run("WindowsParseAndWriteUNIXPaths", func(t *testing.T) {
+		for _, data := range [][]string{
+			{"C:\\",                       "/",                         },
+			{"C:\\hello\\",                "/hello/",                  },
+			{"C:\\hello\\..",              "/hello/..",                },
+			{"C:\\hello\\..\\world",       "/hello/../world",         },
+			{"C:\\hello\\..\\world\\",     "/hello/../world/",       },
+			{"C:\\hello\\..\\world\\foo",  "/hello/../world/foo",    },
+		} {
+			p := data[0]
+			expected := data[1]
+			t.Run(p, func(t *testing.T) {
+				builder1, scopewalker1 := path.EmptyBuilder.Join(path.VoidScopeWalker)
+				require.NoError(t, path.Resolve(path.MustNewWindowsParser(p), scopewalker1))
+				require.Equal(t, expected, builder1.GetUNIXString())
+
+				builder2, scopewalker2 := path.EmptyBuilder.Join(path.VoidScopeWalker)
+				require.NoError(t, path.Resolve(builder1, scopewalker2))
+				require.Equal(t, expected, builder2.GetUNIXString())
+			})
+		}
+	})
+
 	// The following paths can be normalized, even when making no
 	// assumptions about the layout of the underlying file system.
 	t.Run("Normalized", func(t *testing.T) {

pkg/filesystem/path/component_walker.go

@@ -56,9 +56,10 @@ 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 OnRelative() to
-	// signal whether resolution should continue at the root directory
-	// or at the directory that contained the symbolic link.
+	// Resolve() function will call into OnAbsolute(), OnRelative() or
+	// OnDriveLetter() to signal whether resolution should continue at
+	// the root directory or at the directory that contained the
+	// symbolic link.
 	OnDirectory(name Component) (GotDirectoryOrSymlink, error)
 
 	// OnTerminal is called for the potentially last pathname

pkg/filesystem/path/local_windows.go

@@ -9,13 +9,11 @@ import (
 // NewLocalParser creates a pathname parser for paths that are native to
 // the locally running operating system.
 func NewLocalParser(path string) (Parser, error) {
-	// TODO: Implement an actual Windows pathname parser.
-	return NewUNIXParser(filepath.ToSlash(path))
+	return NewWindowsParser(filepath.ToSlash(path))
 }
 
 // GetLocalString converts a path to a string representation that is
 // supported by the locally running operating system.
 func GetLocalString(s Stringer) (string, error) {
-	// TODO: Implement an actual Windows pathname formatter.
-	return filepath.FromSlash(s.GetUNIXString()), nil
+	return s.GetWindowsString()
 }

pkg/filesystem/path/loop_detecting_scope_walker.go

@@ -34,6 +34,17 @@ func (w *loopDetectingScopeWalker) OnAbsolute() (ComponentWalker, error) {
 	}, nil
 }
 
+func (w *loopDetectingScopeWalker) OnDriveLetter(drive rune) (ComponentWalker, error) {
+	componentWalker, err := w.base.OnDriveLetter(drive)
+	if err != nil {
+		return nil, err
+	}
+	return &loopDetectingComponentWalker{
+		base:         componentWalker,
+		symlinksLeft: w.symlinksLeft,
+	}, nil
+}
+
 func (w *loopDetectingScopeWalker) OnRelative() (ComponentWalker, error) {
 	componentWalker, err := w.base.OnRelative()
 	if err != nil {

pkg/filesystem/path/relative_scope_walker.go

@@ -21,6 +21,10 @@ func (pw *relativeScopeWalker) OnAbsolute() (ComponentWalker, error) {
 	return nil, status.Error(codes.InvalidArgument, "Path is absolute, while a relative path was expected")
 }
 
+func (pw *relativeScopeWalker) OnDriveLetter(drive rune) (ComponentWalker, error) {
+	return nil, status.Error(codes.InvalidArgument, "Path has a drive letter, while a relative path was expected")
+}
+
 func (pw *relativeScopeWalker) OnRelative() (ComponentWalker, error) {
 	return pw.componentWalker, nil
 }

pkg/filesystem/path/scope_walker.go

@@ -7,7 +7,9 @@ type ScopeWalker interface {
 	// One of these functions is called right before processing the
 	// first component in the path (if any). Based on the
 	// characteristics of the path. Absolute paths are handled through
-	// OnAbsolute(), and relative paths require OnRelative().
+	// OnAbsolute(), and relative paths require OnRelative(), on Windows
+	// absolute paths can also start with a drive letter, which is handled
+	// through OnDriveLetter().
 	//
 	// These functions can be used by the implementation to determine
 	// whether path resolution needs to be relative to the current
@@ -25,4 +27,5 @@ type ScopeWalker interface {
 	// system.
 	OnAbsolute() (ComponentWalker, error)
 	OnRelative() (ComponentWalker, error)
+	OnDriveLetter(drive rune) (ComponentWalker, error)
 }

pkg/filesystem/path/stringer.go

@@ -4,4 +4,5 @@ package path
 // converted to string representations.
 type Stringer interface {
 	GetUNIXString() string
+	GetWindowsString() (string, error)
 }

pkg/filesystem/path/trace.go

@@ -44,3 +44,13 @@ func (t *Trace) GetUNIXString() string {
 	t.writeToStringBuilder(&sb)
 	return sb.String()
 }
+
+// GetWindowsString returns a string representation of the path for use on Windows.
+func (t *Trace) GetWindowsString() (string, error) {
+	if t == nil {
+		return ".", nil
+	}
+	var sb strings.Builder
+	t.writeToStringBuilder(&sb)
+	return sb.String(), nil
+}