Refactor tests and make `toLocalPath` a method on `Server`

packet.go

@@ -1247,7 +1247,7 @@ func (p *sshFxpExtendedPacketPosixRename) UnmarshalBinary(b []byte) error {
 }
 
 func (p *sshFxpExtendedPacketPosixRename) respond(s *Server) responsePacket {
-	err := os.Rename(toLocalPath(s.workDir, p.Oldpath), toLocalPath(s.workDir, p.Newpath))
+	err := os.Rename(s.toLocalPath(p.Oldpath), s.toLocalPath(p.Newpath))
 	return statusFromError(p.ID, err)
 }
 
@@ -1276,6 +1276,6 @@ func (p *sshFxpExtendedPacketHardlink) UnmarshalBinary(b []byte) error {
 }
 
 func (p *sshFxpExtendedPacketHardlink) respond(s *Server) responsePacket {
-	err := os.Link(toLocalPath(s.workDir, p.Oldpath), toLocalPath(s.workDir, p.Newpath))
+	err := os.Link(s.toLocalPath(p.Oldpath), s.toLocalPath(p.Newpath))
 	return statusFromError(p.ID, err)
 }

request-plan9.go

@@ -3,8 +3,6 @@
 package sftp
 
 import (
-	"path"
-	"path/filepath"
 	"syscall"
 )
 
@@ -15,24 +13,3 @@ func fakeFileInfoSys() interface{} {
 func testOsSys(sys interface{}) error {
 	return nil
 }
-
-func toLocalPath(workDir, p string) string {
-	if workDir != "" && !path.IsAbs(p) {
-		p = path.Join(workDir, p)
-	}
-
-	lp := filepath.FromSlash(p)
-
-	if path.IsAbs(p) {
-		tmp := lp[1:]
-
-		if filepath.IsAbs(tmp) {
-			// If the FromSlash without any starting slashes is absolute,
-			// then we have a filepath encoded with a prefix '/'.
-			// e.g. "/#s/boot" to "#s/boot"
-			return tmp
-		}
-	}
-
-	return lp
-}

request-unix.go

@@ -4,7 +4,6 @@ package sftp
 
 import (
 	"errors"
-	"path"
 	"syscall"
 )
 
@@ -22,11 +21,3 @@ func testOsSys(sys interface{}) error {
 	}
 	return nil
 }
-
-func toLocalPath(workDir, p string) string {
-	if workDir != "" && !path.IsAbs(p) {
-		p = path.Join(workDir, p)
-	}
-
-	return p
-}

request_test.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"io"
 	"os"
-	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -248,113 +247,3 @@ func TestOpendirHandleReuse(t *testing.T) {
 	rpkt = request.call(handlers, pkt, nil, 0)
 	assert.IsType(t, &sshFxpNamePacket{}, rpkt)
 }
-
-func Test_toLocalPath(t *testing.T) {
-	type args struct {
-		workDir string
-		p       string
-	}
-	tests := []struct {
-		name string
-		goos string
-		args args
-		want string
-	}{
-		{
-			name: "empty path with no workdir",
-			args: args{p: ""},
-			want: "",
-		},
-		{
-			name: "relative path with no workdir",
-			args: args{p: "file"},
-			want: "file",
-		},
-		{
-			name: "absolute path with no workdir",
-			args: args{p: "/file"},
-			want: "/file",
-		},
-		{
-			name: "workdir and empty path on Unix",
-			goos: "linux",
-			args: args{workDir: cleanPath("/home/user"), p: ""},
-			want: "/home/user",
-		},
-		{
-			name: "workdir and relative path on Unix",
-			goos: "linux",
-			args: args{workDir: cleanPath("/home/user"), p: "file"},
-			want: "/home/user/file",
-		},
-		{
-			name: "workdir and relative path with . on Unix",
-			goos: "linux",
-			args: args{workDir: cleanPath("/home/user"), p: "."},
-			want: "/home/user",
-		},
-		{
-			name: "workdir and relative path with . and file on Unix",
-			goos: "linux",
-			args: args{workDir: cleanPath("/home/user"), p: "./file"},
-			want: "/home/user/file",
-		},
-		{
-			name: "workdir and absolute path on Unix",
-			goos: "linux",
-			args: args{workDir: cleanPath("/home/user"), p: "/file"},
-			want: "/file",
-		},
-		{
-			name: "workdir and non-unixy path on Unix prefixes workdir",
-			goos: "linux",
-			args: args{workDir: cleanPath("/home/user"), p: "C:\\file"},
-			want: "/home/user/C:\\file",
-		},
-		{
-			name: "workdir and empty path on Windows",
-			goos: "windows",
-			args: args{workDir: cleanPath("C:\\Users\\User"), p: ""},
-			want: "C:\\Users\\User",
-		},
-		{
-			name: "workdir and relative path on Windows",
-			goos: "windows",
-			args: args{workDir: cleanPath("C:\\Users\\User"), p: "file"},
-			want: "C:\\Users\\User\\file",
-		},
-		{
-			name: "workdir and relative path with . on Windows",
-			goos: "windows",
-			args: args{workDir: cleanPath("C:\\Users\\User"), p: "."},
-			want: "C:\\Users\\User",
-		},
-		{
-			name: "workdir and relative path with . and file on Windows",
-			goos: "windows",
-			args: args{workDir: cleanPath("C:\\Users\\User"), p: "./file"},
-			want: "C:\\Users\\User\\file",
-		},
-		{
-			name: "workdir and absolute path on Windows",
-			goos: "windows",
-			args: args{workDir: cleanPath("C:\\Users\\User"), p: "/C:/file"},
-			want: "C:\\file",
-		},
-		{
-			name: "workdir and non-unixy path on Windows prefixes workdir",
-			goos: "windows",
-			args: args{workDir: cleanPath("C:\\Users\\User"), p: "C:\\file"},
-			want: "C:\\Users\\User\\C:\\file",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if tt.goos != "" && runtime.GOOS != tt.goos {
-				t.Skipf("Skipping test for %s on %s", tt.goos, runtime.GOOS)
-			}
-
-			assert.Equal(t, tt.want, toLocalPath(tt.args.workDir, tt.args.p), "wrong local path")
-		})
-	}
-}

request_windows.go

@@ -1,8 +1,6 @@
 package sftp
 
 import (
-	"path"
-	"path/filepath"
 	"syscall"
 )
 
@@ -13,36 +11,3 @@ func fakeFileInfoSys() interface{} {
 func testOsSys(sys interface{}) error {
 	return nil
 }
-
-func toLocalPath(workDir, p string) string {
-	if workDir != "" && !path.IsAbs(p) {
-		p = path.Join(workDir, p)
-	}
-
-	lp := filepath.FromSlash(p)
-
-	if path.IsAbs(p) {
-		tmp := lp
-		for len(tmp) > 0 && tmp[0] == '\\' {
-			tmp = tmp[1:]
-		}
-
-		if filepath.IsAbs(tmp) {
-			// If the FromSlash without any starting slashes is absolute,
-			// then we have a filepath encoded with a prefix '/'.
-			// e.g. "/C:/Windows" to "C:\\Windows"
-			return tmp
-		}
-
-		tmp += "\\"
-
-		if filepath.IsAbs(tmp) {
-			// If the FromSlash without any starting slashes but with extra end slash is absolute,
-			// then we have a filepath encoded with a prefix '/' and a dropped '/' at the end.
-			// e.g. "/C:" to "C:\\"
-			return tmp
-		}
-	}
-
-	return lp
-}

server.go

@@ -185,7 +185,7 @@ func handlePacket(s *Server, p orderedRequest) error {
 		}
 	case *sshFxpStatPacket:
 		// stat the requested file
-		info, err := os.Stat(toLocalPath(s.workDir, p.Path))
+		info, err := os.Stat(s.toLocalPath(p.Path))
 		rpkt = &sshFxpStatResponse{
 			ID:   p.ID,
 			info: info,
@@ -195,7 +195,7 @@ func handlePacket(s *Server, p orderedRequest) error {
 		}
 	case *sshFxpLstatPacket:
 		// stat the requested file
-		info, err := os.Lstat(toLocalPath(s.workDir, p.Path))
+		info, err := os.Lstat(s.toLocalPath(p.Path))
 		rpkt = &sshFxpStatResponse{
 			ID:   p.ID,
 			info: info,
@@ -219,24 +219,24 @@ func handlePacket(s *Server, p orderedRequest) error {
 		}
 	case *sshFxpMkdirPacket:
 		// TODO FIXME: ignore flags field
-		err := os.Mkdir(toLocalPath(s.workDir, p.Path), 0o755)
+		err := os.Mkdir(s.toLocalPath(p.Path), 0o755)
 		rpkt = statusFromError(p.ID, err)
 	case *sshFxpRmdirPacket:
-		err := os.Remove(toLocalPath(s.workDir, p.Path))
+		err := os.Remove(s.toLocalPath(p.Path))
 		rpkt = statusFromError(p.ID, err)
 	case *sshFxpRemovePacket:
-		err := os.Remove(toLocalPath(s.workDir, p.Filename))
+		err := os.Remove(s.toLocalPath(p.Filename))
 		rpkt = statusFromError(p.ID, err)
 	case *sshFxpRenamePacket:
-		err := os.Rename(toLocalPath(s.workDir, p.Oldpath), toLocalPath(s.workDir, p.Newpath))
+		err := os.Rename(s.toLocalPath(p.Oldpath), s.toLocalPath(p.Newpath))
 		rpkt = statusFromError(p.ID, err)
 	case *sshFxpSymlinkPacket:
-		err := os.Symlink(toLocalPath(s.workDir, p.Targetpath), toLocalPath(s.workDir, p.Linkpath))
+		err := os.Symlink(s.toLocalPath(p.Targetpath), s.toLocalPath(p.Linkpath))
 		rpkt = statusFromError(p.ID, err)
 	case *sshFxpClosePacket:
 		rpkt = statusFromError(p.ID, s.closeHandle(p.Handle))
 	case *sshFxpReadlinkPacket:
-		f, err := os.Readlink(toLocalPath(s.workDir, p.Path))
+		f, err := os.Readlink(s.toLocalPath(p.Path))
 		rpkt = &sshFxpNamePacket{
 			ID: p.ID,
 			NameAttrs: []*sshFxpNameAttr{
@@ -251,7 +251,7 @@ func handlePacket(s *Server, p orderedRequest) error {
 			rpkt = statusFromError(p.ID, err)
 		}
 	case *sshFxpRealpathPacket:
-		f, err := filepath.Abs(toLocalPath(s.workDir, p.Path))
+		f, err := filepath.Abs(s.toLocalPath(p.Path))
 		f = cleanPath(f)
 		rpkt = &sshFxpNamePacket{
 			ID: p.ID,
@@ -267,7 +267,7 @@ func handlePacket(s *Server, p orderedRequest) error {
 			rpkt = statusFromError(p.ID, err)
 		}
 	case *sshFxpOpendirPacket:
-		lp := toLocalPath(s.workDir, p.Path)
+		lp := s.toLocalPath(p.Path)
 
 		if stat, err := os.Stat(lp); err != nil {
 			rpkt = statusFromError(p.ID, err)
@@ -458,7 +458,7 @@ func (p *sshFxpOpenPacket) respond(svr *Server) responsePacket {
 		osFlags |= os.O_EXCL
 	}
 
-	f, err := os.OpenFile(toLocalPath(svr.workDir, p.Path), osFlags, 0o644)
+	f, err := os.OpenFile(svr.toLocalPath(p.Path), osFlags, 0o644)
 	if err != nil {
 		return statusFromError(p.ID, err)
 	}
@@ -496,7 +496,7 @@ func (p *sshFxpSetstatPacket) respond(svr *Server) responsePacket {
 	b := p.Attrs.([]byte)
 	var err error
 
-	p.Path = toLocalPath(svr.workDir, p.Path)
+	p.Path = svr.toLocalPath(p.Path)
 
 	debug("setstat name \"%s\"", p.Path)
 	if (p.Flags & sshFileXferAttrSize) != 0 {

server_plan9.go

@@ -0,0 +1,30 @@
+//go:build plan9
+// +build plan9
+
+package sftp
+
+import (
+	"path"
+	"path/filepath"
+)
+
+func (s *Server) toLocalPath(p string) string {
+	if s.workDir != "" && !path.IsAbs(p) {
+		p = path.Join(s.workDir, p)
+	}
+
+	lp := filepath.FromSlash(p)
+
+	if path.IsAbs(p) {
+		tmp := lp[1:]
+
+		if filepath.IsAbs(tmp) {
+			// If the FromSlash without any starting slashes is absolute,
+			// then we have a filepath encoded with a prefix '/'.
+			// e.g. "/#s/boot" to "#s/boot"
+			return tmp
+		}
+	}
+
+	return lp
+}

server_unix.go

@@ -0,0 +1,16 @@
+//go:build !windows && !plan9
+// +build !windows,!plan9
+
+package sftp
+
+import (
+	"path"
+)
+
+func (s *Server) toLocalPath(p string) string {
+	if s.workDir != "" && !path.IsAbs(p) {
+		p = path.Join(s.workDir, p)
+	}
+
+	return p
+}

server_unix_test.go

@@ -0,0 +1,87 @@
+//go:build !windows && !plan9
+// +build !windows,!plan9
+
+package sftp
+
+import (
+	"testing"
+)
+
+func TestServer_toLocalPath(t *testing.T) {
+	tests := []struct {
+		name        string
+		withWorkDir string
+		p           string
+		want        string
+	}{
+		{
+			name: "empty path with no workdir",
+			p:    "",
+			want: "",
+		},
+		{
+			name: "relative path with no workdir",
+			p:    "file",
+			want: "file",
+		},
+		{
+			name: "absolute path with no workdir",
+			p:    "/file",
+			want: "/file",
+		},
+		{
+			name:        "workdir and empty path",
+			withWorkDir: "/home/user",
+			p:           "",
+			want:        "/home/user",
+		},
+		{
+			name:        "workdir and relative path",
+			withWorkDir: "/home/user",
+			p:           "file",
+			want:        "/home/user/file",
+		},
+		{
+			name:        "workdir and relative path with .",
+			withWorkDir: "/home/user",
+			p:           ".",
+			want:        "/home/user",
+		},
+		{
+			name:        "workdir and relative path with . and file",
+			withWorkDir: "/home/user",
+			p:           "./file",
+			want:        "/home/user/file",
+		},
+		{
+			name:        "workdir and absolute path",
+			withWorkDir: "/home/user",
+			p:           "/file",
+			want:        "/file",
+		},
+		{
+			name:        "workdir and non-unixy path prefixes workdir",
+			withWorkDir: "/home/user",
+			p:           "C:\\file",
+			// This may look like a bug but it is the result of passing
+			// invalid input (a non-unixy path) to the server.
+			want: "/home/user/C:\\file",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// We don't need to initialize the Server further to test
+			// toLocalPath behavior.
+			s := &Server{}
+			if tt.withWorkDir != "" {
+				if err := WithServerWorkingDirectory(tt.withWorkDir)(s); err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			if got := s.toLocalPath(tt.p); got != tt.want {
+				t.Errorf("Server.toLocalPath() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

server_windows.go

@@ -0,0 +1,39 @@
+package sftp
+
+import (
+	"path"
+	"path/filepath"
+)
+
+func (s *Server) toLocalPath(p string) string {
+	if s.workDir != "" && !path.IsAbs(p) {
+		p = path.Join(s.workDir, p)
+	}
+
+	lp := filepath.FromSlash(p)
+
+	if path.IsAbs(p) {
+		tmp := lp
+		for len(tmp) > 0 && tmp[0] == '\\' {
+			tmp = tmp[1:]
+		}
+
+		if filepath.IsAbs(tmp) {
+			// If the FromSlash without any starting slashes is absolute,
+			// then we have a filepath encoded with a prefix '/'.
+			// e.g. "/C:/Windows" to "C:\\Windows"
+			return tmp
+		}
+
+		tmp += "\\"
+
+		if filepath.IsAbs(tmp) {
+			// If the FromSlash without any starting slashes but with extra end slash is absolute,
+			// then we have a filepath encoded with a prefix '/' and a dropped '/' at the end.
+			// e.g. "/C:" to "C:\\"
+			return tmp
+		}
+	}
+
+	return lp
+}

server_windows_test.go

@@ -0,0 +1,87 @@
+//go:build windows
+// +build windows
+
+package sftp
+
+import (
+	"testing"
+)
+
+func TestServer_toLocalPath(t *testing.T) {
+	tests := []struct {
+		name        string
+		withWorkDir string
+		p           string
+		want        string
+	}{
+		{
+			name: "empty path with no workdir",
+			p:    "",
+			want: "",
+		},
+		{
+			name: "relative path with no workdir",
+			p:    "file",
+			want: "file",
+		},
+		{
+			name: "absolute path with no workdir",
+			p:    "/file",
+			want: "\\file",
+		},
+		{
+			name:        "workdir and empty path",
+			withWorkDir: "C:\\Users\\User",
+			p:           "",
+			want:        "C:\\Users\\User",
+		},
+		{
+			name:        "workdir and relative path",
+			withWorkDir: "C:\\Users\\User",
+			p:           "file",
+			want:        "C:\\Users\\User\\file",
+		},
+		{
+			name:        "workdir and relative path with .",
+			withWorkDir: "C:\\Users\\User",
+			p:           ".",
+			want:        "C:\\Users\\User",
+		},
+		{
+			name:        "workdir and relative path with . and file",
+			withWorkDir: "C:\\Users\\User",
+			p:           "./file",
+			want:        "C:\\Users\\User\\file",
+		},
+		{
+			name:        "workdir and absolute path",
+			withWorkDir: "C:\\Users\\User",
+			p:           "/C:/file",
+			want:        "C:\\file",
+		},
+		{
+			name:        "workdir and non-unixy path prefixes workdir",
+			withWorkDir: "C:\\Users\\User",
+			p:           "C:\\file",
+			// This may look like a bug but it is the result of passing
+			// invalid input (a non-unixy path) to the server.
+			want: "C:\\Users\\User\\C:\\file",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// We don't need to initialize the Server further to test
+			// toLocalPath behavior.
+			s := &Server{}
+			if tt.withWorkDir != "" {
+				if err := WithServerWorkingDirectory(tt.withWorkDir)(s); err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			if got := s.toLocalPath(tt.p); got != tt.want {
+				t.Errorf("Server.toLocalPath() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}