Merge pull request #215 from JRF63/issue199

Fix #199

Author: jgarzik

tree/common/mod.rs

@@ -175,13 +175,25 @@ where
             }
 
             unsafe {
+                // Creates the target directory with the same file permission bits as the source,
+                // modified by the umask of the process. Copying the permission bits without the
+                // umask is postponed to the `postprocess_dir` closure on the call to
+                // `traverse_directory` inside `copy_file`.
                 let ret = libc::mkdirat(
                     target_dirfd,
                     target_filename,
-                    source_md.mode() as libc::mode_t,
+                    // OR'ed with S_IRWXU according to the spec
+                    source_md.mode() as libc::mode_t | libc::S_IRWXU,
                 );
+
                 if ret != 0 {
-                    return Err(io::Error::last_os_error());
+                    let e = io::Error::last_os_error();
+                    let err_str = gettext!(
+                        "cannot create directory '{}': {}",
+                        target.display(),
+                        error_string(&e)
+                    );
+                    return Err(io::Error::other(err_str));
                 }
             }
         }
@@ -515,14 +527,25 @@ where
                             // not allow atomically creating a directory then opening it:
                             //
                             // https://stackoverflow.com/questions/45818628/whats-the-expected-behavior-of-openname-o-creato-directory-mode/48693137#48693137
-                            let new_target_dirfd = ftw::FileDescriptor::open_at(
+                            let new_target_dirfd = match ftw::FileDescriptor::open_at(
                                 target_dirfd,
                                 target_filename_cstr.as_ptr(),
                                 libc::O_RDONLY,
-                            )
-                            .map_err(|_| ())?;
-                            target_dirfd_stack_borrowed.push(new_target_dirfd);
+                            ) {
+                                Ok(fd) => fd,
+                                Err(e) => {
+                                    let err_str = gettext!(
+                                        "cannot open directory '{}': {}",
+                                        target.display(),
+                                        error_string(&e)
+                                    );
+                                    *last_error.borrow_mut() = Some(io::Error::other(err_str));
+                                    *terminate_borrowed = true;
+                                    return Ok(false);
+                                }
+                            };
 
+                            target_dirfd_stack_borrowed.push(new_target_dirfd);
                             target_dir_path_borrowed.push(target_filename);
 
                             true

tree/tests/cp/mod.rs

@@ -342,7 +342,7 @@ fn test_cp_fail_perm() {
         1,
     );
 
-    for f in [test_dir, d, d_a] {
+    for f in [test_dir, d, d_a, dd] {
         fs::set_permissions(f, fs::Permissions::from_mode(0o777)).unwrap();
     }
     fs::remove_dir_all(test_dir).unwrap();
@@ -939,3 +939,34 @@ fn test_cp_special_bits() {
 
     fs::remove_dir_all(test_dir).unwrap();
 }
+
+// Replicates failure to copy D/D on `test_cp_fail_perm` due to not OR'ing with S_IRWXU:
+// https://github.com/rustcoreutils/posixutils-rs/issues/199
+#[test]
+fn test_cp_issue199() {
+    let test_dir = &format!("{}/test_cp_issue199", env!("CARGO_TARGET_TMPDIR"));
+    let d = &format!("{test_dir}/D");
+    let dd = &format!("{test_dir}/DD");
+    let d_d = &format!("{test_dir}/D/D");
+
+    let setgid = libc::S_ISGID as u32;
+    let setuid = libc::S_ISUID as u32;
+
+    fs::create_dir(test_dir).unwrap();
+
+    let mode = fs::symlink_metadata(test_dir).unwrap().mode();
+    fs::set_permissions(test_dir, fs::Permissions::from_mode(mode & !setgid)).unwrap();
+
+    fs::create_dir(d).unwrap();
+    fs::create_dir(d_d).unwrap();
+
+    let nonperm_bits = fs::symlink_metadata(d).unwrap().mode() & !(setgid | setuid) & !0o777;
+    fs::set_permissions(d, fs::Permissions::from_mode(0o500 | nonperm_bits)).unwrap();
+
+    cp_test(&["-pR", d, dd], "", "", 0);
+
+    fs::set_permissions(d, fs::Permissions::from_mode(0o777)).unwrap();
+    fs::set_permissions(dd, fs::Permissions::from_mode(0o777)).unwrap();
+
+    fs::remove_dir_all(test_dir).unwrap();
+}