From 94a320f23cd9e6ad27a03a2ac3c833f8871129f8 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 22 Aug 2023 00:36:17 +0100 Subject: [PATCH] serve ftp: update to goftp.io/server v2.0.1 - fixes #7237 --- cmd/serve/ftp/ftp.go | 157 +++++++++++++++++--------------------- cmd/serve/ftp/ftp_test.go | 2 +- go.mod | 2 +- go.sum | 4 +- 4 files changed, 74 insertions(+), 91 deletions(-) diff --git a/cmd/serve/ftp/ftp.go b/cmd/serve/ftp/ftp.go index 0406206ae..bb5dddef0 100644 --- a/cmd/serve/ftp/ftp.go +++ b/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 -type server struct { +// driver contains everything to run the driver for the FTP server +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, - CertFile: s.opt.TLSCert, - KeyFile: s.opt.TLSKey, + TLS: d.useTLS, + CertFile: d.opt.TLSCert, + KeyFile: d.opt.TLSKey, //TODO implement a maximum of https://godoc.org/goftp.io/server#ServerOpts } - s.srv = ftp.NewServer(ftpopt) - return s, nil + d.srv, err = ftp.NewServer(ftpopt) + 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 { - fs.Logf(s.f, "Serving FTP on %s", s.srv.Hostname+":"+strconv.Itoa(s.srv.Port)) - return s.srv.ListenAndServe() +func (d *driver) serve() error { + fs.Logf(d.f, "Serving FTP on %s", d.srv.Hostname+":"+strconv.Itoa(d.srv.Port)) + return d.srv.ListenAndServe() } // close stops the ftp server // //lint:ignore U1000 unused when not building linux -func (s *server) close() error { - fs.Logf(s.f, "Stopping FTP on %s", s.srv.Hostname+":"+strconv.Itoa(s.srv.Port)) - return s.srv.Shutdown() +func (d *driver) close() error { + fs.Logf(d.f, "Stopping FTP on %s", d.srv.Hostname+":"+strconv.Itoa(d.srv.Port)) + 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 -// -// This is not used - the one in Driver should be called instead -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 { +func (d *driver) CheckPasswd(sctx *ftp.Context, user, pass string) (ok bool, err error) { + if d.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() { - return 0, errors.New("a dir has the same name") + if fi.IsDir() { + 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 { - appendData = false + if offset > -1 && !isExist { + 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) - bytes, err := io.Copy(f, data) + defer fs.CheckClose(f, &err) + 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) - } -} diff --git a/cmd/serve/ftp/ftp_test.go b/cmd/serve/ftp/ftp_test.go index 6ac0d344c..59fd59b8d 100644 --- a/cmd/serve/ftp/ftp_test.go +++ b/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 ( diff --git a/go.mod b/go.mod index dcc6ffe4e..260f0beda 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 24259f3e2..757dc92c4 100644 --- a/go.sum +++ b/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 v1.0.0-rc1/go.mod h1:hFZeR656ErRt3ojMKt7H10vQ5nuWV1e0YeUTeorlR6k= +goftp.io/server/v2 v2.0.1 h1:H+9UbCX2N206ePDSVNCjBftOKOgil6kQ5RAQNx5hJwE= +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=