From 2cd9c7ef16d44270b3b872000a53877f5e13ea39 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Wed, 1 Jan 2020 17:26:38 +0100 Subject: [PATCH 1/4] sftp: Use MkdirAll provided by the client Closes #2518 --- internal/backend/sftp/sftp.go | 37 ++--------------------------------- 1 file changed, 2 insertions(+), 35 deletions(-) diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index 5ac60da82..bebcb4c84 100644 --- a/internal/backend/sftp/sftp.go +++ b/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) - debug.Log("mkdirAll %v -> %v", d, err) + err := r.c.MkdirAll(d) 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 { From 63c67be9087751bf03fb07a484b680700c9ec0c0 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Mon, 6 Jan 2020 21:37:57 +0100 Subject: [PATCH 2/4] Add hint abouth absolute sftp paths for Synology NAS --- doc/faq.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/faq.rst b/doc/faq.rst index e6d956d0c..a93c4ed86 100644 --- a/doc/faq.rst +++ b/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 -and ls to explore the SFTP file system hierarchy on your NAS. +Try removing the ``/volume1`` prefix in your paths. If this does not work, use +sftp and ls to explore the SFTP file system hierarchy on your NAS. The following may work: From 6e9778ae0ee290e638a602f1030e4b28ae27d0a5 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Mon, 6 Jan 2020 21:46:22 +0100 Subject: [PATCH 3/4] Add changelog file --- changelog/unreleased/issue-2518 | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 changelog/unreleased/issue-2518 diff --git a/changelog/unreleased/issue-2518 b/changelog/unreleased/issue-2518 new file mode 100644 index 000000000..b2ae6bde7 --- /dev/null +++ b/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 oft 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 From bf199e5ca7b965d35e10434f6d6326b164090458 Mon Sep 17 00:00:00 2001 From: rawtaz Date: Thu, 13 Feb 2020 00:47:09 +0100 Subject: [PATCH 4/4] Minor speling corection. --- changelog/unreleased/issue-2518 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/issue-2518 b/changelog/unreleased/issue-2518 index b2ae6bde7..6fad486f5 100644 --- a/changelog/unreleased/issue-2518 +++ b/changelog/unreleased/issue-2518 @@ -3,7 +3,7 @@ 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 oft the sftp library instead of our own +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