Add instance name verification

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 ## [Unreleased]
 
+### Added
+
+- File name verification to avoid creating corrupted instances (blocks some characters and names)
+
 ### Fixed
 
 - `debug` command no longer errors even when succeeding on Windows

src/core/processor/write.rs

@@ -10,14 +10,14 @@ use std::{
 use crate::{
 	argon_error,
 	core::{
-		meta::{Meta, NodePath, Source, SourceEntry, SourceKind, SyncbackFilter},
+		meta::{Meta, NodePath, Source, SourceEntry, SourceKind},
 		snapshot::{AddedSnapshot, Snapshot, UpdatedSnapshot},
 		tree::Tree,
 	},
 	ext::PathExt,
 	middleware::{data, dir, Middleware},
 	project::{Project, ProjectNode},
-	resolution::UnresolvedValue,
+	syncback::{serialize_properties, validate_properties, verify_name},
 	vfs::Vfs,
 	Properties,
 };
@@ -31,13 +31,19 @@ macro_rules! path_exists {
 	};
 }
 
+macro_rules! bad_name {
+	($err:expr) => {
+		argon_error!("Instance name is corrupted: {}", $err);
+	};
+}
+
 macro_rules! filter_warn {
 	($id:expr) => {
-		warn!("Instance {:?} does not match syncback filter! Skipping..", $id);
+		warn!("Instance {:?} does not pass syncback filter! Skipping..", $id);
 	};
 	($id:expr, $path:expr) => {
 		warn!(
-			"Path: {:?} (source of instance: {:?}) does not match syncback filter! Skipping..",
+			"Path: {:?} (source of instance: {:?}) does not pass syncback filter! Skipping..",
 			$path, $id
 		);
 	};
@@ -73,6 +79,11 @@ pub fn apply_addition(snapshot: AddedSnapshot, tree: &mut Tree, vfs: &Vfs) -> Re
 		parent_meta: &Meta,
 		vfs: &Vfs,
 	) -> Result<Option<Meta>> {
+		if let Err(err) = verify_name(&snapshot.name) {
+			bad_name!(err);
+			return Ok(None);
+		}
+
 		let mut meta = snapshot.meta.clone().with_context(&parent_meta.context);
 		let filter = parent_meta.context.syncback_filter();
 		let properties = snapshot.properties.clone();
@@ -183,8 +194,13 @@ pub fn apply_addition(snapshot: AddedSnapshot, tree: &mut Tree, vfs: &Vfs) -> Re
 				.with_context(|| format!("Failed to find sync rule for path: {}", parent_path.display()))?;
 
 			let name = sync_rule.get_name(&parent_path);
-
 			let folder_path = parent_path.with_file_name(&name);
+
+			if vfs.exists(&folder_path) {
+				path_exists!(folder_path);
+				return Ok(parent_meta.source.clone());
+			}
+
 			let file_path = sync_rule
 				.locate(&folder_path, &name, true)
 				.with_context(|| format!("Failed to locate file path for parent: {}", folder_path.display()))?;
@@ -423,10 +439,25 @@ pub fn apply_update(snapshot: UpdatedSnapshot, tree: &mut Tree, vfs: &Vfs) -> Re
 
 	match meta.source.get().clone() {
 		SourceKind::Path(path) => {
+			if let Some(properties) = snapshot.properties {
+				update_non_project_properties(&path, properties, instance, &mut meta, vfs)?;
+			}
+
+			// It has to be done after updating properties as it may change the file path
 			if let Some(name) = snapshot.name {
+				if let Err(err) = verify_name(&name) {
+					bad_name!(err);
+					return Ok(());
+				}
+
 				let new_path = path.with_file_name(path.get_name().replace(&instance.name, &name));
 				*meta.source.get_mut() = SourceKind::Path(new_path.clone());
 
+				if vfs.exists(&new_path) {
+					path_exists!(new_path);
+					return Ok(());
+				}
+
 				let filter = meta.context.syncback_filter();
 
 				if let Some(SourceEntry::Folder(path)) = meta.source.get_folder_mut() {
@@ -461,10 +492,6 @@ pub fn apply_update(snapshot: UpdatedSnapshot, tree: &mut Tree, vfs: &Vfs) -> Re
 				instance.name = name;
 			}
 
-			if let Some(properties) = snapshot.properties {
-				update_non_project_properties(&path, properties, instance, &mut meta, vfs)?;
-			}
-
 			tree.update_meta(snapshot.id, meta);
 
 			if let Some(_class) = snapshot.class {
@@ -510,6 +537,7 @@ pub fn apply_update(snapshot: UpdatedSnapshot, tree: &mut Tree, vfs: &Vfs) -> Re
 				}
 			}
 
+			// It has to be done after updating properties as it may change the node path
 			if let Some(new_name) = snapshot.name {
 				let parent_node = project.find_node_by_path(&node_path.parent()).with_context(|| {
 					format!("Failed to find parent project node with path {:?}", node_path.parent())
@@ -661,27 +689,3 @@ pub fn apply_removal(id: Ref, tree: &mut Tree, vfs: &Vfs) -> Result<()> {
 
 	Ok(())
 }
-
-fn serialize_properties(class: &str, properties: Properties) -> HashMap<String, UnresolvedValue> {
-	properties
-		.iter()
-		.map(|(property, varaint)| {
-			(
-				property.to_owned(),
-				UnresolvedValue::from_variant(varaint.clone(), class, property),
-			)
-		})
-		.collect()
-}
-
-fn validate_properties(properties: Properties, filter: &SyncbackFilter) -> Properties {
-	// Temporary solution for serde failing to deserialize empty HashMap
-	if properties.contains_key("ArgonEmpty") {
-		HashMap::new()
-	} else {
-		properties
-			.into_iter()
-			.filter(|(property, _)| !filter.matches_property(property))
-			.collect()
-	}
-}

src/lib.rs

@@ -21,6 +21,7 @@ pub mod server;
 pub mod sessions;
 pub mod stats;
 pub mod studio;
+pub mod syncback;
 pub mod updater;
 pub mod util;
 pub mod vfs;

src/syncback.rs

@@ -0,0 +1,84 @@
+use crate::{core::meta::SyncbackFilter, resolution::UnresolvedValue, Properties};
+use anyhow::{bail, Result};
+use colored::Colorize;
+use std::collections::HashMap;
+
+#[cfg(not(windows))]
+const FORBIDDEN_CHARACTERS: [char; 1] = ['/'];
+
+#[cfg(windows)]
+const FORBIDDEN_FILE_NAMES: [&str; 22] = [
+	"CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2",
+	"LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9",
+];
+
+#[cfg(windows)]
+const FORBIDDEN_CHARACTERS: [char; 9] = ['<', '>', ':', '"', '/', '\\', '|', '?', '*'];
+
+pub fn verify_name(name: &str) -> Result<()> {
+	if name.is_empty() {
+		bail!("file name cannot be empty");
+	}
+
+	if name.len() > 255 {
+		bail!("file name cannot be longer than 255 characters");
+	}
+
+	#[cfg(windows)]
+	if name.ends_with('.') {
+		bail!("file name cannot end with a period");
+	}
+
+	#[cfg(windows)]
+	if name.ends_with(' ') {
+		bail!("file name cannot end with a space");
+	}
+
+	for char in name.chars() {
+		if FORBIDDEN_CHARACTERS.contains(&char) {
+			bail!("file name cannot contain {} character", char.to_string().bold());
+		}
+
+		if char.is_whitespace() {
+			bail!("file name cannot contain whitespace");
+		}
+
+		#[cfg(windows)]
+		if char.is_control() {
+			bail!("file name cannot contain ASCII control characters");
+		}
+	}
+
+	#[cfg(windows)]
+	for file_name in FORBIDDEN_FILE_NAMES {
+		if name == file_name {
+			bail!("file cannot be named {}", file_name.bold());
+		}
+	}
+
+	Ok(())
+}
+
+pub fn validate_properties(properties: Properties, filter: &SyncbackFilter) -> Properties {
+	// Temporary solution for serde failing to deserialize empty HashMap
+	if properties.contains_key("ArgonEmpty") {
+		HashMap::new()
+	} else {
+		properties
+			.into_iter()
+			.filter(|(property, _)| !filter.matches_property(property))
+			.collect()
+	}
+}
+
+pub fn serialize_properties(class: &str, properties: Properties) -> HashMap<String, UnresolvedValue> {
+	properties
+		.iter()
+		.map(|(property, varaint)| {
+			(
+				property.to_owned(),
+				UnresolvedValue::from_variant(varaint.clone(), class, property),
+			)
+		})
+		.collect()
+}