nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: pkg/sftp
Pull request #392 (2020-11-05 12:45:22)

request-server: don't return EOF if there is an unexpected error
io.EOF means [no error](https://github.com/pkg/sftp/blob/master/examples/request-server/main.go#L124) so convert to ErrUnexpectedEOF if we detect a transfer error

drakkan: > Please add a test. Thank you

Done, thank you

Patch:
diff --git a/request-server.go b/request-server.go
index f808880..0b9d5a4 100644
--- a/request-server.go
+++ b/request-server.go
@@ -167,6 +167,9 @@ func (rs *RequestServer) Serve() error {
 	// make sure all open requests are properly closed
 	// (eg. possible on dropped connections, client crashes, etc.)
 	for handle, req := range rs.openRequests {
+		if err == io.EOF {
+			err = io.ErrUnexpectedEOF
+		}
 		req.transferError(err)
 
 		delete(rs.openRequests, handle)


Test patch:
diff --git a/request-server_test.go b/request-server_test.go
index aaec7ae..7243dab 100644
--- a/request-server_test.go
+++ b/request-server_test.go
@@ -17,8 +17,9 @@ import (
 var _ = fmt.Print
 
 type csPair struct {
-	cli *Client
-	svr *RequestServer
+	cli       *Client
+	svr       *RequestServer
+	svrResult chan error
 }
 
 // these must be closed in order, else client.Close will hang
@@ -39,6 +40,9 @@ func clientRequestServerPair(t *testing.T) *csPair {
 	skipIfPlan9(t)
 	ready := make(chan bool)
 	os.Remove(sock) // either this or signal handling
+	pair := &csPair{
+		svrResult: make(chan error, 1),
+	}
 	var server *RequestServer
 	go func() {
 		l, err := net.Listen("unix", sock)
@@ -55,7 +59,8 @@ func clientRequestServerPair(t *testing.T) *csPair {
 			options = append(options, WithRSAllocator())
 		}
 		server = NewRequestServer(fd, handlers, options...)
-		server.Serve()
+		err = server.Serve()
+		pair.svrResult <- err
 	}()
 	<-ready
 	defer os.Remove(sock)
@@ -65,7 +70,9 @@ func clientRequestServerPair(t *testing.T) *csPair {
 	if err != nil {
 		t.Fatalf("%+v\n", err)
 	}
-	return &csPair{client, server}
+	pair.svr = server
+	pair.cli = client
+	return pair
 }
 
 func checkRequestServerAllocator(t *testing.T, p *csPair) {
@@ -718,6 +725,34 @@ func TestRequestReaddir(t *testing.T) {
 	checkRequestServerAllocator(t, p)
 }
 
+func TestCleanDisconnect(t *testing.T) {
+	p := clientRequestServerPair(t)
+	defer p.Close()
+
+	err := p.cli.conn.Close()
+	require.NoError(t, err)
+	// server must return io.EOF after a clean client close
+	// with no pending open requests
+	err = <-p.svrResult
+	require.EqualError(t, err, io.EOF.Error())
+	checkRequestServerAllocator(t, p)
+}
+
+func TestUncleanDisconnect(t *testing.T) {
+	p := clientRequestServerPair(t)
+	defer p.Close()
+
+	foo := NewRequest("", "foo")
+	p.svr.nextRequest(foo)
+	err := p.cli.conn.Close()
+	require.NoError(t, err)
+	// the foo request above is still open after the client disconnects
+	// so the server will convert io.EOF to io.ErrUnexpectedEOF
+	err = <-p.svrResult
+	require.EqualError(t, err, io.ErrUnexpectedEOF.Error())
+	checkRequestServerAllocator(t, p)
+}
+
 func TestCleanPath(t *testing.T) {
 	assert.Equal(t, "/", cleanPath("/"))
 	assert.Equal(t, "/", cleanPath("."))
Repository: pkg/sftp
Pull request #393 (2020-11-16 08:43:31)

request server: fix handles leak in error case
For open/opendir requests we create a new handle and close it
when the client sends an SSH_FXP_CLOSE but if we return an error the
client will never send the close packet so we have to close the handle
ourselves.

You can easily see the bug running the modified test cases without the patch.

The open handles will be closed when the client closes the whole connection, but if we have a client connected for a long time we accumulate handles every time we return an error for open/opendir.

This bug is more evident after applying #392 since the `EOF` will be converted to `ErrUnexpectedEOF` if open requests are found when the client disconnects 


Patch:
diff --git a/request-server.go b/request-server.go
index 2264994..f808880 100644
--- a/request-server.go
+++ b/request-server.go
@@ -198,12 +198,20 @@ func (rs *RequestServer) packetWorker(
 			rpkt = cleanPacketPath(pkt)
 		case *sshFxpOpendirPacket:
 			request := requestFromPacket(ctx, pkt)
-			rs.nextRequest(request)
+			handle := rs.nextRequest(request)
 			rpkt = request.opendir(rs.Handlers, pkt)
+			if _, ok := rpkt.(*sshFxpHandlePacket); !ok {
+				// if we return an error we have to remove the handle from the active ones
+				rs.closeRequest(handle)
+			}
 		case *sshFxpOpenPacket:
 			request := requestFromPacket(ctx, pkt)
-			rs.nextRequest(request)
+			handle := rs.nextRequest(request)
 			rpkt = request.open(rs.Handlers, pkt)
+			if _, ok := rpkt.(*sshFxpHandlePacket); !ok {
+				// if we return an error we have to remove the handle from the active ones
+				rs.closeRequest(handle)
+			}
 		case *sshFxpFstatPacket:
 			handle := pkt.getHandle()
 			request, ok := rs.getRequest(handle)


Test patch:
diff --git a/request-server_test.go b/request-server_test.go
index 7161006..aaec7ae 100644
--- a/request-server_test.go
+++ b/request-server_test.go
@@ -228,6 +228,9 @@ func TestRequestOpenFail(t *testing.T) {
 	rf, err := p.cli.Open("/foo")
 	assert.Exactly(t, os.ErrNotExist, err)
 	assert.Nil(t, rf)
+	// if we return an error the sftp client will not close the handle
+	// ensure that we close it ourself
+	assert.Len(t, p.svr.openRequests, 0)
 	checkRequestServerAllocator(t, p)
 }
 
@@ -711,6 +714,7 @@ func TestRequestReaddir(t *testing.T) {
 	require.Len(t, di, 100)
 	names := []string{di[18].Name(), di[81].Name()}
 	assert.Equal(t, []string{"foo_18", "foo_81"}, names)
+	assert.Len(t, p.svr.openRequests, 0)
 	checkRequestServerAllocator(t, p)
 }
Repository: pkg/sftp
Pull request #401 (2020-12-08 11:05:04)

Client perm
normalise permission denied error as file not found error


Patch:
diff --git a/client.go b/client.go
index e329ed5..cdd5669 100644
--- a/client.go
+++ b/client.go
@@ -655,14 +655,18 @@ func (c *Client) Join(elem ...string) string { return path.Join(elem...) }
 // is not empty.
 func (c *Client) Remove(path string) error {
 	err := c.removeFile(path)
+	// some servers, *cough* osx *cough*, return EPERM, not ENODIR.
+	// serv-u returns ssh_FX_FILE_IS_A_DIRECTORY
+	// EPERM is converted to os.ErrPermission so it is not a StatusError
 	if err, ok := err.(*StatusError); ok {
 		switch err.Code {
-		// some servers, *cough* osx *cough*, return EPERM, not ENODIR.
-		// serv-u returns ssh_FX_FILE_IS_A_DIRECTORY
-		case sshFxPermissionDenied, sshFxFailure, sshFxFileIsADirectory:
+		case sshFxFailure, sshFxFileIsADirectory:
 			return c.RemoveDirectory(path)
 		}
 	}
+	if os.IsPermission(err) {
+		return c.RemoveDirectory(path)
+	}
 	return err
 }
 
@@ -1363,6 +1367,8 @@ func normaliseError(err error) error {
 			return io.EOF
 		case sshFxNoSuchFile:
 			return os.ErrNotExist
+		case sshFxPermissionDenied:
+			return os.ErrPermission
 		case sshFxOk:
 			return nil
 		default:


Test patch:
diff --git a/client_integration_test.go b/client_integration_test.go
index eda5d32..caf15cb 100644
--- a/client_integration_test.go
+++ b/client_integration_test.go
@@ -453,16 +453,13 @@ func TestClientCreateFailed(t *testing.T) {
 	defer sftp.Close()
 
 	f, err := ioutil.TempFile("", "sftptest-createfailed")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
+
 	defer f.Close()
 	defer os.Remove(f.Name())
 
 	f2, err := sftp.Create(f.Name())
-	if err1, ok := err.(*StatusError); !ok || err1.Code != sshFxPermissionDenied {
-		t.Fatalf("Create: want: %v, got %#v", sshFxPermissionDenied, err)
-	}
+	require.True(t, os.IsPermission(err))
 	if err == nil {
 		f2.Close()
 	}
Repository: pkg/sftp
Pull request #411 (2021-03-10 11:28:57)

client fix potential crash if we receive a short packet
This was discovered using  continuous fuzzing, see [here](https://github.com/google/oss-fuzz/pull/5324)


Patch:
diff --git a/client.go b/client.go
index 81b2d6c..df981fe 100644
--- a/client.go
+++ b/client.go
@@ -273,7 +273,10 @@ func (c *Client) recvVersion() error {
 		return &unexpectedPacketErr{sshFxpVersion, typ}
 	}
 
-	version, data := unmarshalUint32(data)
+	version, data, err := unmarshalUint32Safe(data)
+	if err != nil {
+		return err
+	}
 	if version != sftpProtocolVersion {
 		return &unexpectedVersionErr{sftpProtocolVersion, version}
 	}


Test patch:
diff --git a/client_test.go b/client_test.go
index b2eacbc..05b885c 100644
--- a/client_test.go
+++ b/client_test.go
@@ -228,3 +228,14 @@ func TestClientZeroLengthPacket(t *testing.T) {
 		c.Close()
 	}
 }
+
+func TestClientShortPacket(t *testing.T) {
+	// init packet too short.
+	packet := []byte{0, 0, 0, 1, 2}
+
+	r := bytes.NewReader(packet)
+	_, err := NewClientPipe(r, &sink{})
+	if !errors.Is(err, errShortPacket) {
+		t.Fatalf("expected error: %v, got: %v", errShortPacket, err)
+	}
+}