diff --git a/internal/model/rwfolder.go b/internal/model/rwfolder.go index f4ed74f1..69df99b5 100644 --- a/internal/model/rwfolder.go +++ b/internal/model/rwfolder.go @@ -509,7 +509,7 @@ func (p *rwFolder) handleDir(file protocol.FileInfo) { // Most likely a file/link is getting replaced with a directory. // Remove the file/link and fall through to directory creation. case err == nil && (!info.IsDir() || info.Mode()&os.ModeSymlink != 0): - err = osutil.InWritableDir(os.Remove, realName) + err = osutil.InWritableDir(osutil.Remove, realName) if err != nil { l.Infof("Puller (folder %q, dir %q): %v", p.folder, file.Name, err) return @@ -581,11 +581,11 @@ func (p *rwFolder) deleteDir(file protocol.FileInfo) { files, _ := dir.Readdirnames(-1) for _, file := range files { if defTempNamer.IsTemporary(file) { - osutil.InWritableDir(os.Remove, filepath.Join(realName, file)) + osutil.InWritableDir(osutil.Remove, filepath.Join(realName, file)) } } } - err = osutil.InWritableDir(os.Remove, realName) + err = osutil.InWritableDir(osutil.Remove, realName) if err == nil || os.IsNotExist(err) { p.dbUpdates <- file } else { @@ -624,7 +624,7 @@ func (p *rwFolder) deleteFile(file protocol.FileInfo) { } else if p.versioner != nil { err = osutil.InWritableDir(p.versioner.Archive, realName) } else { - err = osutil.InWritableDir(os.Remove, realName) + err = osutil.InWritableDir(osutil.Remove, realName) } if err != nil && !os.IsNotExist(err) { @@ -700,7 +700,7 @@ func (p *rwFolder) renameFile(source, target protocol.FileInfo) { // get rid of. Attempt to delete it instead so that we make *some* // progress. The target is unhandled. - err = osutil.InWritableDir(os.Remove, from) + err = osutil.InWritableDir(osutil.Remove, from) if err != nil { l.Infof("Puller (folder %q, file %q): delete %q after failed rename: %v", p.folder, target.Name, source.Name, err) return @@ -1067,7 +1067,7 @@ func (p *rwFolder) performFinish(state *sharedPullerState) { // over it, hence remove it before proceeding. stat, err := osutil.Lstat(state.realName) if err == nil && (stat.IsDir() || stat.Mode()&os.ModeSymlink != 0) { - osutil.InWritableDir(os.Remove, state.realName) + osutil.InWritableDir(osutil.Remove, state.realName) } // Replace the original content with the new one err = osutil.Rename(state.tempName, state.realName) diff --git a/internal/osutil/osutil.go b/internal/osutil/osutil.go index 285a0278..6639ba53 100644 --- a/internal/osutil/osutil.go +++ b/internal/osutil/osutil.go @@ -88,6 +88,20 @@ func InWritableDir(fn func(string) error, path string) error { return fn(path) } +// On Windows, removes the read-only attribute from the target prior deletion. +func Remove(path string) error { + if runtime.GOOS == "windows" { + info, err := os.Stat(path) + if err != nil { + return err + } + if info.Mode()&0200 == 0 { + os.Chmod(path, 0700) + } + } + return os.Remove(path) +} + func ExpandTilde(path string) (string, error) { if path == "~" { return getHomeDir() diff --git a/internal/osutil/osutil_test.go b/internal/osutil/osutil_test.go index c9648329..8ba20905 100644 --- a/internal/osutil/osutil_test.go +++ b/internal/osutil/osutil_test.go @@ -8,6 +8,7 @@ package osutil_test import ( "os" + "runtime" "testing" "github.com/syncthing/syncthing/internal/osutil" @@ -68,3 +69,97 @@ func TestInWriteableDir(t *testing.T) { t.Error("testdata/file/foo returned nil error") } } + +func TestInWritableDirWindowsRemove(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skipf("Tests not required") + return + } + + err := os.RemoveAll("testdata") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll("testdata") + + create := func(name string) error { + fd, err := os.Create(name) + if err != nil { + return err + } + fd.Close() + return nil + } + + os.Mkdir("testdata", 0700) + + os.Mkdir("testdata/windows", 0500) + os.Mkdir("testdata/windows/ro", 0500) + create("testdata/windows/ro/readonly") + os.Chmod("testdata/windows/ro/readonly", 0500) + + for _, path := range []string{"testdata/windows/ro/readonly", "testdata/windows/ro", "testdata/windows"} { + err := os.Remove(path) + if err == nil { + t.Errorf("Expected error %s", path) + } + } + + for _, path := range []string{"testdata/windows/ro/readonly", "testdata/windows/ro", "testdata/windows"} { + err := osutil.InWritableDir(osutil.Remove, path) + if err != nil { + t.Errorf("Unexpected error %s: %s", path, err) + } + } +} + +func TestInWritableDirWindowsRename(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skipf("Tests not required") + return + } + + err := os.RemoveAll("testdata") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll("testdata") + + create := func(name string) error { + fd, err := os.Create(name) + if err != nil { + return err + } + fd.Close() + return nil + } + + os.Mkdir("testdata", 0700) + + os.Mkdir("testdata/windows", 0500) + os.Mkdir("testdata/windows/ro", 0500) + create("testdata/windows/ro/readonly") + os.Chmod("testdata/windows/ro/readonly", 0500) + + for _, path := range []string{"testdata/windows/ro/readonly", "testdata/windows/ro", "testdata/windows"} { + err := os.Rename(path, path+"new") + if err == nil { + t.Errorf("Expected error %s", path) + } + } + + rename := func(path string) error { + return osutil.Rename(path, path+"new") + } + + for _, path := range []string{"testdata/windows/ro/readonly", "testdata/windows/ro", "testdata/windows"} { + err := osutil.InWritableDir(rename, path) + if err != nil { + t.Errorf("Unexpected error %s: %s", path, err) + } + _, err = os.Stat(path + "new") + if err != nil { + t.Errorf("Unexpected error %s: %s", path, err) + } + } +}