serve ftp: update to goftp.io/server v2.0.1 - fixes #7237

cmd/serve/ftp/ftp.go

@@ -9,6 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	iofs "io/fs"
 	"net"
 	"os"
 	"os/user"
@@ -29,7 +30,7 @@ import (
 	"github.com/rclone/rclone/vfs/vfsflags"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	ftp "goftp.io/server/core"
+	ftp "goftp.io/server/v2"
 )
 
 // Options contains options for the http Server
@@ -121,8 +122,8 @@ You can set a single username and password with the --user and --pass flags.
 	},
 }
 
-// server contains everything to run the server
+// driver contains everything to run the driver for the FTP server
-type server struct {
+type driver struct {
 	f      fs.Fs
 	srv    *ftp.Server
 	ctx    context.Context // for global config
@@ -130,12 +131,13 @@ type server struct {
 	vfs    *vfs.VFS
 	proxy  *proxy.Proxy
 	useTLS bool
+	lock   sync.Mutex
 }
 
 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) (*server, error) {
+func newServer(ctx context.Context, f fs.Fs, opt *Options) (*driver, error) {
 	host, port, err := net.SplitHostPort(opt.ListenAddr)
 	if err != nil {
 		return nil, errors.New("failed to parse host:port")
@@ -145,54 +147,58 @@ func newServer(ctx context.Context, f fs.Fs, opt *Options) (*server, error) {
 		return nil, errors.New("failed to parse host:port")
 	}
 
-	s := &server{
+	d := &driver{
 		f:   f,
 		ctx: ctx,
 		opt: *opt,
 	}
 	if proxyflags.Opt.AuthProxy != "" {
-		s.proxy = proxy.New(ctx, &proxyflags.Opt)
+		d.proxy = proxy.New(ctx, &proxyflags.Opt)
 	} else {
-		s.vfs = vfs.New(f, &vfsflags.Opt)
+		d.vfs = vfs.New(f, &vfsflags.Opt)
 	}
-	s.useTLS = s.opt.TLSKey != ""
+	d.useTLS = d.opt.TLSKey != ""
 
 	// Check PassivePorts format since the server library doesn't!
 	if !passivePortsRe.MatchString(opt.PassivePorts) {
 		return nil, fmt.Errorf("invalid format for passive ports %q", opt.PassivePorts)
 	}
 
-	ftpopt := &ftp.ServerOpts{
+	ftpopt := &ftp.Options{
 		Name:           "Rclone FTP Server",
 		WelcomeMessage: "Welcome to Rclone " + fs.Version + " FTP Server",
-		Factory:        s, // implemented by NewDriver method
+		Driver:         d,
 		Hostname:       host,
 		Port:           portNum,
 		PublicIP:       opt.PublicIP,
 		PassivePorts:   opt.PassivePorts,
-		Auth:           s, // implemented by CheckPasswd method
+		Auth:           d,
+		Perm:           ftp.NewSimplePerm("ftp", "ftp"), // fake user and group
 		Logger:         &Logger{},
-		TLS:            s.useTLS,
+		TLS:            d.useTLS,
-		CertFile:       s.opt.TLSCert,
+		CertFile:       d.opt.TLSCert,
-		KeyFile:        s.opt.TLSKey,
+		KeyFile:        d.opt.TLSKey,
 		//TODO implement a maximum of https://godoc.org/goftp.io/server#ServerOpts
 	}
-	s.srv = ftp.NewServer(ftpopt)
+	d.srv, err = ftp.NewServer(ftpopt)
-	return s, nil
+	if err != nil {
+		return nil, fmt.Errorf("failed to create new FTP server: %w", err)
+	}
+	return d, nil
 }
 
 // serve runs the ftp server
-func (s *server) serve() error {
+func (d *driver) serve() error {
-	fs.Logf(s.f, "Serving FTP on %s", s.srv.Hostname+":"+strconv.Itoa(s.srv.Port))
+	fs.Logf(d.f, "Serving FTP on %s", d.srv.Hostname+":"+strconv.Itoa(d.srv.Port))
-	return s.srv.ListenAndServe()
+	return d.srv.ListenAndServe()
 }
 
 // close stops the ftp server
 //
 //lint:ignore U1000 unused when not building linux
-func (s *server) close() error {
+func (d *driver) close() error {
-	fs.Logf(s.f, "Stopping FTP on %s", s.srv.Hostname+":"+strconv.Itoa(s.srv.Port))
+	fs.Logf(d.f, "Stopping FTP on %s", d.srv.Hostname+":"+strconv.Itoa(d.srv.Port))
-	return s.srv.Shutdown()
+	return d.srv.Shutdown()
 }
 
 // Logger ftp logger output formatted message
@@ -223,44 +229,17 @@ func (l *Logger) PrintResponse(sessionID string, code int, message string) {
 }
 
 // CheckPasswd handle auth based on configuration
-//
+func (d *driver) CheckPasswd(sctx *ftp.Context, user, pass string) (ok bool, err error) {
-// This is not used - the one in Driver should be called instead
+	if d.proxy != nil {
-func (s *server) CheckPasswd(user, pass string) (ok bool, err error) {
-	err = errors.New("internal error: server.CheckPasswd should never be called")
-	fs.Errorf(nil, "Error: %v", err)
-	return false, err
-}
-
-// NewDriver starts a new session for each client connection
-func (s *server) NewDriver() (ftp.Driver, error) {
-	log.Trace("", "Init driver")("")
-	d := &Driver{
-		s:   s,
-		vfs: s.vfs, // this can be nil if proxy set
-	}
-	return d, nil
-}
-
-// Driver implementation of ftp server
-type Driver struct {
-	s    *server
-	vfs  *vfs.VFS
-	lock sync.Mutex
-}
-
-// CheckPasswd handle auth based on configuration
-func (d *Driver) CheckPasswd(user, pass string) (ok bool, err error) {
-	s := d.s
-	if s.proxy != nil {
 		var VFS *vfs.VFS
-		VFS, _, err = s.proxy.Call(user, pass, false)
+		VFS, _, err = d.proxy.Call(user, pass, false)
 		if err != nil {
 			fs.Infof(nil, "proxy login failed: %v", err)
 			return false, nil
 		}
 		d.vfs = VFS
 	} else {
-		ok = s.opt.BasicUser == user && (s.opt.BasicPass == "" || s.opt.BasicPass == pass)
+		ok = d.opt.BasicUser == user && (d.opt.BasicPass == "" || d.opt.BasicPass == pass)
 		if !ok {
 			fs.Infof(nil, "login failed: bad credentials")
 			return false, nil
@@ -270,7 +249,7 @@ func (d *Driver) CheckPasswd(user, pass string) (ok bool, err error) {
 }
 
 // Stat get information on file or folder
-func (d *Driver) Stat(path string) (fi ftp.FileInfo, err error) {
+func (d *driver) Stat(sctx *ftp.Context, path string) (fi iofs.FileInfo, err error) {
 	defer log.Trace(path, "")("fi=%+v, err = %v", &fi, &err)
 	n, err := d.vfs.Stat(path)
 	if err != nil {
@@ -280,7 +259,7 @@ func (d *Driver) Stat(path string) (fi ftp.FileInfo, err error) {
 }
 
 // ChangeDir move current folder
-func (d *Driver) ChangeDir(path string) (err error) {
+func (d *driver) ChangeDir(sctx *ftp.Context, path string) (err error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 	defer log.Trace(path, "")("err = %v", &err)
@@ -295,7 +274,7 @@ func (d *Driver) ChangeDir(path string) (err error) {
 }
 
 // ListDir list content of a folder
-func (d *Driver) ListDir(path string, callback func(ftp.FileInfo) error) (err error) {
+func (d *driver) ListDir(sctx *ftp.Context, path string, callback func(iofs.FileInfo) error) (err error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 	defer log.Trace(path, "")("err = %v", &err)
@@ -318,7 +297,7 @@ func (d *Driver) ListDir(path string, callback func(ftp.FileInfo) error) (err er
 	// Account the transfer
 	tr := accounting.GlobalStats().NewTransferRemoteSize(path, node.Size())
 	defer func() {
-		tr.Done(d.s.ctx, err)
+		tr.Done(d.ctx, err)
 	}()
 
 	for _, file := range dirEntries {
@@ -331,7 +310,7 @@ func (d *Driver) ListDir(path string, callback func(ftp.FileInfo) error) (err er
 }
 
 // DeleteDir delete a folder and his content
-func (d *Driver) DeleteDir(path string) (err error) {
+func (d *driver) DeleteDir(sctx *ftp.Context, path string) (err error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 	defer log.Trace(path, "")("err = %v", &err)
@@ -350,7 +329,7 @@ func (d *Driver) DeleteDir(path string) (err error) {
 }
 
 // DeleteFile delete a file
-func (d *Driver) DeleteFile(path string) (err error) {
+func (d *driver) DeleteFile(sctx *ftp.Context, path string) (err error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 	defer log.Trace(path, "")("err = %v", &err)
@@ -369,7 +348,7 @@ func (d *Driver) DeleteFile(path string) (err error) {
 }
 
 // Rename rename a file or folder
-func (d *Driver) Rename(oldName, newName string) (err error) {
+func (d *driver) Rename(sctx *ftp.Context, oldName, newName string) (err error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 	defer log.Trace(oldName, "newName=%q", newName)("err = %v", &err)
@@ -377,7 +356,7 @@ func (d *Driver) Rename(oldName, newName string) (err error) {
 }
 
 // MakeDir create a folder
-func (d *Driver) MakeDir(path string) (err error) {
+func (d *driver) MakeDir(sctx *ftp.Context, path string) (err error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 	defer log.Trace(path, "")("err = %v", &err)
@@ -390,7 +369,7 @@ func (d *Driver) MakeDir(path string) (err error) {
 }
 
 // GetFile download a file
-func (d *Driver) GetFile(path string, offset int64) (size int64, fr io.ReadCloser, err error) {
+func (d *driver) GetFile(sctx *ftp.Context, path string, offset int64) (size int64, fr io.ReadCloser, err error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 	defer log.Trace(path, "offset=%v", offset)("err = %v", &err)
@@ -416,22 +395,23 @@ func (d *Driver) GetFile(path string, offset int64) (size int64, fr io.ReadClose
 
 	// Account the transfer
 	tr := accounting.GlobalStats().NewTransferRemoteSize(path, node.Size())
-	defer tr.Done(d.s.ctx, nil)
+	defer tr.Done(d.ctx, nil)
 
 	return node.Size(), handle, nil
 }
 
 // PutFile upload a file
-func (d *Driver) PutFile(path string, data io.Reader, appendData bool) (n int64, err error) {
+func (d *driver) PutFile(sctx *ftp.Context, path string, data io.Reader, offset int64) (n int64, err error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
-	defer log.Trace(path, "append=%v", appendData)("err = %v", &err)
+	defer log.Trace(path, "offset=%d", offset)("err = %v", &err)
+
 	var isExist bool
-	node, err := d.vfs.Stat(path)
+	fi, err := d.vfs.Stat(path)
 	if err == nil {
 		isExist = true
-		if node.IsDir() {
+		if fi.IsDir() {
-			return 0, errors.New("a dir has the same name")
+			return 0, errors.New("can't create file - directory exists")
 		}
 	} else {
 		if os.IsNotExist(err) {
@@ -441,41 +421,51 @@ func (d *Driver) PutFile(path string, data io.Reader, appendData bool) (n int64,
 		}
 	}
 
-	if appendData && !isExist {
+	if offset > -1 && !isExist {
-		appendData = false
+		offset = -1
 	}
 
-	if !appendData {
+	var f vfs.Handle
+
+	if offset == -1 {
 		if isExist {
-			err = node.Remove()
+			err = d.vfs.Remove(path)
 			if err != nil {
 				return 0, err
 			}
 		}
-		f, err := d.vfs.OpenFile(path, os.O_RDWR|os.O_CREATE, 0660)
+		f, err = d.vfs.Create(path)
 		if err != nil {
 			return 0, err
 		}
-		defer closeIO(path, f)
+		defer fs.CheckClose(f, &err)
-		bytes, err := io.Copy(f, data)
+		n, err = io.Copy(f, data)
 		if err != nil {
 			return 0, err
 		}
-		return bytes, nil
+		return n, nil
 	}
 
-	of, err := d.vfs.OpenFile(path, os.O_APPEND|os.O_RDWR, 0660)
+	f, err = d.vfs.OpenFile(path, os.O_APPEND|os.O_RDWR, 0660)
 	if err != nil {
 		return 0, err
 	}
-	defer closeIO(path, of)
+	defer fs.CheckClose(f, &err)
 
-	_, err = of.Seek(0, io.SeekEnd)
+	info, err := f.Stat()
+	if err != nil {
+		return 0, err
+	}
+	if offset > info.Size() {
+		return 0, fmt.Errorf("offset %d is beyond file size %d", offset, info.Size())
+	}
+
+	_, err = f.Seek(offset, io.SeekStart)
 	if err != nil {
 		return 0, err
 	}
 
-	bytes, err := io.Copy(of, data)
+	bytes, err := io.Copy(f, data)
 	if err != nil {
 		return 0, err
 	}
@@ -521,10 +511,3 @@ func (f *FileInfo) Group() string {
 func (f *FileInfo) ModTime() time.Time {
 	return f.FileInfo.ModTime().UTC()
 }
-
-func closeIO(path string, c io.Closer) {
-	err := c.Close()
-	if err != nil {
-		log.Trace(path, "")("err = %v", &err)
-	}
-}

cmd/serve/ftp/ftp_test.go

@@ -18,7 +18,7 @@ import (
 	"github.com/rclone/rclone/fs/config/configmap"
 	"github.com/rclone/rclone/fs/config/obscure"
 	"github.com/stretchr/testify/assert"
-	ftp "goftp.io/server/core"
+	ftp "goftp.io/server/v2"
 )
 
 const (

go.mod

@@ -64,7 +64,7 @@ require (
 	github.com/youmark/pkcs8 v0.0.0-20201027041543-1326539a0a0a
 	github.com/yunify/qingstor-sdk-go/v3 v3.2.0
 	go.etcd.io/bbolt v1.3.7
-	goftp.io/server v1.0.0-rc1
+	goftp.io/server/v2 v2.0.1
 	golang.org/x/crypto v0.11.0
 	golang.org/x/net v0.12.0
 	golang.org/x/oauth2 v0.10.0

go.sum

@@ -542,8 +542,8 @@ go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0=
 go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo=
 go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
 go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A=
-goftp.io/server v1.0.0-rc1 h1:gdu6Dq8dK4Qllrhc7oEclGUH+6gBYRQtO43ELnI7fjc=
+goftp.io/server/v2 v2.0.1 h1:H+9UbCX2N206ePDSVNCjBftOKOgil6kQ5RAQNx5hJwE=
-goftp.io/server v1.0.0-rc1/go.mod h1:hFZeR656ErRt3ojMKt7H10vQ5nuWV1e0YeUTeorlR6k=
+goftp.io/server/v2 v2.0.1/go.mod h1:7+H/EIq7tXdfo1Muu5p+l3oQ6rYkDZ8lY7IM5d5kVdQ=
 golang.org/x/arch v0.3.0 h1:02VY4/ZcO/gBOH6PUaoiptASxtXU10jazRCP865E97k=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=