Merge pull request #2530 from restic/fix-sftp-mkdirall

sftp: Use MkdirAll provided by the client

changelog/unreleased/issue-2518

@@ -0,0 +1,16 @@
+Bugfix: Do not crash with Synology NAS sftp server
+
+It was found that when restic is used to store data on an sftp server on a
+Synology NAS with a relative path (one which does not start with a slash), it
+may go into an endless loop trying to create directories on the server. We've
+fixed this bug by using a function in the sftp library instead of our own
+implementation.
+
+The bug was discovered because the Synology sftp server behaves erratic with
+non-absolute path (e.g. `home/restic-repo`). This can be resolved by just using
+an absolute path instead (`/home/restic-repo`). We've also added a paragraph in
+the FAQ.
+
+https://github.com/restic/restic/issues/2518
+https://github.com/restic/restic/issues/2363
+https://github.com/restic/restic/pull/2530

doc/faq.rst

@@ -142,6 +142,9 @@ is not slowed down, which is particularly useful for servers.
 Creating new repo on a Synology NAS via sftp fails
 --------------------------------------------------
 
+For using restic with a Synology NAS via sftp, please make sure that the
+specified path is absolute, it must start with a slash (``/``).
+
 Sometimes creating a new restic repository on a Synology NAS via sftp fails
 with an error similar to the following:
 
@@ -160,8 +163,8 @@ different than the directory structure on the device and maybe even as exposed
 via other protocols.
 
 
-Try removing the /volume1 prefix in your paths. If this does not work, use sftp
+Try removing the ``/volume1`` prefix in your paths. If this does not work, use
-and ls to explore the SFTP file system hierarchy on your NAS.
+sftp and ls to explore the SFTP file system hierarchy on your NAS.
 
 The following may work:
 

internal/backend/sftp/sftp.go

@@ -138,8 +138,7 @@ func Open(cfg Config) (*SFTP, error) {
 
 func (r *SFTP) mkdirAllDataSubdirs() error {
 	for _, d := range r.Paths() {
-		err := r.mkdirAll(d, backend.Modes.Dir)
+		err := r.c.MkdirAll(d)
-		debug.Log("mkdirAll %v -> %v", d, err)
 		if err != nil {
 			return err
 		}
@@ -250,38 +249,6 @@ func (r *SFTP) Location() string {
 	return r.p
 }
 
-func (r *SFTP) mkdirAll(dir string, mode os.FileMode) error {
-	// check if directory already exists
-	fi, err := r.c.Lstat(dir)
-	if err == nil {
-		if fi.IsDir() {
-			return nil
-		}
-
-		return errors.Errorf("mkdirAll(%s): entry exists but is not a directory", dir)
-	}
-
-	// create parent directories
-	errMkdirAll := r.mkdirAll(path.Dir(dir), backend.Modes.Dir)
-
-	// create directory
-	errMkdir := r.c.Mkdir(dir)
-
-	// test if directory was created successfully
-	fi, err = r.c.Lstat(dir)
-	if err != nil {
-		// return previous errors
-		return errors.Errorf("mkdirAll(%s): unable to create directories: %v, %v", dir, errMkdirAll, errMkdir)
-	}
-
-	if !fi.IsDir() {
-		return errors.Errorf("mkdirAll(%s): entry exists but is not a directory", dir)
-	}
-
-	// set mode
-	return r.c.Chmod(dir, mode)
-}
-
 // Join joins the given paths and cleans them afterwards. This always uses
 // forward slashes, which is required by sftp.
 func Join(parts ...string) string {
@@ -306,7 +273,7 @@ func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader
 
 	if r.IsNotExist(err) {
 		// error is caused by a missing directory, try to create it
-		mkdirErr := r.mkdirAll(r.Dirname(h), backend.Modes.Dir)
+		mkdirErr := r.c.MkdirAll(r.Dirname(h))
 		if mkdirErr != nil {
 			debug.Log("error creating dir %v: %v", r.Dirname(h), mkdirErr)
 		} else {