Merge branch 'pr/909'

* pr/909:
  Add Vilbrekin
  Correctly check whether parent directory is writable for current user. "&04" was checking if file is readable by others, while "&0200" checks if it's writable for current user.

CONTRIBUTORS

@@ -22,3 +22,4 @@ Phill Luby <phill.luby@newredo.com>
 Ryan Sullivan <kayoticsully@gmail.com>
 Tully Robinson <tully@tojr.org>
 Veeti Paananen <veeti.paananen@rojekti.fi>
+Vil Brekin <vilbrekin@gmail.com>

gui/index.html

@@ -789,11 +789,11 @@
           <li>Daniel Martí</li>
           <li>Felix Ableitner</li>
           <li>Felix Unterpaintner</li>
+          <li>Gilli Sigurdsson</li>
         </ul>
       </div>
       <div class="col-md-6">
         <ul>
-          <li>Gilli Sigurdsson</li>
           <li>James Patterson</li>
           <li>Jens Diemer</li>
           <li>Jochen Voss</li>
@@ -805,6 +805,7 @@
           <li>Ryan Sullivan</li>
           <li>Tully Robinson</li>
           <li>Veeti Paananen</li>
+          <li>Vil Brekin</li>
         </ul>
       </div>
     </div>

internal/model/sharedpullerstate.go

@@ -20,6 +20,7 @@ import (
 	"path/filepath"
 	"sync"
 
+	"github.com/syncthing/syncthing/internal/osutil"
 	"github.com/syncthing/syncthing/internal/protocol"
 )
 
@@ -68,7 +69,7 @@ func (s *sharedPullerState) tempFile() (*os.File, error) {
 	if info, err := os.Stat(dir); err != nil {
 		s.earlyCloseLocked("dst stat dir", err)
 		return nil, err
-	} else if info.Mode()&04 == 0 {
+	} else if info.Mode()&0200 == 0 {
 		err := os.Chmod(dir, 0755)
 		if err == nil {
 			defer func() {
@@ -136,7 +137,8 @@ func (s *sharedPullerState) earlyCloseLocked(context string, err error) {
 	s.err = err
 	if s.fd != nil {
 		s.fd.Close()
-		os.Remove(s.tempName)
+		// Delete temporary file, even if parent dir is read-only
+		osutil.InWritableDir(func(string) error { os.Remove(s.tempName); return nil }, s.tempName)
 	}
 	s.closed = true
 }

internal/model/sharedpullerstate_test.go

@@ -15,7 +15,11 @@
 
 package model
 
-import "testing"
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
 
 func TestSourceFileOK(t *testing.T) {
 	s := sharedPullerState{
@@ -61,3 +65,23 @@ func TestSourceFileBad(t *testing.T) {
 		t.Fatal("Unexpected nil failed()")
 	}
 }
+
+// Test creating temporary file inside read-only directory
+func TestReadOnlyDir(t *testing.T) {
+	s := sharedPullerState{
+		tempName: "testdata/read_only_dir/.temp_name",
+	}
+
+	// Ensure dir is read-only (git doesn't store full permissions)
+	os.Chmod(filepath.Dir(s.tempName), 0555)
+
+	fd, err := s.tempFile()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if fd == nil {
+		t.Fatal("Unexpected nil fd")
+	}
+
+	s.earlyClose("Test done", nil)
+}

internal/osutil/osutil.go

@@ -66,7 +66,7 @@ func Rename(from, to string) 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()&04 == 0 {
+	if info, err := os.Stat(dir); err == nil && info.IsDir() && 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.