Repository: pkg/sftp
Pull request #456 (2021-08-12 09:12:12)

Regroup marshling/unmarshaling and lint
This covers a few different things that were somewhat unrelated to starting to use `filexfer` but will make that transfer easier.

* Collects and isolates marshal/unmarshal code into `packets.go`
* Removing `clamp` and `min` functions that both did the same thing, and didn’t really contribute to more understandable code.
* Fix corner cases in `os.ModeType → uint32` where we treat the various types as bitfields that can be OR’ed together rather than mutually exclusive values.
* a small bit of linting on `request-server.go`
* `fileInfo` can use the values in the wrapped `FileStat`, which we also don’t need to treat as an `any`.
* bring over the `ls_formatting` from PR-432, as this is an improvement beyond just bringing in `filexfer` as it extracts more UID/GID values for the various `GOOS`, supports setting UID/GID values from a `FileStat`, and also converts UIDs/GIDs to usernames and groupnames when possible. (Do we really want to use system-wide username/groupnames? Might be good to provide an ability to override this? Since `request-server` might be operating entirely disjoint from the underlying OS.)
* removed a few `cgo` build conditions that were not actually necessary
* verified that the code compiles for all `GOOS` supported in `go1.16` (not verified that it works, just that it compiles)


Patch:
diff --git a/attrs.go b/attrs.go
index 7020d3a..2bb2d57 100644
--- a/attrs.go
+++ b/attrs.go
@@ -21,29 +21,26 @@ const (
 
 // fileInfo is an artificial type designed to satisfy os.FileInfo.
 type fileInfo struct {
-	name  string
-	size  int64
-	mode  os.FileMode
-	mtime time.Time
-	sys   interface{}
+	name string
+	stat *FileStat
 }
 
 // Name returns the base name of the file.
 func (fi *fileInfo) Name() string { return fi.name }
 
 // Size returns the length in bytes for regular files; system-dependent for others.
-func (fi *fileInfo) Size() int64 { return fi.size }
+func (fi *fileInfo) Size() int64 { return int64(fi.stat.Size) }
 
 // Mode returns file mode bits.
-func (fi *fileInfo) Mode() os.FileMode { return fi.mode }
+func (fi *fileInfo) Mode() os.FileMode { return toFileMode(fi.stat.Mode) }
 
 // ModTime returns the last modification time of the file.
-func (fi *fileInfo) ModTime() time.Time { return fi.mtime }
+func (fi *fileInfo) ModTime() time.Time { return time.Unix(int64(fi.stat.Mtime), 0) }
 
 // IsDir returns true if the file is a directory.
 func (fi *fileInfo) IsDir() bool { return fi.Mode().IsDir() }
 
-func (fi *fileInfo) Sys() interface{} { return fi.sys }
+func (fi *fileInfo) Sys() interface{} { return fi.stat }
 
 // FileStat holds the original unmarshalled values from a call to READDIR or
 // *STAT. It is exported for the purposes of accessing the raw values via
@@ -65,25 +62,21 @@ type StatExtended struct {
 	ExtData string
 }
 
-func fileInfoFromStat(st *FileStat, name string) os.FileInfo {
-	fs := &fileInfo{
-		name:  name,
-		size:  int64(st.Size),
-		mode:  toFileMode(st.Mode),
-		mtime: time.Unix(int64(st.Mtime), 0),
-		sys:   st,
+func fileInfoFromStat(stat *FileStat, name string) os.FileInfo {
+	return &fileInfo{
+		name: name,
+		stat: stat,
 	}
-	return fs
 }
 
-func fileStatFromInfo(fi os.FileInfo) (uint32, FileStat) {
+func fileStatFromInfo(fi os.FileInfo) (uint32, *FileStat) {
 	mtime := fi.ModTime().Unix()
 	atime := mtime
 	var flags uint32 = sshFileXferAttrSize |
 		sshFileXferAttrPermissions |
 		sshFileXferAttrACmodTime
 
-	fileStat := FileStat{
+	fileStat := &FileStat{
 		Size:  uint64(fi.Size()),
 		Mode:  fromFileMode(fi.Mode()),
 		Mtime: uint32(mtime),
@@ -91,83 +84,7 @@ func fileStatFromInfo(fi os.FileInfo) (uint32, FileStat) {
 	}
 
 	// os specific file stat decoding
-	fileStatFromInfoOs(fi, &flags, &fileStat)
+	fileStatFromInfoOs(fi, &flags, fileStat)
 
 	return flags, fileStat
 }
-
-func unmarshalAttrs(b []byte) (*FileStat, []byte) {
-	flags, b := unmarshalUint32(b)
-	return getFileStat(flags, b)
-}
-
-func getFileStat(flags uint32, b []byte) (*FileStat, []byte) {
-	var fs FileStat
-	if flags&sshFileXferAttrSize == sshFileXferAttrSize {
-		fs.Size, b, _ = unmarshalUint64Safe(b)
-	}
-	if flags&sshFileXferAttrUIDGID == sshFileXferAttrUIDGID {
-		fs.UID, b, _ = unmarshalUint32Safe(b)
-	}
-	if flags&sshFileXferAttrUIDGID == sshFileXferAttrUIDGID {
-		fs.GID, b, _ = unmarshalUint32Safe(b)
-	}
-	if flags&sshFileXferAttrPermissions == sshFileXferAttrPermissions {
-		fs.Mode, b, _ = unmarshalUint32Safe(b)
-	}
-	if flags&sshFileXferAttrACmodTime == sshFileXferAttrACmodTime {
-		fs.Atime, b, _ = unmarshalUint32Safe(b)
-		fs.Mtime, b, _ = unmarshalUint32Safe(b)
-	}
-	if flags&sshFileXferAttrExtended == sshFileXferAttrExtended {
-		var count uint32
-		count, b, _ = unmarshalUint32Safe(b)
-		ext := make([]StatExtended, count)
-		for i := uint32(0); i < count; i++ {
-			var typ string
-			var data string
-			typ, b, _ = unmarshalStringSafe(b)
-			data, b, _ = unmarshalStringSafe(b)
-			ext[i] = StatExtended{typ, data}
-		}
-		fs.Extended = ext
-	}
-	return &fs, b
-}
-
-func marshalFileInfo(b []byte, fi os.FileInfo) []byte {
-	// attributes variable struct, and also variable per protocol version
-	// spec version 3 attributes:
-	// uint32   flags
-	// uint64   size           present only if flag SSH_FILEXFER_ATTR_SIZE
-	// uint32   uid            present only if flag SSH_FILEXFER_ATTR_UIDGID
-	// uint32   gid            present only if flag SSH_FILEXFER_ATTR_UIDGID
-	// uint32   permissions    present only if flag SSH_FILEXFER_ATTR_PERMISSIONS
-	// uint32   atime          present only if flag SSH_FILEXFER_ACMODTIME
-	// uint32   mtime          present only if flag SSH_FILEXFER_ACMODTIME
-	// uint32   extended_count present only if flag SSH_FILEXFER_ATTR_EXTENDED
-	// string   extended_type
-	// string   extended_data
-	// ...      more extended data (extended_type - extended_data pairs),
-	// 	   so that number of pairs equals extended_count
-
-	flags, fileStat := fileStatFromInfo(fi)
-
-	b = marshalUint32(b, flags)
-	if flags&sshFileXferAttrSize != 0 {
-		b = marshalUint64(b, fileStat.Size)
-	}
-	if flags&sshFileXferAttrUIDGID != 0 {
-		b = marshalUint32(b, fileStat.UID)
-		b = marshalUint32(b, fileStat.GID)
-	}
-	if flags&sshFileXferAttrPermissions != 0 {
-		b = marshalUint32(b, fileStat.Mode)
-	}
-	if flags&sshFileXferAttrACmodTime != 0 {
-		b = marshalUint32(b, fileStat.Atime)
-		b = marshalUint32(b, fileStat.Mtime)
-	}
-
-	return b
-}
diff --git a/attrs_stubs.go b/attrs_stubs.go
index ba72e30..c01f336 100644
--- a/attrs_stubs.go
+++ b/attrs_stubs.go
@@ -1,4 +1,4 @@
-// +build !cgo plan9 windows android
+// +build plan9 windows android
 
 package sftp
 
diff --git a/attrs_unix.go b/attrs_unix.go
index 846b208..d1f4452 100644
--- a/attrs_unix.go
+++ b/attrs_unix.go
@@ -1,5 +1,4 @@
-// +build darwin dragonfly freebsd !android,linux netbsd openbsd solaris aix
-// +build cgo
+// +build darwin dragonfly freebsd !android,linux netbsd openbsd solaris aix js
 
 package sftp
 
diff --git a/client.go b/client.go
index 0569094..d623404 100644
--- a/client.go
+++ b/client.go
@@ -1863,13 +1863,6 @@ func (f *File) Truncate(size int64) error {
 	return f.c.setfstat(f.handle, sshFileXferAttrSize, uint64(size))
 }
 
-func min(a, b int) int {
-	if a > b {
-		return b
-	}
-	return a
-}
-
 // normaliseError normalises an error into a more standard form that can be
 // checked against stdlib errors like io.EOF or os.ErrNotExist.
 func normaliseError(err error) error {
@@ -1892,28 +1885,6 @@ func normaliseError(err error) error {
 	}
 }
 
-func unmarshalStatus(id uint32, data []byte) error {
-	sid, data := unmarshalUint32(data)
-	if sid != id {
-		return &unexpectedIDErr{id, sid}
-	}
-	code, data := unmarshalUint32(data)
-	msg, data, _ := unmarshalStringSafe(data)
-	lang, _, _ := unmarshalStringSafe(data)
-	return &StatusError{
-		Code: code,
-		msg:  msg,
-		lang: lang,
-	}
-}
-
-func marshalStatus(b []byte, err StatusError) []byte {
-	b = marshalUint32(b, err.Code)
-	b = marshalString(b, err.msg)
-	b = marshalString(b, err.lang)
-	return b
-}
-
 // flags converts the flags passed to OpenFile into ssh flags.
 // Unsupported flags are ignored.
 func flags(f int) uint32 {
diff --git a/ls_formatting.go b/ls_formatting.go
new file mode 100644
index 0000000..e083e22
--- /dev/null
+++ b/ls_formatting.go
@@ -0,0 +1,81 @@
+package sftp
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"os/user"
+	"strconv"
+	"time"
+
+	sshfx "github.com/pkg/sftp/internal/encoding/ssh/filexfer"
+)
+
+func lsFormatID(id uint32) string {
+	return strconv.FormatUint(uint64(id), 10)
+}
+
+type osIDLookup struct{}
+
+func (osIDLookup) Filelist(*Request) (ListerAt, error) {
+	return nil, errors.New("unimplemented stub")
+}
+
+func (osIDLookup) LookupUserName(uid string) string {
+	u, err := user.LookupId(uid)
+	if err != nil {
+		return uid
+	}
+
+	return u.Username
+}
+
+func (osIDLookup) LookupGroupName(gid string) string {
+	g, err := user.LookupGroupId(gid)
+	if err != nil {
+		return gid
+	}
+
+	return g.Name
+}
+
+// runLs formats the FileInfo as per `ls -l` style, which is in the 'longname' field of a SSH_FXP_NAME entry.
+// This is a fairly simple implementation, just enough to look close to openssh in simple cases.
+func runLs(idLookup NameLookupFileLister, dirent os.FileInfo) string {
+	// example from openssh sftp server:
+	// crw-rw-rw-    1 root     wheel           0 Jul 31 20:52 ttyvd
+	// format:
+	// {directory / char device / etc}{rwxrwxrwx}  {number of links} owner group size month day [time (this year) | year (otherwise)] name
+
+	symPerms := sshfx.FileMode(fromFileMode(dirent.Mode())).String()
+
+	var numLinks uint64 = 1
+	uid, gid := "0", "0"
+
+	switch sys := dirent.Sys().(type) {
+	case *sshfx.Attributes:
+		uid = lsFormatID(sys.UID)
+		gid = lsFormatID(sys.GID)
+	case *FileStat:
+		uid = lsFormatID(sys.UID)
+		gid = lsFormatID(sys.GID)
+	default:
+		numLinks, uid, gid = lsLinksUIDGID(dirent)
+	}
+
+	if idLookup != nil {
+		uid, gid = idLookup.LookupUserName(uid), idLookup.LookupGroupName(gid)
+	}
+
+	mtime := dirent.ModTime()
+	date := mtime.Format("Jan 2")
+
+	var yearOrTime string
+	if mtime.Before(time.Now().AddDate(0, -6, 0)) {
+		yearOrTime = mtime.Format("2006")
+	} else {
+		yearOrTime = mtime.Format("15:04")
+	}
+
+	return fmt.Sprintf("%s %4d %-8s %-8s %8d %s %5s %s", symPerms, numLinks, uid, gid, dirent.Size(), date, yearOrTime, dirent.Name())
+}
diff --git a/ls_plan9.go b/ls_plan9.go
new file mode 100644
index 0000000..a16a3ea
--- /dev/null
+++ b/ls_plan9.go
@@ -0,0 +1,21 @@
+// +build plan9
+
+package sftp
+
+import (
+	"os"
+	"syscall"
+)
+
+func lsLinksUIDGID(fi os.FileInfo) (numLinks uint64, uid, gid string) {
+	numLinks = 1
+	uid, gid = "0", "0"
+
+	switch sys := fi.Sys().(type) {
+	case *syscall.Dir:
+		uid = sys.Uid
+		gid = sys.Gid
+	}
+
+	return numLinks, uid, gid
+}
diff --git a/ls_stub.go b/ls_stub.go
new file mode 100644
index 0000000..6dec393
--- /dev/null
+++ b/ls_stub.go
@@ -0,0 +1,11 @@
+// +build windows android
+
+package sftp
+
+import (
+	"os"
+)
+
+func lsLinksUIDGID(fi os.FileInfo) (numLinks uint64, uid, gid string) {
+	return 1, "0", "0"
+}
diff --git a/ls_unix.go b/ls_unix.go
new file mode 100644
index 0000000..59ccffd
--- /dev/null
+++ b/ls_unix.go
@@ -0,0 +1,23 @@
+// +build aix darwin dragonfly freebsd !android,linux netbsd openbsd solaris js
+
+package sftp
+
+import (
+	"os"
+	"syscall"
+)
+
+func lsLinksUIDGID(fi os.FileInfo) (numLinks uint64, uid, gid string) {
+	numLinks = 1
+	uid, gid = "0", "0"
+
+	switch sys := fi.Sys().(type) {
+	case *syscall.Stat_t:
+		numLinks = uint64(sys.Nlink)
+		uid = lsFormatID(sys.Uid)
+		gid = lsFormatID(sys.Gid)
+	default:
+	}
+
+	return numLinks, uid, gid
+}
diff --git a/packet.go b/packet.go
index 2b2e592..50ca069 100644
--- a/packet.go
+++ b/packet.go
@@ -37,6 +37,50 @@ func marshalString(b []byte, v string) []byte {
 	return append(marshalUint32(b, uint32(len(v))), v...)
 }
 
+func marshalFileInfo(b []byte, fi os.FileInfo) []byte {
+	// attributes variable struct, and also variable per protocol version
+	// spec version 3 attributes:
+	// uint32   flags
+	// uint64   size           present only if flag SSH_FILEXFER_ATTR_SIZE
+	// uint32   uid            present only if flag SSH_FILEXFER_ATTR_UIDGID
+	// uint32   gid            present only if flag SSH_FILEXFER_ATTR_UIDGID
+	// uint32   permissions    present only if flag SSH_FILEXFER_ATTR_PERMISSIONS
+	// uint32   atime          present only if flag SSH_FILEXFER_ACMODTIME
+	// uint32   mtime          present only if flag SSH_FILEXFER_ACMODTIME
+	// uint32   extended_count present only if flag SSH_FILEXFER_ATTR_EXTENDED
+	// string   extended_type
+	// string   extended_data
+	// ...      more extended data (extended_type - extended_data pairs),
+	// 	   so that number of pairs equals extended_count
+
+	flags, fileStat := fileStatFromInfo(fi)
+
+	b = marshalUint32(b, flags)
+	if flags&sshFileXferAttrSize != 0 {
+		b = marshalUint64(b, fileStat.Size)
+	}
+	if flags&sshFileXferAttrUIDGID != 0 {
+		b = marshalUint32(b, fileStat.UID)
+		b = marshalUint32(b, fileStat.GID)
+	}
+	if flags&sshFileXferAttrPermissions != 0 {
+		b = marshalUint32(b, fileStat.Mode)
+	}
+	if flags&sshFileXferAttrACmodTime != 0 {
+		b = marshalUint32(b, fileStat.Atime)
+		b = marshalUint32(b, fileStat.Mtime)
+	}
+
+	return b
+}
+
+func marshalStatus(b []byte, err StatusError) []byte {
+	b = marshalUint32(b, err.Code)
+	b = marshalString(b, err.msg)
+	b = marshalString(b, err.lang)
+	return b
+}
+
 func marshal(b []byte, v interface{}) []byte {
 	if v == nil {
 		return b
@@ -115,6 +159,63 @@ func unmarshalStringSafe(b []byte) (string, []byte, error) {
 	return string(b[:n]), b[n:], nil
 }
 
+func unmarshalAttrs(b []byte) (*FileStat, []byte) {
+	flags, b := unmarshalUint32(b)
+	return unmarshalFileStat(flags, b)
+}
+
+func unmarshalFileStat(flags uint32, b []byte) (*FileStat, []byte) {
+	var fs FileStat
+	if flags&sshFileXferAttrSize == sshFileXferAttrSize {
+		fs.Size, b, _ = unmarshalUint64Safe(b)
+	}
+	if flags&sshFileXferAttrUIDGID == sshFileXferAttrUIDGID {
+		fs.UID, b, _ = unmarshalUint32Safe(b)
+	}
+	if flags&sshFileXferAttrUIDGID == sshFileXferAttrUIDGID {
+		fs.GID, b, _ = unmarshalUint32Safe(b)
+	}
+	if flags&sshFileXferAttrPermissions == sshFileXferAttrPermissions {
+		fs.Mode, b, _ = unmarshalUint32Safe(b)
+	}
+	if flags&sshFileXferAttrACmodTime == sshFileXferAttrACmodTime {
+		fs.Atime, b, _ = unmarshalUint32Safe(b)
+		fs.Mtime, b, _ = unmarshalUint32Safe(b)
+	}
+	if flags&sshFileXferAttrExtended == sshFileXferAttrExtended {
+		var count uint32
+		count, b, _ = unmarshalUint32Safe(b)
+		ext := make([]StatExtended, count)
+		for i := uint32(0); i < count; i++ {
+			var typ string
+			var data string
+			typ, b, _ = unmarshalStringSafe(b)
+			data, b, _ = unmarshalStringSafe(b)
+			ext[i] = StatExtended{
+				ExtType: typ,
+				ExtData: data,
+			}
+		}
+		fs.Extended = ext
+	}
+	return &fs, b
+}
+
+func unmarshalStatus(id uint32, data []byte) error {
+	sid, data := unmarshalUint32(data)
+	if sid != id {
+		return &unexpectedIDErr{id, sid}
+	}
+	code, data := unmarshalUint32(data)
+	msg, data, _ := unmarshalStringSafe(data)
+	lang, _, _ := unmarshalStringSafe(data)
+	return &StatusError{
+		Code: code,
+		msg:  msg,
+		lang: lang,
+	}
+}
+
 type packetMarshaler interface {
 	marshalPacket() (header, payload []byte, err error)
 }
@@ -638,12 +739,17 @@ func (p *sshFxpReadPacket) UnmarshalBinary(b []byte) error {
 const dataHeaderLen = 4 + 1 + 4 + 4
 
 func (p *sshFxpReadPacket) getDataSlice(alloc *allocator, orderID uint32) []byte {
-	dataLen := clamp(p.Len, maxTxPacket)
+	dataLen := p.Len
+	if dataLen > maxTxPacket {
+		dataLen = maxTxPacket
+	}
+
 	if alloc != nil {
 		// GetPage returns a slice with capacity = maxMsgLength this is enough to avoid new allocations in
 		// sshFxpDataPacket.MarshalBinary
 		return alloc.GetPage(orderID)[:dataLen]
 	}
+
 	// allocate with extra space for the header
 	return make([]byte, dataLen, dataLen+dataHeaderLen)
 }
@@ -1016,6 +1122,7 @@ func (p *StatVFS) marshalPacket() ([]byte, []byte, error) {
 	return header, buf.Bytes(), err
 }
 
+// MarshalBinary encodes the StatVFS as an SSH_FXP_EXTENDED_REPLY packet.
 func (p *StatVFS) MarshalBinary() ([]byte, error) {
 	header, payload, err := p.marshalPacket()
 	return append(header, payload...), err
diff --git a/request-attrs.go b/request-attrs.go
index 7c2e5c1..b5c95b4 100644
--- a/request-attrs.go
+++ b/request-attrs.go
@@ -58,6 +58,6 @@ func (a FileStat) FileMode() os.FileMode {
 // Attributes parses file attributes byte blob and return them in a
 // FileStat object.
 func (r *Request) Attributes() *FileStat {
-	fs, _ := getFileStat(r.Flags, r.Attrs)
+	fs, _ := unmarshalFileStat(r.Flags, r.Attrs)
 	return fs
 }
diff --git a/request-interfaces.go b/request-interfaces.go
index 41e3327..c8c424c 100644
--- a/request-interfaces.go
+++ b/request-interfaces.go
@@ -95,6 +95,14 @@ type RealPathFileLister interface {
 	RealPath(string) string
 }
 
+// NameLookupFileLister is a FileLister that implmeents the LookupUsername and LookupGroupName methods.
+// If this interface is implemented, then longname ls formatting will use these to convert usernames and groupnames.
+type NameLookupFileLister interface {
+	FileLister
+	LookupUserName(string) string
+	LookupGroupName(string) string
+}
+
 // ListerAt does for file lists what io.ReaderAt does for files.
 // ListAt should return the number of entries copied and an io.EOF
 // error if at end of list. This is testable by comparing how many you
diff --git a/request-server.go b/request-server.go
index 62f5aa5..5fa828b 100644
--- a/request-server.go
+++ b/request-server.go
@@ -22,12 +22,14 @@ type Handlers struct {
 
 // RequestServer abstracts the sftp protocol with an http request-like protocol
 type RequestServer struct {
+	Handlers Handlers
+
 	*serverConn
-	Handlers        Handlers
-	pktMgr          *packetManager
-	openRequests    map[string]*Request
-	openRequestLock sync.RWMutex
-	handleCount     int
+	pktMgr *packetManager
+
+	mu           sync.RWMutex
+	handleCount  int
+	openRequests map[string]*Request
 }
 
 // A RequestServerOption is a function which applies configuration to a RequestServer.
@@ -55,9 +57,11 @@ func NewRequestServer(rwc io.ReadWriteCloser, h Handlers, options ...RequestServ
 		},
 	}
 	rs := &RequestServer{
-		serverConn:   svrConn,
-		Handlers:     h,
-		pktMgr:       newPktMgr(svrConn),
+		Handlers: h,
+
+		serverConn: svrConn,
+		pktMgr:     newPktMgr(svrConn),
+
 		openRequests: make(map[string]*Request),
 	}
 
@@ -69,13 +73,15 @@ func NewRequestServer(rwc io.ReadWriteCloser, h Handlers, options ...RequestServ
 
 // New Open packet/Request
 func (rs *RequestServer) nextRequest(r *Request) string {
-	rs.openRequestLock.Lock()
-	defer rs.openRequestLock.Unlock()
+	rs.mu.Lock()
+	defer rs.mu.Unlock()
+
 	rs.handleCount++
-	handle := strconv.Itoa(rs.handleCount)
-	r.handle = handle
-	rs.openRequests[handle] = r
-	return handle
+
+	r.handle = strconv.Itoa(rs.handleCount)
+	rs.openRequests[r.handle] = r
+
+	return r.handle
 }
 
 // Returns Request from openRequests, bool is false if it is missing.
@@ -84,20 +90,23 @@ func (rs *RequestServer) nextRequest(r *Request) string {
 // you can do different things with. What you are doing with it are denoted by
 // the first packet of that type (read/write/etc).
 func (rs *RequestServer) getRequest(handle string) (*Request, bool) {
-	rs.openRequestLock.RLock()
-	defer rs.openRequestLock.RUnlock()
+	rs.mu.RLock()
+	defer rs.mu.RUnlock()
+
 	r, ok := rs.openRequests[handle]
 	return r, ok
 }
 
 // Close the Request and clear from openRequests map
 func (rs *RequestServer) closeRequest(handle string) error {
-	rs.openRequestLock.Lock()
-	defer rs.openRequestLock.Unlock()
+	rs.mu.Lock()
+	defer rs.mu.Unlock()
+
 	if r, ok := rs.openRequests[handle]; ok {
 		delete(rs.openRequests, handle)
 		return r.close()
 	}
+
 	return EBADF
 }
 
@@ -142,8 +151,10 @@ func (rs *RequestServer) Serve() error {
 			rs.pktMgr.alloc.Free()
 		}
 	}()
+
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
+
 	var wg sync.WaitGroup
 	runWorker := func(ch chan orderedRequest) {
 		wg.Add(1)
@@ -160,8 +171,8 @@ func (rs *RequestServer) Serve() error {
 
 	wg.Wait() // wait for all workers to exit
 
-	rs.openRequestLock.Lock()
-	defer rs.openRequestLock.Unlock()
+	rs.mu.Lock()
+	defer rs.mu.Unlock()
 
 	// make sure all open requests are properly closed
 	// (eg. possible on dropped connections, client crashes, etc.)
@@ -178,9 +189,7 @@ func (rs *RequestServer) Serve() error {
 	return err
 }
 
-func (rs *RequestServer) packetWorker(
-	ctx context.Context, pktChan chan orderedRequest,
-) error {
+func (rs *RequestServer) packetWorker(ctx context.Context, pktChan chan orderedRequest) error {
 	for pkt := range pktChan {
 		orderID := pkt.orderID()
 		if epkt, ok := pkt.requestPacket.(*sshFxpExtendedPacket); ok {
diff --git a/request.go b/request.go
index d842787..c6da4b6 100644
--- a/request.go
+++ b/request.go
@@ -505,10 +505,14 @@ func filelist(h FileLister, r *Request, pkt requestPacket) responsePacket {
 
 		nameAttrs := make([]*sshFxpNameAttr, 0, len(finfo))
 
+		// If the type conversion fails, we get untyped `nil`,
+		// which is handled by not looking up any names.
+		idLookup, _ := h.(NameLookupFileLister)
+
 		for _, fi := range finfo {
 			nameAttrs = append(nameAttrs, &sshFxpNameAttr{
 				Name:     fi.Name(),
-				LongName: runLs(fi),
+				LongName: runLs(idLookup, fi),
 				Attrs:    []interface{}{fi},
 			})
 		}
diff --git a/server.go b/server.go
index a38d09b..529052b 100644
--- a/server.go
+++ b/server.go
@@ -466,11 +466,13 @@ func (p *sshFxpReaddirPacket) respond(svr *Server) responsePacket {
 		return statusFromError(p.ID, err)
 	}
 
+	idLookup := osIDLookup{}
+
 	ret := &sshFxpNamePacket{ID: p.ID}
 	for _, dirent := range dirents {
 		ret.NameAttrs = append(ret.NameAttrs, &sshFxpNameAttr{
 			Name:     dirent.Name(),
-			LongName: runLs(dirent),
+			LongName: runLs(idLookup, dirent),
 			Attrs:    []interface{}{dirent},
 		})
 	}
@@ -612,99 +614,3 @@ func statusFromError(id uint32, err error) *sshFxpStatusPacket {
 
 	return ret
 }
-
-func clamp(v, max uint32) uint32 {
-	if v > max {
-		return max
-	}
-	return v
-}
-
-func runLsTypeWord(dirent os.FileInfo) string {
-	// find first character, the type char
-	// b     Block special file.
-	// c     Character special file.
-	// d     Directory.
-	// l     Symbolic link.
-	// s     Socket link.
-	// p     FIFO.
-	// -     Regular file.
-	tc := '-'
-	mode := dirent.Mode()
-	if (mode & os.ModeDir) != 0 {
-		tc = 'd'
-	} else if (mode & os.ModeDevice) != 0 {
-		tc = 'b'
-		if (mode & os.ModeCharDevice) != 0 {
-			tc = 'c'
-		}
-	} else if (mode & os.ModeSymlink) != 0 {
-		tc = 'l'
-	} else if (mode & os.ModeSocket) != 0 {
-		tc = 's'
-	} else if (mode & os.ModeNamedPipe) != 0 {
-		tc = 'p'
-	}
-
-	// owner
-	orc := '-'
-	if (mode & 0400) != 0 {
-		orc = 'r'
-	}
-	owc := '-'
-	if (mode & 0200) != 0 {
-		owc = 'w'
-	}
-	oxc := '-'
-	ox := (mode & 0100) != 0
-	setuid := (mode & os.ModeSetuid) != 0
-	if ox && setuid {
-		oxc = 's'
-	} else if setuid {
-		oxc = 'S'
-	} else if ox {
-		oxc = 'x'
-	}
-
-	// group
-	grc := '-'
-	if (mode & 040) != 0 {
-		grc = 'r'
-	}
-	gwc := '-'
-	if (mode & 020) != 0 {
-		gwc = 'w'
-	}
-	gxc := '-'
-	gx := (mode & 010) != 0
-	setgid := (mode & os.ModeSetgid) != 0
-	if gx && setgid {
-		gxc = 's'
-	} else if setgid {
-		gxc = 'S'
-	} else if gx {
-		gxc = 'x'
-	}
-
-	// all / others
-	arc := '-'
-	if (mode & 04) != 0 {
-		arc = 'r'
-	}
-	awc := '-'
-	if (mode & 02) != 0 {
-		awc = 'w'
-	}
-	axc := '-'
-	ax := (mode & 01) != 0
-	sticky := (mode & os.ModeSticky) != 0
-	if ax && sticky {
-		axc = 't'
-	} else if sticky {
-		axc = 'T'
-	} else if ax {
-		axc = 'x'
-	}
-
-	return fmt.Sprintf("%c%c%c%c%c%c%c%c%c%c", tc, orc, owc, oxc, grc, gwc, gxc, arc, awc, axc)
-}
diff --git a/server_stubs.go b/server_stubs.go
deleted file mode 100644
index 84a7d7f..0000000
--- a/server_stubs.go
+++ /dev/null
@@ -1,32 +0,0 @@
-// +build !cgo plan9 windows android
-
-package sftp
-
-import (
-	"fmt"
-	"os"
-	"time"
-)
-
-func runLs(dirent os.FileInfo) string {
-	typeword := runLsTypeWord(dirent)
-	numLinks := 1
-	if dirent.IsDir() {
-		numLinks = 0
-	}
-	username := "root"
-	groupname := "root"
-	mtime := dirent.ModTime()
-	monthStr := mtime.Month().String()[0:3]
-	day := mtime.Day()
-	year := mtime.Year()
-	now := time.Now()
-	isOld := mtime.Before(now.Add(-time.Hour * 24 * 365 / 2))
-
-	yearOrTime := fmt.Sprintf("%02d:%02d", mtime.Hour(), mtime.Minute())
-	if isOld {
-		yearOrTime = fmt.Sprintf("%d", year)
-	}
-
-	return fmt.Sprintf("%s %4d %-8s %-8s %8d %s %2d %5s %s", typeword, numLinks, username, groupname, dirent.Size(), monthStr, day, yearOrTime, dirent.Name())
-}
diff --git a/server_unix.go b/server_unix.go
deleted file mode 100644
index 9ba54b6..0000000
--- a/server_unix.go
+++ /dev/null
@@ -1,55 +0,0 @@
-// +build darwin dragonfly freebsd !android,linux netbsd openbsd solaris aix
-// +build cgo
-
-package sftp
-
-import (
-	"fmt"
-	"os"
-	"syscall"
-	"time"
-)
-
-// ls -l style output for a file, which is in the 'long output' section of a readdir response packet
-// this is a very simple (lazy) implementation, just enough to look almost like openssh in a few basic cases
-func runLs(dirent os.FileInfo) string {
-	// example from openssh sftp server:
-	// crw-rw-rw-    1 root     wheel           0 Jul 31 20:52 ttyvd
-	// format:
-	// {directory / char device / etc}{rwxrwxrwx}  {number of links} owner group size month day [time (this year) | year (otherwise)] name
-
-	typeword := runLsTypeWord(dirent)
-
-	var numLinks uint64 = 1
-	if dirent.IsDir() {
-		numLinks = 0
-	}
-
-	var uid, gid uint32
-
-	if statt, ok := dirent.Sys().(*syscall.Stat_t); ok {
-		// The type of Nlink varies form int16 (aix-ppc64) to uint64 (linux-amd64),
-		// we cast up to uint64 to make all OS/ARCH combos source compatible.
-		numLinks = uint64(statt.Nlink)
-		uid = statt.Uid
-		gid = statt.Gid
-	}
-
-	username := fmt.Sprintf("%d", uid)
-	groupname := fmt.Sprintf("%d", gid)
-	// TODO FIXME: uid -> username, gid -> groupname lookup for ls -l format output
-
-	mtime := dirent.ModTime()
-	monthStr := mtime.Month().String()[0:3]
-	day := mtime.Day()
-	year := mtime.Year()
-	now := time.Now()
-	isOld := mtime.Before(now.Add(-time.Hour * 24 * 365 / 2))
-
-	yearOrTime := fmt.Sprintf("%02d:%02d", mtime.Hour(), mtime.Minute())
-	if isOld {
-		yearOrTime = fmt.Sprintf("%d", year)
-	}
-
-	return fmt.Sprintf("%s %4d %-8s %-8s %8d %s %2d %5s %s", typeword, numLinks, username, groupname, dirent.Size(), monthStr, day, yearOrTime, dirent.Name())
-}
diff --git a/stat_plan9.go b/stat_plan9.go
index 52da020..761abdf 100644
--- a/stat_plan9.go
+++ b/stat_plan9.go
@@ -49,6 +49,7 @@ func isRegular(mode uint32) bool {
 // toFileMode converts sftp filemode bits to the os.FileMode specification
 func toFileMode(mode uint32) os.FileMode {
 	var fm = os.FileMode(mode & 0777)
+
 	switch mode & S_IFMT {
 	case syscall.S_IFBLK:
 		fm |= os.ModeDevice
@@ -65,37 +66,30 @@ func toFileMode(mode uint32) os.FileMode {
 	case syscall.S_IFSOCK:
 		fm |= os.ModeSocket
 	}
+
 	return fm
 }
 
 // fromFileMode converts from the os.FileMode specification to sftp filemode bits
 func fromFileMode(mode os.FileMode) uint32 {
-	ret := uint32(0)
+	ret := uint32(mode & os.ModePerm)
 
-	if mode&os.ModeDevice != 0 {
-		if mode&os.ModeCharDevice != 0 {
-			ret |= syscall.S_IFCHR
-		} else {
-			ret |= syscall.S_IFBLK
-		}
-	}
-	if mode&os.ModeDir != 0 {
+	switch mode & os.ModeType {
+	case os.ModeDevice | os.ModeCharDevice:
+		ret |= syscall.S_IFCHR
+	case os.ModeDevice:
+		ret |= syscall.S_IFBLK
+	case os.ModeDir:
 		ret |= syscall.S_IFDIR
-	}
-	if mode&os.ModeSymlink != 0 {
-		ret |= syscall.S_IFLNK
-	}
-	if mode&os.ModeNamedPipe != 0 {
+	case os.ModeNamedPipe:
 		ret |= syscall.S_IFIFO
-	}
-	if mode&os.ModeSocket != 0 {
-		ret |= syscall.S_IFSOCK
-	}
-
-	if mode&os.ModeType == 0 {
+	case os.ModeSymlink:
+		ret |= syscall.S_IFLNK
+	case 0:
 		ret |= syscall.S_IFREG
+	case os.ModeSocket:
+		ret |= syscall.S_IFSOCK
 	}
-	ret |= uint32(mode & os.ModePerm)
 
 	return ret
 }
diff --git a/stat_posix.go b/stat_posix.go
index 98b60e7..92f76e2 100644
--- a/stat_posix.go
+++ b/stat_posix.go
@@ -51,6 +51,7 @@ func isRegular(mode uint32) bool {
 // toFileMode converts sftp filemode bits to the os.FileMode specification
 func toFileMode(mode uint32) os.FileMode {
 	var fm = os.FileMode(mode & 0777)
+
 	switch mode & S_IFMT {
 	case syscall.S_IFBLK:
 		fm |= os.ModeDevice
@@ -67,55 +68,50 @@ func toFileMode(mode uint32) os.FileMode {
 	case syscall.S_IFSOCK:
 		fm |= os.ModeSocket
 	}
-	if mode&syscall.S_ISGID != 0 {
-		fm |= os.ModeSetgid
-	}
+
 	if mode&syscall.S_ISUID != 0 {
 		fm |= os.ModeSetuid
 	}
+	if mode&syscall.S_ISGID != 0 {
+		fm |= os.ModeSetgid
+	}
 	if mode&syscall.S_ISVTX != 0 {
 		fm |= os.ModeSticky
 	}
+
 	return fm
 }
 
 // fromFileMode converts from the os.FileMode specification to sftp filemode bits
 func fromFileMode(mode os.FileMode) uint32 {
-	ret := uint32(0)
+	ret := uint32(mode & os.ModePerm)
 
-	if mode&os.ModeDevice != 0 {
-		if mode&os.ModeCharDevice != 0 {
-			ret |= syscall.S_IFCHR
-		} else {
-			ret |= syscall.S_IFBLK
-		}
-	}
-	if mode&os.ModeDir != 0 {
+	switch mode & os.ModeType {
+	case os.ModeDevice | os.ModeCharDevice:
+		ret |= syscall.S_IFCHR
+	case os.ModeDevice:
+		ret |= syscall.S_IFBLK
+	case os.ModeDir:
 		ret |= syscall.S_IFDIR
-	}
-	if mode&os.ModeSymlink != 0 {
+	case os.ModeNamedPipe:
+		ret |= syscall.S_IFIFO
+	case os.ModeSymlink:
 		ret |= syscall.S_IFLNK
+	case 0:
+		ret |= syscall.S_IFREG
+	case os.ModeSocket:
+		ret |= syscall.S_IFSOCK
 	}
-	if mode&os.ModeNamedPipe != 0 {
-		ret |= syscall.S_IFIFO
+
+	if mode&os.ModeSetuid != 0 {
+		ret |= syscall.S_ISUID
 	}
 	if mode&os.ModeSetgid != 0 {
 		ret |= syscall.S_ISGID
 	}
-	if mode&os.ModeSetuid != 0 {
-		ret |= syscall.S_ISUID
-	}
 	if mode&os.ModeSticky != 0 {
 		ret |= syscall.S_ISVTX
 	}
-	if mode&os.ModeSocket != 0 {
-		ret |= syscall.S_IFSOCK
-	}
-
-	if mode&os.ModeType == 0 {
-		ret |= syscall.S_IFREG
-	}
-	ret |= uint32(mode & os.ModePerm)
 
 	return ret
 }


Test patch:
diff --git a/attrs_test.go b/attrs_test.go
index 18d4f5c..a755df6 100644
--- a/attrs_test.go
+++ b/attrs_test.go
@@ -1,45 +1,8 @@
 package sftp
 
 import (
-	"bytes"
 	"os"
-	"reflect"
-	"testing"
-	"time"
 )
 
 // ensure that attrs implemenst os.FileInfo
 var _ os.FileInfo = new(fileInfo)
-
-var unmarshalAttrsTests = []struct {
-	b    []byte
-	want *fileInfo
-	rest []byte
-}{
-	{marshal(nil, struct{ Flags uint32 }{}), &fileInfo{mtime: time.Unix(int64(0), 0)}, nil},
-	{marshal(nil, struct {
-		Flags uint32
-		Size  uint64
-	}{sshFileXferAttrSize, 20}), &fileInfo{size: 20, mtime: time.Unix(int64(0), 0)}, nil},
-	{marshal(nil, struct {
-		Flags       uint32
-		Size        uint64
-		Permissions uint32
-	}{sshFileXferAttrSize | sshFileXferAttrPermissions, 20, 0644}), &fileInfo{size: 20, mode: os.FileMode(0644), mtime: time.Unix(int64(0), 0)}, nil},
-	{marshal(nil, struct {
-		Flags                 uint32
-		Size                  uint64
-		UID, GID, Permissions uint32
-	}{sshFileXferAttrSize | sshFileXferAttrUIDGID | sshFileXferAttrUIDGID | sshFileXferAttrPermissions, 20, 1000, 1000, 0644}), &fileInfo{size: 20, mode: os.FileMode(0644), mtime: time.Unix(int64(0), 0)}, nil},
-}
-
-func TestUnmarshalAttrs(t *testing.T) {
-	for _, tt := range unmarshalAttrsTests {
-		stat, rest := unmarshalAttrs(tt.b)
-		got := fileInfoFromStat(stat, "")
-		tt.want.sys = got.Sys()
-		if !reflect.DeepEqual(got, tt.want) || !bytes.Equal(tt.rest, rest) {
-			t.Errorf("unmarshalAttrs(%#v): want %#v, %#v, got: %#v, %#v", tt.b, tt.want, tt.rest, got, rest)
-		}
-	}
-}
diff --git a/client_integration_test.go b/client_integration_test.go
index 59fdb83..54615ab 100644
--- a/client_integration_test.go
+++ b/client_integration_test.go
@@ -2313,7 +2313,12 @@ func benchmarkWrite(b *testing.B, bufsize int, delay time.Duration) {
 		}
 
 		for offset < size {
-			n, err := f2.Write(data[offset:min(len(data), offset+bufsize)])
+			buf := data[offset:]
+			if len(buf) > bufsize {
+				buf = buf[:bufsize]
+			}
+
+			n, err := f2.Write(buf)
 			if err != nil {
 				b.Fatal(err)
 			}
diff --git a/client_test.go b/client_test.go
index 8a8c51b..4577ca2 100644
--- a/client_test.go
+++ b/client_test.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"io"
 	"os"
-	"reflect"
 	"testing"
 
 	"github.com/kr/fs"
@@ -89,64 +88,6 @@ func TestFlags(t *testing.T) {
 	}
 }
 
-func TestUnmarshalStatus(t *testing.T) {
-	requestID := uint32(1)
-
-	id := marshalUint32([]byte{}, requestID)
-	idCode := marshalUint32(id, sshFxFailure)
-	idCodeMsg := marshalString(idCode, "err msg")
-	idCodeMsgLang := marshalString(idCodeMsg, "lang tag")
-
-	var tests = []struct {
-		desc   string
-		reqID  uint32
-		status []byte
-		want   error
-	}{
-		{
-			desc:   "well-formed status",
-			reqID:  1,
-			status: idCodeMsgLang,
-			want: &StatusError{
-				Code: sshFxFailure,
-				msg:  "err msg",
-				lang: "lang tag",
-			},
-		},
-		{
-			desc:   "missing error message and language tag",
-			reqID:  1,
-			status: idCode,
-			want: &StatusError{
-				Code: sshFxFailure,
-			},
-		},
-		{
-			desc:   "missing language tag",
-			reqID:  1,
-			status: idCodeMsg,
-			want: &StatusError{
-				Code: sshFxFailure,
-				msg:  "err msg",
-			},
-		},
-		{
-			desc:   "request identifier mismatch",
-			reqID:  2,
-			status: idCodeMsgLang,
-			want:   &unexpectedIDErr{2, requestID},
-		},
-	}
-
-	for _, tt := range tests {
-		got := unmarshalStatus(tt.reqID, tt.status)
-		if !reflect.DeepEqual(got, tt.want) {
-			t.Errorf("unmarshalStatus(%v, %v), test %q\n- want: %#v\n-  got: %#v",
-				requestID, tt.status, tt.desc, tt.want, got)
-		}
-	}
-}
-
 type packetSizeTest struct {
 	size  int
 	valid bool
diff --git a/ls_formatting_test.go b/ls_formatting_test.go
new file mode 100644
index 0000000..c9c63a1
--- /dev/null
+++ b/ls_formatting_test.go
@@ -0,0 +1,172 @@
+package sftp
+
+import (
+	"os"
+	"regexp"
+	"strings"
+	"testing"
+	"time"
+)
+
+const (
+	typeDirectory = "d"
+	typeFile      = "[^d]"
+)
+
+func TestRunLsWithExamplesDirectory(t *testing.T) {
+	path := "examples"
+	item, _ := os.Stat(path)
+	result := runLs(nil, item)
+	runLsTestHelper(t, result, typeDirectory, path)
+}
+
+func TestRunLsWithLicensesFile(t *testing.T) {
+	path := "LICENSE"
+	item, _ := os.Stat(path)
+	result := runLs(nil, item)
+	runLsTestHelper(t, result, typeFile, path)
+}
+
+func TestRunLsWithExamplesDirectoryWithOSLookup(t *testing.T) {
+	path := "examples"
+	item, _ := os.Stat(path)
+	result := runLs(osIDLookup{}, item)
+	runLsTestHelper(t, result, typeDirectory, path)
+}
+
+func TestRunLsWithLicensesFileWithOSLookup(t *testing.T) {
+	path := "LICENSE"
+	item, _ := os.Stat(path)
+	result := runLs(osIDLookup{}, item)
+	runLsTestHelper(t, result, typeFile, path)
+}
+
+/*
+   The format of the `longname' field is unspecified by this protocol.
+   It MUST be suitable for use in the output of a directory listing
+   command (in fact, the recommended operation for a directory listing
+   command is to simply display this data).  However, clients SHOULD NOT
+   attempt to parse the longname field for file attributes; they SHOULD
+   use the attrs field instead.
+
+    The recommended format for the longname field is as follows:
+
+        -rwxr-xr-x   1 mjos     staff      348911 Mar 25 14:29 t-filexfer
+        1234567890 123 12345678 12345678 12345678 123456789012
+
+   Here, the first line is sample output, and the second field indicates
+   widths of the various fields.  Fields are separated by spaces.  The
+   first field lists file permissions for user, group, and others; the
+   second field is link count; the third field is the name of the user
+   who owns the file; the fourth field is the name of the group that
+   owns the file; the fifth field is the size of the file in bytes; the
+   sixth field (which actually may contain spaces, but is fixed to 12
+   characters) is the file modification time, and the seventh field is
+   the file name.  Each field is specified to be a minimum of certain
+   number of character positions (indicated by the second line above),
+   but may also be longer if the data does not fit in the specified
+   length.
+
+    The SSH_FXP_ATTRS response has the following format:
+
+        uint32     id
+        ATTRS      attrs
+
+   where `id' is the request identifier, and `attrs' is the returned
+   file attributes as described in Section ``File Attributes''.
+
+   N.B.: FileZilla does parse this ls formatting, and so not rendering it
+   on any particular GOOS/GOARCH can cause compatibility issues with this client.
+*/
+func runLsTestHelper(t *testing.T, result, expectedType, path string) {
+	// using regular expressions to make tests work on all systems
+	// a virtual file system (like afero) would be needed to mock valid filesystem checks
+	// expected layout is:
+	// drwxr-xr-x   8 501      20            272 Aug  9 19:46 examples
+
+	t.Log(result)
+
+	sparce := strings.Split(result, " ")
+
+	var fields []string
+	for _, field := range sparce {
+		if field == "" {
+			continue
+		}
+
+		fields = append(fields, field)
+	}
+
+	perms, linkCnt, user, group, size := fields[0], fields[1], fields[2], fields[3], fields[4]
+	dateTime := strings.Join(fields[5:8], " ")
+	filename := fields[8]
+
+	// permissions (len 10, "drwxr-xr-x")
+	const (
+		rwxs = "[-r][-w][-xsS]"
+		rwxt = "[-r][-w][-xtT]"
+	)
+	if ok, err := regexp.MatchString("^"+expectedType+rwxs+rwxs+rwxt+"$", perms); !ok {
+		if err != nil {
+			t.Fatal("unexpected error:", err)
+		}
+
+		t.Errorf("runLs(%q): permission field mismatch, expected dir, got: %#v, err: %#v", path, perms, err)
+	}
+
+	// link count (len 3, number)
+	const (
+		number = "(?:[0-9]+)"
+	)
+	if ok, err := regexp.MatchString("^"+number+"$", linkCnt); !ok {
+		if err != nil {
+			t.Fatal("unexpected error:", err)
+		}
+
+		t.Errorf("runLs(%q): link count field mismatch, got: %#v, err: %#v", path, linkCnt, err)
+	}
+
+	// username / uid (len 8, number or string)
+	const (
+		name = "(?:[a-z_][a-z0-9_]*)"
+	)
+	if ok, err := regexp.MatchString("^(?:"+number+"|"+name+")+$", user); !ok {
+		if err != nil {
+			t.Fatal("unexpected error:", err)
+		}
+
+		t.Errorf("runLs(%q): username / uid mismatch, expected user, got: %#v, err: %#v", path, user, err)
+	}
+
+	// groupname / gid (len 8, number or string)
+	if ok, err := regexp.MatchString("^(?:"+number+"|"+name+")+$", group); !ok {
+		if err != nil {
+			t.Fatal("unexpected error:", err)
+		}
+
+		t.Errorf("runLs(%q): groupname / gid mismatch, expected group, got: %#v, err: %#v", path, group, err)
+	}
+
+	// filesize (len 8)
+	if ok, err := regexp.MatchString("^"+number+"$", size); !ok {
+		if err != nil {
+			t.Fatal("unexpected error:", err)
+		}
+
+		t.Errorf("runLs(%q): filesize field mismatch, expected size in bytes, got: %#v, err: %#v", path, size, err)
+	}
+
+	// mod time (len 12, e.g. Aug  9 19:46)
+	_, err := time.Parse("Jan 2 15:04", dateTime)
+	if err != nil {
+		_, err = time.Parse("Jan 2 2006", dateTime)
+		if err != nil {
+			t.Errorf("runLs.dateTime = %#v should match `Jan 2 15:04` or `Jan 2 2006`: %+v", dateTime, err)
+		}
+	}
+
+	// filename
+	if path != filename {
+		t.Errorf("runLs.filename = %#v, expected: %#v", filename, path)
+	}
+}
diff --git a/packet_test.go b/packet_test.go
index c7deb5a..cbee5e4 100644
--- a/packet_test.go
+++ b/packet_test.go
@@ -6,6 +6,7 @@ import (
 	"errors"
 	"io/ioutil"
 	"os"
+	"reflect"
 	"testing"
 )
 
@@ -217,6 +218,138 @@ func TestUnmarshalString(t *testing.T) {
 	}
 }
 
+func TestUnmarshalAttrs(t *testing.T) {
+	var tests = []struct {
+		b    []byte
+		want *FileStat
+	}{
+		{
+			b:    []byte{0x00, 0x00, 0x00, 0x00},
+			want: &FileStat{},
+		},
+		{
+			b: []byte{
+				0x00, 0x00, 0x00, byte(sshFileXferAttrSize),
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 20,
+			},
+			want: &FileStat{
+				Size: 20,
+			},
+		},
+		{
+			b: []byte{
+				0x00, 0x00, 0x00, byte(sshFileXferAttrSize | sshFileXferAttrPermissions),
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 20,
+				0x00, 0x00, 0x01, 0xA4,
+			},
+			want: &FileStat{
+				Size: 20,
+				Mode: 0644,
+			},
+		},
+		{
+			b: []byte{
+				0x00, 0x00, 0x00, byte(sshFileXferAttrSize | sshFileXferAttrPermissions | sshFileXferAttrUIDGID),
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 20,
+				0x00, 0x00, 0x03, 0xE8,
+				0x00, 0x00, 0x03, 0xE9,
+				0x00, 0x00, 0x01, 0xA4,
+			},
+			want: &FileStat{
+				Size: 20,
+				Mode: 0644,
+				UID:  1000,
+				GID:  1001,
+			},
+		},
+		{
+			b: []byte{
+				0x00, 0x00, 0x00, byte(sshFileXferAttrSize | sshFileXferAttrPermissions | sshFileXferAttrUIDGID | sshFileXferAttrACmodTime),
+				0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 20,
+				0x00, 0x00, 0x03, 0xE8,
+				0x00, 0x00, 0x03, 0xE9,
+				0x00, 0x00, 0x01, 0xA4,
+				0x00, 0x00, 0x00, 42,
+				0x00, 0x00, 0x00, 13,
+			},
+			want: &FileStat{
+				Size:  20,
+				Mode:  0644,
+				UID:   1000,
+				GID:   1001,
+				Atime: 42,
+				Mtime: 13,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		got, _ := unmarshalAttrs(tt.b)
+		if !reflect.DeepEqual(got, tt.want) {
+			t.Errorf("unmarshalAttrs(% X):\n-  got: %#v\n- want: %#v", tt.b, got, tt.want)
+		}
+	}
+}
+
+func TestUnmarshalStatus(t *testing.T) {
+	var requestID uint32 = 1
+
+	id := marshalUint32(nil, requestID)
+	idCode := marshalUint32(id, sshFxFailure)
+	idCodeMsg := marshalString(idCode, "err msg")
+	idCodeMsgLang := marshalString(idCodeMsg, "lang tag")
+
+	var tests = []struct {
+		desc   string
+		reqID  uint32
+		status []byte
+		want   error
+	}{
+		{
+			desc:   "well-formed status",
+			status: idCodeMsgLang,
+			want: &StatusError{
+				Code: sshFxFailure,
+				msg:  "err msg",
+				lang: "lang tag",
+			},
+		},
+		{
+			desc:   "missing language tag",
+			status: idCodeMsg,
+			want: &StatusError{
+				Code: sshFxFailure,
+				msg:  "err msg",
+			},
+		},
+		{
+			desc:   "missing error message and language tag",
+			status: idCode,
+			want: &StatusError{
+				Code: sshFxFailure,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			got := unmarshalStatus(1, tt.status)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("unmarshalStatus(1, % X):\n-  got: %#v\n- want: %#v", tt.status, got, tt.want)
+			}
+		})
+	}
+
+	got := unmarshalStatus(2, idCodeMsgLang)
+	want := &unexpectedIDErr{
+		want: 2,
+		got:  1,
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("unmarshalStatus(2, % X):\n-  got: %#v\n- want: %#v", idCodeMsgLang, got, want)
+	}
+}
+
 func TestSendPacket(t *testing.T) {
 	var tests = []struct {
 		packet encoding.BinaryMarshaler
diff --git a/request-attrs_test.go b/request-attrs_test.go
index 423a17c..3e1b096 100644
--- a/request-attrs_test.go
+++ b/request-attrs_test.go
@@ -33,7 +33,7 @@ func TestRequestAttributes(t *testing.T) {
 	at := []byte{}
 	at = marshalUint32(at, 1)
 	at = marshalUint32(at, 2)
-	testFs, _ := getFileStat(fl, at)
+	testFs, _ := unmarshalFileStat(fl, at)
 	assert.Equal(t, fa, *testFs)
 	// Size and Mode
 	fa = FileStat{Mode: 700, Size: 99}
@@ -41,7 +41,7 @@ func TestRequestAttributes(t *testing.T) {
 	at = []byte{}
 	at = marshalUint64(at, 99)
 	at = marshalUint32(at, 700)
-	testFs, _ = getFileStat(fl, at)
+	testFs, _ = unmarshalFileStat(fl, at)
 	assert.Equal(t, fa, *testFs)
 	// FileMode
 	assert.True(t, testFs.FileMode().IsRegular())
@@ -50,7 +50,7 @@ func TestRequestAttributes(t *testing.T) {
 }
 
 func TestRequestAttributesEmpty(t *testing.T) {
-	fs, b := getFileStat(sshFileXferAttrAll, nil)
+	fs, b := unmarshalFileStat(sshFileXferAttrAll, nil)
 	assert.Equal(t, &FileStat{
 		Extended: []StatExtended{},
 	}, fs)
diff --git a/request-server_test.go b/request-server_test.go
index 62cf311..bac7f64 100644
--- a/request-server_test.go
+++ b/request-server_test.go
@@ -440,9 +440,9 @@ func TestRequestStat(t *testing.T) {
 	require.NoError(t, err)
 	fi, err := p.cli.Stat("/foo")
 	require.NoError(t, err)
-	assert.Equal(t, fi.Name(), "foo")
-	assert.Equal(t, fi.Size(), int64(5))
-	assert.Equal(t, fi.Mode(), os.FileMode(0644))
+	assert.Equal(t, "foo", fi.Name())
+	assert.Equal(t, int64(5), fi.Size())
+	assert.Equal(t, os.FileMode(0644), fi.Mode())
 	assert.NoError(t, testOsSys(fi.Sys()))
 	checkRequestServerAllocator(t, p)
 }
@@ -459,9 +459,9 @@ func TestRequestSetstat(t *testing.T) {
 	require.NoError(t, err)
 	fi, err := p.cli.Stat("/foo")
 	require.NoError(t, err)
-	assert.Equal(t, fi.Name(), "foo")
-	assert.Equal(t, fi.Size(), int64(5))
-	assert.Equal(t, fi.Mode(), os.FileMode(0644))
+	assert.Equal(t, "foo", fi.Name())
+	assert.Equal(t, int64(5), fi.Size())
+	assert.Equal(t, os.FileMode(0644), fi.Mode())
 	assert.NoError(t, testOsSys(fi.Sys()))
 	checkRequestServerAllocator(t, p)
 }
@@ -475,9 +475,9 @@ func TestRequestFstat(t *testing.T) {
 	require.NoError(t, err)
 	fi, err := fp.Stat()
 	require.NoError(t, err)
-	assert.Equal(t, fi.Name(), "foo")
-	assert.Equal(t, fi.Size(), int64(5))
-	assert.Equal(t, fi.Mode(), os.FileMode(0644))
+	assert.Equal(t, "foo", fi.Name())
+	assert.Equal(t, int64(5), fi.Size())
+	assert.Equal(t, os.FileMode(0644), fi.Mode())
 	assert.NoError(t, testOsSys(fi.Sys()))
 	checkRequestServerAllocator(t, p)
 }
diff --git a/server_test.go b/server_test.go
index 74da0c6..ae61eec 100644
--- a/server_test.go
+++ b/server_test.go
@@ -6,162 +6,15 @@ import (
 	"io"
 	"os"
 	"path"
-	"regexp"
 	"runtime"
 	"sync"
 	"syscall"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-const (
-	typeDirectory = "d"
-	typeFile      = "[^d]"
-)
-
-func TestRunLsWithExamplesDirectory(t *testing.T) {
-	path := "examples"
-	item, _ := os.Stat(path)
-	result := runLs(item)
-	runLsTestHelper(t, result, typeDirectory, path)
-}
-
-func TestRunLsWithLicensesFile(t *testing.T) {
-	path := "LICENSE"
-	item, _ := os.Stat(path)
-	result := runLs(item)
-	runLsTestHelper(t, result, typeFile, path)
-}
-
-/*
-   The format of the `longname' field is unspecified by this protocol.
-   It MUST be suitable for use in the output of a directory listing
-   command (in fact, the recommended operation for a directory listing
-   command is to simply display this data).  However, clients SHOULD NOT
-   attempt to parse the longname field for file attributes; they SHOULD
-   use the attrs field instead.
-
-    The recommended format for the longname field is as follows:
-
-        -rwxr-xr-x   1 mjos     staff      348911 Mar 25 14:29 t-filexfer
-        1234567890 123 12345678 12345678 12345678 123456789012
-
-   Here, the first line is sample output, and the second field indicates
-   widths of the various fields.  Fields are separated by spaces.  The
-   first field lists file permissions for user, group, and others; the
-   second field is link count; the third field is the name of the user
-   who owns the file; the fourth field is the name of the group that
-   owns the file; the fifth field is the size of the file in bytes; the
-   sixth field (which actually may contain spaces, but is fixed to 12
-   characters) is the file modification time, and the seventh field is
-   the file name.  Each field is specified to be a minimum of certain
-   number of character positions (indicated by the second line above),
-   but may also be longer if the data does not fit in the specified
-   length.
-
-    The SSH_FXP_ATTRS response has the following format:
-
-        uint32     id
-        ATTRS      attrs
-
-   where `id' is the request identifier, and `attrs' is the returned
-   file attributes as described in Section ``File Attributes''.
-*/
-func runLsTestHelper(t *testing.T, result, expectedType, path string) {
-	// using regular expressions to make tests work on all systems
-	// a virtual file system (like afero) would be needed to mock valid filesystem checks
-	// expected layout is:
-	// drwxr-xr-x   8 501      20            272 Aug  9 19:46 examples
-
-	// permissions (len 10, "drwxr-xr-x")
-	got := result[0:10]
-	if ok, err := regexp.MatchString("^"+expectedType+"[rwx-]{9}$", got); !ok {
-		t.Errorf("runLs(*FileInfo): permission field mismatch, expected dir, got: %#v, err: %#v", got, err)
-	}
-
-	// space
-	got = result[10:11]
-	if ok, err := regexp.MatchString("^\\s$", got); !ok {
-		t.Errorf("runLs(*FileInfo): spacer 1 mismatch, expected whitespace, got: %#v, err: %#v", got, err)
-	}
-
-	// link count (len 3, number)
-	got = result[12:15]
-	if ok, err := regexp.MatchString("^\\s*[0-9]+$", got); !ok {
-		t.Errorf("runLs(*FileInfo): link count field mismatch, got: %#v, err: %#v", got, err)
-	}
-
-	// spacer
-	got = result[15:16]
-	if ok, err := regexp.MatchString("^\\s$", got); !ok {
-		t.Errorf("runLs(*FileInfo): spacer 2 mismatch, expected whitespace, got: %#v, err: %#v", got, err)
-	}
-
-	// username / uid (len 8, number or string)
-	got = result[16:24]
-	if ok, err := regexp.MatchString("^[^\\s]{1,8}\\s*$", got); !ok {
-		t.Errorf("runLs(*FileInfo): username / uid mismatch, expected user, got: %#v, err: %#v", got, err)
-	}
-
-	// spacer
-	got = result[24:25]
-	if ok, err := regexp.MatchString("^\\s$", got); !ok {
-		t.Errorf("runLs(*FileInfo): spacer 3 mismatch, expected whitespace, got: %#v, err: %#v", got, err)
-	}
-
-	// groupname / gid (len 8, number or string)
-	got = result[25:33]
-	if ok, err := regexp.MatchString("^[^\\s]{1,8}\\s*$", got); !ok {
-		t.Errorf("runLs(*FileInfo): groupname / gid mismatch, expected group, got: %#v, err: %#v", got, err)
-	}
-
-	// spacer
-	got = result[33:34]
-	if ok, err := regexp.MatchString("^\\s$", got); !ok {
-		t.Errorf("runLs(*FileInfo): spacer 4 mismatch, expected whitespace, got: %#v, err: %#v", got, err)
-	}
-
-	// filesize (len 8)
-	got = result[34:42]
-	if ok, err := regexp.MatchString("^\\s*[0-9]+$", got); !ok {
-		t.Errorf("runLs(*FileInfo): filesize field mismatch, expected size in bytes, got: %#v, err: %#v", got, err)
-	}
-
-	// spacer
-	got = result[42:43]
-	if ok, err := regexp.MatchString("^\\s$", got); !ok {
-		t.Errorf("runLs(*FileInfo): spacer 5 mismatch, expected whitespace, got: %#v, err: %#v", got, err)
-	}
-
-	// mod time (len 12, e.g. Aug  9 19:46)
-	got = result[43:55]
-	layout := "Jan  2 15:04"
-	_, err := time.Parse(layout, got)
-
-	if err != nil {
-		layout = "Jan  2 2006"
-		_, err = time.Parse(layout, got)
-	}
-	if err != nil {
-		t.Errorf("runLs(*FileInfo): mod time field mismatch, expected date layout %s, got: %#v, err: %#v", layout, got, err)
-	}
-
-	// spacer
-	got = result[55:56]
-	if ok, err := regexp.MatchString("^\\s$", got); !ok {
-		t.Errorf("runLs(*FileInfo): spacer 6 mismatch, expected whitespace, got: %#v, err: %#v", got, err)
-	}
-
-	// filename
-	got = result[56:]
-	if ok, err := regexp.MatchString("^"+path+"$", got); !ok {
-		t.Errorf("runLs(*FileInfo): name field mismatch, expected examples, got: %#v, err: %#v", got, err)
-	}
-}
-
 func clientServerPair(t *testing.T) (*Client, *Server) {
 	cr, sw := io.Pipe()
 	sr, cw := io.Pipe()