From c25dd83587312b3b53e0c0918144fa8746e591cd Mon Sep 17 00:00:00 2001 From: Ed Schouten Date: Fri, 8 Mar 2024 10:44:18 +0100 Subject: [PATCH] Change Symlink()/Readlink() to take/return path.Parser On Windows, symlink targets may contain drive letters and are supposed to use backslashes. This is problematic, because the Directory.Symlink() and Directory.Readlink() methods are currently supposed to return UNIX-like pathname strings. By altering these functions to use path.Parser, users of this API don't need to care about pathname conventions used on the host operating system. This change has the side-effect that OutputHierarchy will now yield SymlinkNode and OutputSymlink entries that contain sanitized symlink target strings. Any redundant slashes and "." components are automatically removed. Symlink target paths are now strongly guaranteed to be non-empty as well. --- go.mod | 2 +- go.sum | 2 + go_dependencies.bzl | 4 +- pkg/builder/local_build_executor_test.go | 2 +- pkg/builder/naive_build_directory.go | 6 ++- pkg/builder/naive_build_directory_test.go | 23 ++++++++++-- pkg/builder/output_hierarchy.go | 37 ++++++++++++------- pkg/builder/output_hierarchy_test.go | 12 +++--- pkg/builder/uploadable_directory.go | 2 +- pkg/builder/virtual_build_directory.go | 6 +-- pkg/filesystem/lazy_directory.go | 6 +-- pkg/filesystem/lazy_directory_test.go | 17 ++++++--- .../virtual/base_symlink_factory.go | 22 ++++++++--- .../virtual/blob_access_cas_file_factory.go | 4 +- pkg/filesystem/virtual/native_leaf.go | 2 +- .../virtual/pool_backed_file_allocator.go | 4 +- pkg/filesystem/virtual/special_file.go | 4 +- .../virtual/user_settable_symlink.go | 4 +- 18 files changed, 105 insertions(+), 54 deletions(-) diff --git a/go.mod b/go.mod index fb37e790..119c6db0 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ replace github.com/grpc-ecosystem/grpc-gateway/v2 => github.com/grpc-ecosystem/g require ( cloud.google.com/go/longrunning v0.5.5 github.com/bazelbuild/remote-apis v0.0.0-20240215191509-9ff14cecffe5 - github.com/buildbarn/bb-storage v0.0.0-20240307194821-4d0d7a3d85b5 + github.com/buildbarn/bb-storage v0.0.0-20240308085957-e8fd6935d2ef github.com/buildbarn/go-xdr v0.0.0-20231115101217-a9e2aa4cf64b github.com/golang/mock v1.6.0 github.com/golang/protobuf v1.5.3 diff --git a/go.sum b/go.sum index bee582e0..f271dabf 100644 --- a/go.sum +++ b/go.sum @@ -59,6 +59,8 @@ github.com/buildbarn/bb-storage v0.0.0-20240222075657-f52022cbad94 h1:/39JibX2Gc github.com/buildbarn/bb-storage v0.0.0-20240222075657-f52022cbad94/go.mod h1:2JFdqOUodMQHyZ3kX21n0hYlY1zmca0BEPHEpPf6wEw= github.com/buildbarn/bb-storage v0.0.0-20240307194821-4d0d7a3d85b5 h1:tBbub4L03HT0PNKToki245kaXjwb+AOzyhfy8oosL6w= github.com/buildbarn/bb-storage v0.0.0-20240307194821-4d0d7a3d85b5/go.mod h1:0uISGKJD6Owt29w2sUlK0TeLtYdLWtBiC43yVHdgMAY= +github.com/buildbarn/bb-storage v0.0.0-20240308085957-e8fd6935d2ef h1:4Mq41hEzT6G4F1dxNiwbwHsQf/pNSFk//pkVgZbGbcw= +github.com/buildbarn/bb-storage v0.0.0-20240308085957-e8fd6935d2ef/go.mod h1:0uISGKJD6Owt29w2sUlK0TeLtYdLWtBiC43yVHdgMAY= github.com/buildbarn/go-xdr v0.0.0-20231115101217-a9e2aa4cf64b h1:/sKWC0Fs5fXNo/t72BRZRLERg4v2gFoEeg2Mk+a8xak= github.com/buildbarn/go-xdr v0.0.0-20231115101217-a9e2aa4cf64b/go.mod h1:VwInghBSUyPtNBhl7o2oCUnxOCTGgySJnRTO1Kh7XuI= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= diff --git a/go_dependencies.bzl b/go_dependencies.bzl index c568f2ca..72fdccfd 100644 --- a/go_dependencies.bzl +++ b/go_dependencies.bzl @@ -173,8 +173,8 @@ def go_dependencies(): go_repository( name = "com_github_buildbarn_bb_storage", importpath = "github.com/buildbarn/bb-storage", - sum = "h1:tBbub4L03HT0PNKToki245kaXjwb+AOzyhfy8oosL6w=", - version = "v0.0.0-20240307194821-4d0d7a3d85b5", + sum = "h1:4Mq41hEzT6G4F1dxNiwbwHsQf/pNSFk//pkVgZbGbcw=", + version = "v0.0.0-20240308085957-e8fd6935d2ef", ) go_repository( name = "com_github_buildbarn_go_xdr", diff --git a/pkg/builder/local_build_executor_test.go b/pkg/builder/local_build_executor_test.go index b572f02f..e8a669ec 100644 --- a/pkg/builder/local_build_executor_test.go +++ b/pkg/builder/local_build_executor_test.go @@ -448,7 +448,7 @@ func TestLocalBuildExecutorOutputSymlinkReadingFailure(t *testing.T) { fooDirectory.EXPECT().ReadDir().Return([]filesystem.FileInfo{ filesystem.NewFileInfo(path.MustNewComponent("bar"), filesystem.FileTypeSymlink, false), }, nil) - fooDirectory.EXPECT().Readlink(path.MustNewComponent("bar")).Return("", status.Error(codes.Internal, "Cosmic rays caused interference")) + fooDirectory.EXPECT().Readlink(path.MustNewComponent("bar")).Return(nil, status.Error(codes.Internal, "Cosmic rays caused interference")) fooDirectory.EXPECT().Close() inputRootDirectory.EXPECT().Close() serverLogsDirectory := mock.NewMockUploadableDirectory(ctrl) diff --git a/pkg/builder/naive_build_directory.go b/pkg/builder/naive_build_directory.go index 2b60a9a8..90b8b29c 100644 --- a/pkg/builder/naive_build_directory.go +++ b/pkg/builder/naive_build_directory.go @@ -124,7 +124,11 @@ func (d *naiveBuildDirectory) mergeDirectoryContents(ctx context.Context, digest return status.Errorf(codes.InvalidArgument, "Symlink %#v has an invalid name", symlink.Name) } childPathTrace := pathTrace.Append(component) - if err := inputDirectory.Symlink(symlink.Target, component); err != nil { + targetParser, err := path.NewUNIXParser(symlink.Target) + if err != nil { + return util.StatusWrapf(err, "Symlink %#v has an invalid target", childPathTrace.String()) + } + if err := inputDirectory.Symlink(targetParser, component); err != nil { return util.StatusWrapf(err, "Failed to create input symlink %#v", childPathTrace.String()) } } diff --git a/pkg/builder/naive_build_directory_test.go b/pkg/builder/naive_build_directory_test.go index 283caaa1..dfe735bc 100644 --- a/pkg/builder/naive_build_directory_test.go +++ b/pkg/builder/naive_build_directory_test.go @@ -78,7 +78,12 @@ func TestNaiveBuildDirectorySuccess(t *testing.T) { buildDirectory.EXPECT().Mkdir(path.MustNewComponent("directory"), os.FileMode(0o777)).Return(nil) nestedDirectory := mock.NewMockDirectoryCloser(ctrl) buildDirectory.EXPECT().EnterDirectory(path.MustNewComponent("directory")).Return(nestedDirectory, nil) - nestedDirectory.EXPECT().Symlink("../non-executable", path.MustNewComponent("link-to-non-executable")).Return(nil) + nestedDirectory.EXPECT().Symlink(gomock.Any(), path.MustNewComponent("link-to-non-executable")). + Do(func(targetParser path.Parser, name path.Component) { + targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + require.NoError(t, path.Resolve(targetParser, scopeWalker)) + require.Equal(t, "../non-executable", targetPath.String()) + }) nestedDirectory.EXPECT().Close() fileFetcher := mock.NewMockFileFetcher(ctrl) fileFetcher.EXPECT().GetFile( @@ -93,8 +98,12 @@ func TestNaiveBuildDirectorySuccess(t *testing.T) { buildDirectory, path.MustNewComponent("executable"), true).Return(nil) - buildDirectory.EXPECT().Symlink("executable", - path.MustNewComponent("link-to-executable")).Return(nil) + buildDirectory.EXPECT().Symlink(gomock.Any(), path.MustNewComponent("link-to-executable")). + Do(func(targetParser path.Parser, name path.Component) { + targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + require.NoError(t, path.Resolve(targetParser, scopeWalker)) + require.Equal(t, "executable", targetPath.String()) + }) contentAddressableStorage := mock.NewMockBlobAccess(ctrl) inputRootPopulator := builder.NewNaiveBuildDirectory(buildDirectory, directoryFetcher, fileFetcher, contentAddressableStorage) @@ -413,7 +422,13 @@ func TestNaiveBuildDirectorySymlinkCreationFailure(t *testing.T) { buildDirectory.EXPECT().Mkdir(path.MustNewComponent("Hello"), os.FileMode(0o777)).Return(nil) helloDirectory := mock.NewMockDirectoryCloser(ctrl) buildDirectory.EXPECT().EnterDirectory(path.MustNewComponent("Hello")).Return(helloDirectory, nil) - helloDirectory.EXPECT().Symlink("/etc/passwd", path.MustNewComponent("World")).Return(status.Error(codes.Unimplemented, "This filesystem does not support symbolic links")) + helloDirectory.EXPECT().Symlink(gomock.Any(), path.MustNewComponent("World")). + DoAndReturn(func(targetParser path.Parser, name path.Component) error { + targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + require.NoError(t, path.Resolve(targetParser, scopeWalker)) + require.Equal(t, "/etc/passwd", targetPath.String()) + return status.Error(codes.Unimplemented, "This filesystem does not support symbolic links") + }) helloDirectory.EXPECT().Close() fileFetcher := mock.NewMockFileFetcher(ctrl) contentAddressableStorage := mock.NewMockBlobAccess(ctrl) diff --git a/pkg/builder/output_hierarchy.go b/pkg/builder/output_hierarchy.go index 80303d9e..a630f201 100644 --- a/pkg/builder/output_hierarchy.go +++ b/pkg/builder/output_hierarchy.go @@ -319,14 +319,20 @@ func (s *uploadOutputsState) uploadOutputFile(d UploadableDirectory, name path.C // UploadOutputDirectory is called to read the attributes of a single // output symlink. func (s *uploadOutputsState) uploadOutputSymlink(d UploadableDirectory, name path.Component, childPath *path.Trace, outputSymlinks *[]*remoteexecution.OutputSymlink, paths []string) { - if target, err := d.Readlink(name); err == nil { - for _, path := range paths { - *outputSymlinks = append( - *outputSymlinks, - &remoteexecution.OutputSymlink{ - Path: path, - Target: target, - }) + if targetParser, err := d.Readlink(name); err == nil { + targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + if path.Resolve(targetParser, scopeWalker); err == nil { + target := targetPath.String() + for _, path := range paths { + *outputSymlinks = append( + *outputSymlinks, + &remoteexecution.OutputSymlink{ + Path: path, + Target: target, + }) + } + } else { + s.saveError(util.StatusWrapf(err, "Failed to resolve target of output symlink %#v", childPath.String())) } } else { s.saveError(util.StatusWrapf(err, "Failed to read output symlink %#v", childPath.String())) @@ -382,11 +388,16 @@ func (s *uploadOutputDirectoryState) uploadDirectory(d UploadableDirectory, dPat s.saveError(util.StatusWrapf(err, "Failed to enter output directory %#v", childPath.String())) } case filesystem.FileTypeSymlink: - if target, err := d.Readlink(name); err == nil { - directory.Symlinks = append(directory.Symlinks, &remoteexecution.SymlinkNode{ - Name: name.String(), - Target: target, - }) + if targetParser, err := d.Readlink(name); err == nil { + targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + if path.Resolve(targetParser, scopeWalker); err == nil { + directory.Symlinks = append(directory.Symlinks, &remoteexecution.SymlinkNode{ + Name: name.String(), + Target: targetPath.String(), + }) + } else { + s.saveError(util.StatusWrapf(err, "Failed to resolve target of output symlink %#v", childPath.String())) + } } else { s.saveError(util.StatusWrapf(err, "Failed to read output symlink %#v", childPath.String())) } diff --git a/pkg/builder/output_hierarchy_test.go b/pkg/builder/output_hierarchy_test.go index 542b1913..21382ace 100644 --- a/pkg/builder/output_hierarchy_test.go +++ b/pkg/builder/output_hierarchy_test.go @@ -286,17 +286,17 @@ func TestOutputHierarchyUploadOutputs(t *testing.T) { foo.EXPECT().Lstat(path.MustNewComponent("path-enoent")).Return(filesystem.FileInfo{}, syscall.ENOENT) // Inspection/uploading of all non-directory outputs. - foo.EXPECT().Readlink(path.MustNewComponent("directory-symlink")).Return("directory-symlink-target", nil) + foo.EXPECT().Readlink(path.MustNewComponent("directory-symlink")).Return(path.MustNewUNIXParser("directory-symlink-target"), nil) foo.EXPECT().UploadFile(ctx, path.MustNewComponent("file-regular"), gomock.Any(), writableFileUploadDelay). Return(digest.MustNewDigest("example", remoteexecution.DigestFunction_MD5, "a58c2f2281011ca2e631b39baa1ab657", 12), nil) foo.EXPECT().UploadFile(ctx, path.MustNewComponent("file-executable"), gomock.Any(), writableFileUploadDelay). Return(digest.MustNewDigest("example", remoteexecution.DigestFunction_MD5, "7590e1b46240ecb5ea65a80db7ee6fae", 15), nil) - foo.EXPECT().Readlink(path.MustNewComponent("file-symlink")).Return("file-symlink-target", nil) + foo.EXPECT().Readlink(path.MustNewComponent("file-symlink")).Return(path.MustNewUNIXParser("file-symlink-target"), nil) foo.EXPECT().UploadFile(ctx, path.MustNewComponent("path-regular"), gomock.Any(), writableFileUploadDelay). Return(digest.MustNewDigest("example", remoteexecution.DigestFunction_MD5, "44206648b7bb2f3b0d2ed0c52ad2e269", 12), nil) foo.EXPECT().UploadFile(ctx, path.MustNewComponent("path-executable"), gomock.Any(), writableFileUploadDelay). Return(digest.MustNewDigest("example", remoteexecution.DigestFunction_MD5, "87729325cd08d300fb0e238a3a8da443", 15), nil) - foo.EXPECT().Readlink(path.MustNewComponent("path-symlink")).Return("path-symlink-target", nil) + foo.EXPECT().Readlink(path.MustNewComponent("path-symlink")).Return(path.MustNewUNIXParser("path-symlink-target"), nil) // Uploading of /foo/directory-directory. Files with an // unknown type (UNIX sockets, FIFOs) should be ignored. @@ -319,7 +319,7 @@ func TestOutputHierarchyUploadOutputs(t *testing.T) { Return(digest.MustNewDigest("example", remoteexecution.DigestFunction_MD5, "ee7004c7949d83f130592f15d98ca343", 10), nil) directoryDirectory.EXPECT().UploadFile(ctx, path.MustNewComponent("regular"), gomock.Any(), writableFileUploadDelay). Return(digest.MustNewDigest("example", remoteexecution.DigestFunction_MD5, "af37d08ae228a87dc6b265fd1019c97d", 7), nil) - directoryDirectory.EXPECT().Readlink(path.MustNewComponent("symlink")).Return("symlink-target", nil) + directoryDirectory.EXPECT().Readlink(path.MustNewComponent("symlink")).Return(path.MustNewUNIXParser("symlink-target"), nil) directoryDirectory.EXPECT().Close() contentAddressableStorage.EXPECT().Put( ctx, @@ -905,7 +905,7 @@ func TestOutputHierarchyUploadOutputs(t *testing.T) { }, nil) root.EXPECT().UploadFile(ctx, path.MustNewComponent("file1"), gomock.Any(), writableFileUploadDelay). Return(digest.MustNewDigest("example", remoteexecution.DigestFunction_MD5, "132d36a32eb9e41afb86d8ba65fe9657", 123), nil) - root.EXPECT().Readlink(path.MustNewComponent("symlink1")).Return("target1", nil) + root.EXPECT().Readlink(path.MustNewComponent("symlink1")).Return(path.MustNewUNIXParser("target1"), nil) directory1 := mock.NewMockUploadableDirectory(ctrl) root.EXPECT().EnterUploadableDirectory(path.MustNewComponent("directory1")).Return(directory1, nil) @@ -915,7 +915,7 @@ func TestOutputHierarchyUploadOutputs(t *testing.T) { }, nil) directory1.EXPECT().UploadFile(ctx, path.MustNewComponent("file2"), gomock.Any(), writableFileUploadDelay). Return(digest.MustNewDigest("example", remoteexecution.DigestFunction_MD5, "09ae70542cc258d5c1007d774da5ccb1", 456), nil) - directory1.EXPECT().Readlink(path.MustNewComponent("symlink2")).Return("target2", nil) + directory1.EXPECT().Readlink(path.MustNewComponent("symlink2")).Return(path.MustNewUNIXParser("target2"), nil) directory1.EXPECT().Close() rootDirectory := &remoteexecution.Directory{ diff --git a/pkg/builder/uploadable_directory.go b/pkg/builder/uploadable_directory.go index c43eb837..6bddb7cb 100644 --- a/pkg/builder/uploadable_directory.go +++ b/pkg/builder/uploadable_directory.go @@ -18,7 +18,7 @@ type UploadableDirectory interface { EnterUploadableDirectory(name path.Component) (UploadableDirectory, error) Lstat(name path.Component) (filesystem.FileInfo, error) ReadDir() ([]filesystem.FileInfo, error) - Readlink(name path.Component) (string, error) + Readlink(name path.Component) (path.Parser, error) // Upload a file into the Content Addressable Storage. // diff --git a/pkg/builder/virtual_build_directory.go b/pkg/builder/virtual_build_directory.go index 1fff1de8..258c3558 100644 --- a/pkg/builder/virtual_build_directory.go +++ b/pkg/builder/virtual_build_directory.go @@ -150,13 +150,13 @@ func (d *virtualBuildDirectory) Mknod(name path.Component, perm os.FileMode, dev return nil } -func (d *virtualBuildDirectory) Readlink(name path.Component) (string, error) { +func (d *virtualBuildDirectory) Readlink(name path.Component) (path.Parser, error) { child, err := d.LookupChild(name) if err != nil { - return "", err + return nil, err } if _, leaf := child.GetPair(); leaf != nil { return leaf.Readlink() } - return "", syscall.EISDIR + return nil, syscall.EISDIR } diff --git a/pkg/filesystem/lazy_directory.go b/pkg/filesystem/lazy_directory.go index 2d935559..30251d36 100644 --- a/pkg/filesystem/lazy_directory.go +++ b/pkg/filesystem/lazy_directory.go @@ -147,10 +147,10 @@ func (d *lazyDirectory) ReadDir() ([]filesystem.FileInfo, error) { return underlying.ReadDir() } -func (d *lazyDirectory) Readlink(name path.Component) (string, error) { +func (d *lazyDirectory) Readlink(name path.Component) (path.Parser, error) { underlying, err := d.openUnderlying() if err != nil { - return "", err + return nil, err } defer underlying.Close() return underlying.Readlink(name) @@ -192,7 +192,7 @@ func (d *lazyDirectory) Rename(oldName path.Component, newDirectory filesystem.D return underlying.Rename(oldName, newDirectory, newName) } -func (d *lazyDirectory) Symlink(oldName string, newName path.Component) error { +func (d *lazyDirectory) Symlink(oldName path.Parser, newName path.Component) error { underlying, err := d.openUnderlying() if err != nil { return err diff --git a/pkg/filesystem/lazy_directory_test.go b/pkg/filesystem/lazy_directory_test.go index 4cb2396f..14339bda 100644 --- a/pkg/filesystem/lazy_directory_test.go +++ b/pkg/filesystem/lazy_directory_test.go @@ -115,13 +115,15 @@ func TestLazyDirectory(t *testing.T) { t.Run("ReadlinkSuccess", func(t *testing.T) { underlyingDirectory := mock.NewMockDirectoryCloser(ctrl) directoryOpener.EXPECT().Call().Return(underlyingDirectory, nil) - underlyingDirectory.EXPECT().Readlink(path.MustNewComponent("symlink")).Return("target", nil) + underlyingDirectory.EXPECT().Readlink(path.MustNewComponent("symlink")).Return(path.MustNewUNIXParser("target"), nil) underlyingDirectory.EXPECT().Close().Return(nil) // Call should be forwarded literally. - target, err := directory.Readlink(path.MustNewComponent("symlink")) + targetParser, err := directory.Readlink(path.MustNewComponent("symlink")) require.NoError(t, err) - require.Equal(t, target, "target") + targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + require.NoError(t, path.Resolve(targetParser, scopeWalker)) + require.Equal(t, "target", targetPath.String()) }) t.Run("RemoveSuccess", func(t *testing.T) { @@ -170,11 +172,16 @@ func TestLazyDirectory(t *testing.T) { t.Run("SymlinkSuccess", func(t *testing.T) { underlyingDirectory := mock.NewMockDirectoryCloser(ctrl) directoryOpener.EXPECT().Call().Return(underlyingDirectory, nil) - underlyingDirectory.EXPECT().Symlink("old", path.MustNewComponent("new")).Return(nil) + underlyingDirectory.EXPECT().Symlink(gomock.Any(), path.MustNewComponent("new")). + Do(func(targetParser path.Parser, name path.Component) { + targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + require.NoError(t, path.Resolve(targetParser, scopeWalker)) + require.Equal(t, "old", targetPath.String()) + }) underlyingDirectory.EXPECT().Close().Return(nil) // Call should be forwarded literally. - err := directory.Symlink("old", path.MustNewComponent("new")) + err := directory.Symlink(path.MustNewUNIXParser("old"), path.MustNewComponent("new")) require.NoError(t, err) }) diff --git a/pkg/filesystem/virtual/base_symlink_factory.go b/pkg/filesystem/virtual/base_symlink_factory.go index 97713507..2379a0cc 100644 --- a/pkg/filesystem/virtual/base_symlink_factory.go +++ b/pkg/filesystem/virtual/base_symlink_factory.go @@ -30,15 +30,27 @@ type symlink struct { target []byte } -func (f symlink) Readlink() (string, error) { +func (f symlink) Readlink() (path.Parser, error) { if !utf8.Valid(f.target) { - return "", status.Error(codes.InvalidArgument, "Symbolic link contents are not valid UTF-8") + return nil, status.Error(codes.InvalidArgument, "Symbolic link contents are not valid UTF-8") } - return string(f.target), nil + return path.NewUNIXParser(string(f.target)) +} + +func (f symlink) readlinkString() (string, error) { + targetParser, err := f.Readlink() + if err != nil { + return "", err + } + targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker) + if err := path.Resolve(targetParser, scopeWalker); err != nil { + return "", err + } + return targetPath.String(), nil } func (f symlink) GetBazelOutputServiceStat(digestFunction *digest.Function) (*bazeloutputservice.BatchStatResponse_Stat, error) { - target, err := f.Readlink() + target, err := f.readlinkString() if err != nil { return nil, err } @@ -52,7 +64,7 @@ func (f symlink) GetBazelOutputServiceStat(digestFunction *digest.Function) (*ba } func (f symlink) AppendOutputPathPersistencyDirectoryNode(directory *outputpathpersistency.Directory, name path.Component) { - if target, err := f.Readlink(); err == nil { + if target, err := f.readlinkString(); err == nil { directory.Symlinks = append(directory.Symlinks, &remoteexecution.SymlinkNode{ Name: name.String(), Target: target, diff --git a/pkg/filesystem/virtual/blob_access_cas_file_factory.go b/pkg/filesystem/virtual/blob_access_cas_file_factory.go index 5f07cd6c..1f081ec1 100644 --- a/pkg/filesystem/virtual/blob_access_cas_file_factory.go +++ b/pkg/filesystem/virtual/blob_access_cas_file_factory.go @@ -66,8 +66,8 @@ func (f *blobAccessCASFile) Link() Status { return StatusOK } -func (f *blobAccessCASFile) Readlink() (string, error) { - return "", syscall.EINVAL +func (f *blobAccessCASFile) Readlink() (path.Parser, error) { + return nil, syscall.EINVAL } func (f *blobAccessCASFile) Unlink() { diff --git a/pkg/filesystem/virtual/native_leaf.go b/pkg/filesystem/virtual/native_leaf.go index 322268d7..a19963e7 100644 --- a/pkg/filesystem/virtual/native_leaf.go +++ b/pkg/filesystem/virtual/native_leaf.go @@ -29,7 +29,7 @@ type NativeLeaf interface { // PrepopulatedDirectory and InitialContentsFetcher into // parameterized types, where the leaves could be any type // that's based on NativeLeaf. - Readlink() (string, error) + Readlink() (path.Parser, error) UploadFile(ctx context.Context, contentAddressableStorage blobstore.BlobAccess, digestFunction digest.Function, writableFileUploadDelay <-chan struct{}) (digest.Digest, error) // GetContainingDigests() returns a set of digests of objects in // the Content Addressable Storage that back the contents of diff --git a/pkg/filesystem/virtual/pool_backed_file_allocator.go b/pkg/filesystem/virtual/pool_backed_file_allocator.go index b54444f2..3f09f321 100644 --- a/pkg/filesystem/virtual/pool_backed_file_allocator.go +++ b/pkg/filesystem/virtual/pool_backed_file_allocator.go @@ -184,8 +184,8 @@ func (f *fileBackedFile) Link() Status { return StatusOK } -func (f *fileBackedFile) Readlink() (string, error) { - return "", syscall.EINVAL +func (f *fileBackedFile) Readlink() (path.Parser, error) { + return nil, syscall.EINVAL } func (f *fileBackedFile) Unlink() { diff --git a/pkg/filesystem/virtual/special_file.go b/pkg/filesystem/virtual/special_file.go index d080f626..59ca88f5 100644 --- a/pkg/filesystem/virtual/special_file.go +++ b/pkg/filesystem/virtual/special_file.go @@ -29,8 +29,8 @@ func NewSpecialFile(fileType filesystem.FileType, deviceNumber *filesystem.Devic } } -func (f *specialFile) Readlink() (string, error) { - return "", syscall.EINVAL +func (f *specialFile) Readlink() (path.Parser, error) { + return nil, syscall.EINVAL } func (f *specialFile) GetBazelOutputServiceStat(digestFunction *digest.Function) (*bazeloutputservice.BatchStatResponse_Stat, error) { diff --git a/pkg/filesystem/virtual/user_settable_symlink.go b/pkg/filesystem/virtual/user_settable_symlink.go index 12100f0b..9c59cfd1 100644 --- a/pkg/filesystem/virtual/user_settable_symlink.go +++ b/pkg/filesystem/virtual/user_settable_symlink.go @@ -80,8 +80,8 @@ func (f *UserSettableSymlink) InstallTemporaryDirectory(ctx context.Context, req // Readlink returns the target of the symbolic link. This method always // fails, as it's called in places where no Context is available. -func (f *UserSettableSymlink) Readlink() (string, error) { - return "", status.Error(codes.InvalidArgument, "Target of user settable symlinks can only be obtained through the virtual file system") +func (f *UserSettableSymlink) Readlink() (path.Parser, error) { + return nil, status.Error(codes.InvalidArgument, "Target of user settable symlinks can only be obtained through the virtual file system") } // GetBazelOutputServiceStat returns the status of the symbolic link, so