proper fix

This commit is contained in:
Shreyas Goenka 2023-06-19 01:37:01 +02:00
parent 5368972172
commit 90b80eb91b
No known key found for this signature in database
GPG Key ID: 92A07DF49CCB0622
8 changed files with 67 additions and 50 deletions

View File

@ -35,7 +35,7 @@ func filerForPath(ctx context.Context, fullPath string) (filer.Filer, string, er
if runtime.GOOS == "windows" { if runtime.GOOS == "windows" {
parts := strings.SplitN(path, ":", 2) parts := strings.SplitN(path, ":", 2)
if len(parts) < 2 { if len(parts) < 2 {
return nil, "", fmt.Errorf("no volume specfied for path: %s", path) return nil, "", fmt.Errorf("no volume specified for path: %s", path)
} }
volume := parts[0] + ":" volume := parts[0] + ":"
relPath := parts[1] relPath := parts[1]

View File

@ -67,14 +67,14 @@ type DbfsClient struct {
workspaceClient *databricks.WorkspaceClient workspaceClient *databricks.WorkspaceClient
// File operations will be relative to this path. // File operations will be relative to this path.
root RootPath root UnixRootPath
} }
func NewDbfsClient(w *databricks.WorkspaceClient, root string) (Filer, error) { func NewDbfsClient(w *databricks.WorkspaceClient, root string) (Filer, error) {
return &DbfsClient{ return &DbfsClient{
workspaceClient: w, workspaceClient: w,
root: NewRootPath(root), root: NewUnixRootPath(root),
}, nil }, nil
} }

View File

@ -19,12 +19,13 @@ type LocalClient struct {
func NewLocalClient(root string) (Filer, error) { func NewLocalClient(root string) (Filer, error) {
if runtime.GOOS == "windows" && root == "/" { if runtime.GOOS == "windows" && root == "/" {
// Windows paths require a Volume/Drive letter specified. This allows us // Windows file systems do not have a "root" directory. Instead paths require
// to create local filers at the root of a windows file system without specifying one // a Volume/Drive letter specified. This allows us to refer to files across
return &LocalClient{root: NewRootPath("")}, nil // different drives from a single client
return &LocalClient{root: NopRootPath{}}, nil
} }
return &LocalClient{ return &LocalClient{
root: NewRootPath(root), root: NewUnixRootPath(root),
}, nil }, nil
} }
@ -106,7 +107,7 @@ func (w *LocalClient) Delete(ctx context.Context, name string, mode ...DeleteMod
} }
// Illegal to delete the root path. // Illegal to delete the root path.
if absPath == w.root.rootPath { if absPath == w.root.Root() {
return CannotDeleteRootError{} return CannotDeleteRootError{}
} }

View File

@ -3,17 +3,13 @@ package filer
import ( import (
"context" "context"
"path/filepath" "path/filepath"
"runtime"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func TestLocalClientAtRootForWindows(t *testing.T) { func TestLocalClientAtRootOfFilesystem(t *testing.T) {
if runtime.GOOS != "windows" {
t.Skip("this test is only meant for windows")
}
tmpDir := t.TempDir() tmpDir := t.TempDir()
ctx := context.Background() ctx := context.Background()

View File

@ -1,34 +1,16 @@
package filer package filer
import ( type RootPath interface {
"fmt" Join(string) (string, error)
"path" Root() string
"strings"
)
// RootPath can be joined with a relative path and ensures that
// the returned path is always a strict child of the root path.
type RootPath struct {
rootPath string
} }
// NewRootPath constructs and returns [RootPath]. type NopRootPath struct{}
// The named path is cleaned on construction.
func NewRootPath(name string) RootPath { func (rp NopRootPath) Join(name string) (string, error) {
return RootPath{ return name, nil
rootPath: path.Clean(name),
}
} }
// Join returns the specified path name joined to the root. func (rp NopRootPath) Root() string {
// It returns an error if the resulting path is not a strict child of the root path. return ""
func (p *RootPath) Join(name string) (string, error) {
absPath := path.Join(p.rootPath, name)
// Don't allow escaping the specified root using relative paths.
if !strings.HasPrefix(absPath, p.rootPath) {
return "", fmt.Errorf("relative path escapes root: %s", name)
}
return absPath, nil
} }

View File

@ -0,0 +1,38 @@
package filer
import (
"fmt"
"path"
"strings"
)
// UnixRootPath can be joined with a relative path and ensures that
// the returned path is always a strict child of the root path.
type UnixRootPath struct {
rootPath string
}
// NewUnixRootPath constructs and returns [RootPath].
// The named path is cleaned on construction.
func NewUnixRootPath(name string) UnixRootPath {
return UnixRootPath{
rootPath: path.Clean(name),
}
}
// Join returns the specified path name joined to the root.
// It returns an error if the resulting path is not a strict child of the root path.
func (p UnixRootPath) Join(name string) (string, error) {
absPath := path.Join(p.rootPath, name)
// Don't allow escaping the specified root using relative paths.
if !strings.HasPrefix(absPath, p.rootPath) {
return "", fmt.Errorf("relative path escapes root: %s", name)
}
return absPath, nil
}
func (p UnixRootPath) Root() string {
return p.rootPath
}

View File

@ -7,9 +7,9 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
func testRootPath(t *testing.T, uncleanRoot string) { func testUnixRootPath(t *testing.T, uncleanRoot string) {
cleanRoot := path.Clean(uncleanRoot) cleanRoot := path.Clean(uncleanRoot)
rp := NewRootPath(uncleanRoot) rp := NewUnixRootPath(uncleanRoot)
remotePath, err := rp.Join("a/b/c") remotePath, err := rp.Join("a/b/c")
assert.NoError(t, err) assert.NoError(t, err)
@ -79,12 +79,12 @@ func testRootPath(t *testing.T, uncleanRoot string) {
assert.ErrorContains(t, err, `relative path escapes root: ../..`) assert.ErrorContains(t, err, `relative path escapes root: ../..`)
} }
func TestRootPathClean(t *testing.T) { func TestUnixRootPathClean(t *testing.T) {
testRootPath(t, "/some/root/path") testUnixRootPath(t, "/some/root/path")
} }
func TestRootPathUnclean(t *testing.T) { func TestUnixRootPathUnclean(t *testing.T) {
testRootPath(t, "/some/root/path/") testUnixRootPath(t, "/some/root/path/")
testRootPath(t, "/some/root/path/.") testUnixRootPath(t, "/some/root/path/.")
testRootPath(t, "/some/root/../path/") testUnixRootPath(t, "/some/root/../path/")
} }

View File

@ -79,7 +79,7 @@ type WorkspaceFilesClient struct {
apiClient *client.DatabricksClient apiClient *client.DatabricksClient
// File operations will be relative to this path. // File operations will be relative to this path.
root RootPath root UnixRootPath
} }
func NewWorkspaceFilesClient(w *databricks.WorkspaceClient, root string) (Filer, error) { func NewWorkspaceFilesClient(w *databricks.WorkspaceClient, root string) (Filer, error) {
@ -92,7 +92,7 @@ func NewWorkspaceFilesClient(w *databricks.WorkspaceClient, root string) (Filer,
workspaceClient: w, workspaceClient: w,
apiClient: apiClient, apiClient: apiClient,
root: NewRootPath(root), root: NewUnixRootPath(root),
}, nil }, nil
} }