From 909d60464e9be25295894df686528311e826391f Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 13 Jul 2015 12:44:59 +0200 Subject: [PATCH] Revert "Merge pull request #2053 from calmh/atomicwriter" (fixes #2058) This reverts commit b611f72e0829f60f3410e6f07652794f437d35ad, reversing changes made to a04b005e932244aa12882dfb95443df0c182163f. --- cmd/syncthing/gui_csrf.go | 27 ++++++---- internal/config/wrapper.go | 14 ++++-- internal/model/model.go | 26 +++++++--- internal/osutil/atomic.go | 90 ---------------------------------- internal/osutil/atomic_test.go | 52 -------------------- 5 files changed, 48 insertions(+), 161 deletions(-) delete mode 100644 internal/osutil/atomic.go delete mode 100644 internal/osutil/atomic_test.go diff --git a/cmd/syncthing/gui_csrf.go b/cmd/syncthing/gui_csrf.go index cabaaf21..47bf6ccb 100644 --- a/cmd/syncthing/gui_csrf.go +++ b/cmd/syncthing/gui_csrf.go @@ -12,6 +12,7 @@ import ( "net/http" "os" "strings" + "time" "github.com/syncthing/syncthing/internal/osutil" "github.com/syncthing/syncthing/internal/sync" @@ -90,20 +91,28 @@ func newCsrfToken() string { } func saveCsrfTokens() { - // We're ignoring errors in here. It's not super critical and there's - // nothing relevant we can do about them anyway... - name := locations[locCsrfTokens] - f, err := osutil.CreateAtomic(name, 0600) + tmp := fmt.Sprintf("%s.tmp.%d", name, time.Now().UnixNano()) + + f, err := os.OpenFile(tmp, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0644) + if err != nil { + return + } + defer os.Remove(tmp) + + for _, t := range csrfTokens { + _, err := fmt.Fprintln(f, t) + if err != nil { + return + } + } + + err = f.Close() if err != nil { return } - for _, t := range csrfTokens { - fmt.Fprintln(f, t) - } - - f.Close() + osutil.Rename(tmp, name) } func loadCsrfTokens() { diff --git a/internal/config/wrapper.go b/internal/config/wrapper.go index aeaf8b7d..64106383 100644 --- a/internal/config/wrapper.go +++ b/internal/config/wrapper.go @@ -7,7 +7,9 @@ package config import ( + "io/ioutil" "os" + "path/filepath" "github.com/syncthing/protocol" "github.com/syncthing/syncthing/internal/events" @@ -281,20 +283,24 @@ 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 := osutil.CreateAtomic(w.path, 0600) + fd, err := ioutil.TempFile(filepath.Dir(w.path), "cfg") if err != nil { return err } + defer os.Remove(fd.Name()) - if err := w.cfg.WriteXML(fd); err != nil { + err = w.cfg.WriteXML(fd) + if err != nil { fd.Close() return err } - if err := fd.Close(); err != nil { + err = fd.Close() + if err != nil { return err } events.Default.Log(events.ConfigSaved, w.cfg) - return nil + + return osutil.Rename(fd.Name(), w.path) } diff --git a/internal/model/model.go b/internal/model/model.go index 96dd68a7..af24da5c 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -13,6 +13,7 @@ import ( "errors" "fmt" "io" + "io/ioutil" "net" "os" "path/filepath" @@ -935,17 +936,30 @@ func (m *Model) SetIgnores(folder string, content []string) error { return fmt.Errorf("Folder %s does not exist", folder) } - fd, err := osutil.CreateAtomic(filepath.Join(cfg.Path(), ".stignore"), 0644) + fd, err := ioutil.TempFile(cfg.Path(), ".syncthing.stignore-"+folder) + 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 + } + } + + err = fd.Close() if err != nil { l.Warnln("Saving .stignore:", err) return err } - for _, line := range content { - fmt.Fprintln(fd, line) - } - - if err := fd.Close(); err != nil { + file := filepath.Join(cfg.Path(), ".stignore") + err = osutil.Rename(fd.Name(), file) + if err != nil { l.Warnln("Saving .stignore:", err) return err } diff --git a/internal/osutil/atomic.go b/internal/osutil/atomic.go deleted file mode 100644 index 5205eef9..00000000 --- a/internal/osutil/atomic.go +++ /dev/null @@ -1,90 +0,0 @@ -// 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 deleted file mode 100644 index bc1c6b09..00000000 --- a/internal/osutil/atomic_test.go +++ /dev/null @@ -1,52 +0,0 @@ -// 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") - } -}