diff --git a/Cargo.toml b/Cargo.toml index ead1e5e..9cbd9c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,3 @@ - - [package] name = "trash" version = "5.2.0" @@ -29,6 +27,7 @@ rand = "0.8.5" once_cell = "1.18.0" env_logger = "0.10.0" tempfile = "3.8.0" +defer = "0.2.1" [target.'cfg(target_os = "macos")'.dependencies] @@ -39,6 +38,7 @@ objc2-foundation = { version = "0.2.0", features = [ "NSString", "NSURL", ] } +percent-encoding = "2.3.1" [target.'cfg(all(unix, not(target_os = "macos"), not(target_os = "ios"), not(target_os = "android")))'.dependencies] chrono = { version = "0.4.31", optional = true, default-features = false, features = [ diff --git a/src/macos.rs b/src/macos.rs index 3f33279..5f2031f 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -1,4 +1,8 @@ -use std::{ffi::OsString, path::PathBuf, process::Command}; +use std::{ + ffi::OsString, + path::{Path, PathBuf}, + process::Command, +}; use log::trace; use objc2_foundation::{NSFileManager, NSString, NSURL}; @@ -71,22 +75,25 @@ impl TrashContextExtMacos for TrashContext { } impl TrashContext { pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec) -> Result<(), Error> { - let full_paths = full_paths.into_iter().map(to_string).collect::, _>>()?; match self.platform_specific.delete_method { - DeleteMethod::Finder => delete_using_finder(full_paths), - DeleteMethod::NsFileManager => delete_using_file_mgr(full_paths), + DeleteMethod::Finder => delete_using_finder(&full_paths), + DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths), } } } -fn delete_using_file_mgr(full_paths: Vec) -> Result<(), Error> { +fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> { trace!("Starting delete_using_file_mgr"); let file_mgr = unsafe { NSFileManager::defaultManager() }; for path in full_paths { - let string = NSString::from_str(&path); + let path = path.as_ref().as_os_str().as_encoded_bytes(); + let path = match std::str::from_utf8(path) { + Ok(path_utf8) => NSString::from_str(path_utf8), // utf-8 path, use as is + Err(_) => NSString::from_str(&percent_encode(path)), // binary path, %-encode it + }; trace!("Starting fileURLWithPath"); - let url = unsafe { NSURL::fileURLWithPath(&string) }; + let url = unsafe { NSURL::fileURLWithPath(&path) }; trace!("Finished fileURLWithPath"); trace!("Calling trashItemAtURL"); @@ -95,19 +102,29 @@ fn delete_using_file_mgr(full_paths: Vec) -> Result<(), Error> { if let Err(err) = res { return Err(Error::Unknown { - description: format!("While deleting '{path}', `trashItemAtURL` failed: {err}"), + description: format!("While deleting '{:?}', `trashItemAtURL` failed: {err}", path.as_ref()), }); } } Ok(()) } -fn delete_using_finder(full_paths: Vec) -> Result<(), Error> { +fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { // AppleScript command to move files (or directories) to Trash looks like // osascript -e 'tell application "Finder" to delete { POSIX file "file1", POSIX "file2" }' // The `-e` flag is used to execute only one line of AppleScript. let mut command = Command::new("osascript"); - let posix_files = full_paths.into_iter().map(|p| format!("POSIX file \"{p}\"")).collect::>().join(", "); + let posix_files = full_paths + .iter() + .map(|p| { + let path_b = p.as_ref().as_os_str().as_encoded_bytes(); + match std::str::from_utf8(path_b) { + Ok(path_utf8) => format!("POSIX file \"{path_utf8}\""), // utf-8 path, use as is + Err(_) => format!("POSIX file \"{}\"", &percent_encode(path_b)), // binary path, %-encode it + } + }) + .collect::>() + .join(", "); let script = format!("tell application \"Finder\" to delete {{ {posix_files} }}"); let argv: Vec = vec!["-e".into(), script.into()]; @@ -135,24 +152,47 @@ fn delete_using_finder(full_paths: Vec) -> Result<(), Error> { Ok(()) } -fn to_string>(str_in: T) -> Result { - let os_string = str_in.into(); - let s = os_string.to_str(); - match s { - Some(s) => Ok(s.to_owned()), - None => Err(Error::ConvertOsString { original: os_string }), +/// std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by a special symbol. +/// Valid utf8, including `%`, are not escaped. +use std::borrow::Cow; +fn percent_encode(input: &[u8]) -> Cow<'_, str> { + use percent_encoding::percent_encode_byte as b2pc; + + let mut iter = input.utf8_chunks().peekable(); + if let Some(chunk) = iter.peek() { + if chunk.invalid().is_empty() { + return Cow::Borrowed(chunk.valid()); + } + } else { + return Cow::Borrowed(""); + }; + + let mut res = String::with_capacity(input.len()); + for chunk in iter { + res.push_str(chunk.valid()); + let invalid = chunk.invalid(); + if !invalid.is_empty() { + for byte in invalid { + res.push_str(b2pc(*byte)); + } + } } + Cow::Owned(res) } #[cfg(test)] mod tests { use crate::{ - macos::{DeleteMethod, TrashContextExtMacos}, + macos::{percent_encode, DeleteMethod, TrashContextExtMacos}, tests::{get_unique_name, init_logging}, TrashContext, }; use serial_test::serial; + use std::ffi::OsStr; use std::fs::File; + use std::os::unix::ffi::OsStrExt; + use std::path::PathBuf; + use std::process::Command; #[test] #[serial] @@ -166,4 +206,69 @@ mod tests { trash_ctx.delete(&path).unwrap(); assert!(File::open(&path).is_err()); } + + #[test] + #[serial] + fn test_delete_binary_path_with_ns_file_manager() { + let (_cleanup, tmp) = create_hfs_volume().unwrap(); + let parent_fs_supports_binary = tmp.path(); + + init_logging(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::NsFileManager); + + let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) + let mut path_invalid = parent_fs_supports_binary.join(get_unique_name()); + path_invalid.set_extension(OsStr::from_bytes(invalid_utf8)); //...trash-test-111-0.\x80 (not push to avoid fail unexisting dir) + + File::create_new(&path_invalid).unwrap(); + + assert!(path_invalid.exists()); + trash_ctx.delete(&path_invalid).unwrap(); + assert!(!path_invalid.exists()); + } + + #[test] + fn test_path_byte() { + let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) + let percent_encoded = "%80"; // valid macOS path in a %-escaped encoding + + let mut expected_path = PathBuf::from(get_unique_name()); + let mut path_with_invalid_utf8 = expected_path.clone(); + + path_with_invalid_utf8.push(OsStr::from_bytes(invalid_utf8)); // trash-test-111-0/\x80 + expected_path.push(percent_encoded); // trash-test-111-0/%80 + + let actual = percent_encode(&path_with_invalid_utf8.as_os_str().as_encoded_bytes()); // trash-test-111-0/%80 + assert_eq!(std::path::Path::new(actual.as_ref()), expected_path); + } + + fn create_hfs_volume() -> std::io::Result<(impl Drop, tempfile::TempDir)> { + let tmp = tempfile::tempdir()?; + let dmg_file = tmp.path().join("fs.dmg"); + let cleanup = { + // Create dmg file + Command::new("hdiutil").args(["create", "-size", "1m", "-fs", "HFS+"]).arg(&dmg_file).status()?; + + // Mount dmg file into temporary location + Command::new("hdiutil") + .args(["attach", "-nobrowse", "-mountpoint"]) + .arg(tmp.path()) + .arg(&dmg_file) + .status()?; + + // Ensure that the mount point is always cleaned up + defer::defer({ + let mount_point = tmp.path().to_owned(); + move || { + Command::new("hdiutil") + .arg("detach") + .arg(&mount_point) + .status() + .expect("detach temporary test dmg filesystem successfully"); + } + }) + }; + Ok((cleanup, tmp)) + } } diff --git a/tests/trash.rs b/tests/trash.rs index 70f6400..817a24c 100644 --- a/tests/trash.rs +++ b/tests/trash.rs @@ -1,4 +1,3 @@ -use std::ffi::OsStr; use std::fs::{create_dir, File}; use std::path::{Path, PathBuf}; @@ -143,6 +142,7 @@ fn create_remove_single_file() { #[test] #[serial] fn create_remove_single_file_invalid_utf8() { + use std::ffi::OsStr; let name = unsafe { OsStr::from_encoded_bytes_unchecked(&[168]) }; File::create_new(name).unwrap(); trash::delete(name).unwrap();