diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 6b1f8395f..7788c27a2 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -33,8 +33,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [libs/bits] \#5720 Validate `BitArray` in `FromProto`, which now returns an error (@melekes) - [proto/p2p] Renamed `DefaultNodeInfo` and `DefaultNodeInfoOther` to `NodeInfo` and `NodeInfoOther` (@erikgrinaker) - [proto/p2p] Rename `NodeInfo.default_node_id` to `node_id` (@erikgrinaker) - -- [libs/os] Kill() and {Must,}{Read,Write}File() functions have been removed. (@alessio) + - [libs/os] `EnsureDir` now propagates IO errors and checks the file type (@erikgrinaker) + - [libs/os] Kill() and {Must,}{Read,Write}File() functions have been removed. (@alessio) - Blockchain Protocol diff --git a/libs/os/os.go b/libs/os/os.go index 733f7e942..09ffcbd6d 100644 --- a/libs/os/os.go +++ b/libs/os/os.go @@ -33,12 +33,19 @@ func Exit(s string) { os.Exit(1) } +// EnsureDir ensures the given directory exists, creating it if necessary. +// Errors if the path already exists as a non-directory. func EnsureDir(dir string, mode os.FileMode) error { - if _, err := os.Stat(dir); os.IsNotExist(err) { - err := os.MkdirAll(dir, mode) - if err != nil { - return fmt.Errorf("could not create directory %v: %w", dir, err) - } + info, err := os.Stat(dir) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to stat %q: %w", dir, err) + } + if info != nil && !info.IsDir() { + return fmt.Errorf("path %q already exists as a non-directory", dir) + } + err = os.MkdirAll(dir, mode) + if err != nil { + return fmt.Errorf("could not create directory %q: %w", dir, err) } return nil } diff --git a/libs/os/os_test.go b/libs/os/os_test.go index 2b44dc8e7..3228ed840 100644 --- a/libs/os/os_test.go +++ b/libs/os/os_test.go @@ -6,10 +6,12 @@ import ( "io/ioutil" "os" "os/exec" + "path/filepath" "syscall" "testing" "time" + "github.com/stretchr/testify/require" tmos "github.com/tendermint/tendermint/libs/os" ) @@ -67,6 +69,39 @@ func TestTrapSignal(t *testing.T) { t.Fatal("this error should not be triggered") } +func TestEnsureDir(t *testing.T) { + tmp, err := ioutil.TempDir("", "ensure-dir") + require.NoError(t, err) + defer os.RemoveAll(tmp) + + // Should be possible to create a new directory. + err = tmos.EnsureDir(filepath.Join(tmp, "dir"), 0755) + require.NoError(t, err) + require.DirExists(t, filepath.Join(tmp, "dir")) + + // Should succeed on existing directory. + err = tmos.EnsureDir(filepath.Join(tmp, "dir"), 0755) + require.NoError(t, err) + + // Should fail on file. + err = ioutil.WriteFile(filepath.Join(tmp, "file"), []byte{}, 0644) + require.NoError(t, err) + err = tmos.EnsureDir(filepath.Join(tmp, "file"), 0755) + require.Error(t, err) + + // Should allow symlink to dir. + err = os.Symlink(filepath.Join(tmp, "dir"), filepath.Join(tmp, "linkdir")) + require.NoError(t, err) + err = tmos.EnsureDir(filepath.Join(tmp, "linkdir"), 0755) + require.NoError(t, err) + + // Should error on symlink to file. + err = os.Symlink(filepath.Join(tmp, "file"), filepath.Join(tmp, "linkfile")) + require.NoError(t, err) + err = tmos.EnsureDir(filepath.Join(tmp, "linkfile"), 0755) + require.Error(t, err) +} + type mockLogger struct{} func (ml mockLogger) Info(msg string, keyvals ...interface{}) {}