Use github.com/google/renameio for atomic file updates

This also fixes a very unlikely race condition which could delete a
temporary file from a concurrent nsscash run if it chooses the same
temporary file name after os.Rename() was called but before our deferred
os.Remove() remove it. This race would cause the second nsscash run to
fail but not cause any corruption of persistent files.

diff --git a/README.adoc b/README.adoc
index 93deb4041f52964295fc582ad0542e6103483d8e..8821d8a218b8f0b7dacb23d71f73c80a4d30e42e 100644 (file)
--- a/README.adoc
+++ b/README.adoc
@@ -68,8 +68,9 @@ Nsscash is licensed under AGPL version 3 or later.
 == REQUIREMENTS
 
 - Go, for `nsscash`
-  * github.com/pkg/errors
   * github.com/BurntSushi/toml
+  * github.com/google/renameio
+  * github.com/pkg/errors
 - C compiler, for `libnss_cash.so.2`
 
 - HTTP(S) server to provide the passwd/group/etc. files

diff --git a/file.go b/file.go
index c7e580990d7ba54ada969e6d454acecc54c07704..f382ba4ef8bdcae5100d568cdd09009fef0b33b4 100644 (file)
--- a/file.go
+++ b/file.go
@@ -30,6 +30,7 @@ import (
        "syscall"
        "time"
 
+       "github.com/google/renameio"
        "github.com/pkg/errors"
 )
 
@@ -164,18 +165,11 @@ func deployFile(file *File) error {
                return fmt.Errorf("refusing to write empty file")
        }
 
-       // Write the file in an atomic fashion by creating a temporary file
-       // and renaming it over the target file
-
-       dir := filepath.Dir(file.Path)
-       name := filepath.Base(file.Path)
-
-       f, err := ioutil.TempFile(dir, "tmp-"+name+"-")
+       f, err := renameio.TempFile(filepath.Dir(file.Path), file.Path)
        if err != nil {
                return err
        }
-       defer os.Remove(f.Name())
-       defer f.Close()
+       defer f.Cleanup()
 
        // Apply permissions/user/group from the target file but remove the
        // write permissions to discourage manual modifications, use Stat
@@ -204,9 +198,5 @@ func deployFile(file *File) error {
        if err != nil {
                return err
        }
-       err = f.Sync()
-       if err != nil {
-               return err
-       }
-       return os.Rename(f.Name(), file.Path)
+       return f.CloseAtomicallyReplace()
 }

diff --git a/go.mod b/go.mod
index 60a4e0ea761135255e2646862a3b27ae05308189..46f14159d0b3f409d3945ab0c3ce93a86239acb1 100644 (file)
--- a/go.mod
+++ b/go.mod
@@ -4,5 +4,6 @@ go 1.12
 
 require (
        github.com/BurntSushi/toml v0.3.1
+       github.com/google/renameio v0.1.1-0.20200217212219-353f81969824
        github.com/pkg/errors v0.8.1
 )

diff --git a/go.sum b/go.sum
index 69a45f8efd673a79f736859bae12b72f761e8675..9169c0838f16c80ea8727d986e2c80453a837fcc 100644 (file)
--- a/go.sum
+++ b/go.sum
@@ -1,4 +1,6 @@
 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
+github.com/google/renameio v0.1.1-0.20200217212219-353f81969824 h1:9q700G0beHecUuiZOuKgNqNsGQixTeDLnzVZ5nsW3lc=
+github.com/google/renameio v0.1.1-0.20200217212219-353f81969824/go.mod h1:t/HQoYBZSsWSNK35C6CO/TpPLDVWvxOHboWUAweKUpk=
 github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

diff --git a/main.go b/main.go
index 31ef0493a770cffebf38eb6c4d57c628743d17fd..5248a77b8794b6ba8eccf6f46ca396053cae3471 100644 (file)
--- a/main.go
+++ b/main.go
@@ -25,6 +25,8 @@ import (
        "log"
        "os"
        "path/filepath"
+
+       "github.com/google/renameio"
 )
 
 func main() {
@@ -130,29 +132,24 @@ func mainConvert(typ, srcPath, dstPath string) error {
                return fmt.Errorf("unsupported file type %v", t)
        }
 
-       dstDir := filepath.Dir(dstPath)
-       dstName := filepath.Base(dstPath)
-
        // We must create the file first or deployFile() will abort; this is
        // ugly because deployFile() already performs an atomic replacement
        // but the simplest solution with the least duplicate code
-       f, err := ioutil.TempFile(dstDir, "tmp-"+dstName+"-")
+       f, err := renameio.TempFile(filepath.Dir(dstPath), dstPath)
        if err != nil {
                return err
        }
-       tmpPath := f.Name()
-       defer os.Remove(tmpPath)
-       f.Close()
+       defer f.Cleanup()
 
        err = deployFile(&File{
                Type: t,
                Url:  srcPath,
-               Path: tmpPath,
+               Path: f.Name(),
                body: x.Bytes(),
        })
        if err != nil {
                return err
        }
 
-       return os.Rename(tmpPath, dstPath)
+       return f.CloseAtomicallyReplace()
 }

diff --git a/state.go b/state.go
index 4b3124137530213e5444867c797afa29e3ff5dea..43fc598b1814bc17a92821587a40ed35b697c87a 100644 (file)
--- a/state.go
+++ b/state.go
@@ -23,6 +23,8 @@ import (
        "os"
        "path/filepath"
        "time"
+
+       "github.com/google/renameio"
 )
 
 type State struct {
@@ -68,26 +70,15 @@ func WriteState(path string, state *State) error {
                return err
        }
 
-       // Write the file in an atomic fashion by creating a temporary file
-       // and renaming it over the target file
-
-       dir := filepath.Dir(path)
-       name := filepath.Base(path)
-
-       f, err := ioutil.TempFile(dir, "tmp-"+name+"-")
+       f, err := renameio.TempFile(filepath.Dir(path), path)
        if err != nil {
                return err
        }
-       defer os.Remove(f.Name())
-       defer f.Close()
+       defer f.Cleanup()
 
        _, err = f.Write(x)
        if err != nil {
                return err
        }
-       err = f.Sync()
-       if err != nil {
-               return err
-       }
-       return os.Rename(f.Name(), path)
+       return f.CloseAtomicallyReplace()
 }