Skip to content

Commit a8d51a2

Browse files
sylvestreRenjiSann
authored andcommitted
mkdir: simplify umask shaping and add regression tests
1 parent f8bad1f commit a8d51a2

2 files changed

Lines changed: 41 additions & 14 deletions

File tree

src/uu/mkdir/src/mkdir.rs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -259,9 +259,10 @@ impl Drop for UmaskGuard {
259259

260260
/// Create a directory with the exact mode specified, bypassing umask.
261261
///
262-
/// GNU mkdir temporarily sets umask to shaped mask before calling mkdir(2), ensuring the
263-
/// directory is created atomically with the correct permissions. This avoids a
264-
/// race condition where the directory briefly exists with umask-based permissions.
262+
/// GNU mkdir temporarily sets umask to a shaped umask before calling mkdir(2),
263+
/// ensuring the directory is created atomically with the correct permissions.
264+
/// This avoids a race condition where the directory briefly exists with
265+
/// umask-based permissions.
265266
#[cfg(unix)]
266267
fn create_dir_with_mode(
267268
path: &Path,
@@ -282,30 +283,24 @@ fn create_dir_with_mode(path: &Path, _mode: u32, _shaped_umask: u32) -> std::io:
282283

283284
// Helper function to create a single directory with appropriate permissions
284285
// `is_parent` argument is not used on windows
285-
#[allow(unused_variables)]
286+
#[cfg_attr(not(unix), allow(unused_variables))]
286287
fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
287288
#[cfg(unix)]
288289
let (mkdir_mode, shaped_umask) = {
289-
let umask = mode::get_umask();
290-
let umask_bits = rustix::fs::Mode::from_bits_truncate(umask as rustix::fs::RawMode);
290+
let mode_bits = |x: u32| rustix::fs::Mode::from_bits_truncate(x as rustix::fs::RawMode);
291+
let umask_bits = mode_bits(mode::get_umask());
291292
if is_parent {
292293
// Parent directories are never affected by -m (matches GNU behavior).
293294
// We pass 0o777 as the mode and shape the umask so it cannot block
294295
// owner write or execute (u+wx), ensuring the owner can traverse and
295296
// write into the parent to create children. All other umask bits are
296297
// preserved so the kernel applies them — and any default ACL on the
297298
// grandparent — through the normal mkdir(2) path.
298-
(
299-
DEFAULT_PERM,
300-
umask_bits & !rustix::fs::Mode::from_bits_truncate(0o300 as rustix::fs::RawMode),
301-
)
299+
(DEFAULT_PERM, umask_bits & !mode_bits(0o300))
302300
} else {
303301
match config.mode {
304302
// Explicit -m: shape umask so it cannot block explicitly requested bits.
305-
Some(m) => (
306-
m,
307-
umask_bits & !rustix::fs::Mode::from_bits_truncate(m as rustix::fs::RawMode),
308-
),
303+
Some(m) => (m, umask_bits & !mode_bits(m)),
309304
// No -m: leave umask fully intact; kernel applies umask + ACL naturally.
310305
None => (DEFAULT_PERM, umask_bits),
311306
}

tests/by-util/test_mkdir.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,38 @@ fn test_mkdir_acl_inheritance_with_restrictive_mask() {
414414
);
415415
}
416416

417+
#[test]
418+
#[cfg(unix)]
419+
fn test_mkdir_p_respects_umask_without_acl() {
420+
use std::os::unix::fs::PermissionsExt;
421+
let (at, mut ucmd) = at_and_ucmd!();
422+
ucmd.arg("-p").arg("a/b/c").umask(0o022).succeeds();
423+
let perms = at.metadata("a/b/c").permissions().mode();
424+
assert_eq!(perms & 0o777, 0o755);
425+
}
426+
427+
#[test]
428+
#[cfg(unix)]
429+
fn test_mkdir_explicit_mode_zero() {
430+
use std::os::unix::fs::PermissionsExt;
431+
let (at, mut ucmd) = at_and_ucmd!();
432+
ucmd.arg("-m").arg("0").arg("d").umask(0o022).succeeds();
433+
let perms = at.metadata("d").permissions().mode();
434+
assert_eq!(perms & 0o777, 0o000);
435+
}
436+
437+
#[test]
438+
#[cfg(unix)]
439+
fn test_mkdir_explicit_mode_with_umask() {
440+
// -m must win over umask: requesting 0o777 with a restrictive umask must
441+
// still yield 0o777, since the umask is shaped to not block requested bits.
442+
use std::os::unix::fs::PermissionsExt;
443+
let (at, mut ucmd) = at_and_ucmd!();
444+
ucmd.arg("-m").arg("777").arg("d").umask(0o077).succeeds();
445+
let perms = at.metadata("d").permissions().mode();
446+
assert_eq!(perms & 0o777, 0o777);
447+
}
448+
417449
#[test]
418450
fn test_mkdir_trailing_dot() {
419451
new_ucmd!().arg("-p").arg("-v").arg("test_dir").succeeds();

0 commit comments

Comments
 (0)