Merge pull request #195 from puerco/clean-paths

Use path.Clean instead of filepath.Clean in iofs.Open

Author: pjbgf

helper/iofs/iofs.go

@@ -5,7 +5,7 @@ package iofs
 import (
 	"io"
 	"io/fs"
-	"path/filepath"
+	"strings"
 
 	billyfs "github.com/go-git/go-billy/v6"
 	"github.com/go-git/go-billy/v6/helper/polyfill"
@@ -30,9 +30,16 @@ var (
 
 // TODO: implement fs.GlobFS, which will be a fair bit more code.
 
+// validPath checks that name is a valid io/fs path. In addition to the
+// standard fs.ValidPath checks, it rejects backslashes which on Windows
+// would be interpreted as path separators by the underlying billy filesystem.
+func validPath(name string) bool {
+	return fs.ValidPath(name) && !strings.Contains(name, `\`)
+}
+
 // Open opens the named file on the underlying FS, implementing fs.FS (returning a file or error).
 func (a *adapterFs) Open(name string) (fs.File, error) {
-	if name[0] == '/' || name != filepath.Clean(name) {
+	if !validPath(name) {
 		// fstest.TestFS explicitly checks that these should return error.
 		// MemFS performs the clean internally, so we need to block that here for testing purposes.
 		return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid}
@@ -54,16 +61,25 @@ func (a *adapterFs) Open(name string) (fs.File, error) {
 
 // ReadDir reads the named directory, implementing fs.ReadDirFS (returning a listing or error).
 func (a *adapterFs) ReadDir(name string) ([]fs.DirEntry, error) {
+	if !validPath(name) {
+		return nil, &fs.PathError{Op: "readdir", Path: name, Err: fs.ErrInvalid}
+	}
 	return a.fs.ReadDir(name)
 }
 
 // Stat returns information on the named file, implementing fs.StatFS (returning FileInfo or error).
 func (a *adapterFs) Stat(name string) (fs.FileInfo, error) {
+	if !validPath(name) {
+		return nil, &fs.PathError{Op: "stat", Path: name, Err: fs.ErrInvalid}
+	}
 	return a.fs.Stat(name)
 }
 
 // ReadFile reads the named file and returns its contents, implementing fs.ReadFileFS (returning contents or error).
 func (a *adapterFs) ReadFile(name string) ([]byte, error) {
+	if !validPath(name) {
+		return nil, &fs.PathError{Op: "readfile", Path: name, Err: fs.ErrInvalid}
+	}
 	stat, err := a.fs.Stat(name)
 	if err != nil {
 		return nil, err

helper/iofs/iofs_test.go

@@ -25,26 +25,58 @@ func TestWithFSTest(t *testing.T) {
 	iofs := New(memfs)
 
 	files := map[string]string{
-		"foo.txt":                       "hello, world",
-		"bar.txt":                       "goodbye, world",
-		filepath.Join("dir", "baz.txt"): "こんにちわ, world",
+		"foo.txt":     "hello, world",
+		"bar.txt":     "goodbye, world",
+		"dir/baz.txt": "こんにちわ, world",
 	}
 	createdFiles := make([]string, 0, len(files))
 	for filename, contents := range files {
 		makeFile(t, memfs, filename, contents)
 		createdFiles = append(createdFiles, filename)
 	}
 
-	if runtime.GOOS == "windows" {
-		t.Skip("fstest.TestFS is not yet windows path aware")
-	}
-
 	err := fstest.TestFS(iofs, createdFiles...)
 	if err != nil {
 		checkFsTestError(t, err, files)
 	}
 }
 
+// TestOpenForwardSlashPath verifies that Open works with forward-slash paths
+// on all platforms. This is a regression test ensuring filepath.Clean is not
+// used when cleaning paths in Open() (it should use path.Clean) so valid
+// fs.FS paths like "dir/file.txt" are not rejected on Windows.
+func TestOpenForwardSlashPath(t *testing.T) {
+	t.Parallel()
+	mem := memfs.New()
+	adapter := New(mem)
+
+	makeFile(t, mem, "dir/subdir/file.txt", "content")
+
+	tests := []struct {
+		name    string
+		path    string
+		wantErr bool
+	}{
+		{"simple-nested", "dir/subdir/file.txt", false},
+		{"directory", "dir/subdir", false},
+		{"dot-path", ".", false},
+		{"absolute-path-rejected", "/dir/subdir/file.txt", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			f, err := adapter.Open(tt.path)
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			require.NoError(t, f.Close())
+		})
+	}
+}
+
 func TestDeletes(t *testing.T) {
 	t.Parallel()
 	memfs := memfs.New()