diff --git a/internal/serving/disk/symlink/path_test.go b/internal/serving/disk/symlink/path_test.go index 6b0a41f3f8b3ea6e78e54c0a48c6acd8c4311623..23978037d9737d2d26c87a2e1d689192bc864b6b 100644 --- a/internal/serving/disk/symlink/path_test.go +++ b/internal/serving/disk/symlink/path_test.go @@ -26,6 +26,10 @@ func tmpDir(t *testing.T) (vfs.Root, string, func()) { tmpDir, err := ioutil.TempDir("", "symlink_tests") require.NoError(t, err) + // On some systems `/tmp` can be a symlink + tmpDir, err = filepath.EvalSymlinks(tmpDir) + require.NoError(t, err) + root, err := fs.Root(context.Background(), tmpDir) require.NoError(t, err) @@ -70,7 +74,7 @@ var EvalSymlinksTests = []EvalSymlinksTest{ {"test/link2/..", "test"}, {"test/dir/link3", "."}, {"test/link2/link3/test", "test"}, - {"test/linkabs", "../.."}, + {"test/linkabs", "/"}, {"test/link4/..", "test"}, {"src/versions/current/modules/test", "src/pool/test"}, } diff --git a/internal/vfs/local/root.go b/internal/vfs/local/root.go index 0ed2206df03cbd32af8739302f653d66f8d7b415..9cd67a9f05d24266dd32a5949b5ce01b4fc7d5df 100644 --- a/internal/vfs/local/root.go +++ b/internal/vfs/local/root.go @@ -65,10 +65,18 @@ func (r *Root) Readlink(ctx context.Context, name string) (string, error) { return "", err } - if filepath.IsAbs(target) { + // If `target` is local to `rootPath` return relative + if strings.HasPrefix(target, r.rootPath+"/") { return filepath.Rel(filepath.Dir(fullPath), target) } + // If `target` is absolute return as-is making `EvalSymlinks` + // to discover misuse of a root path + if filepath.IsAbs(target) { + return target, nil + } + + // If `target` is relative, return as-is return target, nil } diff --git a/internal/vfs/local/root_test.go b/internal/vfs/local/root_test.go index c8ee6616ebf9e1a3bda57464080aa80e0ed6761d..f89e7736d25e34bb551be43996a4279c055ebc33 100644 --- a/internal/vfs/local/root_test.go +++ b/internal/vfs/local/root_test.go @@ -119,7 +119,8 @@ func TestReadlink(t *testing.T) { func TestReadlinkAbsolutePath(t *testing.T) { // create structure as: // /tmp/dir: directory - // /tmp/dir/symlink: points to `/tmp/file` + // /tmp/dir/symlink: points to `/tmp/file` outside of the `/tmp/dir` + // /tmp/dir/symlink2: points to `/tmp/dir/file` tmpDir, cleanup := tmpDir(t) defer cleanup() @@ -132,13 +133,36 @@ func TestReadlinkAbsolutePath(t *testing.T) { err = os.Symlink(filePath, symlinkPath) require.NoError(t, err) - root, err := localVFS.Root(context.Background(), dirPath) + symlinkPath = filepath.Join(dirPath, "symlink2") + dirFilePath := filepath.Join(dirPath, "file") + err = os.Symlink(dirFilePath, symlinkPath) require.NoError(t, err) - targetPath, err := root.Readlink(context.Background(), "symlink") + root, err := localVFS.Root(context.Background(), dirPath) require.NoError(t, err) - assert.Equal(t, "../file", targetPath, "the relative path is returned") + tests := map[string]struct { + path string + expectedTarget string + }{ + "the absolute path is returned for file outside of `/tmp/dir": { + path: "symlink", + expectedTarget: filePath, + }, + "the relative path is returned for file inside the `/tmp/dir": { + path: "symlink2", + expectedTarget: "file", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + targetPath, err := root.Readlink(context.Background(), test.path) + require.NoError(t, err) + + assert.Equal(t, test.expectedTarget, targetPath) + }) + } } func TestLstat(t *testing.T) {