diff --git a/cmd/syncthing/gui_csrf.go b/cmd/syncthing/gui_csrf.go index 47bf6ccb7..cabaaf210 100644 --- a/cmd/syncthing/gui_csrf.go +++ b/cmd/syncthing/gui_csrf.go @@ -12,7 +12,6 @@ import ( "net/http" "os" "strings" - "time" "github.com/syncthing/syncthing/internal/osutil" "github.com/syncthing/syncthing/internal/sync" @@ -91,28 +90,20 @@ func newCsrfToken() string { } func saveCsrfTokens() { - name := locations[locCsrfTokens] - tmp := fmt.Sprintf("%s.tmp.%d", name, time.Now().UnixNano()) + // We're ignoring errors in here. It's not super critical and there's + // nothing relevant we can do about them anyway... - f, err := os.OpenFile(tmp, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0644) + name := locations[locCsrfTokens] + f, err := osutil.CreateAtomic(name, 0600) if err != nil { return } - defer os.Remove(tmp) for _, t := range csrfTokens { - _, err := fmt.Fprintln(f, t) - if err != nil { - return - } + fmt.Fprintln(f, t) } - err = f.Close() - if err != nil { - return - } - - osutil.Rename(tmp, name) + f.Close() } func loadCsrfTokens() { diff --git a/internal/config/wrapper.go b/internal/config/wrapper.go index 64106383c..aeaf8b7d7 100644 --- a/internal/config/wrapper.go +++ b/internal/config/wrapper.go @@ -7,9 +7,7 @@ package config import ( - "io/ioutil" "os" - "path/filepath" "github.com/syncthing/protocol" "github.com/syncthing/syncthing/internal/events" @@ -283,24 +281,20 @@ func (w *Wrapper) IgnoredDevice(id protocol.DeviceID) bool { // Save writes the configuration to disk, and generates a ConfigSaved event. func (w *Wrapper) Save() error { - fd, err := ioutil.TempFile(filepath.Dir(w.path), "cfg") + fd, err := osutil.CreateAtomic(w.path, 0600) if err != nil { return err } - defer os.Remove(fd.Name()) - err = w.cfg.WriteXML(fd) - if err != nil { + if err := w.cfg.WriteXML(fd); err != nil { fd.Close() return err } - err = fd.Close() - if err != nil { + if err := fd.Close(); err != nil { return err } events.Default.Log(events.ConfigSaved, w.cfg) - - return osutil.Rename(fd.Name(), w.path) + return nil } diff --git a/internal/model/model.go b/internal/model/model.go index af24da5c8..96dd68a72 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -13,7 +13,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net" "os" "path/filepath" @@ -936,30 +935,17 @@ func (m *Model) SetIgnores(folder string, content []string) error { return fmt.Errorf("Folder %s does not exist", folder) } - fd, err := ioutil.TempFile(cfg.Path(), ".syncthing.stignore-"+folder) + fd, err := osutil.CreateAtomic(filepath.Join(cfg.Path(), ".stignore"), 0644) if err != nil { l.Warnln("Saving .stignore:", err) return err } - defer os.Remove(fd.Name()) for _, line := range content { - _, err = fmt.Fprintln(fd, line) - if err != nil { - l.Warnln("Saving .stignore:", err) - return err - } + fmt.Fprintln(fd, line) } - err = fd.Close() - if err != nil { - l.Warnln("Saving .stignore:", err) - return err - } - - file := filepath.Join(cfg.Path(), ".stignore") - err = osutil.Rename(fd.Name(), file) - if err != nil { + if err := fd.Close(); err != nil { l.Warnln("Saving .stignore:", err) return err } diff --git a/internal/osutil/atomic.go b/internal/osutil/atomic.go new file mode 100644 index 000000000..5205eef96 --- /dev/null +++ b/internal/osutil/atomic.go @@ -0,0 +1,90 @@ +// Copyright (C) 2015 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at http://mozilla.org/MPL/2.0/. + +package osutil + +import ( + "errors" + "io/ioutil" + "os" + "path/filepath" +) + +var ( + ErrClosed = errors.New("write to closed writer") + TempPrefix = ".syncthing.tmp." +) + +// An AtomicWriter is an *os.File that writes to a temporary file in the same +// directory as the final path. On successfull Close the file is renamed to +// it's final path. Any error on Write or during Close is accumulated and +// returned on Close, so a lazy user can ignore errors until Close. +type AtomicWriter struct { + path string + next *os.File + err error +} + +// CreateAtomic is like os.Create with a FileMode, except a temporary file +// name is used instead of the given name. +func CreateAtomic(path string, mode os.FileMode) (*AtomicWriter, error) { + fd, err := ioutil.TempFile(filepath.Dir(path), TempPrefix) + if err != nil { + return nil, err + } + + if err := os.Chmod(fd.Name(), mode); err != nil { + fd.Close() + os.Remove(fd.Name()) + return nil, err + } + + w := &AtomicWriter{ + path: path, + next: fd, + } + + return w, nil +} + +// Write is like io.Writer, but is a no-op on an already failed AtomicWriter. +func (w *AtomicWriter) Write(bs []byte) (int, error) { + if w.err != nil { + return 0, w.err + } + n, err := w.next.Write(bs) + if err != nil { + w.err = err + w.next.Close() + } + return n, err +} + +// Close closes the temporary file and renames it to the final path. It is +// invalid to call Write() or Close() after Close(). +func (w *AtomicWriter) Close() error { + if w.err != nil { + return w.err + } + + // Try to not leave temp file around, but ignore error. + defer os.Remove(w.next.Name()) + + if err := w.next.Close(); err != nil { + w.err = err + return err + } + + if err := os.Rename(w.next.Name(), w.path); err != nil { + w.err = err + return err + } + + // Set w.err to return appropriately for any future operations. + w.err = ErrClosed + + return nil +} diff --git a/internal/osutil/atomic_test.go b/internal/osutil/atomic_test.go new file mode 100644 index 000000000..bc1c6b098 --- /dev/null +++ b/internal/osutil/atomic_test.go @@ -0,0 +1,52 @@ +// Copyright (C) 2015 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at http://mozilla.org/MPL/2.0/. + +package osutil + +import ( + "bytes" + "io/ioutil" + "os" + "testing" +) + +func TestCreateAtomic(t *testing.T) { + os.RemoveAll("testdata") + defer os.RemoveAll("testdata") + + if err := os.Mkdir("testdata", 0755); err != nil { + t.Fatal(err) + } + + w, err := CreateAtomic("testdata/file", 0644) + if err != nil { + t.Fatal(err) + } + + n, err := w.Write([]byte("hello")) + if err != nil { + t.Fatal(err) + } + if n != 5 { + t.Fatal("written bytes", n, "!= 5") + } + + if _, err := ioutil.ReadFile("testdata/file"); err == nil { + t.Fatal("file should not exist") + } + + if err := w.Close(); err != nil { + t.Fatal(err) + } + + bs, err := ioutil.ReadFile("testdata/file") + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(bs, []byte("hello")) { + t.Error("incorrect data") + } +}