From 06f90ebfa6f3e0a744584513e2786988b26569da Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 08 Jul 2026 11:11:48 -0700 Subject: [PATCH] os: properly handle trailing / in Root.MkdirAll MkdirAll("dir/") should create "dir", not return an error. Fixes #80308 Change-Id: I75458c30b6cb94be3a3d368f6024caef6a6a6964 --- diff --git a/src/os/root_noopenat.go b/src/os/root_noopenat.go index 228b580..a84e952 100644 --- a/src/os/root_noopenat.go +++ b/src/os/root_noopenat.go @@ -158,6 +158,9 @@ if err := checkPathEscapes(r, name); err == errPathEscapes { return &PathError{Op: "mkdirat", Path: name, Err: err} } + if name == "" { + return &PathError{Op: "mkdirat", Path: name, Err: syscall.ENOENT} + } prefix := r.root.name + string(PathSeparator) if err := MkdirAll(prefix+name, perm); err != nil { if pe, ok := err.(*PathError); ok { diff --git a/src/os/root_openat.go b/src/os/root_openat.go index d7c9334..92865e4 100644 --- a/src/os/root_openat.go +++ b/src/os/root_openat.go @@ -167,6 +167,12 @@ fi, e := r.Stat(fullname) if e == nil && fi.Mode().IsDir() { err = nil + } else if e == nil { + err = syscall.ENOTDIR + } else if !IsNotExist(e) { + // EPERM, ELOOP, etc., + // probably more useful than EEXIST. + err = e } } } @@ -177,7 +183,12 @@ } return struct{}{}, &PathError{Op: "mkdirat", Err: err} } - _, err := doInRoot(r, fullname, 0, openDirFunc, openLastComponentFunc) + flags := uint(doInRootCreatingDirectory) + switch runtime.GOOS { + case "linux", "windows": + flags = doInRootNoHandleTerminalSlash // see rootMkdir + } + _, err := doInRoot(r, fullname, flags, openDirFunc, openLastComponentFunc) if err != nil { if _, ok := err.(*PathError); !ok { err = &PathError{Op: "mkdirat", Path: fullname, Err: err} diff --git a/src/os/root_test.go b/src/os/root_test.go index 5e0acf9..33a2f33 100644 --- a/src/os/root_test.go +++ b/src/os/root_test.go @@ -585,6 +585,11 @@ func TestRootMkdirAll(t *testing.T) { for _, test := range rootTestCases { + if test.name == "directory does not exist" { + // Test expects error, mkdirall creates the missing directory. + // TestRootMultiMkdirAll covers this case better anyway, just skip. + continue + } test.run(t, func(t *testing.T, target string, root *os.Root) { wantError := test.wantError if test.ltarget != "" { @@ -593,7 +598,7 @@ wantError = true } - err := root.Mkdir(test.open, 0o777) + err := root.MkdirAll(test.open, 0o777) if errEndsTest(t, err, wantError, "root.MkdirAll(%q)", test.open) { return } @@ -3133,6 +3138,52 @@ }) } +func TestRootMultiMkdirAllShallow(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + return testRootMultiMkdirAll(t, test, test.targetPath) + }) +} + +func TestRootMultiMkdirAllDeep(t *testing.T) { + runRootMultiTest(t, func(t *testing.T, test *rootMultiTest) (string, error) { + targetPath := test.targetPath + if len(targetPath) > 0 && os.IsPathSeparator(targetPath[len(targetPath)-1]) { + targetPath += "a/b/" + } else { + targetPath += "/a/b" + } + return testRootMultiMkdirAll(t, test, targetPath) + }) +} + +func testRootMultiMkdirAll(t *testing.T, test *rootMultiTest, targetPath string) (string, error) { + var mkdirAll = os.MkdirAll + if test.root != nil { + mkdirAll = test.root.MkdirAll + } + + test.setOp("MkdirAll(%q, 0o777)", targetPath) + gotErr := mkdirAll(targetPath, 0o777) + + switch { + case test.root != nil && test.target.lescapes(): + // "mkdir ../target", or equivalent escaping path. + test.wantError(t, gotErr, os.ErrPathEscapes) + case test.root != nil && test.target.escapes(): + // "mkdir ../target", or equivalent escaping path. + test.wantError(t, gotErr, errAny) + return "", errSkipRootConsistencyCheck + case test.root != nil && test.target.kind == testFileSymlink && test.target.target.kind == testFileAbsent && targetPath != test.targetPath: + // A minor inconsistency between Root.MkdirAll and os.MkdirAll: + // When an intermediate component of the tree being constructed is a + // dangling symlink, Root.MkdirAll will follow the symlink and create + // its target directory, while os.MkdirAll will fail with an error. + return "", errSkipRootConsistencyCheck + default: + } + return "", gotErr +} + func TestRootMultiRename(t *testing.T) { if runtime.GOOS == "wasip1" { switch os.Getenv("GOWASIRUNTIME") {