Repository: rclone/rclone
Pull request #4664 (2020-10-08 12:35:45)

sftp: remember entered password in AskPass mode
#### What is the purpose of this change?

After switching to a password callback function, if the ssh connection
aborts and needs to be reconnected then the user is-reprompted for their
password.  Instead we now remember the password they entered and just give
that back.  We do lose the ability for them to correct mistakes, but that's
the situation from before switching to callbacks.  We keep the benefits
of not asking for passwords until the SSH connection succeeds (right
known_hosts entry, for example).

#### Was the change discussed in an issue or in the forum before?

  https://github.com/rclone/rclone/issues/4660#issuecomment-705502792

#### Checklist

- [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request).
- [ ] I have added tests for all changes in this PR if appropriate.
- [ ] I have added documentation for the changes if appropriate.
- [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [x] I'm done, this Pull Request is ready for review :-)



Patch:
diff --git a/backend/local/local.go b/backend/local/local.go
index d785fee85..a4a45cd91 100644
--- a/backend/local/local.go
+++ b/backend/local/local.go
@@ -1231,6 +1231,15 @@ func (o *Object) setMetadata(info os.FileInfo) {
 	o.modTime = info.ModTime()
 	o.mode = info.Mode()
 	o.fs.objectMetaMu.Unlock()
+	// On Windows links read as 0 size so set the correct size here
+	if runtime.GOOS == "windows" && o.translatedLink {
+		linkdst, err := os.Readlink(o.path)
+		if err != nil {
+			fs.Errorf(o, "Failed to read link size: %v", err)
+		} else {
+			o.size = int64(len(linkdst))
+		}
+	}
 }
 
 // Stat an Object into info
diff --git a/backend/sftp/sftp.go b/backend/sftp/sftp.go
index d5848b01d..a50f1cc3a 100644
--- a/backend/sftp/sftp.go
+++ b/backend/sftp/sftp.go
@@ -235,6 +235,7 @@ type Fs struct {
 	poolMu       sync.Mutex
 	pool         []*conn
 	pacer        *fs.Pacer // pacer for operations
+	savedpswd    string
 }
 
 // Object is a remote SFTP file that has been stat'd (so it exists, but is not necessarily open for reading)
@@ -413,6 +414,10 @@ func (f *Fs) putSftpConnection(pc **conn, err error) {
 // NewFs creates a new Fs object from the name and root. It connects to
 // the host specified in the config file.
 func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
+	// This will hold the Fs object.  We need to create it here
+	// so we can refer to it in the SSH callback, but it's populated
+	// in NewFsWithConnection
+	f := &Fs{}
 	ctx := context.Background()
 	// Parse config into Options struct
 	opt := new(Options)
@@ -566,33 +571,42 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
 	// Config for password if none was defined and we're allowed to
 	// We don't ask now; we ask if the ssh connection succeeds
 	if opt.Pass == "" && opt.AskPassword {
-		sshConfig.Auth = append(sshConfig.Auth, ssh.PasswordCallback(getPass))
+		sshConfig.Auth = append(sshConfig.Auth, ssh.PasswordCallback(f.getPass))
 	}
 
-	return NewFsWithConnection(ctx, name, root, m, opt, sshConfig)
+	return NewFsWithConnection(ctx, f, name, root, m, opt, sshConfig)
 }
 
 // If we're in password mode and ssh connection succeeds then this
-// callback is called
-func getPass() (string, error) {
-	_, _ = fmt.Fprint(os.Stderr, "Enter SFTP password: ")
-	return config.ReadPassword(), nil
+// callback is called.  First time around we ask the user, and then
+// save it so on reconnection we give back the previous string.
+// This removes the ability to let the user correct a mistaken entry,
+// but means that reconnects are transparent.
+// We'll re-use config.Pass for this, 'cos we know it's not been
+// specified.
+func (f *Fs) getPass() (string, error) {
+	for f.savedpswd == "" {
+		_, _ = fmt.Fprint(os.Stderr, "Enter SFTP password: ")
+		f.savedpswd = config.ReadPassword()
+	}
+	return f.savedpswd, nil
 }
 
 // NewFsWithConnection creates a new Fs object from the name and root and an ssh.ClientConfig. It connects to
 // the host specified in the ssh.ClientConfig
-func NewFsWithConnection(ctx context.Context, name string, root string, m configmap.Mapper, opt *Options, sshConfig *ssh.ClientConfig) (fs.Fs, error) {
-	f := &Fs{
-		name:      name,
-		root:      root,
-		absRoot:   root,
-		opt:       *opt,
-		m:         m,
-		config:    sshConfig,
-		url:       "sftp://" + opt.User + "@" + opt.Host + ":" + opt.Port + "/" + root,
-		mkdirLock: newStringLock(),
-		pacer:     fs.NewPacer(pacer.NewDefault(pacer.MinSleep(minSleep), pacer.MaxSleep(maxSleep), pacer.DecayConstant(decayConstant))),
-	}
+func NewFsWithConnection(ctx context.Context, f *Fs, name string, root string, m configmap.Mapper, opt *Options, sshConfig *ssh.ClientConfig) (fs.Fs, error) {
+	// Populate the Filesystem Object
+	f.name = name
+	f.root = root
+	f.absRoot = root
+	f.opt = *opt
+	f.m = m
+	f.config = sshConfig
+	f.url = "sftp://" + opt.User + "@" + opt.Host + ":" + opt.Port + "/" + root
+	f.mkdirLock = newStringLock()
+	f.pacer = fs.NewPacer(pacer.NewDefault(pacer.MinSleep(minSleep), pacer.MaxSleep(maxSleep), pacer.DecayConstant(decayConstant)))
+	f.savedpswd = ""
+
 	f.features = (&fs.Features{
 		CanHaveEmptyDirectories: true,
 		SlowHash:                true,


Test patch:
diff --git a/backend/local/local_internal_test.go b/backend/local/local_internal_test.go
index abdd74dad..f5e651398 100644
--- a/backend/local/local_internal_test.go
+++ b/backend/local/local_internal_test.go
@@ -6,7 +6,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"runtime"
 	"testing"
 	"time"
 
@@ -89,9 +88,6 @@ func TestSymlink(t *testing.T) {
 
 	// Object viewed as symlink
 	file2 := fstest.NewItem("symlink.txt"+linkSuffix, "file.txt", modTime2)
-	if runtime.GOOS == "windows" {
-		file2.Size = 0 // symlinks are 0 length under Windows
-	}
 
 	// Object viewed as destination
 	file2d := fstest.NewItem("symlink.txt", "hello", modTime1)
@@ -121,9 +117,6 @@ func TestSymlink(t *testing.T) {
 	// Create a symlink
 	modTime3 := fstest.Time("2002-03-03T04:05:10.123123123Z")
 	file3 := r.WriteObjectTo(ctx, r.Flocal, "symlink2.txt"+linkSuffix, "file.txt", modTime3, false)
-	if runtime.GOOS == "windows" {
-		file3.Size = 0 // symlinks are 0 length under Windows
-	}
 	fstest.CheckListingWithPrecision(t, r.Flocal, []fstest.Item{file1, file2, file3}, nil, fs.ModTimeNotSupported)
 	if haveLChtimes {
 		fstest.CheckItems(t, r.Flocal, file1, file2, file3)
@@ -142,9 +135,7 @@ func TestSymlink(t *testing.T) {
 	o, err := r.Flocal.NewObject(ctx, "symlink2.txt"+linkSuffix)
 	require.NoError(t, err)
 	assert.Equal(t, "symlink2.txt"+linkSuffix, o.Remote())
-	if runtime.GOOS != "windows" {
-		assert.Equal(t, int64(8), o.Size())
-	}
+	assert.Equal(t, int64(8), o.Size())
 
 	// Check that NewObject doesn't see the non suffixed version
 	_, err = r.Flocal.NewObject(ctx, "symlink2.txt")