[ENG-516] Prevent copies/cuts to the same source/dest (#748)

* add basic UI protection to stop cut/copies to the same path

* add rust src/dest protections

* fix pasting logic

* fix path canonicalization

* skip paths instead of overwriting them

* Using non-blocking ops and fixing some warnings

---------

Co-authored-by: Ericson Soares <ericson.ds999@gmail.com>

core/src/api/mod.rs

@@ -1,7 +1,6 @@
 use chrono::{DateTime, Utc};
 use prisma_client_rust::{operator::or, Direction};
 use rspc::{Config, Type};
-use sd_file_ext::kind::ObjectKind;
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;
 
@@ -84,7 +83,7 @@ pub(crate) fn mount() -> Arc<Router> {
 			})
 		})
 		.library_query("search", |t| {
-			#[derive(Deserialize, Type, Debug)]
+			#[derive(Deserialize, Type, Debug, Clone, Copy)]
 			#[serde(rename_all = "camelCase")]
 			#[specta(inline)]
 			enum Ordering {
@@ -145,7 +144,7 @@ pub(crate) fn mount() -> Arc<Router> {
 					.search
 					.map(|search| {
 						search
-							.split(" ")
+							.split(' ')
 							.map(str::to_string)
 							.map(file_path::materialized_path::contains)
 							.map(Some)
@@ -158,7 +157,7 @@ pub(crate) fn mount() -> Arc<Router> {
 						args.kind
 							.map(|kind| file_path::object::is(vec![object::kind::equals(kind)])),
 						args.extension.map(file_path::extension::equals),
-						(args.tags.len() > 0).then(|| {
+						(!args.tags.is_empty()).then(|| {
 							file_path::object::is(vec![object::tags::some(vec![
 								tag_on_object::tag::is(vec![or(args
 									.tags

core/src/job/mod.rs

@@ -8,6 +8,7 @@ use std::{
 	collections::{hash_map::DefaultHasher, VecDeque},
 	fmt::Debug,
 	hash::{Hash, Hasher},
+	path::PathBuf,
 	sync::Arc,
 };
 
@@ -63,6 +64,10 @@ pub enum JobError {
 	IdentifierError(#[from] FileIdentifierJobError),
 	#[error("Crypto error: {0}")]
 	CryptoError(#[from] CryptoError),
+	#[error("source and destination path are the same: {}", .0.display())]
+	MatchingSrcDest(PathBuf),
+	#[error("action would overwrite another file: {}", .0.display())]
+	WouldOverwrite(PathBuf),
 
 	// Not errors
 	#[error("Job had a early finish: <name='{name}', reason='{reason}'>")]

core/src/object/fs/copy.rs

@@ -9,7 +9,8 @@ use std::{hash::Hash, path::PathBuf};
 
 use serde::{Deserialize, Serialize};
 use specta::Type;
-use tracing::trace;
+use tokio::fs;
+use tracing::{trace, warn};
 
 use super::{context_menu_fs_info, get_path_from_location_id, osstr_to_string, FsInfo};
 
@@ -138,9 +139,28 @@ impl StatefulJob for FileCopierJob {
 					);
 				}
 
-				trace!("Copying from {:?} to {:?}", path, target_path);
+				if fs::canonicalize(path.parent().ok_or(JobError::Path)?).await?
+					== fs::canonicalize(target_path.parent().ok_or(JobError::Path)?).await?
+				{
+					return Err(JobError::MatchingSrcDest(path.clone()));
+				}
 
-				tokio::fs::copy(&path, &target_path).await?;
+				if fs::metadata(&target_path).await.is_ok() {
+					// only skip as it could be half way through a huge directory copy and run into an issue
+					warn!(
+						"Skipping {} as it would be overwritten",
+						target_path.display()
+					);
+				// TODO(brxken128): could possibly return an error if the skipped file was the *only* file to be copied?
+				} else {
+					trace!(
+						"Copying from {} to {}",
+						path.display(),
+						target_path.display()
+					);
+
+					fs::copy(&path, &target_path).await?;
+				}
 			}
 			FileCopierJobStep::Directory { path } => {
 				// if this is the very first path, create the target dir
@@ -148,10 +168,10 @@ impl StatefulJob for FileCopierJob {
 				if job_state.source_fs_info.path_data.is_dir
 					&& &job_state.source_fs_info.fs_path == path
 				{
-					tokio::fs::create_dir_all(&job_state.target_path).await?;
+					fs::create_dir_all(&job_state.target_path).await?;
 				}
 
-				let mut dir = tokio::fs::read_dir(&path).await?;
+				let mut dir = fs::read_dir(&path).await?;
 
 				while let Some(entry) = dir.next_entry().await? {
 					if entry.metadata().await?.is_dir() {
@@ -159,7 +179,7 @@ impl StatefulJob for FileCopierJob {
 							.steps
 							.push_back(FileCopierJobStep::Directory { path: entry.path() });
 
-						tokio::fs::create_dir_all(
+						fs::create_dir_all(
 							job_state.target_path.join(
 								entry
 									.path()

core/src/object/fs/cut.rs

@@ -9,7 +9,8 @@ use std::{hash::Hash, path::PathBuf};
 
 use serde::{Deserialize, Serialize};
 use specta::Type;
-use tracing::trace;
+use tokio::fs;
+use tracing::{trace, warn};
 
 use super::{context_menu_fs_info, get_path_from_location_id, FsInfo};
 
@@ -84,9 +85,34 @@ impl StatefulJob for FileCutterJob {
 			.target_directory
 			.join(source_info.fs_path.file_name().ok_or(JobError::OsStr)?);
 
-		trace!("Cutting {:?} to {:?}", source_info.fs_path, full_output);
+		if fs::canonicalize(
+			source_info
+				.fs_path
+				.parent()
+				.map_or(Err(JobError::Path), Ok)?,
+		)
+		.await? == fs::canonicalize(full_output.parent().map_or(Err(JobError::Path), Ok)?)
+			.await?
+		{
+			return Err(JobError::MatchingSrcDest(source_info.fs_path.clone()));
+		}
 
-		tokio::fs::rename(&source_info.fs_path, &full_output).await?;
+		if fs::metadata(&full_output).await.is_ok() {
+			warn!(
+				"Skipping {} as it would be overwritten",
+				full_output.display()
+			);
+
+			return Err(JobError::WouldOverwrite(full_output));
+		}
+
+		trace!(
+			"Cutting {} to {}",
+			source_info.fs_path.display(),
+			full_output.display()
+		);
+
+		fs::rename(&source_info.fs_path, &full_output).await?;
 
 		ctx.progress(vec![JobReportUpdate::CompletedTaskCount(
 			state.step_number + 1,

crates/file-ext/src/kind.rs

@@ -1,5 +1,4 @@
 use serde::{Deserialize, Serialize};
-use specta::Type;
 
 #[repr(i32)]
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, Eq, PartialEq)]

interface/app/$libraryId/Explorer/ContextMenu.tsx

@@ -46,6 +46,13 @@ export default (props: PropsWithChildren) => {
 	const copyFiles = useLibraryMutation('files.copyFiles');
 	const cutFiles = useLibraryMutation('files.cutFiles');
 
+	const isPastable =
+		store.cutCopyState.sourceLocationId !== store.locationId
+			? true
+			: store.cutCopyState.sourcePath !== params.path
+			? true
+			: false;
+
 	return (
 		<CM.Root trigger={props.children}>
 			<OpenInNativeExplorer />
@@ -74,32 +81,34 @@ export default (props: PropsWithChildren) => {
 				icon={Repeat}
 			/>
 
-			<CM.Item
-				label="Paste"
-				keybind="⌘V"
-				hidden={!store.cutCopyState.active}
-				onClick={() => {
-					if (store.cutCopyState.actionType == 'Copy') {
-						store.locationId &&
-							copyFiles.mutate({
-								source_location_id: store.cutCopyState.sourceLocationId,
-								source_path_id: store.cutCopyState.sourcePathId,
-								target_location_id: store.locationId,
-								target_path: params.path,
-								target_file_name_suffix: null
-							});
-					} else {
-						store.locationId &&
-							cutFiles.mutate({
-								source_location_id: store.cutCopyState.sourceLocationId,
-								source_path_id: store.cutCopyState.sourcePathId,
-								target_location_id: store.locationId,
-								target_path: params.path
-							});
-					}
-				}}
-				icon={Clipboard}
-			/>
+			{isPastable && (
+				<CM.Item
+					label="Paste"
+					keybind="⌘V"
+					hidden={!store.cutCopyState.active}
+					onClick={() => {
+						if (store.cutCopyState.actionType == 'Copy') {
+							store.locationId &&
+								copyFiles.mutate({
+									source_location_id: store.cutCopyState.sourceLocationId,
+									source_path_id: store.cutCopyState.sourcePathId,
+									target_location_id: store.locationId,
+									target_path: params.path,
+									target_file_name_suffix: null
+								});
+						} else {
+							store.locationId &&
+								cutFiles.mutate({
+									source_location_id: store.cutCopyState.sourceLocationId,
+									source_path_id: store.cutCopyState.sourcePathId,
+									target_location_id: store.locationId,
+									target_path: params.path
+								});
+						}
+					}}
+					icon={Clipboard}
+				/>
+			)}
 
 			<CM.Item
 				label="Deselect"

interface/app/$libraryId/Explorer/File/ContextMenu.tsx

@@ -80,6 +80,7 @@ export default ({ data, className, ...props }: Props) => {
 					keybind="⌘X"
 					onClick={() => {
 						getExplorerStore().cutCopyState = {
+							sourcePath: params.path,
 							sourceLocationId: store.locationId!,
 							sourcePathId: data.item.id,
 							actionType: 'Cut',
@@ -94,6 +95,7 @@ export default ({ data, className, ...props }: Props) => {
 					keybind="⌘C"
 					onClick={() => {
 						getExplorerStore().cutCopyState = {
+							sourcePath: params.path,
 							sourceLocationId: store.locationId!,
 							sourcePathId: data.item.id,
 							actionType: 'Copy',

interface/hooks/useExplorerStore.tsx

@@ -26,6 +26,7 @@ const state = {
 	contextMenuActiveObject: null as object | null,
 	newThumbnails: {} as Record<string, boolean>,
 	cutCopyState: {
+		sourcePath: '', // this is used solely for preventing copy/cutting to the same path (as that will truncate the file)
 		sourceLocationId: 0,
 		sourcePathId: 0,
 		actionType: 'Cut',