diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index dc59b2b99c0..485ec961630 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -216,8 +216,8 @@ fn chmod(_path: &Path, _mode: u32) -> UResult<()> { Ok(()) } -// Return true if the directory at `path` has been created by this call. -// `is_parent` argument is not used on windows +// Create a directory at the given path. +// Uses iterative approach instead of recursion to avoid stack overflow with deep nesting. #[allow(unused_variables)] fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> { let path_exists = path.exists(); @@ -231,15 +231,41 @@ fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> { return Ok(()); } + // Iterative implementation: collect all directories to create, then create them + // This avoids stack overflow with deeply nested directories if config.recursive { - match path.parent() { - Some(p) => create_dir(p, true, config)?, - None => { - USimpleError::new(1, translate!("mkdir-error-failed-to-create-tree")); + // Pre-allocate approximate capacity to avoid reallocations + let mut dirs_to_create = Vec::with_capacity(16); + let mut current = path; + + // First pass: collect all parent directories + while let Some(parent) = current.parent() { + if parent == Path::new("") { + break; + } + dirs_to_create.push(parent); + current = parent; + } + + // Second pass: create directories from root to leaf + // Only create those that don't exist + for dir in dirs_to_create.iter().rev() { + if !dir.exists() { + create_single_dir(dir, true, config)?; } } } + // Create the target directory + create_single_dir(path, is_parent, config) +} + +// Helper function to create a single directory with appropriate permissions +// `is_parent` argument is not used on windows +#[allow(unused_variables)] +fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> { + let path_exists = path.exists(); + match std::fs::create_dir(path) { Ok(()) => { if config.verbose { @@ -287,7 +313,24 @@ fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> { Ok(()) } - Err(_) if path.is_dir() => Ok(()), + Err(_) if path.is_dir() => { + // Directory already exists - check if this is a logical directory creation + // (i.e., not just a parent reference like "test_dir/..") + let ends_with_parent_dir = matches!( + path.components().next_back(), + Some(std::path::Component::ParentDir) + ); + + // Print verbose message for logical directories, even if they exist + // This matches GNU behavior for paths like "test_dir/../test_dir_a" + if config.verbose && is_parent && config.recursive && !ends_with_parent_dir { + println!( + "{}", + translate!("mkdir-verbose-created-directory", "util_name" => uucore::util_name(), "path" => path.quote()) + ); + } + Ok(()) + } Err(e) => Err(e.into()), } } diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index 1c734449f4f..5a77cb7fa56 100644 --- a/tests/by-util/test_mkdir.rs +++ b/tests/by-util/test_mkdir.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore bindgen testtest +// spell-checker:ignore bindgen testtest casetest CASETEST #![allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] @@ -362,6 +362,31 @@ fn test_mkdir_trailing_dot_and_slash() { println!("ls dest {}", result.stdout_str()); } +#[test] +fn test_mkdir_trailing_spaces_and_dots() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Test trailing space + scene.ucmd().arg("-p").arg("test ").succeeds(); + assert!(at.dir_exists("test ")); + + // Test multiple trailing spaces + scene.ucmd().arg("-p").arg("test ").succeeds(); + assert!(at.dir_exists("test ")); + + // Test leading dot (hidden on Unix) + scene.ucmd().arg("-p").arg(".hidden").succeeds(); + assert!(at.dir_exists(".hidden")); + + // Test trailing dot (should work) + scene.ucmd().arg("-p").arg("test.").succeeds(); + assert!(at.dir_exists("test.")); + + // Test multiple leading dots + scene.ucmd().arg("-p").arg("...test").succeeds(); + assert!(at.dir_exists("...test")); +} #[test] #[cfg(not(windows))] fn test_umask_compliance() { @@ -430,3 +455,356 @@ fn test_selinux_invalid() { // invalid context, so, no directory assert!(!at.dir_exists(dest)); } + +#[test] +fn test_mkdir_deep_nesting() { + // Regression test for stack overflow with deeply nested directories. + // The iterative implementation should handle arbitrary depth without stack overflow. + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create a path with 350 levels of nesting + let depth = 350; + let dir_name = "d"; + let mut path = std::path::PathBuf::new(); + for _ in 0..depth { + path.push(dir_name); + } + + scene.ucmd().arg("-p").arg(&path).succeeds(); + + assert!(at.dir_exists(&path)); +} + +#[test] +fn test_mkdir_dot_components() { + // Test handling of "." (current directory) components + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Test case from review comment: test/././test2/././test3/././test4 + // GNU mkdir normalizes this path and creates test/test2/test3/test4 + let path = "test/././test2/././test3/././test4"; + scene.ucmd().arg("-p").arg(path).succeeds(); + + // Verify expected structure exists (GNU compatibility) + assert!(at.dir_exists("test/test2/test3/test4")); + + // Test leading "." - should create test_dot/test_dot2 + scene + .ucmd() + .arg("-p") + .arg("./test_dot/test_dot2") + .succeeds(); + assert!(at.dir_exists("test_dot/test_dot2")); + + // Test mixed "." and normal components + scene + .ucmd() + .arg("-p") + .arg("mixed/./normal/./path") + .succeeds(); + assert!(at.dir_exists("mixed/normal/path")); + + // Test that the command works without creating redundant directories + // The key test is that it doesn't fail or create incorrect structure + // The actual filesystem behavior (whether literal "." dirs exist) + // may vary, but the logical result should be correct +} + +#[test] +fn test_mkdir_parent_dir_components() { + // Test handling of ".." (parent directory) components + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.mkdir("base"); + at.mkdir("base/child"); + + scene + .ucmd() + .arg("-p") + .arg("base/child/../sibling") + .succeeds(); + assert!(at.dir_exists("base/sibling")); + + scene + .ucmd() + .arg("-p") + .arg("base/child/../../other") + .succeeds(); + assert!(at.dir_exists("other")); + + scene + .ucmd() + .arg("-p") + .arg("base/child/../sibling") + .succeeds(); + assert!(at.dir_exists("base/sibling")); +} + +#[test] +fn test_mkdir_mixed_special_components() { + // Test complex paths with both "." and ".." components + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + scene + .ucmd() + .arg("-p") + .arg("./start/./middle/../end/./final") + .succeeds(); + assert!(at.dir_exists("start/end/final")); +} + +#[test] +fn test_mkdir_control_characters() { + // Test handling of control characters in filenames + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Test NUL byte (should fail on most systems) + // Note: This test is skipped because NUL bytes cause panics in the test framework + // let result = scene.ucmd().arg("test\x00name").run(); + // assert!(!result.succeeded()); + + // Test newline character (may fail on Windows) + let result = scene.ucmd().arg("-p").arg("test\nname").run(); + if result.succeeded() { + assert!(at.dir_exists("test\nname")); + } + + // Test tab character (may fail on Windows) + let result = scene.ucmd().arg("-p").arg("test\tname").run(); + if result.succeeded() { + assert!(at.dir_exists("test\tname")); + } + + // Test space character in directory name (should work on all systems) + scene.ucmd().arg("-p").arg("test name").succeeds(); + assert!(at.dir_exists("test name")); + + // Test double quotes in path - platform-specific behavior expected + // On Linux/Unix: Should succeed and create directory with quotes + // On Windows: Should fail with "Invalid argument" error + #[cfg(unix)] + { + scene.ucmd().arg("-pv").arg("a/\"\"/b/c").succeeds(); + assert!(at.dir_exists("a/\"\"/b/c")); + } + #[cfg(windows)] + { + let result = scene.ucmd().arg("-pv").arg("a/\"\"/b/c").run(); + // Should fail after creating 'a' directory + assert!(at.dir_exists("a")); + assert!(!at.dir_exists("a/\"\"")); + assert!(!result.succeeded()); + // Windows should reject directory names with quotes + // We don't assert on specific error message as it varies by Windows version + } + + // Test single quotes in path - should work on both Unix and Windows + scene.ucmd().arg("-p").arg("a/''/b/c").succeeds(); + assert!(at.dir_exists("a/''/b/c")); +} + +#[test] +fn test_mkdir_maximum_path_length() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Test moderate length path (should work on all systems) + let long_path = "a".repeat(50) + "/" + &"b".repeat(50) + "/" + &"c".repeat(50); + scene.ucmd().arg("-p").arg(&long_path).succeeds(); + assert!(at.dir_exists(&long_path)); + + // Test longer but reasonable path + let longer_path = "x".repeat(100) + "/" + &"y".repeat(50) + "/" + &"z".repeat(30); + scene.ucmd().arg("-p").arg(&longer_path).succeeds(); + assert!(at.dir_exists(&longer_path)); + + // Test extremely long path (may fail on some systems) + let very_long_path = "very_long_directory_name_".repeat(20) + "/final"; + let result = scene.ucmd().arg("-p").arg(&very_long_path).run(); + if result.succeeded() { + assert!(at.dir_exists(&very_long_path)); + } + // If it fails, that's acceptable on some systems with path limits +} + +#[test] +fn test_mkdir_reserved_device_names() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // These should work on Unix but may fail on Windows + let result = scene.ucmd().arg("-p").arg("CON").run(); + if result.succeeded() { + assert!(at.dir_exists("CON")); + } + + let result = scene.ucmd().arg("-p").arg("PRN").run(); + if result.succeeded() { + assert!(at.dir_exists("PRN")); + } + + let result = scene.ucmd().arg("-p").arg("AUX").run(); + if result.succeeded() { + assert!(at.dir_exists("AUX")); + } + + // Test device names with extensions + let result = scene.ucmd().arg("-p").arg("COM1").run(); + if result.succeeded() { + assert!(at.dir_exists("COM1")); + } + + let result = scene.ucmd().arg("-p").arg("LPT1").run(); + if result.succeeded() { + assert!(at.dir_exists("LPT1")); + } +} + +#[test] +fn test_mkdir_case_sensitivity() { + // Test case sensitivity behavior (varies by filesystem) + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create directory with lowercase name + scene.ucmd().arg("-p").arg("CaseTest").succeeds(); + assert!(at.dir_exists("CaseTest")); + + // Try to create directory with different case + // On case-sensitive filesystems: creates separate directory + // On case-insensitive filesystems: fails (directory already exists) + let result = scene.ucmd().arg("-p").arg("casetest").run(); + + // The test passes regardless of filesystem behavior + // We just verify the command doesn't crash + if result.succeeded() { + // Case-sensitive filesystem - both exist + assert!(at.dir_exists("CaseTest")); + assert!(at.dir_exists("casetest")); + } else { + // Case-insensitive filesystem - only one exists + assert!(at.dir_exists("CaseTest")); + } + + // Test mixed case variations + scene.ucmd().arg("-p").arg("CASETEST").succeeds(); + scene.ucmd().arg("-p").arg("caseTEST").succeeds(); +} + +#[test] +fn test_mkdir_network_paths() { + // Test network path formats (UNC paths on Windows) + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Test UNC-style path prefix (may fail on some systems) + let result = scene.ucmd().arg("-p").arg("//server/share/test").run(); + if result.succeeded() { + assert!(at.dir_exists("//server/share/test")); + } + // If it fails, that's acceptable on some systems with read-only restrictions + + // Test path that looks like network but is actually local + scene.ucmd().arg("-p").arg("server_share_test").succeeds(); + assert!(at.dir_exists("server_share_test")); + + // Test path with double slash in middle (should work) + scene.ucmd().arg("-p").arg("test//double//slash").succeeds(); + assert!(at.dir_exists("test//double//slash")); +} + +#[test] +fn test_mkdir_environment_expansion() { + // Test that mkdir doesn't expand environment variables (unlike some shells) + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + unsafe { + std::env::set_var("TEST_VAR", "expanded_value"); + } + + // Create directory with literal $VAR (should not expand) + scene.ucmd().arg("-p").arg("$TEST_VAR/dir").succeeds(); + assert!(at.dir_exists("$TEST_VAR/dir")); + + // Verify the literal name exists, not the expanded value + assert!(!at.dir_exists("expanded_value/dir")); + + // Test with braces + scene + .ucmd() + .arg("-p") + .arg("${TEST_VAR}_braced/dir") + .succeeds(); + assert!(at.dir_exists("${TEST_VAR}_braced/dir")); + + // Test with tilde (should not expand to home directory) + scene.ucmd().arg("-p").arg("~/test_dir").succeeds(); + assert!(at.dir_exists("~/test_dir")); + + unsafe { + std::env::remove_var("TEST_VAR"); + } +} + +#[test] +fn test_mkdir_concurrent_creation() { + // Test concurrent mkdir -p operations: 10 iterations, 8 threads, 40 levels nesting + use std::thread; + + for _ in 0..10 { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let mut dir = at.plus("concurrent_test"); + dir.push("a"); + + for _ in 0..40 { + dir.push("a"); + } + + let path_str = dir.to_string_lossy().to_string(); + let bin_path = scene.bin_path.clone(); + + let mut handles = vec![]; + + for _ in 0..8 { + let path_clone = path_str.clone(); + let bin_path_clone = bin_path.clone(); + + let handle = thread::spawn(move || { + // Use the actual uutils mkdir binary to test the real implementation + let result = std::process::Command::new(&bin_path_clone) + .arg("mkdir") + .arg("-p") + .arg(&path_clone) + .current_dir(std::env::current_dir().unwrap()) + .output(); + + match result { + Ok(output) => { + assert!( + output.status.success(), + "mkdir failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + Err(e) => panic!("Failed to execute mkdir: {e}"), + } + }); + handles.push(handle); + } + + handles + .drain(..) + .map(|handle| handle.join().unwrap()) + .count(); + + assert!(at.dir_exists(&path_str)); + } +}