serve ftp: fix VFS leak when the server fails to start

The deferred cleanup in the constructor checked a local error variable
rather than the error being returned, so failures after the VFS was
created (such as an invalid --passive-port) never shut it down. Name
the error return so the cleanup sees the returned error.

cmd/serve/ftp/ftp.go

@@ -189,7 +189,7 @@ func init() {
 var passivePortsRe = regexp.MustCompile(`^\s*\d+\s*-\s*\d+\s*$`)
 
 // Make a new FTP to serve the remote
-func newServer(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Options, proxyOpt *proxy.Options) (*driver, error) {
+func newServer(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Options, proxyOpt *proxy.Options) (_ *driver, err error) {
 	host, port, err := net.SplitHostPort(opt.ListenAddr)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse host:port from %q", opt.ListenAddr)

cmd/serve/ftp/ftp_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/rclone/rclone/fs/config/obscure"
 	"github.com/rclone/rclone/fs/rc"
 	"github.com/rclone/rclone/lib/israce"
+	"github.com/rclone/rclone/vfs"
 	"github.com/rclone/rclone/vfs/vfscommon"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -131,3 +132,20 @@ func TestRc(t *testing.T) {
 		"vfs_cache_mode": "off",
 	})
 }
+
+// TestNewServerError checks that a server initialisation failure is
+// returned as an error and does not leak the VFS it created.
+func TestNewServerError(t *testing.T) {
+	f, err := fs.NewFs(context.Background(), t.TempDir())
+	require.NoError(t, err)
+
+	opt := Opt
+	opt.ListenAddr = testHOST + ":" + testPORT
+	opt.PassivePorts = "not-a-port-range"
+
+	before := vfs.ActiveCount()
+	d, err := newServer(context.Background(), f, &opt, &vfscommon.Opt, &proxy.Opt)
+	require.Error(t, err)
+	assert.Nil(t, d)
+	assert.Equal(t, before, vfs.ActiveCount(), "VFS leaked after failed server creation")
+}