From 59b1b0e1dc4e4c14958eb89ea1c439375a9796a6 Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Tue, 7 Apr 2020 15:38:55 +0200
Subject: [PATCH] lib/osutil: Fix "atomic" rename on Windows (ref #6495, ref
 #6493) (#6510)

So, in a funny plot twist, it turns out that WriteFile in Go 1.13
doesn't actually set the read only bit on Windows when called with
permissions 0444 so my test was broken. With an improved test it turns
out that Rename does not, in fact, overwrite a read-only file on
Windows. This adds a fix for that.

(Rename might get improved in Go 1.15...)
---
 lib/osutil/atomic.go      | 11 ++++++++++-
 lib/osutil/atomic_test.go | 35 +++++++++++++++++++++++++++--------
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/lib/osutil/atomic.go b/lib/osutil/atomic.go
index 939fefdd1..d5f7b1217 100644
--- a/lib/osutil/atomic.go
+++ b/lib/osutil/atomic.go
@@ -9,6 +9,7 @@ package osutil
 import (
 	"errors"
 	"path/filepath"
+	"runtime"
 
 	"github.com/syncthing/syncthing/lib/fs"
 )
@@ -92,7 +93,15 @@ func (w *AtomicWriter) Close() error {
 		return err
 	}
 
-	if err := w.fs.Rename(w.next.Name(), w.path); err != nil {
+	err := w.fs.Rename(w.next.Name(), w.path)
+	if runtime.GOOS == "windows" && fs.IsPermission(err) {
+		// On Windows, we might not be allowed to rename over the file
+		// because it's read-only. Get us some write permissions and try
+		// again.
+		_ = w.fs.Chmod(w.path, 0644)
+		err = w.fs.Rename(w.next.Name(), w.path)
+	}
+	if err != nil {
 		w.err = err
 		return err
 	}
diff --git a/lib/osutil/atomic_test.go b/lib/osutil/atomic_test.go
index 9ee30064c..3de74bff3 100644
--- a/lib/osutil/atomic_test.go
+++ b/lib/osutil/atomic_test.go
@@ -10,6 +10,7 @@ import (
 	"bytes"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 )
 
@@ -52,27 +53,45 @@ func TestCreateAtomicCreate(t *testing.T) {
 }
 
 func TestCreateAtomicReplace(t *testing.T) {
-	testCreateAtomicReplace(t, 0644)
+	testCreateAtomicReplace(t, 0666)
 }
 func TestCreateAtomicReplaceReadOnly(t *testing.T) {
-	testCreateAtomicReplace(t, 0400)
+	testCreateAtomicReplace(t, 0444) // windows compatible read-only bits
 }
 
 func testCreateAtomicReplace(t *testing.T, oldPerms os.FileMode) {
 	t.Helper()
 
-	os.RemoveAll("testdata")
-	defer os.RemoveAll("testdata")
+	testdir, err := ioutil.TempDir("", "syncthing")
+	if err != nil {
+		t.Fatal(err)
+	}
+	testfile := filepath.Join(testdir, "testfile")
 
-	if err := os.Mkdir("testdata", 0755); err != nil {
+	os.RemoveAll(testdir)
+	defer os.RemoveAll(testdir)
+
+	if err := os.Mkdir(testdir, 0755); err != nil {
 		t.Fatal(err)
 	}
 
-	if err := ioutil.WriteFile("testdata/file", []byte("some old data"), oldPerms); err != nil {
+	if err := ioutil.WriteFile(testfile, []byte("some old data"), oldPerms); err != nil {
 		t.Fatal(err)
 	}
 
-	w, err := CreateAtomic("testdata/file")
+	// Go < 1.14 has a bug in WriteFile where it does not use the requested
+	// permissions on Windows. Chmod to make sure.
+	if err := os.Chmod(testfile, oldPerms); err != nil {
+		t.Fatal(err)
+	}
+	// Trust, but verify.
+	if info, err := os.Stat(testfile); err != nil {
+		t.Fatal(err)
+	} else if info.Mode() != oldPerms {
+		t.Fatalf("Wrong perms 0%o", info.Mode())
+	}
+
+	w, err := CreateAtomic(testfile)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -84,7 +103,7 @@ func testCreateAtomicReplace(t *testing.T, oldPerms os.FileMode) {
 		t.Fatal(err)
 	}
 
-	bs, err := ioutil.ReadFile("testdata/file")
+	bs, err := ioutil.ReadFile(testfile)
 	if err != nil {
 		t.Fatal(err)
 	}