diff --git a/internal/osutil/osutil.go b/internal/osutil/osutil.go index 5ce2704a4..9f6b7b578 100644 --- a/internal/osutil/osutil.go +++ b/internal/osutil/osutil.go @@ -69,7 +69,14 @@ func Copy(from, to string) (err error) { // containing `path` is writable for the duration of the call. func InWritableDir(fn func(string) error, path string) error { dir := filepath.Dir(path) - if info, err := os.Stat(dir); err == nil && info.IsDir() && info.Mode()&0200 == 0 { + info, err := os.Stat(dir) + if err != nil { + return err + } + if !info.IsDir() { + return errors.New("Not a directory: " + path) + } + if info.Mode()&0200 == 0 { // A non-writeable directory (for this user; we assume that's the // relevant part). Temporarily change the mode so we can delete the // file or directory inside it. diff --git a/internal/osutil/osutil_test.go b/internal/osutil/osutil_test.go index 74abd1f8d..40fcae469 100644 --- a/internal/osutil/osutil_test.go +++ b/internal/osutil/osutil_test.go @@ -15,4 +15,65 @@ package osutil_test -// Empty test file to generate 0% coverage rather than no coverage +import ( + "os" + "testing" + + "github.com/syncthing/syncthing/internal/osutil" +) + +func TestInWriteableDir(t *testing.T) { + err := os.RemoveAll("testdata") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll("testdata") + + os.Mkdir("testdata", 0700) + os.Mkdir("testdata/rw", 0700) + os.Mkdir("testdata/ro", 0500) + + create := func(name string) error { + fd, err := os.Create(name) + if err != nil { + return err + } + fd.Close() + return nil + } + + // These should succeed + + err = osutil.InWritableDir(create, "testdata/file") + if err != nil { + t.Error("testdata/file:", err) + } + err = osutil.InWritableDir(create, "testdata/rw/foo") + if err != nil { + t.Error("testdata/rw/foo:", err) + } + err = osutil.InWritableDir(os.Remove, "testdata/rw/foo") + if err != nil { + t.Error("testdata/rw/foo:", err) + } + + err = osutil.InWritableDir(create, "testdata/ro/foo") + if err != nil { + t.Error("testdata/ro/foo:", err) + } + err = osutil.InWritableDir(os.Remove, "testdata/ro/foo") + if err != nil { + t.Error("testdata/ro/foo:", err) + } + + // These should not + + err = osutil.InWritableDir(create, "testdata/nonexistent/foo") + if err == nil { + t.Error("testdata/nonexistent/foo returned nil error") + } + err = osutil.InWritableDir(create, "testdata/file/foo") + if err == nil { + t.Error("testdata/file/foo returned nil error") + } +}