Skip to content

Commit 279fa6d

Browse files
authored
Merge pull request #92 from thaJeztah/fix_implied_directories
archive: create implied parents for directory entries
2 parents 1c23372 + 517985a commit 279fa6d

2 files changed

Lines changed: 60 additions & 55 deletions

File tree

archive.go

Lines changed: 52 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,71 +1034,68 @@ func unrepresentableOnWindows(hdr *tar.Header) error {
10341034
// destination at the OS level (openat(2) semantics), preventing escape via
10351035
// symlinks in the destination tree.
10361036
func createImpliedDirectories(root *os.Root, hdr *tar.Header, options *TarOptions) error {
1037-
// For non-directory entries, ensure that the parent directory exists.
1038-
if hdr.Typeflag != tar.TypeDir {
1039-
parent := filepath.FromSlash(path.Dir(strings.TrimSuffix(hdr.Name, "/")))
1040-
// Skip when the parent is the root itself; nothing to create.
1041-
if parent == "." || parent == "" {
1042-
return nil
1043-
}
1044-
if _, err := root.Lstat(parent); err == nil {
1045-
return nil
1046-
} else if !os.IsNotExist(err) {
1047-
return err
1037+
parent := filepath.FromSlash(path.Dir(strings.TrimSuffix(hdr.Name, "/")))
1038+
// Skip when the parent is the root itself; nothing to create.
1039+
if parent == "." || parent == "" {
1040+
return nil
1041+
}
1042+
if _, err := root.Lstat(parent); err == nil {
1043+
return nil
1044+
} else if !os.IsNotExist(err) {
1045+
return err
1046+
}
1047+
// RootPair() is confined inside this loop as most cases will not require a call, so we can spend some
1048+
// unneeded function calls in the uncommon case to encapsulate logic -- implied directories are a niche
1049+
// usage that reduces the portability of an image.
1050+
uid, gid := options.IDMap.RootPair()
1051+
1052+
// Similar to [user.MkdirAllAndChown]
1053+
//
1054+
// [user.MkdirAllAndChown]: https://pkg.go.dev/github.com/moby/sys/user#MkdirAllAndChown
1055+
var cur string
1056+
for c := range strings.SplitSeq(parent, string(os.PathSeparator)) {
1057+
if c == "" {
1058+
continue
10481059
}
1049-
// RootPair() is confined inside this loop as most cases will not require a call, so we can spend some
1050-
// unneeded function calls in the uncommon case to encapsulate logic -- implied directories are a niche
1051-
// usage that reduces the portability of an image.
1052-
uid, gid := options.IDMap.RootPair()
1053-
1054-
// Similar to [user.MkdirAllAndChown]
1055-
//
1056-
// [user.MkdirAllAndChown]: https://pkg.go.dev/github.com/moby/sys/user#MkdirAllAndChown
1057-
var cur string
1058-
for c := range strings.SplitSeq(parent, string(os.PathSeparator)) {
1059-
if c == "" {
1060-
continue
1060+
cur = filepath.Join(cur, c)
1061+
if err := root.Mkdir(cur, ImpliedDirectoryMode); err != nil {
1062+
if !errors.Is(err, os.ErrExist) {
1063+
return err
10611064
}
1062-
cur = filepath.Join(cur, c)
1063-
if err := root.Mkdir(cur, ImpliedDirectoryMode); err != nil {
1064-
if !errors.Is(err, os.ErrExist) {
1065-
return err
1066-
}
10671065

1068-
fi, err := root.Stat(cur)
1069-
if err != nil {
1070-
return err
1071-
}
1072-
if fi.IsDir() {
1073-
continue
1074-
}
1075-
return &os.PathError{Op: "mkdir", Path: cur, Err: syscall.ENOTDIR}
1076-
}
1077-
if options.NoLchown {
1078-
continue
1079-
}
1080-
// Only the successful Mkdir case is newly-created.
1081-
dir, err := root.Open(cur)
1066+
fi, err := root.Stat(cur)
10821067
if err != nil {
10831068
return err
10841069
}
1085-
if uid != 0 || gid != 0 {
1086-
if err := dir.Chown(uid, gid); err != nil {
1087-
_ = dir.Close()
1088-
return err
1089-
}
1070+
if fi.IsDir() {
1071+
continue
10901072
}
1091-
// root.Mkdir applies the mode subject to the process umask, so
1092-
// re-apply it with Chmod to guarantee ImpliedDirectoryMode
1093-
// independent of umask, matching the previous MkdirAllAndChown
1094-
// behavior.
1095-
if err := dir.Chmod(ImpliedDirectoryMode); err != nil {
1073+
return &os.PathError{Op: "mkdir", Path: cur, Err: syscall.ENOTDIR}
1074+
}
1075+
if options.NoLchown {
1076+
continue
1077+
}
1078+
// Only the successful Mkdir case is newly-created.
1079+
dir, err := root.Open(cur)
1080+
if err != nil {
1081+
return err
1082+
}
1083+
if uid != 0 || gid != 0 {
1084+
if err := dir.Chown(uid, gid); err != nil {
10961085
_ = dir.Close()
10971086
return err
10981087
}
1099-
if err := dir.Close(); err != nil {
1100-
return err
1101-
}
1088+
}
1089+
// root.Mkdir applies the mode subject to the process umask, so
1090+
// re-apply it with Chmod to guarantee ImpliedDirectoryMode
1091+
// independent of umask, matching the previous MkdirAllAndChown
1092+
// behavior.
1093+
if err := dir.Chmod(ImpliedDirectoryMode); err != nil {
1094+
_ = dir.Close()
1095+
return err
1096+
}
1097+
if err := dir.Close(); err != nil {
1098+
return err
11021099
}
11031100
}
11041101

archive_unix_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,14 @@ func TestImpliedDirectoryPermissions(t *testing.T) {
9090
Name: "explicit/permissions/umask/file",
9191
Typeflag: tar.TypeReg,
9292
Mode: 0o666,
93+
}, {
94+
// Deliberately omit the trailing slash to match the normalized path passed
95+
// to createImpliedDirectories; Typeflag is the authoritative directory marker.
96+
//
97+
// Regression test for https://github.com/moby/moby/issues/53257
98+
Name: "implied/dir-without-trailing-slash",
99+
Typeflag: tar.TypeDir,
100+
Mode: 0o700,
93101
}}
94102

95103
w := tar.NewWriter(buf)

0 commit comments

Comments
 (0)