Skip to content

Commit 077e871

Browse files
tomgrtomgr
committed
Fix resolving chained symlinks with the WinFSP VFS
It turns out that WinFSP's FspFileSystemFindReparsePoint only splits on backslashes when calculating reparse depth. Forward slashes in symlink targets (e.g. from VirtualSymlink) caused chained symlinks to fail. We now convert forward slashes to back slashes when returning the reparse point.
1 parent 749fb40 commit 077e871

File tree

3 files changed

+156
-59
lines changed

3 files changed

+156
-59
lines changed

‎pkg/filesystem/virtual/winfsp/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ go_test(
7474
"//pkg/proto/configuration/filesystem/virtual",
7575
"@com_github_buildbarn_bb_storage//pkg/blockdevice",
7676
"@com_github_buildbarn_bb_storage//pkg/clock",
77+
"@com_github_buildbarn_bb_storage//pkg/filesystem/path",
7778
"@com_github_buildbarn_bb_storage//pkg/program",
7879
"@com_github_buildbarn_bb_storage//pkg/util",
7980
"@com_github_stretchr_testify//require",

‎pkg/filesystem/virtual/winfsp/file_system.go‎

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,30 +1318,6 @@ func FillSymlinkReparseBuffer(target string, flags uint32, buffer []byte) (int,
13181318
return requiredSize, nil
13191319
}
13201320

1321-
type relativePathChecker struct {
1322-
isRelative bool
1323-
}
1324-
1325-
func (d *relativePathChecker) OnAbsolute() (path.ComponentWalker, error) {
1326-
// This counts as relative as it's interpreted as relative to the
1327-
// current drive.
1328-
d.isRelative = true
1329-
return path.VoidComponentWalker, nil
1330-
}
1331-
1332-
func (relativePathChecker) OnDriveLetter(drive rune) (path.ComponentWalker, error) {
1333-
return path.VoidComponentWalker, nil
1334-
}
1335-
1336-
func (d *relativePathChecker) OnRelative() (path.ComponentWalker, error) {
1337-
d.isRelative = true
1338-
return path.VoidComponentWalker, nil
1339-
}
1340-
1341-
func (relativePathChecker) OnShare(server, share string) (path.ComponentWalker, error) {
1342-
return path.VoidComponentWalker, nil
1343-
}
1344-
13451321
func getReparsePointForLeaf(ctx context.Context, leaf virtual.Leaf, buffer []byte) (int, error) {
13461322
target, status := leaf.VirtualReadlink(ctx)
13471323
if status != virtual.StatusOK {
@@ -1352,17 +1328,27 @@ func getReparsePointForLeaf(ctx context.Context, leaf virtual.Leaf, buffer []byt
13521328
return 0, nil
13531329
}
13541330

1355-
// Parse the path to determine if it's absolute
1356-
w := relativePathChecker{}
1357-
if err := path.Resolve(path.LocalFormat.NewParser(string(target)), &w); err != nil {
1331+
// Normalize to backslashes. After reparse resolution the
1332+
// I/O manager re-issues the request with the substituted
1333+
// path. FspFileSystemFindReparsePoint splits on backslashes
1334+
// only, so forward slashes cause it to misidentify the
1335+
// reparse point's depth, breaking chained symlinks.
1336+
targetParser := path.LocalFormat.NewParser(string(target))
1337+
cleanPathBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
1338+
if err := path.Resolve(targetParser, scopeWalker); err != nil {
13581339
return 0, err
13591340
}
1360-
var flags int
1361-
if w.isRelative {
1362-
flags = windowsext.SYMLINK_FLAG_RELATIVE
1341+
var flags uint32
1342+
switch cleanPathBuilder.WindowsPathKind() {
1343+
case path.WindowsPathKindRelative, path.WindowsPathKindDriveRelative:
1344+
flags = uint32(windowsext.SYMLINK_FLAG_RELATIVE)
1345+
case path.WindowsPathKindAbsolute:
13631346
}
1364-
1365-
return FillSymlinkReparseBuffer(string(target), uint32(flags), buffer)
1347+
targetStr, err := cleanPathBuilder.GetWindowsString(path.WindowsPathFormatStandard)
1348+
if err != nil {
1349+
return 0, err
1350+
}
1351+
return FillSymlinkReparseBuffer(targetStr, flags, buffer)
13661352
}
13671353

13681354
func (fs *FileSystem) GetReparsePoint(ref *ffi.FileSystemRef, file uintptr, name string, buffer []byte) (int, error) {

‎pkg/filesystem/virtual/winfsp/file_system_integration_test.go‎

Lines changed: 137 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
virtual_pb "github.com/buildbarn/bb-remote-execution/pkg/proto/configuration/filesystem/virtual"
2222
"github.com/buildbarn/bb-storage/pkg/blockdevice"
2323
"github.com/buildbarn/bb-storage/pkg/clock"
24+
bb_path "github.com/buildbarn/bb-storage/pkg/filesystem/path"
2425
"github.com/buildbarn/bb-storage/pkg/program"
2526
"github.com/buildbarn/bb-storage/pkg/util"
2627
"github.com/stretchr/testify/require"
@@ -39,7 +40,7 @@ func findFreeDriveLetter() (string, error) {
3940
return "", fmt.Errorf("no free drive letters available")
4041
}
4142

42-
func createWinFSPMountForTest(t *testing.T, terminationGroup program.Group, caseSensitive bool) (string, blockdevice.BlockDevice) {
43+
func createWinFSPForTest(t *testing.T, terminationGroup program.Group, caseSensitive bool) (string, blockdevice.BlockDevice, virtual_configuration.Mount, virtual.Directory) {
4344
// We can't run winfsp-tests at a directory path due to
4445
// https://github.com/winfsp/winfsp/issues/279. Instead find a free drive
4546
// letter and run it there instead.
@@ -75,38 +76,41 @@ func createWinFSPMountForTest(t *testing.T, terminationGroup program.Group, case
7576

7677
// Create a virtual directory to hold new files.
7778
defaultAttributesSetter := func(requested virtual.AttributesMask, attributes *virtual.Attributes) {}
78-
err = mount.Expose(
79-
terminationGroup,
80-
virtual.NewInMemoryPrepopulatedDirectory(
81-
virtual.NewHandleAllocatingFileAllocator(
82-
virtual.NewPoolBackedFileAllocator(
83-
pool.NewBlockDeviceBackedFilePool(
84-
bd,
85-
pool.NewBitmapSectorAllocator(uint32(sectorCount)),
86-
sectorSizeBytes,
87-
),
88-
util.DefaultErrorLogger,
89-
defaultAttributesSetter,
90-
virtual.NoNamedAttributesFactory,
79+
rootDir := virtual.NewInMemoryPrepopulatedDirectory(
80+
virtual.NewHandleAllocatingFileAllocator(
81+
virtual.NewPoolBackedFileAllocator(
82+
pool.NewBlockDeviceBackedFilePool(
83+
bd,
84+
pool.NewBitmapSectorAllocator(uint32(sectorCount)),
85+
sectorSizeBytes,
9186
),
92-
handleAllocator,
93-
),
94-
virtual.NewHandleAllocatingSymlinkFactory(
95-
virtual.BaseSymlinkFactory,
96-
handleAllocator.New(),
87+
util.DefaultErrorLogger,
88+
defaultAttributesSetter,
89+
virtual.NoNamedAttributesFactory,
9790
),
98-
util.DefaultErrorLogger,
9991
handleAllocator,
100-
sort.Sort,
101-
func(s string) bool { return false },
102-
clock.SystemClock,
103-
normalizer,
104-
defaultAttributesSetter,
105-
virtual.NoNamedAttributesFactory,
10692
),
93+
virtual.NewHandleAllocatingSymlinkFactory(
94+
virtual.BaseSymlinkFactory,
95+
handleAllocator.New(),
96+
),
97+
util.DefaultErrorLogger,
98+
handleAllocator,
99+
sort.Sort,
100+
func(s string) bool { return false },
101+
clock.SystemClock,
102+
normalizer,
103+
defaultAttributesSetter,
104+
virtual.NoNamedAttributesFactory,
107105
)
108-
require.NoError(t, err, "Failed to expose mount point")
109106

107+
return vfsPath, bd, mount, rootDir
108+
}
109+
110+
func createWinFSPMountForTest(t *testing.T, terminationGroup program.Group, caseSensitive bool) (string, blockdevice.BlockDevice) {
111+
vfsPath, bd, mount, rootDir := createWinFSPForTest(t, terminationGroup, caseSensitive)
112+
err := mount.Expose(terminationGroup, rootDir)
113+
require.NoError(t, err, "Failed to expose mount point")
110114
return vfsPath, bd
111115
}
112116

@@ -417,6 +421,112 @@ func TestWinFSPFileSystemGetSecurityByNameIntegration(t *testing.T) {
417421
})
418422
}
419423

424+
func TestWinFSPFileSystemStatFollowsSymlink(t *testing.T) {
425+
program.RunLocal(context.Background(), func(ctx context.Context, siblingsGroup, dependenciesGroup program.Group) error {
426+
// Pre-populate the virtual directory through the VFS API
427+
// (not os.Symlink).
428+
vfsPath, bd, mount, rootDir := createWinFSPForTest(t, dependenciesGroup, false)
429+
defer bd.Close()
430+
431+
// Build a pnpm-style node_modules layout through the VFS
432+
// API with chained symlinks.
433+
var attrs virtual.Attributes
434+
435+
// The real directory.
436+
storeDir, _, s := rootDir.VirtualMkdir(
437+
bb_path.MustNewComponent("store"), 0, &attrs)
438+
require.Equal(t, virtual.StatusOK, s)
439+
_, _, s = storeDir.VirtualMkdir(
440+
bb_path.MustNewComponent("pkg"), 0, &attrs)
441+
require.Equal(t, virtual.StatusOK, s)
442+
443+
// Single symlink: store-link -> store/pkg.
444+
_, _, s = rootDir.VirtualSymlink(
445+
ctx,
446+
[]byte("store/pkg"),
447+
bb_path.MustNewComponent("store-link"),
448+
0, &attrs)
449+
require.Equal(t, virtual.StatusOK, s)
450+
451+
// Create node_modules/.pnpm/pkg@1.0.0/node_modules/.
452+
nmDir, _, s := rootDir.VirtualMkdir(
453+
bb_path.MustNewComponent("node_modules"), 0, &attrs)
454+
require.Equal(t, virtual.StatusOK, s)
455+
pnpmDir, _, s := nmDir.VirtualMkdir(
456+
bb_path.MustNewComponent(".pnpm"), 0, &attrs)
457+
require.Equal(t, virtual.StatusOK, s)
458+
pkgVerDir, _, s := pnpmDir.VirtualMkdir(
459+
bb_path.MustNewComponent("pkg@1.0.0"), 0, &attrs)
460+
require.Equal(t, virtual.StatusOK, s)
461+
innerNmDir, _, s := pkgVerDir.VirtualMkdir(
462+
bb_path.MustNewComponent("node_modules"), 0, &attrs)
463+
require.Equal(t, virtual.StatusOK, s)
464+
465+
// Inner symlink.
466+
_, _, s = innerNmDir.VirtualSymlink(
467+
ctx,
468+
[]byte("../../../../store/pkg"),
469+
bb_path.MustNewComponent("pkg"),
470+
0, &attrs)
471+
require.Equal(t, virtual.StatusOK, s)
472+
473+
// Outer symlink.
474+
_, _, s = nmDir.VirtualSymlink(
475+
ctx,
476+
[]byte(".pnpm/pkg@1.0.0/node_modules/pkg"),
477+
bb_path.MustNewComponent("pkg"),
478+
0, &attrs)
479+
require.Equal(t, virtual.StatusOK, s)
480+
481+
require.NoError(t, mount.Expose(dependenciesGroup, rootDir))
482+
483+
// Write a file into the real directory after mounting.
484+
testContent := []byte(`{"name":"pkg"}`)
485+
require.NoError(t, os.WriteFile(
486+
filepath.Join(vfsPath, "store", "pkg", "package.json"),
487+
testContent, 0o644,
488+
))
489+
490+
t.Run("SingleSymlink", func(t *testing.T) {
491+
singleSymlinkPath := filepath.Join(vfsPath, "store-link")
492+
info, err := os.Stat(singleSymlinkPath)
493+
require.NoError(t, err)
494+
require.True(t, info.IsDir())
495+
496+
content, err := os.ReadFile(filepath.Join(singleSymlinkPath, "package.json"))
497+
require.NoError(t, err)
498+
require.Equal(t, testContent, content)
499+
})
500+
501+
t.Run("ChainedSymlinks", func(t *testing.T) {
502+
symlinkPath := filepath.Join(vfsPath, "node_modules", "pkg")
503+
504+
info, err := os.Lstat(symlinkPath)
505+
require.NoError(t, err)
506+
require.NotZero(t, info.Mode()&os.ModeSymlink)
507+
508+
target, err := os.Readlink(symlinkPath)
509+
require.NoError(t, err)
510+
require.Equal(t, `.pnpm\pkg@1.0.0\node_modules\pkg`, target)
511+
512+
info, err = os.Stat(symlinkPath)
513+
require.NoError(t, err)
514+
require.True(t, info.IsDir())
515+
516+
content, err := os.ReadFile(filepath.Join(symlinkPath, "package.json"))
517+
require.NoError(t, err)
518+
require.Equal(t, testContent, content)
519+
520+
entries, err := os.ReadDir(symlinkPath)
521+
require.NoError(t, err)
522+
require.Len(t, entries, 1)
523+
require.Equal(t, "package.json", entries[0].Name())
524+
})
525+
526+
return nil
527+
})
528+
}
529+
420530
func TestWinFSPFileSystemCasePreserving(t *testing.T) {
421531
program.RunLocal(context.Background(), func(ctx context.Context, siblingsGroup, dependenciesGroup program.Group) error {
422532
vfsPath, bd := createWinFSPMountForTest(t, dependenciesGroup, false)

0 commit comments

Comments
 (0)