Skip to content

Commit

Permalink
Propagate path.Parser in GotSymlink
Browse files Browse the repository at this point in the history
Right now we assume that symbolic links always contain UNIX style paths.
This is not necessarily true. On systems like Windows, targets may not
only contain backslashes. They could also be prefixed with drive
letters.

Fixes: #190
  • Loading branch information
Nils Wireklint authored and EdSchouten committed Mar 7, 2024
1 parent d8bd9aa commit 4d0d7a3
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 25 deletions.
2 changes: 1 addition & 1 deletion pkg/filesystem/path/component_walker.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type GotSymlink struct {
Parent ScopeWalker

// The contents of the symbolic link.
Target string
Target Parser
}

// GotDirectoryOrSymlink is a union type of GotDirectory and GotSymlink.
Expand Down
4 changes: 2 additions & 2 deletions pkg/filesystem/path/loop_detecting_scope_walker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestLoopDetectingScopeWalker(t *testing.T) {
componentWalker := mock.NewMockComponentWalker(ctrl)
scopeWalker.EXPECT().OnRelative().Return(componentWalker, nil).Times(41)
componentWalker.EXPECT().OnTerminal(path.MustNewComponent("foo")).
Return(&path.GotSymlink{Parent: scopeWalker, Target: "foo"}, nil).
Return(&path.GotSymlink{Parent: scopeWalker, Target: path.MustNewUNIXParser("foo")}, nil).
Times(41)

require.Equal(
Expand All @@ -38,7 +38,7 @@ func TestLoopDetectingScopeWalker(t *testing.T) {
scopeWalker1.EXPECT().OnAbsolute().Return(componentWalker1, nil)
scopeWalker2 := mock.NewMockScopeWalker(ctrl)
componentWalker1.EXPECT().OnTerminal(path.MustNewComponent("tmp")).
Return(&path.GotSymlink{Parent: scopeWalker2, Target: "private/tmp"}, nil)
Return(&path.GotSymlink{Parent: scopeWalker2, Target: path.MustNewUNIXParser("private/tmp")}, nil)
componentWalker2 := mock.NewMockComponentWalker(ctrl)
scopeWalker2.EXPECT().OnRelative().Return(componentWalker2, nil)
componentWalker3 := mock.NewMockComponentWalker(ctrl)
Expand Down
13 changes: 7 additions & 6 deletions pkg/filesystem/path/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,17 @@ func (rs *resolverState) resolve() error {
case GotDirectory:
rs.componentWalker = rv.Child
case GotSymlink:
target, err := NewUNIXParser(rv.Target)
if err != nil {
return err
}
if err := rs.push(rv.Parent, target); err != nil {
if err := rs.push(rv.Parent, rv.Target); err != nil {
return err
}
default:
panic("Missing result")
case nil: // Do nothing
case nil:
// Parsed a terminal component. This should only be permitted at the
// very end of the path resolution process.
if len(rs.stack) > 0 {
panic("Parser.ParseFirstComponent() did not respect mustBeDirectory")
}
}
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/filesystem/path/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestResolve(t *testing.T) {
scopeWalker1.EXPECT().OnRelative().Return(componentWalker1, nil)
scopeWalker2 := mock.NewMockScopeWalker(ctrl)
componentWalker1.EXPECT().OnTerminal(path.MustNewComponent("a")).
Return(&path.GotSymlink{Parent: scopeWalker2, Target: "b"}, nil)
Return(&path.GotSymlink{Parent: scopeWalker2, Target: path.MustNewUNIXParser("b")}, nil)
componentWalker2 := mock.NewMockComponentWalker(ctrl)
scopeWalker2.EXPECT().OnRelative().Return(componentWalker2, nil)
componentWalker2.EXPECT().OnTerminal(path.MustNewComponent("b"))
Expand All @@ -122,7 +122,7 @@ func TestResolve(t *testing.T) {
scopeWalker1.EXPECT().OnRelative().Return(componentWalker1, nil)
scopeWalker2 := mock.NewMockScopeWalker(ctrl)
componentWalker1.EXPECT().OnTerminal(path.MustNewComponent("a")).
Return(&path.GotSymlink{Parent: scopeWalker2, Target: "b/"}, nil)
Return(&path.GotSymlink{Parent: scopeWalker2, Target: path.MustNewUNIXParser("b/")}, nil)
componentWalker2 := mock.NewMockComponentWalker(ctrl)
scopeWalker2.EXPECT().OnRelative().Return(componentWalker2, nil)
componentWalker3 := mock.NewMockComponentWalker(ctrl)
Expand All @@ -138,7 +138,7 @@ func TestResolve(t *testing.T) {
scopeWalker1.EXPECT().OnRelative().Return(componentWalker1, nil)
scopeWalker2 := mock.NewMockScopeWalker(ctrl)
componentWalker1.EXPECT().OnDirectory(path.MustNewComponent("a")).
Return(path.GotSymlink{Parent: scopeWalker2, Target: "b"}, nil)
Return(path.GotSymlink{Parent: scopeWalker2, Target: path.MustNewUNIXParser("b")}, nil)
componentWalker2 := mock.NewMockComponentWalker(ctrl)
scopeWalker2.EXPECT().OnRelative().Return(componentWalker2, nil)
componentWalker3 := mock.NewMockComponentWalker(ctrl)
Expand All @@ -154,17 +154,17 @@ func TestResolve(t *testing.T) {
scopeWalker1.EXPECT().OnRelative().Return(componentWalker1, nil)
scopeWalker2 := mock.NewMockScopeWalker(ctrl)
componentWalker1.EXPECT().OnTerminal(path.MustNewComponent("a")).
Return(&path.GotSymlink{Parent: scopeWalker2, Target: "b/z"}, nil)
Return(&path.GotSymlink{Parent: scopeWalker2, Target: path.MustNewUNIXParser("b/z")}, nil)
componentWalker2 := mock.NewMockComponentWalker(ctrl)
scopeWalker2.EXPECT().OnRelative().Return(componentWalker2, nil)
scopeWalker3 := mock.NewMockScopeWalker(ctrl)
componentWalker2.EXPECT().OnDirectory(path.MustNewComponent("b")).
Return(path.GotSymlink{Parent: scopeWalker3, Target: "c/y"}, nil)
Return(path.GotSymlink{Parent: scopeWalker3, Target: path.MustNewUNIXParser("c/y")}, nil)
componentWalker3 := mock.NewMockComponentWalker(ctrl)
scopeWalker3.EXPECT().OnRelative().Return(componentWalker3, nil)
scopeWalker4 := mock.NewMockScopeWalker(ctrl)
componentWalker3.EXPECT().OnDirectory(path.MustNewComponent("c")).
Return(path.GotSymlink{Parent: scopeWalker4, Target: "x"}, nil)
Return(path.GotSymlink{Parent: scopeWalker4, Target: path.MustNewUNIXParser("x")}, nil)
componentWalker4 := mock.NewMockComponentWalker(ctrl)
scopeWalker4.EXPECT().OnRelative().Return(componentWalker4, nil)
componentWalker5 := mock.NewMockComponentWalker(ctrl)
Expand Down
18 changes: 11 additions & 7 deletions pkg/filesystem/path/virtual_root_scope_walker_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type namelessVirtualRootNode struct {
// that point into the root directory.
type namedVirtualRootNode struct {
nameless namelessVirtualRootNode
target string
target Parser
}

// virtualRootNodeCreator is an implementation of ComponentWalker that
Expand All @@ -37,7 +37,7 @@ type virtualRootNodeCreator struct {
func (cw *virtualRootNodeCreator) OnDirectory(name Component) (GotDirectoryOrSymlink, error) {
n, ok := cw.namelessNode.down[name]
if ok {
if n.nameless.isRoot || n.target != "" {
if n.nameless.isRoot || n.target != nil {
return nil, status.Error(codes.InvalidArgument, "Path resides at or below an already registered path")
}
} else {
Expand Down Expand Up @@ -129,7 +129,7 @@ func NewVirtualRootScopeWalkerFactory(rootPath string, aliases map[string]string

path, err := NewUNIXParser(rootPath)
if err != nil {
return nil, err
return nil, util.StatusWrapf(err, "Failed to parse root path %#v", rootPath)
}
if err := Resolve(path, rootPathWalker); err != nil {
return nil, util.StatusWrapf(err, "Failed to resolve root path %#v", rootPath)
Expand All @@ -139,11 +139,11 @@ func NewVirtualRootScopeWalkerFactory(rootPath string, aliases map[string]string
for alias, target := range aliases {
aliasPath, err := NewUNIXParser(alias)
if err != nil {
return nil, err
return nil, util.StatusWrapf(err, "Failed to parse alias path %#v", rootPath)
}
targetPath, err := NewUNIXParser(target)
if err != nil {
return nil, err
return nil, util.StatusWrapf(err, "Failed to parse target path %#v", targetPath)
}
// Resolve the location at which we want to create a fictive
// symlink that points into the virtual root directory.
Expand All @@ -165,7 +165,11 @@ func NewVirtualRootScopeWalkerFactory(rootPath string, aliases map[string]string
if err := Resolve(targetPath, targetPathWalker); err != nil {
return nil, util.StatusWrapf(err, "Failed to resolve alias target %#v", target)
}
aliasCreator.namedNode.target = targetPathBuilder.String()
parser, err := NewUNIXParser(targetPathBuilder.String())
if err != nil {
return nil, err
}
aliasCreator.namedNode.target = parser
}
return wf, nil
}
Expand Down Expand Up @@ -242,7 +246,7 @@ func (cw *pendingVirtualRootComponentWalker) OnDirectory(name Component) (GotDir
// the full path.
return VoidComponentWalker.OnDirectory(name)
}
if n.target != "" {
if n.target != nil {
// Found an alias to the underlying root directory.
return GotSymlink{
Parent: cw.walker,
Expand Down
6 changes: 3 additions & 3 deletions pkg/filesystem/path/virtual_root_scope_walker_factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestVirtualRootScopeWalkerFactoryCreationSuccess(t *testing.T) {
componentWalker1.EXPECT().OnTerminal(path.MustNewComponent("a")).
Return(&path.GotSymlink{
Parent: scopeWalker2,
Target: "b",
Target: path.MustNewUNIXParser("b"),
}, nil)
componentWalker2 := mock.NewMockComponentWalker(ctrl)
scopeWalker2.EXPECT().OnRelative().Return(componentWalker2, nil)
Expand All @@ -139,7 +139,7 @@ func TestVirtualRootScopeWalkerFactoryCreationSuccess(t *testing.T) {
componentWalker1.EXPECT().OnTerminal(path.MustNewComponent("a")).
Return(&path.GotSymlink{
Parent: scopeWalker2,
Target: "/root/b",
Target: path.MustNewUNIXParser("/root/b"),
}, nil)
componentWalker2 := mock.NewMockComponentWalker(ctrl)
scopeWalker2.EXPECT().OnAbsolute().Return(componentWalker2, nil)
Expand All @@ -159,7 +159,7 @@ func TestVirtualRootScopeWalkerFactoryCreationSuccess(t *testing.T) {
componentWalker.EXPECT().OnTerminal(path.MustNewComponent("a")).
Return(&path.GotSymlink{
Parent: scopeWalker2,
Target: "/hello",
Target: path.MustNewUNIXParser("/hello"),
}, nil)

require.NoError(t, path.Resolve(path.MustNewUNIXParser("a"), factory.New(scopeWalker1)))
Expand Down

0 comments on commit 4d0d7a3

Please sign in to comment.