Repository: pkg/sftp
Pull request #315 (2019-10-26 18:35:02)

make SFTP extensions configurable
This patch allow to configure the supported extensions

eikenb: @drakkan Thanks for the submission and I think it is a good idea. I'll be happy to merge it though there is one thing... Could you add a test or 2 covering the new functions? 
drakkan: @eikenb, sorry for the delay. I added some basic test cases, please let me know if they are enough.
eikenb: @drakkan You don't have to apologize to me for delays... I'm more guilty than anyone on that front around here.

@puellanivis Thanks for reviewing that. Very much appreciated. 
eikenb: @drakkan Thanks for the tests and I just had one more thought (sorry)...

I was looking it over again and wondered if it might not be better if we kept SSHExtensionPair private and made SetSFTPExtensions take a list of the names they wanted. Changing the signature to...
```
func SetSFTPExtensions(extensions ...string) error {
...
```
And calling it...
```
SetSFTPExtensions("hardlink@openssh.com", "posix-rename@openssh.com")
```
Seems like this would both keep the API as is and seems like it be easier to call. 

What do you think?
puellanivis: I considered that as well, I was not sure how important the “Data” field of the extension pair was. If it’s just ”always the value 1” then it makes sense.
drakkan: @eikenb @puellanivis please let me know if there are any remaining fixes/improvements, thanks!
drakkan: @puellanivis removed slice copy, thanks

Patch:
diff --git a/request-server.go b/request-server.go
index d15f3a1..7b03dbb 100644
--- a/request-server.go
+++ b/request-server.go
@@ -190,10 +190,15 @@ func (rs *RequestServer) packetWorker(
 				request = NewRequest("Stat", request.Filepath)
 				rpkt = request.call(rs.Handlers, pkt)
 			}
-		case *sshFxpExtendedPacketPosixRename:
-			request := NewRequest("Rename", pkt.Oldpath)
-			request.Target = pkt.Newpath
-			rpkt = request.call(rs.Handlers, pkt)
+		case *sshFxpExtendedPacket:
+			switch expkt := pkt.SpecificPacket.(type) {
+			default:
+				rpkt = statusFromError(pkt, ErrSSHFxOpUnsupported)
+			case *sshFxpExtendedPacketPosixRename:
+				request := NewRequest("Rename", expkt.Oldpath)
+				request.Target = expkt.Newpath
+				rpkt = request.call(rs.Handlers, pkt)
+			}
 		case hasHandle:
 			handle := pkt.getHandle()
 			request, ok := rs.getRequest(handle)
diff --git a/sftp.go b/sftp.go
index fe3d6bf..28b1a08 100644
--- a/sftp.go
+++ b/sftp.go
@@ -85,10 +85,14 @@ const (
 	sshFxfExcl   = 0x00000020
 )
 
-var sftpExtensions = []sshExtensionPair{
-	{"hardlink@openssh.com", "1"},
-	{"posix-rename@openssh.com", "1"},
-}
+var (
+	// supportedSFTPExtensions defines the supported extensions
+	supportedSFTPExtensions = []sshExtensionPair{
+		{"hardlink@openssh.com", "1"},
+		{"posix-rename@openssh.com", "1"},
+	}
+	sftpExtensions = supportedSFTPExtensions
+)
 
 type fxp uint8
 
@@ -227,3 +231,29 @@ func (s *StatusError) Error() string {
 func (s *StatusError) FxCode() fxerr {
 	return fxerr(s.Code)
 }
+
+func getSupportedExtensionByName(extensionName string) (sshExtensionPair, error) {
+	for _, supportedExtension := range supportedSFTPExtensions {
+		if supportedExtension.Name == extensionName {
+			return supportedExtension, nil
+		}
+	}
+	return sshExtensionPair{}, fmt.Errorf("Unsupported extension: %v", extensionName)
+}
+
+// SetSFTPExtensions allows to customize the supported server extensions.
+// See the variable supportedSFTPExtensions for supported extensions.
+// This method accepts a slice of sshExtensionPair names for example 'hardlink@openssh.com'.
+// If an invalid extension is given an error will be returned and nothing will be changed
+func SetSFTPExtensions(extensions ...string) error {
+	tempExtensions := []sshExtensionPair{}
+	for _, extension := range extensions {
+		sftpExtension, err := getSupportedExtensionByName(extension)
+		if err != nil {
+			return err
+		}
+		tempExtensions = append(tempExtensions, sftpExtension)
+	}
+	sftpExtensions = tempExtensions
+	return nil
+}


Test patch:
diff --git a/request-server_test.go b/request-server_test.go
index 6f06bd2..29c48c9 100644
--- a/request-server_test.go
+++ b/request-server_test.go
@@ -241,20 +241,9 @@ func TestRequestRename(t *testing.T) {
 	err = p.cli.Rename("/foo", "/bar")
 	assert.Nil(t, err)
 	f, err := r.fetch("/bar")
-	if err != nil {
-		t.Fatal("unexpected error:", err)
-	}
 	assert.Equal(t, "bar", f.Name())
-	_, err = r.fetch("/foo")
-	assert.Equal(t, os.ErrNotExist, err)
-	err = p.cli.PosixRename("/bar", "/baz")
 	assert.Nil(t, err)
-	f, err = r.fetch("/baz")
-	if err != nil {
-		t.Fatal("unexpected error:", err)
-	}
-	assert.Equal(t, "baz", f.Name())
-	_, err = r.fetch("/bar")
+	_, err = r.fetch("/foo")
 	assert.Equal(t, os.ErrNotExist, err)
 }
 
diff --git a/sftp_test.go b/sftp_test.go
index 42d5ba5..ee0483c 100644
--- a/sftp_test.go
+++ b/sftp_test.go
@@ -26,3 +26,50 @@ func TestErrFxCode(t *testing.T) {
 		assert.Equal(t, statusErr.FxCode(), tt.fx)
 	}
 }
+
+func TestSupportedExtensions(t *testing.T) {
+	for _, supportedExtension := range supportedSFTPExtensions {
+		_, err := getSupportedExtensionByName(supportedExtension.Name)
+		assert.NoError(t, err)
+	}
+	_, err := getSupportedExtensionByName("invalid@example.com")
+	assert.Error(t, err)
+}
+
+func TestExtensions(t *testing.T) {
+	var supportedExtensions []string
+	for _, supportedExtension := range supportedSFTPExtensions {
+		supportedExtensions = append(supportedExtensions, supportedExtension.Name)
+	}
+
+	testSFTPExtensions := []string{"hardlink@openssh.com"}
+	expectedSFTPExtensions := []sshExtensionPair{
+		{"hardlink@openssh.com", "1"},
+	}
+	err := SetSFTPExtensions(testSFTPExtensions...)
+	assert.NoError(t, err)
+	assert.Equal(t, expectedSFTPExtensions, sftpExtensions)
+
+	invalidSFTPExtensions := []string{"invalid@example.com"}
+	err = SetSFTPExtensions(invalidSFTPExtensions...)
+	assert.Error(t, err)
+	assert.Equal(t, expectedSFTPExtensions, sftpExtensions)
+
+	emptySFTPExtensions := []string{}
+	expectedSFTPExtensions = []sshExtensionPair{}
+	err = SetSFTPExtensions(emptySFTPExtensions...)
+	assert.NoError(t, err)
+	assert.Equal(t, expectedSFTPExtensions, sftpExtensions)
+
+	// if we only have an invalid extension nothing will be modified.
+	invalidSFTPExtensions = []string{
+		"hardlink@openssh.com",
+		"invalid@example.com",
+	}
+	err = SetSFTPExtensions(invalidSFTPExtensions...)
+	assert.Error(t, err)
+	assert.Equal(t, expectedSFTPExtensions, sftpExtensions)
+
+	err = SetSFTPExtensions(supportedExtensions...)
+	assert.Equal(t, supportedSFTPExtensions, sftpExtensions)
+}