From e58e92baee1f3121114befe73e2a7a1d1dba363e Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Tue, 3 Dec 2024 22:50:29 +0700 Subject: [PATCH 01/17] dep: add percent encoding support --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index ead1e5e..f46433b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ coinit_speed_over_memory = [] [dependencies] log = "0.4" +percent-encoding = "2.3.1" [dev-dependencies] serial_test = { version = "2.0.0", default-features = false } From 8481d3cccb561e67a98f79ae1bfa8f7ade87d6c4 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Tue, 3 Dec 2024 23:19:21 +0700 Subject: [PATCH 02/17] add from_utf8_lossy_pc %-encodes invalid bytes in otherwise valid utf8 strings --- src/macos.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/macos.rs b/src/macos.rs index 05eacf3..4391e05 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -135,6 +135,28 @@ fn to_string>(str_in: T) -> Result { } } +use std::borrow::Cow; +use percent_encoding::percent_encode_byte as b2pc; +fn from_utf8_lossy_pc(v:&[u8]) -> Cow<'_, str> { // std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by an special symbol. Valid utf8, including `%`, are not escaped, so this is still lossy. Useful for macOS file paths. + let mut iter = v.utf8_chunks(); + let (first_valid,first_invalid) = if let Some(chunk) = iter.next() { + let valid = chunk.valid(); + let invalid = chunk.invalid(); + if invalid.is_empty() {debug_assert_eq!(valid.len(), v.len()); // invalid=empty → last chunk + return Cow::Borrowed(valid);} + (valid,invalid) + } else {return Cow::Borrowed("" );}; + + let mut res = String::with_capacity(v.len()); res.push_str(first_valid); + first_invalid.iter().for_each(|b| {res.push_str(b2pc(*b));}); + for chunk in iter {res.push_str(chunk.valid()); + let invalid = chunk.invalid(); + if !invalid.is_empty() { + invalid .iter().for_each(|b| {res.push_str(b2pc(*b));});} + } + Cow::Owned(res) +} + #[cfg(test)] mod tests { use crate::{ From dc7dca02ba13b34d57f63244522044a17e88cecc Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 00:53:36 +0700 Subject: [PATCH 03/17] test: add for from_utf8_lossy_pc --- src/macos.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/macos.rs b/src/macos.rs index 4391e05..3aed6d5 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -160,12 +160,16 @@ fn from_utf8_lossy_pc(v:&[u8]) -> Cow<'_, str> { // std's from_utf8_lossy, but n #[cfg(test)] mod tests { use crate::{ - macos::{DeleteMethod, TrashContextExtMacos}, + macos::{DeleteMethod, TrashContextExtMacos, from_utf8_lossy_pc}, tests::{get_unique_name, init_logging}, TrashContext, }; use serial_test::serial; use std::fs::File; + use std::path::PathBuf; + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + use std::borrow::Cow; #[test] #[serial] @@ -179,4 +183,23 @@ mod tests { trash_ctx.delete(&path).unwrap(); assert!(File::open(&path).is_err()); } + + #[test] + fn test_path_byte() { + let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) + let pcvalid_utf8 = "%80"; // valid macOS path in a %-escaped encoding + + let mut p = PathBuf::new(); p.push(get_unique_name()); //trash-test-111-0 + let mut path_pcvalid_utf8 = p.clone(); + let mut path_invalid = p.clone(); + + path_invalid.push(OsStr::from_bytes(invalid_utf8)); // trash-test-111-0/\x80 + path_pcvalid_utf8.push(pcvalid_utf8); // trash-test-111-0/%80 + + let path_invalid_byte = path_invalid.as_os_str().as_encoded_bytes(); + let pc_encode: Cow<'_, str> = from_utf8_lossy_pc(&path_invalid_byte); + let path_invalid_pc = PathBuf::from(pc_encode.as_ref()); // trash-test-111-0/%80 + + assert_eq!(path_pcvalid_utf8, path_invalid_pc); + } } From 3978204c7b5d7ca1038717da3238c82f7bb6a6c6 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 13:27:19 +0700 Subject: [PATCH 04/17] dep: add simdutf8 for fast utf8 validation --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index f46433b..3eab3b7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ coinit_speed_over_memory = [] [dependencies] log = "0.4" percent-encoding = "2.3.1" +simdutf8 = "0.1.5" [dev-dependencies] serial_test = { version = "2.0.0", default-features = false } From d7d218743dc1e4793a49b0aa95e752e740c2a9c3 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 13:34:15 +0700 Subject: [PATCH 05/17] remove automatic panicky conversion of potentially binary paths into non-binary strings Issue: #124 --- src/macos.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/macos.rs b/src/macos.rs index 3aed6d5..595e361 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -62,10 +62,9 @@ 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), } } } @@ -126,15 +125,6 @@ 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 }), - } -} - use std::borrow::Cow; use percent_encoding::percent_encode_byte as b2pc; fn from_utf8_lossy_pc(v:&[u8]) -> Cow<'_, str> { // std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by an special symbol. Valid utf8, including `%`, are not escaped, so this is still lossy. Useful for macOS file paths. From 1d18e7a9ae7a92a909c4dc34b441d799f0466b0b Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 13:32:39 +0700 Subject: [PATCH 06/17] convert delete_using_file_mgr to use binary Paths --- src/macos.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/macos.rs b/src/macos.rs index 595e361..896fb5b 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -1,4 +1,4 @@ -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}; @@ -69,11 +69,15 @@ impl TrashContext { } } -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_b = path.as_ref().as_os_str().as_encoded_bytes(); + let string = match simdutf8::basic::from_utf8(path_b) { + Ok(path_utf8) => NSString::from_str(path_utf8), // utf-8 path, use as is + Err(_) => NSString::from_str(&from_utf8_lossy_pc(path_b)) // binary path, %-encode it + }; trace!("Starting fileURLWithPath"); let url = unsafe { NSURL::fileURLWithPath(&string) }; @@ -85,7 +89,7 @@ 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()), }); } } From c013b9a95bfde455a85dcf5b382ee7a6aaf145a2 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 13:35:42 +0700 Subject: [PATCH 07/17] convert delete_using_finder to use binary Paths --- src/macos.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/macos.rs b/src/macos.rs index 896fb5b..1dc22be 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -96,12 +96,12 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> { Ok(()) } -fn delete_using_finder(full_paths: Vec) -> Result<(), Error> { +fn delete_using_finder + std::fmt::Debug>(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.into_iter().map(|p| format!("POSIX file \"{p:?}\"")).collect::>().join(", "); let script = format!("tell application \"Finder\" to delete {{ {posix_files} }}"); let argv: Vec = vec!["-e".into(), script.into()]; From 6fbad98299ffde1acf2a63552d39e4085664d6f1 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 15:04:03 +0700 Subject: [PATCH 08/17] dep: move macos deps behind macos cfg target --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3eab3b7..981544e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,8 +21,6 @@ coinit_speed_over_memory = [] [dependencies] log = "0.4" -percent-encoding = "2.3.1" -simdutf8 = "0.1.5" [dev-dependencies] serial_test = { version = "2.0.0", default-features = false } @@ -41,6 +39,8 @@ objc2-foundation = { version = "0.2.0", features = [ "NSString", "NSURL", ] } +percent-encoding = "2.3.1" +simdutf8 = "0.1.5" [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 = [ From bfbc394a1aba8cb3f348c77f3dffc18a59dde28f Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 16:52:41 +0700 Subject: [PATCH 09/17] test: new delete illegal bytes Disabled since only works on older FS, but tested manually to work on a USB HFS drive --- src/macos.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/macos.rs b/src/macos.rs index 1dc22be..7dd5302 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -178,6 +178,29 @@ mod tests { assert!(File::open(&path).is_err()); } + // DISABLED: test only works on file systems that support binary paths (not APFS), not sure what CI has + // successfully tested on a local HFS usb flash drive + #[test] + #[serial] + fn test_delete_binary_path_with_ns_file_manager() { + let parent_fs_supports_binary = "/Volumes/Untitled"; // USB drive that supports non-utf8 paths + + 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 p = PathBuf::new(); + p.push(parent_fs_supports_binary); // /Volumes/Untitled + p.push(get_unique_name()); // /Volumes/Untitled/trash-test-111-0 + let mut path_invalid = p.clone(); + 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(); + // trash_ctx.delete(&path_invalid).unwrap(); + // assert!(File::open(&path_invalid).is_err()); + } + #[test] fn test_path_byte() { let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) From ab5c49ba45d94fdfa1ed6919daf1d61627de8ab3 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 17:21:12 +0700 Subject: [PATCH 10/17] cargo fmt --- src/macos.rs | 84 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/src/macos.rs b/src/macos.rs index 7dd5302..a1c8c8b 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -1,4 +1,8 @@ -use std::{ffi::OsString, path::{Path, PathBuf}, process::Command}; +use std::{ + ffi::OsString, + path::{Path, PathBuf}, + process::Command, +}; use log::trace; use objc2_foundation::{NSFileManager, NSString, NSURL}; @@ -69,14 +73,14 @@ impl TrashContext { } } -fn delete_using_file_mgr>(full_paths: &[P]) -> 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 path_b = path.as_ref().as_os_str().as_encoded_bytes(); let string = match simdutf8::basic::from_utf8(path_b) { Ok(path_utf8) => NSString::from_str(path_utf8), // utf-8 path, use as is - Err(_) => NSString::from_str(&from_utf8_lossy_pc(path_b)) // binary path, %-encode it + Err(_) => NSString::from_str(&from_utf8_lossy_pc(path_b)), // binary path, %-encode it }; trace!("Starting fileURLWithPath"); @@ -89,19 +93,20 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> { if let Err(err) = res { return Err(Error::Unknown { - description: format!("While deleting '{:?}', `trashItemAtURL` failed: {err}",path.as_ref()), + description: format!("While deleting '{:?}', `trashItemAtURL` failed: {err}", path.as_ref()), }); } } Ok(()) } -fn delete_using_finder + std::fmt::Debug>(full_paths: &[P]) -> Result<(), Error> { +fn delete_using_finder + std::fmt::Debug>(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.into_iter().map(|p| format!("POSIX file \"{p:?}\"")).collect::>().join(", "); let script = format!("tell application \"Finder\" to delete {{ {posix_files} }}"); let argv: Vec = vec!["-e".into(), script.into()]; @@ -129,41 +134,53 @@ fn delete_using_finder + std::fmt::Debug>(full_paths: &[P]) -> Res Ok(()) } -use std::borrow::Cow; use percent_encoding::percent_encode_byte as b2pc; -fn from_utf8_lossy_pc(v:&[u8]) -> Cow<'_, str> { // std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by an special symbol. Valid utf8, including `%`, are not escaped, so this is still lossy. Useful for macOS file paths. - let mut iter = v.utf8_chunks(); - let (first_valid,first_invalid) = if let Some(chunk) = iter.next() { - let valid = chunk.valid(); - let invalid = chunk.invalid(); - if invalid.is_empty() {debug_assert_eq!(valid.len(), v.len()); // invalid=empty → last chunk - return Cow::Borrowed(valid);} - (valid,invalid) - } else {return Cow::Borrowed("" );}; - - let mut res = String::with_capacity(v.len()); res.push_str(first_valid); - first_invalid.iter().for_each(|b| {res.push_str(b2pc(*b));}); - for chunk in iter {res.push_str(chunk.valid()); - let invalid = chunk.invalid(); - if !invalid.is_empty() { - invalid .iter().for_each(|b| {res.push_str(b2pc(*b));});} - } - Cow::Owned(res) +use std::borrow::Cow; +fn from_utf8_lossy_pc(v: &[u8]) -> Cow<'_, str> { + // std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by an special symbol. Valid utf8, including `%`, are not escaped, so this is still lossy. Useful for macOS file paths. + let mut iter = v.utf8_chunks(); + let (first_valid, first_invalid) = if let Some(chunk) = iter.next() { + let valid = chunk.valid(); + let invalid = chunk.invalid(); + if invalid.is_empty() { + debug_assert_eq!(valid.len(), v.len()); // invalid=empty → last chunk + return Cow::Borrowed(valid); + } + (valid, invalid) + } else { + return Cow::Borrowed(""); + }; + + let mut res = String::with_capacity(v.len()); + res.push_str(first_valid); + first_invalid.iter().for_each(|b| { + res.push_str(b2pc(*b)); + }); + for chunk in iter { + res.push_str(chunk.valid()); + let invalid = chunk.invalid(); + if !invalid.is_empty() { + invalid.iter().for_each(|b| { + res.push_str(b2pc(*b)); + }); + } + } + Cow::Owned(res) } #[cfg(test)] mod tests { use crate::{ - macos::{DeleteMethod, TrashContextExtMacos, from_utf8_lossy_pc}, + macos::{from_utf8_lossy_pc, DeleteMethod, TrashContextExtMacos}, tests::{get_unique_name, init_logging}, TrashContext, }; use serial_test::serial; - use std::fs::File; - use std::path::PathBuf; + use std::borrow::Cow; use std::ffi::OsStr; + use std::fs::File; use std::os::unix::ffi::OsStrExt; - use std::borrow::Cow; + use std::path::PathBuf; #[test] #[serial] @@ -192,7 +209,7 @@ mod tests { let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) let mut p = PathBuf::new(); p.push(parent_fs_supports_binary); // /Volumes/Untitled - p.push(get_unique_name()); // /Volumes/Untitled/trash-test-111-0 + p.push(get_unique_name()); // /Volumes/Untitled/trash-test-111-0 let mut path_invalid = p.clone(); path_invalid.set_extension(OsStr::from_bytes(invalid_utf8)); //...trash-test-111-0.\x80 (not push to avoid fail unexisting dir) @@ -204,11 +221,12 @@ mod tests { #[test] fn test_path_byte() { let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) - let pcvalid_utf8 = "%80"; // valid macOS path in a %-escaped encoding + let pcvalid_utf8 = "%80"; // valid macOS path in a %-escaped encoding - let mut p = PathBuf::new(); p.push(get_unique_name()); //trash-test-111-0 + let mut p = PathBuf::new(); + p.push(get_unique_name()); //trash-test-111-0 let mut path_pcvalid_utf8 = p.clone(); - let mut path_invalid = p.clone(); + let mut path_invalid = p.clone(); path_invalid.push(OsStr::from_bytes(invalid_utf8)); // trash-test-111-0/\x80 path_pcvalid_utf8.push(pcvalid_utf8); // trash-test-111-0/%80 From d7295e8edd0934d3d1b3e7e0f9cc745b5d103ce2 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 17:34:36 +0700 Subject: [PATCH 11/17] fix finder extra escaping --- src/macos.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/macos.rs b/src/macos.rs index a1c8c8b..5966f2a 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -106,7 +106,7 @@ fn delete_using_finder + std::fmt::Debug>(full_paths: &[P]) -> Re // 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(", "); + full_paths.into_iter().map(|p| format!("POSIX file {p:?}")).collect::>().join(", "); let script = format!("tell application \"Finder\" to delete {{ {posix_files} }}"); let argv: Vec = vec!["-e".into(), script.into()]; From 5d17879c0c823209f05d4cbc21629079c67e51ba Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 17:42:45 +0700 Subject: [PATCH 12/17] cargo fmt --- src/macos.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/macos.rs b/src/macos.rs index 5966f2a..39e8952 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -105,8 +105,7 @@ fn delete_using_finder + std::fmt::Debug>(full_paths: &[P]) -> Re // 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.into_iter().map(|p| format!("POSIX file {p:?}")).collect::>().join(", "); let script = format!("tell application \"Finder\" to delete {{ {posix_files} }}"); let argv: Vec = vec!["-e".into(), script.into()]; From 0359a4d13975e2fc5717052b2aa23e0394c7b619 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 17:48:29 +0700 Subject: [PATCH 13/17] fix Finder path generation for AS use the same %-conversion --- src/macos.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/macos.rs b/src/macos.rs index 39e8952..a73dbf0 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -100,12 +100,18 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> Ok(()) } -fn delete_using_finder + std::fmt::Debug>(full_paths: &[P]) -> 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.into_iter().map(|p| { + let path_b = p.as_ref().as_os_str().as_encoded_bytes(); + match simdutf8::basic::from_utf8(path_b) { + Ok(path_utf8) => format!("POSIX file \"{path_utf8}\""), // utf-8 path, use as is + Err(_) => format!("POSIX file \"{}\"",&from_utf8_lossy_pc(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()]; From 175d6f5de323b2fed7c8049eaf6bb91266171b30 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 17:48:48 +0700 Subject: [PATCH 14/17] test: new delete illegal bytes via Finder Disabled since only works on older FS, but tested manually to work on a USB HFS drive --- src/macos.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/macos.rs b/src/macos.rs index a73dbf0..fa7674b 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -223,6 +223,29 @@ mod tests { // assert!(File::open(&path_invalid).is_err()); } + // DISABLED: test only works on file systems that support binary paths (not APFS), not sure what CI has + // successfully tested on a local HFS usb flash drive + #[test] + #[serial] + fn test_delete_binary_path_with_finder() { + let parent_fs_supports_binary = "/Volumes/Untitled"; // USB drive that supports non-utf8 paths + + init_logging(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::Finder); + + let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) + let mut p = PathBuf::new(); + p.push(parent_fs_supports_binary); // /Volumes/Untitled + p.push(get_unique_name()); // /Volumes/Untitled/trash-test-111-0 + let mut path_invalid = p.clone(); + 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(); + // trash_ctx.delete(&path_invalid).unwrap(); + // assert!(File::open(&path_invalid).is_err()); + } + #[test] fn test_path_byte() { let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) From e499a0e1a49dbf3f2e672d1d42cc7b54944b6d9d Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 17:50:12 +0700 Subject: [PATCH 15/17] cargo fmt --- src/macos.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/macos.rs b/src/macos.rs index fa7674b..fae5553 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -105,13 +105,17 @@ fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { // 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| { - let path_b = p.as_ref().as_os_str().as_encoded_bytes(); - match simdutf8::basic::from_utf8(path_b) { - Ok(path_utf8) => format!("POSIX file \"{path_utf8}\""), // utf-8 path, use as is - Err(_) => format!("POSIX file \"{}\"",&from_utf8_lossy_pc(path_b)), // binary path, %-encode it - } - }).collect::>().join(", "); + let posix_files = full_paths + .into_iter() + .map(|p| { + let path_b = p.as_ref().as_os_str().as_encoded_bytes(); + match simdutf8::basic::from_utf8(path_b) { + Ok(path_utf8) => format!("POSIX file \"{path_utf8}\""), // utf-8 path, use as is + Err(_) => format!("POSIX file \"{}\"", &from_utf8_lossy_pc(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()]; From b147384fb820a0577d8228b5872d6dac338d4cf5 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 17:52:55 +0700 Subject: [PATCH 16/17] clippy --- src/macos.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/macos.rs b/src/macos.rs index fae5553..6340f12 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -106,7 +106,7 @@ fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { // 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() + .iter() .map(|p| { let path_b = p.as_ref().as_os_str().as_encoded_bytes(); match simdutf8::basic::from_utf8(path_b) { From d23a59166d52c90d7ee02ca2fb356cad0b330eca Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 5 Dec 2024 07:27:14 +0100 Subject: [PATCH 17/17] various refactors * avoid unused import (it's only used on Linux) * make tests work locally (and possibly on CI) by creating temporary filesystems. --- Cargo.toml | 4 +- src/macos.rs | 133 +++++++++++++++++++++++-------------------------- tests/trash.rs | 2 +- 3 files changed, 65 insertions(+), 74 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 981544e..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] @@ -40,7 +39,6 @@ objc2-foundation = { version = "0.2.0", features = [ "NSURL", ] } percent-encoding = "2.3.1" -simdutf8 = "0.1.5" [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 6340f12..b83af5e 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -77,14 +77,14 @@ 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 path_b = path.as_ref().as_os_str().as_encoded_bytes(); - let string = match simdutf8::basic::from_utf8(path_b) { + 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(&from_utf8_lossy_pc(path_b)), // binary path, %-encode it + 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"); @@ -109,9 +109,9 @@ fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { .iter() .map(|p| { let path_b = p.as_ref().as_os_str().as_encoded_bytes(); - match simdutf8::basic::from_utf8(path_b) { + 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 \"{}\"", &from_utf8_lossy_pc(path_b)), // binary path, %-encode it + Err(_) => format!("POSIX file \"{}\"", &percent_encode(path_b)), // binary path, %-encode it } }) .collect::>() @@ -143,35 +143,29 @@ fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { Ok(()) } -use percent_encoding::percent_encode_byte as b2pc; +/// 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 from_utf8_lossy_pc(v: &[u8]) -> Cow<'_, str> { - // std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by an special symbol. Valid utf8, including `%`, are not escaped, so this is still lossy. Useful for macOS file paths. - let mut iter = v.utf8_chunks(); - let (first_valid, first_invalid) = if let Some(chunk) = iter.next() { - let valid = chunk.valid(); - let invalid = chunk.invalid(); - if invalid.is_empty() { - debug_assert_eq!(valid.len(), v.len()); // invalid=empty → last chunk - return Cow::Borrowed(valid); +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()); } - (valid, invalid) } else { return Cow::Borrowed(""); }; - let mut res = String::with_capacity(v.len()); - res.push_str(first_valid); - first_invalid.iter().for_each(|b| { - res.push_str(b2pc(*b)); - }); + 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() { - invalid.iter().for_each(|b| { - res.push_str(b2pc(*b)); - }); + for byte in invalid { + res.push_str(b2pc(*byte)); + } } } Cow::Owned(res) @@ -180,16 +174,16 @@ fn from_utf8_lossy_pc(v: &[u8]) -> Cow<'_, str> { #[cfg(test)] mod tests { use crate::{ - macos::{from_utf8_lossy_pc, DeleteMethod, TrashContextExtMacos}, + macos::{percent_encode, DeleteMethod, TrashContextExtMacos}, tests::{get_unique_name, init_logging}, TrashContext, }; use serial_test::serial; - use std::borrow::Cow; use std::ffi::OsStr; use std::fs::File; use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; + use std::process::Command; #[test] #[serial] @@ -204,69 +198,68 @@ mod tests { assert!(File::open(&path).is_err()); } - // DISABLED: test only works on file systems that support binary paths (not APFS), not sure what CI has - // successfully tested on a local HFS usb flash drive #[test] #[serial] fn test_delete_binary_path_with_ns_file_manager() { - let parent_fs_supports_binary = "/Volumes/Untitled"; // USB drive that supports non-utf8 paths + 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 p = PathBuf::new(); - p.push(parent_fs_supports_binary); // /Volumes/Untitled - p.push(get_unique_name()); // /Volumes/Untitled/trash-test-111-0 - let mut path_invalid = p.clone(); + 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(); - // trash_ctx.delete(&path_invalid).unwrap(); - // assert!(File::open(&path_invalid).is_err()); - } - - // DISABLED: test only works on file systems that support binary paths (not APFS), not sure what CI has - // successfully tested on a local HFS usb flash drive - #[test] - #[serial] - fn test_delete_binary_path_with_finder() { - let parent_fs_supports_binary = "/Volumes/Untitled"; // USB drive that supports non-utf8 paths - - init_logging(); - let mut trash_ctx = TrashContext::default(); - trash_ctx.set_delete_method(DeleteMethod::Finder); - - let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) - let mut p = PathBuf::new(); - p.push(parent_fs_supports_binary); // /Volumes/Untitled - p.push(get_unique_name()); // /Volumes/Untitled/trash-test-111-0 - let mut path_invalid = p.clone(); - 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(); - // File::create_new(&path_invalid).unwrap(); - // trash_ctx.delete(&path_invalid).unwrap(); - // assert!(File::open(&path_invalid).is_err()); + 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 pcvalid_utf8 = "%80"; // valid macOS path in a %-escaped encoding + let percent_encoded = "%80"; // valid macOS path in a %-escaped encoding - let mut p = PathBuf::new(); - p.push(get_unique_name()); //trash-test-111-0 - let mut path_pcvalid_utf8 = p.clone(); - let mut path_invalid = p.clone(); + let mut expected_path = PathBuf::from(get_unique_name()); + let mut path_with_invalid_utf8 = expected_path.clone(); - path_invalid.push(OsStr::from_bytes(invalid_utf8)); // trash-test-111-0/\x80 - path_pcvalid_utf8.push(pcvalid_utf8); // trash-test-111-0/%80 + 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 path_invalid_byte = path_invalid.as_os_str().as_encoded_bytes(); - let pc_encode: Cow<'_, str> = from_utf8_lossy_pc(&path_invalid_byte); - let path_invalid_pc = PathBuf::from(pc_encode.as_ref()); // 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()?; - assert_eq!(path_pcvalid_utf8, path_invalid_pc); + // 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();