Repository: pkg/sftp
Pull request #422 (2021-03-20 10:51:47)

Minor cleanup
Minor stuff that should not get in the way of major refactors.


Patch:
diff --git a/.gitignore b/.gitignore
index caf2dca..e1ec837 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,6 +5,3 @@ server_standalone/server_standalone
 
 examples/*/id_rsa
 examples/*/id_rsa.pub
-
-memprofile.out
-memprofile.svg
diff --git a/Makefile b/Makefile
index 4d3a007..0afad58 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,3 @@
-.PHONY: integration integration_w_race benchmark
-
 integration:
 	go test -integration -v ./...
 	go test -testserver -v ./...
@@ -16,12 +14,4 @@ integration_w_race:
 	go test -race -testserver -allocator -v ./...
 	go test -race -integration -allocator -testserver -v ./...
 
-COUNT ?= 1
-BENCHMARK_PATTERN ?= "."
-
-benchmark:
-	go test -integration -run=NONE -bench=$(BENCHMARK_PATTERN) -benchmem -count=$(COUNT)
 
-benchmark_w_memprofile:
-	go test -integration -run=NONE -bench=$(BENCHMARK_PATTERN) -benchmem -count=$(COUNT) -memprofile memprofile.out
-	go tool pprof -svg -output=memprofile.svg memprofile.out
diff --git a/attrs.go b/attrs.go
index 9097a4d..7020d3a 100644
--- a/attrs.go
+++ b/attrs.go
@@ -13,10 +13,10 @@ const (
 	sshFileXferAttrUIDGID      = 0x00000002
 	sshFileXferAttrPermissions = 0x00000004
 	sshFileXferAttrACmodTime   = 0x00000008
-	sshFileXferAttrExtented    = 0x80000000
+	sshFileXferAttrExtended    = 0x80000000
 
 	sshFileXferAttrAll = sshFileXferAttrSize | sshFileXferAttrUIDGID | sshFileXferAttrPermissions |
-		sshFileXferAttrACmodTime | sshFileXferAttrExtented
+		sshFileXferAttrACmodTime | sshFileXferAttrExtended
 )
 
 // fileInfo is an artificial type designed to satisfy os.FileInfo.
@@ -119,7 +119,7 @@ func getFileStat(flags uint32, b []byte) (*FileStat, []byte) {
 		fs.Atime, b, _ = unmarshalUint32Safe(b)
 		fs.Mtime, b, _ = unmarshalUint32Safe(b)
 	}
-	if flags&sshFileXferAttrExtented == sshFileXferAttrExtented {
+	if flags&sshFileXferAttrExtended == sshFileXferAttrExtended {
 		var count uint32
 		count, b, _ = unmarshalUint32Safe(b)
 		ext := make([]StatExtended, count)
diff --git a/client.go b/client.go
index de48d19..fee409f 100644
--- a/client.go
+++ b/client.go
@@ -43,10 +43,10 @@ type ClientOption func(*Client) error
 func MaxPacketChecked(size int) ClientOption {
 	return func(c *Client) error {
 		if size < 1 {
-			return errors.New("size must be greater or equal to 1")
+			return errors.Errorf("size must be greater or equal to 1")
 		}
 		if size > 32768 {
-			return errors.New("sizes larger than 32KB might not work with all servers")
+			return errors.Errorf("sizes larger than 32KB might not work with all servers")
 		}
 		c.maxPacket = size
 		return nil
@@ -65,7 +65,7 @@ func MaxPacketChecked(size int) ClientOption {
 func MaxPacketUnchecked(size int) ClientOption {
 	return func(c *Client) error {
 		if size < 1 {
-			return errors.New("size must be greater or equal to 1")
+			return errors.Errorf("size must be greater or equal to 1")
 		}
 		c.maxPacket = size
 		return nil
@@ -90,7 +90,7 @@ func MaxPacket(size int) ClientOption {
 func MaxConcurrentRequestsPerFile(n int) ClientOption {
 	return func(c *Client) error {
 		if n < 1 {
-			return errors.New("n must be greater or equal to 1")
+			return errors.Errorf("n must be greater or equal to 1")
 		}
 		c.maxConcurrentRequests = n
 		return nil
diff --git a/conn.go b/conn.go
index de08a63..952a2be 100644
--- a/conn.go
+++ b/conn.go
@@ -16,6 +16,8 @@ type conn struct {
 	// this is the same allocator used in packet manager
 	alloc      *allocator
 	sync.Mutex // used to serialise writes to sendPacket
+	// sendPacketTest is needed to replicate packet issues in testing
+	sendPacketTest func(w io.Writer, m encoding.BinaryMarshaler) error
 }
 
 // the orderID is used in server mode if the allocator is enabled.
@@ -27,7 +29,9 @@ func (c *conn) recvPacket(orderID uint32) (uint8, []byte, error) {
 func (c *conn) sendPacket(m encoding.BinaryMarshaler) error {
 	c.Lock()
 	defer c.Unlock()
-
+	if c.sendPacketTest != nil {
+		return c.sendPacketTest(c, m)
+	}
 	return sendPacket(c, m)
 }
 
@@ -87,7 +91,7 @@ func (c *clientConn) recv() error {
 			// This is an unexpected occurrence. Send the error
 			// back to all listeners so that they terminate
 			// gracefully.
-			return errors.Errorf("sid not found: %d", sid)
+			return errors.Errorf("sid not found: %v", sid)
 		}
 
 		ch <- result{typ: typ, data: data}
diff --git a/go.sum b/go.sum
index c756602..413dc5f 100644
--- a/go.sum
+++ b/go.sum
@@ -15,12 +15,9 @@ golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWP
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=
 golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/term v0.0.0-20201117132131-f5c789dd3221 h1:/ZHdbVpdR/jk3g30/d4yUL0JU9kksj8+F/bnQUVLGDM=
 golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
diff --git a/packet.go b/packet.go
index faf5ffd..4a68635 100644
--- a/packet.go
+++ b/packet.go
@@ -133,7 +133,7 @@ func marshalPacket(m encoding.BinaryMarshaler) (header, payload []byte, err erro
 func sendPacket(w io.Writer, m encoding.BinaryMarshaler) error {
 	header, payload, err := marshalPacket(m)
 	if err != nil {
-		return errors.Wrap(err, "binary marshaller failed")
+		return errors.Errorf("binary marshaller failed: %v", err)
 	}
 
 	length := len(header) + len(payload) - 4 // subtract the uint32(length) from the start
@@ -146,12 +146,12 @@ func sendPacket(w io.Writer, m encoding.BinaryMarshaler) error {
 	binary.BigEndian.PutUint32(header[:4], uint32(length))
 
 	if _, err := w.Write(header); err != nil {
-		return errors.Wrap(err, "failed to send packet")
+		return errors.Errorf("failed to send packet: %v", err)
 	}
 
 	if len(payload) > 0 {
 		if _, err := w.Write(payload); err != nil {
-			return errors.Wrap(err, "failed to send packet payload")
+			return errors.Errorf("failed to send packet payload: %v", err)
 		}
 	}
 
diff --git a/request-errors.go b/request-errors.go
index e4b4b86..6505b5c 100644
--- a/request-errors.go
+++ b/request-errors.go
@@ -37,18 +37,18 @@ func (e fxerr) Error() string {
 	case ErrSSHFxEOF:
 		return "EOF"
 	case ErrSSHFxNoSuchFile:
-		return "No Such File"
+		return "no such file"
 	case ErrSSHFxPermissionDenied:
-		return "Permission Denied"
+		return "permission denied"
 	case ErrSSHFxBadMessage:
-		return "Bad Message"
+		return "bad message"
 	case ErrSSHFxNoConnection:
-		return "No Connection"
+		return "no connection"
 	case ErrSSHFxConnectionLost:
-		return "Connection Lost"
+		return "connection lost"
 	case ErrSSHFxOpUnsupported:
-		return "Operation Unsupported"
+		return "operation unsupported"
 	default:
-		return "Failure"
+		return "failure"
 	}
 }
diff --git a/sftp.go b/sftp.go
index 7c7250f..556cf5c 100644
--- a/sftp.go
+++ b/sftp.go
@@ -58,9 +58,9 @@ const (
 	sshFxNoMedia                 = 13
 	sshFxNoSpaceOnFilesystem     = 14
 	sshFxQuotaExceeded           = 15
-	sshFxUnlnownPrincipal        = 16
+	sshFxUnknownPrincipal        = 16
 	sshFxLockConflict            = 17
-	sshFxDitNotEmpty             = 18
+	sshFxDirNotEmpty             = 18
 	sshFxNotADirectory           = 19
 	sshFxInvalidFilename         = 20
 	sshFxLinkLoop                = 21
@@ -200,15 +200,15 @@ func unimplementedPacketErr(u uint8) error {
 type unexpectedIDErr struct{ want, got uint32 }
 
 func (u *unexpectedIDErr) Error() string {
-	return fmt.Sprintf("sftp: unexpected id: want %d, got %d", u.want, u.got)
+	return fmt.Sprintf("sftp: unexpected id: want %v, got %v", u.want, u.got)
 }
 
 func unimplementedSeekWhence(whence int) error {
-	return errors.Errorf("sftp: unimplemented seek whence %d", whence)
+	return errors.Errorf("sftp: unimplemented seek whence %v", whence)
 }
 
 func unexpectedCount(want, got uint32) error {
-	return errors.Errorf("sftp: unexpected count: want %d, got %d", want, got)
+	return errors.Errorf("sftp: unexpected count: want %v, got %v", want, got)
 }
 
 type unexpectedVersionErr struct{ want, got uint32 }
@@ -239,7 +239,7 @@ func getSupportedExtensionByName(extensionName string) (sshExtensionPair, error)
 			return supportedExtension, nil
 		}
 	}
-	return sshExtensionPair{}, errors.Errorf("unsupported extension: %s", extensionName)
+	return sshExtensionPair{}, fmt.Errorf("Unsupported extension: %v", extensionName)
 }
 
 // SetSFTPExtensions allows to customize the supported server extensions.


Test patch:
diff --git a/client_integration_test.go b/client_integration_test.go
index b84e963..ea68929 100644
--- a/client_integration_test.go
+++ b/client_integration_test.go
@@ -6,6 +6,7 @@ package sftp
 import (
 	"bytes"
 	"crypto/sha1"
+	"encoding"
 	"errors"
 	"io"
 	"io/ioutil"
@@ -1367,8 +1368,7 @@ func TestClientRead(t *testing.T) {
 // and the hash of the contents.
 func readHash(t *testing.T, r io.Reader) (string, int64) {
 	h := sha1.New()
-	tr := io.TeeReader(r, h)
-	read, err := io.Copy(ioutil.Discard, tr)
+	read, err := io.Copy(h, r)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1498,48 +1498,26 @@ func TestClientReadFrom(t *testing.T) {
 var errFakeNet = errors.New("Fake network issue")
 
 func TestClientReadFromDeadlock(t *testing.T) {
-	for i := 0; i < 5; i++ {
-		clientWriteDeadlock(t, i, func(f *File) {
-			b := make([]byte, 32768*4)
-			content := bytes.NewReader(b)
-			_, err := f.ReadFrom(content)
-			if !errors.Is(err, errFakeNet) {
-				t.Fatal("Didn't recieve correct error:", err)
-			}
-		})
-	}
+	clientWriteDeadlock(t, 1, func(f *File) {
+		b := make([]byte, 32768*4)
+		content := bytes.NewReader(b)
+		_, err := f.ReadFrom(content)
+		if err != errFakeNet {
+			t.Fatal("Didn't recieve correct error:", err)
+		}
+	})
 }
 
 // Write has exact same problem
 func TestClientWriteDeadlock(t *testing.T) {
-	for i := 0; i < 5; i++ {
-		clientWriteDeadlock(t, i, func(f *File) {
-			b := make([]byte, 32768*4)
+	clientWriteDeadlock(t, 1, func(f *File) {
+		b := make([]byte, 32768*4)
 
-			_, err := f.Write(b)
-			if !errors.Is(err, errFakeNet) {
-				t.Fatal("Didn't recieve correct error:", err)
-			}
-		})
-	}
-}
-
-type timeBombWriter struct {
-	count int
-	w     io.WriteCloser
-}
-
-func (w *timeBombWriter) Write(b []byte) (int, error) {
-	if w.count < 1 {
-		return 0, errFakeNet
-	}
-
-	w.count--
-	return w.w.Write(b)
-}
-
-func (w *timeBombWriter) Close() error {
-	return w.w.Close()
+		_, err := f.Write(b)
+		if err != errFakeNet {
+			t.Fatal("Didn't recieve correct error:", err)
+		}
+	})
 }
 
 // shared body for both previous tests
@@ -1564,13 +1542,20 @@ func clientWriteDeadlock(t *testing.T, N int, badfunc func(*File)) {
 	}
 	defer w.Close()
 
-	// Override the clienConn Writer with a failing version
-	// Replicates network error/drop part way through (after N good writes)
-	wrap := sftp.clientConn.conn.WriteCloser
-	sftp.clientConn.conn.WriteCloser = &timeBombWriter{
-		count: N,
-		w:     wrap,
+	// Override sendPacket with failing version
+	// Replicates network error/drop part way through (after 1 good packet)
+	count := 0
+	sendPacketTest := func(w io.Writer, m encoding.BinaryMarshaler) error {
+		count++
+		if count > N {
+			return errFakeNet
+		}
+		return sendPacket(w, m)
 	}
+	sftp.clientConn.conn.sendPacketTest = sendPacketTest
+	defer func() {
+		sftp.clientConn.conn.sendPacketTest = nil
+	}()
 
 	// this locked (before the fix)
 	badfunc(w)
@@ -1578,31 +1563,27 @@ func clientWriteDeadlock(t *testing.T, N int, badfunc func(*File)) {
 
 // Read/WriteTo has this issue as well
 func TestClientReadDeadlock(t *testing.T) {
-	for i := 0; i < 3; i++ {
-		clientReadDeadlock(t, i, func(f *File) {
-			b := make([]byte, 32768*4)
+	clientReadDeadlock(t, 1, func(f *File) {
+		b := make([]byte, 32768*4)
 
-			_, err := f.Read(b)
-			if !errors.Is(err, errFakeNet) {
-				t.Fatal("Didn't recieve correct error:", err)
-			}
-		})
-	}
+		_, err := f.Read(b)
+		if err != errFakeNet {
+			t.Fatal("Didn't recieve correct error:", err)
+		}
+	})
 }
 
 func TestClientWriteToDeadlock(t *testing.T) {
-	for i := 0; i < 3; i++ {
-		clientReadDeadlock(t, i, func(f *File) {
-			b := make([]byte, 32768*4)
+	clientReadDeadlock(t, 2, func(f *File) {
+		b := make([]byte, 32768*4)
 
-			buf := bytes.NewBuffer(b)
+		buf := bytes.NewBuffer(b)
 
-			_, err := f.WriteTo(buf)
-			if !errors.Is(err, errFakeNet) {
-				t.Fatal("Didn't recieve correct error:", err)
-			}
-		})
-	}
+		_, err := f.WriteTo(buf)
+		if err != errFakeNet {
+			t.Fatal("Didn't recieve correct error:", err)
+		}
+	})
 }
 
 func clientReadDeadlock(t *testing.T, N int, badfunc func(*File)) {
@@ -1638,14 +1619,22 @@ func clientReadDeadlock(t *testing.T, N int, badfunc func(*File)) {
 	}
 	defer r.Close()
 
-	// Override the clienConn Writer with a failing version
-	// Replicates network error/drop part way through (after N good writes)
-	wrap := sftp.clientConn.conn.WriteCloser
-	sftp.clientConn.conn.WriteCloser = &timeBombWriter{
-		count: N,
-		w:     wrap,
+	// Override sendPacket with failing version
+	// Replicates network error/drop part way through (after 1 good packet)
+	count := 0
+	sendPacketTest := func(w io.Writer, m encoding.BinaryMarshaler) error {
+		count++
+		if count > N {
+			return errFakeNet
+		}
+		return sendPacket(w, m)
 	}
 
+	sftp.clientConn.conn.sendPacketTest = sendPacketTest
+	defer func() {
+		sftp.clientConn.conn.sendPacketTest = nil
+	}()
+
 	// this locked (before the fix)
 	badfunc(r)
 }
@@ -2482,11 +2471,11 @@ func benchmarkWriteTo(b *testing.B, bufsize int, delay time.Duration) {
 	f.Write(data)
 	f.Close()
 
-	buf := bytes.NewBuffer(make([]byte, 0, size))
-
 	b.ResetTimer()
 	b.SetBytes(int64(size))
 
+	buf := new(bytes.Buffer)
+
 	for i := 0; i < b.N; i++ {
 		buf.Reset()
 
diff --git a/example_test.go b/example_test.go
index 0093eb0..cf85929 100644
--- a/example_test.go
+++ b/example_test.go
@@ -131,7 +131,7 @@ func ExampleClient_Mkdir_parents() {
 					fi, err = client.Stat(parents)
 					if err == nil {
 						if !fi.IsDir() {
-							return fmt.Errorf("file exists: %s", parents)
+							return fmt.Errorf("File exists: %s", parents)
 						}
 					}
 				}
diff --git a/packet_test.go b/packet_test.go
index c7deb5a..976f66f 100644
--- a/packet_test.go
+++ b/packet_test.go
@@ -3,174 +3,120 @@ package sftp
 import (
 	"bytes"
 	"encoding"
-	"errors"
-	"io/ioutil"
 	"os"
 	"testing"
 )
 
-func TestMarshalUint32(t *testing.T) {
-	var tests = []struct {
-		v    uint32
-		want []byte
-	}{
-		{0, []byte{0, 0, 0, 0}},
-		{42, []byte{0, 0, 0, 42}},
-		{42 << 8, []byte{0, 0, 42, 0}},
-		{42 << 16, []byte{0, 42, 0, 0}},
-		{42 << 24, []byte{42, 0, 0, 0}},
-		{^uint32(0), []byte{255, 255, 255, 255}},
-	}
+var marshalUint32Tests = []struct {
+	v    uint32
+	want []byte
+}{
+	{1, []byte{0, 0, 0, 1}},
+	{256, []byte{0, 0, 1, 0}},
+	{^uint32(0), []byte{255, 255, 255, 255}},
+}
 
-	for _, tt := range tests {
+func TestMarshalUint32(t *testing.T) {
+	for _, tt := range marshalUint32Tests {
 		got := marshalUint32(nil, tt.v)
 		if !bytes.Equal(tt.want, got) {
-			t.Errorf("marshalUint32(%d) = %#v, want %#v", tt.v, got, tt.want)
+			t.Errorf("marshalUint32(%d): want %v, got %v", tt.v, tt.want, got)
 		}
 	}
 }
 
-func TestMarshalUint64(t *testing.T) {
-	var tests = []struct {
-		v    uint64
-		want []byte
-	}{
-		{0, []byte{0, 0, 0, 0, 0, 0, 0, 0}},
-		{42, []byte{0, 0, 0, 0, 0, 0, 0, 42}},
-		{42 << 8, []byte{0, 0, 0, 0, 0, 0, 42, 0}},
-		{42 << 16, []byte{0, 0, 0, 0, 0, 42, 0, 0}},
-		{42 << 24, []byte{0, 0, 0, 0, 42, 0, 0, 0}},
-		{42 << 32, []byte{0, 0, 0, 42, 0, 0, 0, 0}},
-		{42 << 40, []byte{0, 0, 42, 0, 0, 0, 0, 0}},
-		{42 << 48, []byte{0, 42, 0, 0, 0, 0, 0, 0}},
-		{42 << 56, []byte{42, 0, 0, 0, 0, 0, 0, 0}},
-		{^uint64(0), []byte{255, 255, 255, 255, 255, 255, 255, 255}},
-	}
+var marshalUint64Tests = []struct {
+	v    uint64
+	want []byte
+}{
+	{1, []byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1}},
+	{256, []byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0}},
+	{^uint64(0), []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}},
+	{1 << 32, []byte{0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0}},
+}
 
-	for _, tt := range tests {
+func TestMarshalUint64(t *testing.T) {
+	for _, tt := range marshalUint64Tests {
 		got := marshalUint64(nil, tt.v)
 		if !bytes.Equal(tt.want, got) {
-			t.Errorf("marshalUint64(%d) = %#v, want %#v", tt.v, got, tt.want)
+			t.Errorf("marshalUint64(%d): want %#v, got %#v", tt.v, tt.want, got)
 		}
 	}
 }
 
-func TestMarshalString(t *testing.T) {
-	var tests = []struct {
-		v    string
-		want []byte
-	}{
-		{"", []byte{0, 0, 0, 0}},
-		{"/", []byte{0x0, 0x0, 0x0, 0x01, '/'}},
-		{"/foo", []byte{0x0, 0x0, 0x0, 0x4, '/', 'f', 'o', 'o'}},
-		{"\x00bar", []byte{0x0, 0x0, 0x0, 0x4, 0, 'b', 'a', 'r'}},
-		{"b\x00ar", []byte{0x0, 0x0, 0x0, 0x4, 'b', 0, 'a', 'r'}},
-		{"ba\x00r", []byte{0x0, 0x0, 0x0, 0x4, 'b', 'a', 0, 'r'}},
-		{"bar\x00", []byte{0x0, 0x0, 0x0, 0x4, 'b', 'a', 'r', 0}},
-	}
+var marshalStringTests = []struct {
+	v    string
+	want []byte
+}{
+	{"", []byte{0, 0, 0, 0}},
+	{"/foo", []byte{0x0, 0x0, 0x0, 0x4, 0x2f, 0x66, 0x6f, 0x6f}},
+}
 
-	for _, tt := range tests {
+func TestMarshalString(t *testing.T) {
+	for _, tt := range marshalStringTests {
 		got := marshalString(nil, tt.v)
 		if !bytes.Equal(tt.want, got) {
-			t.Errorf("marshalString(%q) = %#v, want %#v", tt.v, got, tt.want)
+			t.Errorf("marshalString(%q): want %#v, got %#v", tt.v, tt.want, got)
 		}
 	}
 }
 
-func TestMarshal(t *testing.T) {
-	type Struct struct {
-		X, Y, Z uint32
-	}
-
-	var tests = []struct {
-		v    interface{}
-		want []byte
-	}{
-		{uint8(42), []byte{42}},
-		{uint32(42 << 8), []byte{0, 0, 42, 0}},
-		{uint64(42 << 32), []byte{0, 0, 0, 42, 0, 0, 0, 0}},
-		{"foo", []byte{0x0, 0x0, 0x0, 0x3, 'f', 'o', 'o'}},
-		{Struct{1, 2, 3}, []byte{0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x0, 0x3}},
-		{[]uint32{1, 2, 3}, []byte{0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x0, 0x3}},
-	}
+var marshalTests = []struct {
+	v    interface{}
+	want []byte
+}{
+	{uint8(1), []byte{1}},
+	{byte(1), []byte{1}},
+	{uint32(1), []byte{0, 0, 0, 1}},
+	{uint64(1), []byte{0, 0, 0, 0, 0, 0, 0, 1}},
+	{"foo", []byte{0x0, 0x0, 0x0, 0x3, 0x66, 0x6f, 0x6f}},
+	{[]uint32{1, 2, 3, 4}, []byte{0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x0, 0x3, 0x0, 0x0, 0x0, 0x4}},
+}
 
-	for _, tt := range tests {
+func TestMarshal(t *testing.T) {
+	for _, tt := range marshalTests {
 		got := marshal(nil, tt.v)
 		if !bytes.Equal(tt.want, got) {
-			t.Errorf("marshal(%#v) = %#v, want %#v", tt.v, got, tt.want)
+			t.Errorf("marshal(%v): want %#v, got %#v", tt.v, tt.want, got)
 		}
 	}
 }
 
-func TestUnmarshalUint32(t *testing.T) {
-	testBuffer := []byte{
-		0, 0, 0, 0,
-		0, 0, 0, 42,
-		0, 0, 42, 0,
-		0, 42, 0, 0,
-		42, 0, 0, 0,
-		255, 0, 0, 254,
-	}
-
-	var wants = []uint32{
-		0,
-		42,
-		42 << 8,
-		42 << 16,
-		42 << 24,
-		255<<24 | 254,
-	}
-
-	var i int
-	for len(testBuffer) > 0 {
-		got, rest := unmarshalUint32(testBuffer)
+var unmarshalUint32Tests = []struct {
+	b    []byte
+	want uint32
+	rest []byte
+}{
+	{[]byte{0, 0, 0, 0}, 0, nil},
+	{[]byte{0, 0, 1, 0}, 256, nil},
+	{[]byte{255, 0, 0, 255}, 4278190335, nil},
+}
 
-		if got != wants[i] {
-			t.Fatalf("unmarshalUint32(%#v) = %d, want %d", testBuffer[:4], got, wants[i])
+func TestUnmarshalUint32(t *testing.T) {
+	for _, tt := range unmarshalUint32Tests {
+		got, rest := unmarshalUint32(tt.b)
+		if got != tt.want || !bytes.Equal(rest, tt.rest) {
+			t.Errorf("unmarshalUint32(%v): want %v, %#v, got %v, %#v", tt.b, tt.want, tt.rest, got, rest)
 		}
-
-		i++
-		testBuffer = rest
 	}
 }
 
-func TestUnmarshalUint64(t *testing.T) {
-	testBuffer := []byte{
-		0, 0, 0, 0, 0, 0, 0, 0,
-		0, 0, 0, 0, 0, 0, 0, 42,
-		0, 0, 0, 0, 0, 0, 42, 0,
-		0, 0, 0, 0, 0, 42, 0, 0,
-		0, 0, 0, 0, 42, 0, 0, 0,
-		0, 0, 0, 42, 0, 0, 0, 0,
-		0, 0, 42, 0, 0, 0, 0, 0,
-		0, 42, 0, 0, 0, 0, 0, 0,
-		42, 0, 0, 0, 0, 0, 0, 0,
-		255, 0, 0, 0, 0, 0, 0, 254,
-	}
-
-	var wants = []uint64{
-		0,
-		42,
-		42 << 8,
-		42 << 16,
-		42 << 24,
-		42 << 32,
-		42 << 40,
-		42 << 48,
-		42 << 56,
-		255<<56 | 254,
-	}
-
-	var i int
-	for len(testBuffer) > 0 {
-		got, rest := unmarshalUint64(testBuffer)
+var unmarshalUint64Tests = []struct {
+	b    []byte
+	want uint64
+	rest []byte
+}{
+	{[]byte{0, 0, 0, 0, 0, 0, 0, 0}, 0, nil},
+	{[]byte{0, 0, 0, 0, 0, 0, 1, 0}, 256, nil},
+	{[]byte{255, 0, 0, 0, 0, 0, 0, 255}, 18374686479671623935, nil},
+}
 
-		if got != wants[i] {
-			t.Fatalf("unmarshalUint64(%#v) = %d, want %d", testBuffer[:8], got, wants[i])
+func TestUnmarshalUint64(t *testing.T) {
+	for _, tt := range unmarshalUint64Tests {
+		got, rest := unmarshalUint64(tt.b)
+		if got != tt.want || !bytes.Equal(rest, tt.rest) {
+			t.Errorf("unmarshalUint64(%v): want %v, %#v, got %v, %#v", tt.b, tt.want, tt.rest, got, rest)
 		}
-
-		i++
-		testBuffer = rest
 	}
 }
 
@@ -184,191 +130,85 @@ var unmarshalStringTests = []struct {
 }
 
 func TestUnmarshalString(t *testing.T) {
-	testBuffer := []byte{
-		0, 0, 0, 0,
-		0, 0, 0, 1, '/',
-		0, 0, 0, 4, '/', 'f', 'o', 'o',
-		0, 0, 0, 4, 0, 'b', 'a', 'r',
-		0, 0, 0, 4, 'b', 0, 'a', 'r',
-		0, 0, 0, 4, 'b', 'a', 0, 'r',
-		0, 0, 0, 4, 'b', 'a', 'r', 0,
-	}
-
-	var wants = []string{
-		"",
-		"/",
-		"/foo",
-		"\x00bar",
-		"b\x00ar",
-		"ba\x00r",
-		"bar\x00",
-	}
-
-	var i int
-	for len(testBuffer) > 0 {
-		got, rest := unmarshalString(testBuffer)
-
-		if got != wants[i] {
-			t.Fatalf("unmarshalUint64(%#v...) = %q, want %q", testBuffer[:4], got, wants[i])
+	for _, tt := range unmarshalStringTests {
+		got, rest := unmarshalString(tt.b)
+		if got != tt.want || !bytes.Equal(rest, tt.rest) {
+			t.Errorf("unmarshalUint64(%v): want %q, %#v, got %q, %#v", tt.b, tt.want, tt.rest, got, rest)
 		}
-
-		i++
-		testBuffer = rest
 	}
 }
 
-func TestSendPacket(t *testing.T) {
-	var tests = []struct {
-		packet encoding.BinaryMarshaler
-		want   []byte
-	}{
-		{
-			packet: &sshFxInitPacket{
-				Version: 3,
-				Extensions: []extensionPair{
-					{"posix-rename@openssh.com", "1"},
-				},
-			},
-			want: []byte{
-				0x0, 0x0, 0x0, 0x26,
-				0x1,
-				0x0, 0x0, 0x0, 0x3,
-				0x0, 0x0, 0x0, 0x18,
-				'p', 'o', 's', 'i', 'x', '-', 'r', 'e', 'n', 'a', 'm', 'e', '@', 'o', 'p', 'e', 'n', 's', 's', 'h', '.', 'c', 'o', 'm',
-				0x0, 0x0, 0x0, 0x1,
-				'1',
-			},
-		},
-		{
-			packet: &sshFxpOpenPacket{
-				ID:     1,
-				Path:   "/foo",
-				Pflags: flags(os.O_RDONLY),
-			},
-			want: []byte{
-				0x0, 0x0, 0x0, 0x15,
-				0x3,
-				0x0, 0x0, 0x0, 0x1,
-				0x0, 0x0, 0x0, 0x4, '/', 'f', 'o', 'o',
-				0x0, 0x0, 0x0, 0x1,
-				0x0, 0x0, 0x0, 0x0,
-			},
-		},
-		{
-			packet: &sshFxpWritePacket{
-				ID:     124,
-				Handle: "foo",
-				Offset: 13,
-				Length: uint32(len("bar")),
-				Data:   []byte("bar"),
-			},
-			want: []byte{
-				0x0, 0x0, 0x0, 0x1b,
-				0x6,
-				0x0, 0x0, 0x0, 0x7c,
-				0x0, 0x0, 0x0, 0x3, 'f', 'o', 'o',
-				0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xd,
-				0x0, 0x0, 0x0, 0x3, 'b', 'a', 'r',
-			},
-		},
-		{
-			packet: &sshFxpSetstatPacket{
-				ID:    31,
-				Path:  "/bar",
-				Flags: sshFileXferAttrUIDGID,
-				Attrs: struct {
-					UID uint32
-					GID uint32
-				}{
-					UID: 1000,
-					GID: 100,
-				},
-			},
-			want: []byte{
-				0x0, 0x0, 0x0, 0x19,
-				0x9,
-				0x0, 0x0, 0x0, 0x1f,
-				0x0, 0x0, 0x0, 0x4, '/', 'b', 'a', 'r',
-				0x0, 0x0, 0x0, 0x2,
-				0x0, 0x0, 0x3, 0xe8,
-				0x0, 0x0, 0x0, 0x64,
-			},
+var sendPacketTests = []struct {
+	p    encoding.BinaryMarshaler
+	want []byte
+}{
+	{&sshFxInitPacket{
+		Version: 3,
+		Extensions: []extensionPair{
+			{"posix-rename@openssh.com", "1"},
 		},
-	}
+	}, []byte{0x0, 0x0, 0x0, 0x26, 0x1, 0x0, 0x0, 0x0, 0x3, 0x0, 0x0, 0x0, 0x18, 0x70, 0x6f, 0x73, 0x69, 0x78, 0x2d, 0x72, 0x65, 0x6e, 0x61, 0x6d, 0x65, 0x40, 0x6f, 0x70, 0x65, 0x6e, 0x73, 0x73, 0x68, 0x2e, 0x63, 0x6f, 0x6d, 0x0, 0x0, 0x0, 0x1, 0x31}},
 
-	for _, tt := range tests {
-		b := new(bytes.Buffer)
-		sendPacket(b, tt.packet)
-		if got := b.Bytes(); !bytes.Equal(tt.want, got) {
-			t.Errorf("sendPacket(%v): got %x want %x", tt.packet, tt.want, got)
+	{&sshFxpOpenPacket{
+		ID:     1,
+		Path:   "/foo",
+		Pflags: flags(os.O_RDONLY),
+	}, []byte{0x0, 0x0, 0x0, 0x15, 0x3, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x4, 0x2f, 0x66, 0x6f, 0x6f, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0}},
+
+	{&sshFxpWritePacket{
+		ID:     124,
+		Handle: "foo",
+		Offset: 13,
+		Length: uint32(len([]byte("bar"))),
+		Data:   []byte("bar"),
+	}, []byte{0x0, 0x0, 0x0, 0x1b, 0x6, 0x0, 0x0, 0x0, 0x7c, 0x0, 0x0, 0x0, 0x3, 0x66, 0x6f, 0x6f, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xd, 0x0, 0x0, 0x0, 0x3, 0x62, 0x61, 0x72}},
+
+	{&sshFxpSetstatPacket{
+		ID:    31,
+		Path:  "/bar",
+		Flags: flags(os.O_WRONLY),
+		Attrs: struct {
+			UID uint32
+			GID uint32
+		}{1000, 100},
+	}, []byte{0x0, 0x0, 0x0, 0x19, 0x9, 0x0, 0x0, 0x0, 0x1f, 0x0, 0x0, 0x0, 0x4, 0x2f, 0x62, 0x61, 0x72, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x3, 0xe8, 0x0, 0x0, 0x0, 0x64}},
+}
+
+func TestSendPacket(t *testing.T) {
+	for _, tt := range sendPacketTests {
+		var w bytes.Buffer
+		sendPacket(&w, tt.p)
+		if got := w.Bytes(); !bytes.Equal(tt.want, got) {
+			t.Errorf("sendPacket(%v): want %#v, got %#v", tt.p, tt.want, got)
 		}
 	}
 }
 
-func sp(data encoding.BinaryMarshaler) []byte {
-	b := new(bytes.Buffer)
-	sendPacket(b, data)
-	return b.Bytes()
+func sp(p encoding.BinaryMarshaler) []byte {
+	var w bytes.Buffer
+	sendPacket(&w, p)
+	return w.Bytes()
 }
 
-func TestRecvPacket(t *testing.T) {
-	var recvPacketTests = []struct {
-		b []byte
-
-		want    uint8
-		body    []byte
-		wantErr error
-	}{
-		{
-			b: sp(&sshFxInitPacket{
-				Version: 3,
-				Extensions: []extensionPair{
-					{"posix-rename@openssh.com", "1"},
-				},
-			}),
-			want: sshFxpInit,
-			body: []byte{
-				0x0, 0x0, 0x0, 0x3,
-				0x0, 0x0, 0x0, 0x18,
-				'p', 'o', 's', 'i', 'x', '-', 'r', 'e', 'n', 'a', 'm', 'e', '@', 'o', 'p', 'e', 'n', 's', 's', 'h', '.', 'c', 'o', 'm',
-				0x0, 0x0, 0x0, 0x01,
-				'1',
-			},
-		},
-		{
-			b: []byte{
-				0x0, 0x0, 0x0, 0x0,
-			},
-			wantErr: errShortPacket,
-		},
-		{
-			b: []byte{
-				0xff, 0xff, 0xff, 0xff,
-			},
-			wantErr: errLongPacket,
+var recvPacketTests = []struct {
+	b    []byte
+	want uint8
+	rest []byte
+}{
+	{sp(&sshFxInitPacket{
+		Version: 3,
+		Extensions: []extensionPair{
+			{"posix-rename@openssh.com", "1"},
 		},
-	}
+	}), sshFxpInit, []byte{0x0, 0x0, 0x0, 0x3, 0x0, 0x0, 0x0, 0x18, 0x70, 0x6f, 0x73, 0x69, 0x78, 0x2d, 0x72, 0x65, 0x6e, 0x61, 0x6d, 0x65, 0x40, 0x6f, 0x70, 0x65, 0x6e, 0x73, 0x73, 0x68, 0x2e, 0x63, 0x6f, 0x6d, 0x0, 0x0, 0x0, 0x1, 0x31}},
+}
 
+func TestRecvPacket(t *testing.T) {
 	for _, tt := range recvPacketTests {
 		r := bytes.NewReader(tt.b)
-
-		got, body, err := recvPacket(r, nil, 0)
-		if tt.wantErr == nil {
-			if err != nil {
-				t.Fatalf("recvPacket(%#v): unexpected error: %v", tt.b, err)
-			}
-		} else {
-			if !errors.Is(err, tt.wantErr) {
-				t.Fatalf("recvPacket(%#v) = %v, want %v", tt.b, err, tt.wantErr)
-			}
-		}
-
-		if got != tt.want {
-			t.Errorf("recvPacket(%#v) = %#v, want %#v", tt.b, got, tt.want)
-		}
-
-		if !bytes.Equal(body, tt.body) {
-			t.Errorf("recvPacket(%#v) = %#v, want %#v", tt.b, body, tt.body)
+		got, rest, _ := recvPacket(r, nil, 0)
+		if got != tt.want || !bytes.Equal(rest, tt.rest) {
+			t.Errorf("recvPacket(%#v): want %v, %#v, got %v, %#v", tt.b, tt.want, tt.rest, got, rest)
 		}
 	}
 }
@@ -457,51 +297,49 @@ func TestSSHFxpOpenPackethasPflags(t *testing.T) {
 	}
 }
 
-func benchMarshal(b *testing.B, packet encoding.BinaryMarshaler) {
-	b.ResetTimer()
-
+func BenchmarkMarshalInit(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		sendPacket(ioutil.Discard, packet)
+		sp(&sshFxInitPacket{
+			Version: 3,
+			Extensions: []extensionPair{
+				{"posix-rename@openssh.com", "1"},
+			},
+		})
 	}
 }
 
-func BenchmarkMarshalInit(b *testing.B) {
-	benchMarshal(b, &sshFxInitPacket{
-		Version: 3,
-		Extensions: []extensionPair{
-			{"posix-rename@openssh.com", "1"},
-		},
-	})
-}
-
 func BenchmarkMarshalOpen(b *testing.B) {
-	benchMarshal(b, &sshFxpOpenPacket{
-		ID:     1,
-		Path:   "/home/test/some/random/path",
-		Pflags: flags(os.O_RDONLY),
-	})
+	for i := 0; i < b.N; i++ {
+		sp(&sshFxpOpenPacket{
+			ID:     1,
+			Path:   "/home/test/some/random/path",
+			Pflags: flags(os.O_RDONLY),
+		})
+	}
 }
 
 func BenchmarkMarshalWriteWorstCase(b *testing.B) {
 	data := make([]byte, 32*1024)
-
-	benchMarshal(b, &sshFxpWritePacket{
-		ID:     1,
-		Handle: "someopaquehandle",
-		Offset: 0,
-		Length: uint32(len(data)),
-		Data:   data,
-	})
+	for i := 0; i < b.N; i++ {
+		sp(&sshFxpWritePacket{
+			ID:     1,
+			Handle: "someopaquehandle",
+			Offset: 0,
+			Length: uint32(len(data)),
+			Data:   data,
+		})
+	}
 }
 
 func BenchmarkMarshalWrite1k(b *testing.B) {
-	data := make([]byte, 1025)
-
-	benchMarshal(b, &sshFxpWritePacket{
-		ID:     1,
-		Handle: "someopaquehandle",
-		Offset: 0,
-		Length: uint32(len(data)),
-		Data:   data,
-	})
+	data := make([]byte, 1024)
+	for i := 0; i < b.N; i++ {
+		sp(&sshFxpWritePacket{
+			ID:     1,
+			Handle: "someopaquehandle",
+			Offset: 0,
+			Length: uint32(len(data)),
+			Data:   data,
+		})
+	}
 }
diff --git a/server_integration_test.go b/server_integration_test.go
index 92e2857..c4fb3b9 100644
--- a/server_integration_test.go
+++ b/server_integration_test.go
@@ -408,16 +408,16 @@ func testServer(t *testing.T, useSubsystem bool, readonly bool) (net.Listener, s
 func makeDummyKey() (string, error) {
 	priv, err := ecdsa.GenerateKey(elliptic.P256(), crand.Reader)
 	if err != nil {
-		return "", fmt.Errorf("cannot generate key: %w", err)
+		return "", fmt.Errorf("cannot generate key: %v", err)
 	}
 	der, err := x509.MarshalECPrivateKey(priv)
 	if err != nil {
-		return "", fmt.Errorf("cannot marshal key: %w", err)
+		return "", fmt.Errorf("cannot marshal key: %v", err)
 	}
 	block := &pem.Block{Type: "EC PRIVATE KEY", Bytes: der}
 	f, err := ioutil.TempFile("", "sftp-test-key-")
 	if err != nil {
-		return "", fmt.Errorf("cannot create temp file: %w", err)
+		return "", fmt.Errorf("cannot create temp file: %v", err)
 	}
 	defer func() {
 		if f != nil {
@@ -426,34 +426,16 @@ func makeDummyKey() (string, error) {
 		}
 	}()
 	if err := pem.Encode(f, block); err != nil {
-		return "", fmt.Errorf("cannot write key: %w", err)
+		return "", fmt.Errorf("cannot write key: %v", err)
 	}
 	if err := f.Close(); err != nil {
-		return "", fmt.Errorf("error closing key file: %w", err)
+		return "", fmt.Errorf("error closing key file: %v", err)
 	}
 	path := f.Name()
 	f = nil
 	return path, nil
 }
 
-type execError struct {
-	path   string
-	stderr string
-	err    error
-}
-
-func (e *execError) Error() string {
-	return fmt.Sprintf("%s: %v: %s", e.path, e.err, e.stderr)
-}
-
-func (e *execError) Unwrap() error {
-	return e.err
-}
-
-func (e *execError) Cause() error {
-	return e.err
-}
-
 func runSftpClient(t *testing.T, script string, path string, host string, port int) (string, error) {
 	// if sftp client binary is unavailable, skip test
 	if _, err := os.Stat(*testSftpClientBin); err != nil {
@@ -489,11 +471,7 @@ func runSftpClient(t *testing.T, script string, path string, host string, port i
 	}
 	err = cmd.Wait()
 	if err != nil {
-		err = &execError{
-			path:   cmd.Path,
-			stderr: stderr.String(),
-			err:    err,
-		}
+		err = fmt.Errorf("%v: %s", err, stderr.String())
 	}
 	return stdout.String(), err
 }