From 8e986206ac41c04518599e3df4cb780cbc5c1e85 Mon Sep 17 00:00:00 2001
From: rm-dr <96270320+rm-dr@users.noreply.github.com>
Date: Sat, 23 Nov 2024 17:29:56 -0800
Subject: [PATCH] cp: copy attributes after making subdir

---
 src/uu/cp/src/copydir.rs | 48 ++++++++++++++++++++++++++++++++--------
 src/uu/cp/src/cp.rs      |  2 +-
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs
index 3bfc5524913..42b696909f0 100644
--- a/src/uu/cp/src/copydir.rs
+++ b/src/uu/cp/src/copydir.rs
@@ -12,6 +12,7 @@ use std::collections::{HashMap, HashSet};
 use std::env;
 use std::fs;
 use std::io;
+use std::os::unix::fs::PermissionsExt;
 use std::path::{Path, PathBuf, StripPrefixError};
 
 use indicatif::ProgressBar;
@@ -27,7 +28,7 @@ use walkdir::{DirEntry, WalkDir};
 
 use crate::{
     aligned_ancestors, context_for, copy_attributes, copy_file, copy_link, CopyResult, Error,
-    Options,
+    Options, Preserve,
 };
 
 /// Ensure a Windows path starts with a `\\?`.
@@ -246,7 +247,7 @@ fn copy_direntry(
         if target_is_file {
             return Err("cannot overwrite non-directory with directory".into());
         } else {
-            build_dir(options, &local_to_target, false)?;
+            build_dir(&local_to_target, false, options, Some(&source_absolute))?;
             if options.verbose {
                 println!("{}", context_for(&source_relative, &local_to_target));
             }
@@ -371,7 +372,7 @@ pub(crate) fn copy_directory(
     let tmp = if options.parents {
         if let Some(parent) = root.parent() {
             let new_target = target.join(parent);
-            build_dir(options, &new_target, true)?;
+            build_dir(&new_target, true, options, None)?;
             if options.verbose {
                 // For example, if copying file `a/b/c` and its parents
                 // to directory `d/`, then print
@@ -469,15 +470,22 @@ pub fn path_has_prefix(p1: &Path, p2: &Path) -> io::Result<bool> {
 /// Builds a directory at the specified path with the given options.
 ///
 /// # Notes
-/// - It excludes certain permissions if ownership or special mode bits could
-///   potentially change.
+/// - If `copy_attributes_from` is `Some`, the new directory's attributes will be
+///   copied from the provided file. Otherwise, the new directory will have the default
+///   attributes for the current user.
+/// - This method excludes certain permissions if ownership or special mode bits could
+///   potentially change. (See `test_dir_perm_race_with_preserve_mode_and_ownership``)
 /// - The `recursive` flag determines whether parent directories should be created
 ///   if they do not already exist.
-// we need to allow unused_variable since `options` might be unused in non unix systems
-#[allow(unused_variables)]
-fn build_dir(options: &Options, path: &PathBuf, recursive: bool) -> CopyResult<()> {
+fn build_dir(
+    path: &PathBuf,
+    recursive: bool,
+    options: &Options,
+    copy_attributes_from: Option<&Path>,
+) -> CopyResult<()> {
     let mut builder = fs::DirBuilder::new();
     builder.recursive(recursive);
+
     // To prevent unauthorized access before the folder is ready,
     // exclude certain permissions if ownership or special mode bits
     // could potentially change.
@@ -494,11 +502,33 @@ fn build_dir(options: &Options, path: &PathBuf, recursive: bool) -> CopyResult<(
             } else {
                 0
             } as u32;
-        excluded_perms |= uucore::mode::get_umask();
+
+        let umask = if copy_attributes_from.is_some()
+            && matches!(options.attributes.mode, Preserve::Yes { .. })
+        {
+            !fs::symlink_metadata(copy_attributes_from.unwrap())?
+                .permissions()
+                .mode()
+        } else {
+            uucore::mode::get_umask()
+        };
+
+        excluded_perms |= umask;
         let mode = !excluded_perms & 0o777; //use only the last three octet bits
         std::os::unix::fs::DirBuilderExt::mode(&mut builder, mode);
     }
+
     builder.create(path)?;
+
+    if let Some(f) = copy_attributes_from {
+        // We intentionally exclude certain permissions above,
+        // do not overwrite them.
+        // All other attributes should be copied, though.
+        let mut filtered_permissions = options.attributes.clone();
+        filtered_permissions.mode = Preserve::No { explicit: true };
+        copy_attributes(f, path, &filtered_permissions)?;
+    }
+
     Ok(())
 }
 
diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs
index 32168b09009..438822ca19c 100644
--- a/src/uu/cp/src/cp.rs
+++ b/src/uu/cp/src/cp.rs
@@ -172,7 +172,7 @@ pub enum CopyMode {
 /// For full compatibility with GNU, these options should also combine. We
 /// currently only do a best effort imitation of that behavior, because it is
 /// difficult to achieve in clap, especially with `--no-preserve`.
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 pub struct Attributes {
     #[cfg(unix)]
     pub ownership: Preserve,