Skip to content

Commit

Permalink
idtools.MkdirAs*: error out if dir exists as file
Browse files Browse the repository at this point in the history
Standard golang's `os.MkdirAll()` function returns "not a directory" error
in case a directory to be created already exists but is not a directory
(e.g. a file). Our own `idtools.MkdirAs*()` functions do not replicate
the behavior.

This is a bug since all `Mkdir()`-like functions are expected to ensure
the required directory exists and is indeed a directory, and return an
error otherwise.

As the code is using our in-house `system.Stat()` call returning a type
which is incompatible with that of golang's `os.Stat()`, I had to amend
the `system` package with `IsDir()`.

A test case is also provided.

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Nov 27, 2017
1 parent c672fbd commit 2aa13f8
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/idtools/idtools_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"strings"
"sync"
"syscall"

"github.com/docker/docker/pkg/system"
"github.com/opencontainers/runc/libcontainer/user"
Expand All @@ -29,6 +30,9 @@ func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chown

stat, err := system.Stat(path)
if err == nil {
if !stat.IsDir() {
return &os.PathError{"mkdir", path, syscall.ENOTDIR}
}
if !chownExisting {
return nil
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/idtools/idtools_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,20 @@ func TestLookupUserAndGroupThatDoesNotExist(t *testing.T) {
assert.Error(t, err)
}

// TestMkdirIsNotDir checks that mkdirAs() function (used by MkdirAll...)
// returns a correct error in case a directory which it is about to create
// already exists but is a file (rather than a directory).
func TestMkdirIsNotDir(t *testing.T) {
file, err := ioutil.TempFile("", t.Name())
if err != nil {
t.Fatalf("Couldn't create temp dir: %v", err)
}
defer os.Remove(file.Name())

err = mkdirAs(file.Name(), 0755, 0, 0, false, false)
assert.EqualError(t, err, "mkdir "+file.Name()+": not a directory")
}

func RequiresRoot(t *testing.T) {
skip.IfCondition(t, os.Getuid() != 0, "skipping test that requires root")
}
5 changes: 5 additions & 0 deletions pkg/system/stat_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ func (s StatT) Mtim() syscall.Timespec {
return s.mtim
}

// IsDir reports whether s describes a directory.
func (s StatT) IsDir() bool {
return s.mode&syscall.S_IFDIR != 0
}

// Stat takes a path to a file and returns
// a system.StatT type pertaining to that file.
//
Expand Down

0 comments on commit 2aa13f8

Please sign in to comment.