From 5ff57f0fd6e059d460137fa36fa79dfca92db368 Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 2 Sep 2024 14:07:28 +0800 Subject: [PATCH 1/7] Fix fs can't use Windows path --- plugins/fs/src/commands.rs | 23 ++++++++++++++++++++--- plugins/fs/src/lib.rs | 23 ++++++++++++++++++++--- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/plugins/fs/src/commands.rs b/plugins/fs/src/commands.rs index 72289434c0..25fe8417b1 100644 --- a/plugins/fs/src/commands.rs +++ b/plugins/fs/src/commands.rs @@ -26,10 +26,26 @@ use crate::{scope::Entry, Error, FilePath, FsExt}; #[derive(Debug, serde::Deserialize)] #[serde(untagged)] pub enum SafeFilePath { + #[serde(deserialize_with = "deserialize_url")] Url(url::Url), Path(SafePathBuf), } +fn deserialize_url<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let url = url::Url::deserialize(deserializer)?; + let scheme = url.scheme(); + if scheme.len() != 1 { + Ok(url) + } else { + Err(serde::de::Error::custom(format!( + "Single letter scheme \"{scheme}\" is not supported because it conflicts with Windows paths" + ))) + } +} + impl From for FilePath { fn from(value: SafeFilePath) -> Self { match value { @@ -43,10 +59,11 @@ impl FromStr for SafeFilePath { type Err = CommandError; fn from_str(s: &str) -> Result { if let Ok(url) = url::Url::from_str(s) { - Ok(Self::Url(url)) - } else { - Ok(Self::Path(SafePathBuf::new(s.into())?)) + if url.scheme().len() != 1 { + return Ok(Self::Url(url)); + } } + Ok(Self::Path(SafePathBuf::new(s.into())?)) } } diff --git a/plugins/fs/src/lib.rs b/plugins/fs/src/lib.rs index 6b46ea366e..d37c653905 100644 --- a/plugins/fs/src/lib.rs +++ b/plugins/fs/src/lib.rs @@ -55,18 +55,35 @@ type Result = std::result::Result; #[derive(Debug, serde::Deserialize)] #[serde(untagged)] pub enum FilePath { + #[serde(deserialize_with = "deserialize_url")] Url(url::Url), Path(PathBuf), } +fn deserialize_url<'de, D>(deserializer: D) -> std::result::Result +where + D: serde::Deserializer<'de>, +{ + let url: url::Url = Deserialize::deserialize::(deserializer)?; + let scheme = url.scheme(); + if scheme.len() != 1 { + Ok(url) + } else { + Err(serde::de::Error::custom(format!( + "Single letter scheme \"{scheme}\" is not supported because it conflicts with Windows paths" + ))) + } +} + impl FromStr for FilePath { type Err = Infallible; fn from_str(s: &str) -> std::result::Result { if let Ok(url) = url::Url::from_str(s) { - Ok(Self::Url(url)) - } else { - Ok(Self::Path(PathBuf::from(s))) + if url.scheme().len() != 1 { + return Ok(Self::Url(url)); + } } + Ok(Self::Path(PathBuf::from(s))) } } From ba83100a5e02477bbea34f134a6b4e9487a67020 Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 2 Sep 2024 14:13:57 +0800 Subject: [PATCH 2/7] Add change file --- .changes/fs-windows-path.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/fs-windows-path.md diff --git a/.changes/fs-windows-path.md b/.changes/fs-windows-path.md new file mode 100644 index 0000000000..561e561923 --- /dev/null +++ b/.changes/fs-windows-path.md @@ -0,0 +1,5 @@ +--- +"fs": patch +--- + +Fix can't use Windows paths like `C:/Users/UserName/file.txt` From 2e06cecf7023586f1e741b0088383ba3d1b96180 Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 2 Sep 2024 21:50:56 +0800 Subject: [PATCH 3/7] Implement `Deserialize` instead --- plugins/fs/src/commands.rs | 44 +++++++++++++++++++++++++------------- plugins/fs/src/lib.rs | 44 +++++++++++++++++++++++++------------- 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/plugins/fs/src/commands.rs b/plugins/fs/src/commands.rs index 25fe8417b1..efe98bb95a 100644 --- a/plugins/fs/src/commands.rs +++ b/plugins/fs/src/commands.rs @@ -23,26 +23,40 @@ use std::{ use crate::{scope::Entry, Error, FilePath, FsExt}; -#[derive(Debug, serde::Deserialize)] -#[serde(untagged)] +#[derive(Debug)] pub enum SafeFilePath { - #[serde(deserialize_with = "deserialize_url")] Url(url::Url), Path(SafePathBuf), } -fn deserialize_url<'de, D>(deserializer: D) -> Result -where - D: serde::Deserializer<'de>, -{ - let url = url::Url::deserialize(deserializer)?; - let scheme = url.scheme(); - if scheme.len() != 1 { - Ok(url) - } else { - Err(serde::de::Error::custom(format!( - "Single letter scheme \"{scheme}\" is not supported because it conflicts with Windows paths" - ))) +impl<'de> serde::Deserialize<'de> for SafeFilePath { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct SafeFilePathVisitor; + + impl<'de> serde::de::Visitor<'de> for SafeFilePathVisitor { + type Value = SafeFilePath; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a string representing an file URL or a path") + } + + fn visit_str(self, s: &str) -> std::result::Result + where + E: serde::de::Error, + { + SafeFilePath::from_str(s).map_err(|e| { + serde::de::Error::invalid_value( + serde::de::Unexpected::Str(s), + &e.to_string().as_str(), + ) + }) + } + } + + deserializer.deserialize_str(SafeFilePathVisitor) } } diff --git a/plugins/fs/src/lib.rs b/plugins/fs/src/lib.rs index d37c653905..e26d3dd02d 100644 --- a/plugins/fs/src/lib.rs +++ b/plugins/fs/src/lib.rs @@ -52,26 +52,40 @@ type Result = std::result::Result; /// Represents either a filesystem path or a URI pointing to a file /// such as `file://` URIs or Android `content://` URIs. -#[derive(Debug, serde::Deserialize)] -#[serde(untagged)] +#[derive(Debug)] pub enum FilePath { - #[serde(deserialize_with = "deserialize_url")] Url(url::Url), Path(PathBuf), } -fn deserialize_url<'de, D>(deserializer: D) -> std::result::Result -where - D: serde::Deserializer<'de>, -{ - let url: url::Url = Deserialize::deserialize::(deserializer)?; - let scheme = url.scheme(); - if scheme.len() != 1 { - Ok(url) - } else { - Err(serde::de::Error::custom(format!( - "Single letter scheme \"{scheme}\" is not supported because it conflicts with Windows paths" - ))) +impl<'de> serde::Deserialize<'de> for FilePath { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + struct SafeFilePathVisitor; + + impl<'de> serde::de::Visitor<'de> for SafeFilePathVisitor { + type Value = FilePath; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a string representing an file URL or a path") + } + + fn visit_str(self, s: &str) -> std::result::Result + where + E: serde::de::Error, + { + FilePath::from_str(s).map_err(|e| { + serde::de::Error::invalid_value( + serde::de::Unexpected::Str(s), + &e.to_string().as_str(), + ) + }) + } + } + + deserializer.deserialize_str(SafeFilePathVisitor) } } From 79c00d71adcc5bb46a9267e918e9f13fcbc3f5d8 Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 2 Sep 2024 21:52:08 +0800 Subject: [PATCH 4/7] Rename FilePath's visitor to FilePathVisitor --- plugins/fs/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/fs/src/lib.rs b/plugins/fs/src/lib.rs index e26d3dd02d..a86432c5bf 100644 --- a/plugins/fs/src/lib.rs +++ b/plugins/fs/src/lib.rs @@ -63,9 +63,9 @@ impl<'de> serde::Deserialize<'de> for FilePath { where D: serde::Deserializer<'de>, { - struct SafeFilePathVisitor; + struct FilePathVisitor; - impl<'de> serde::de::Visitor<'de> for SafeFilePathVisitor { + impl<'de> serde::de::Visitor<'de> for FilePathVisitor { type Value = FilePath; fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { @@ -85,7 +85,7 @@ impl<'de> serde::Deserialize<'de> for FilePath { } } - deserializer.deserialize_str(SafeFilePathVisitor) + deserializer.deserialize_str(FilePathVisitor) } } From 58748a09f10c6f180680ba593ccc5097e8fdba61 Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 2 Sep 2024 23:10:51 +0800 Subject: [PATCH 5/7] Add todo and test --- plugins/fs/src/commands.rs | 27 +++++++++++++++++++++++++++ plugins/fs/src/lib.rs | 1 + 2 files changed, 28 insertions(+) diff --git a/plugins/fs/src/commands.rs b/plugins/fs/src/commands.rs index efe98bb95a..4123698b75 100644 --- a/plugins/fs/src/commands.rs +++ b/plugins/fs/src/commands.rs @@ -23,6 +23,7 @@ use std::{ use crate::{scope::Entry, Error, FilePath, FsExt}; +// TODO: Combine this with FilePath #[derive(Debug)] pub enum SafeFilePath { Url(url::Url), @@ -1199,3 +1200,29 @@ fn get_stat(metadata: std::fs::Metadata) -> FileInfo { blocks: usm!(blocks), } } + +mod test { + use super::*; + + #[test] + fn windows_path() { + assert_eq!( + matches!( + serde_json::from_str::("\"C:/Users\""), + Ok(SafeFilePath::Path(_)) + ), + true + ); + } + + #[test] + fn url() { + assert_eq!( + matches!( + serde_json::from_str::("\"file:///C:/Users\""), + Ok(SafeFilePath::Url(_)) + ), + true + ); + } +} diff --git a/plugins/fs/src/lib.rs b/plugins/fs/src/lib.rs index a86432c5bf..975c126254 100644 --- a/plugins/fs/src/lib.rs +++ b/plugins/fs/src/lib.rs @@ -50,6 +50,7 @@ pub use scope::{Event as ScopeEvent, Scope}; type Result = std::result::Result; +// TODO: Combine this with SafeFilePath /// Represents either a filesystem path or a URI pointing to a file /// such as `file://` URIs or Android `content://` URIs. #[derive(Debug)] From a01580d7bfa45bd31cd5d38d56ff743f8e25f3e0 Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 2 Sep 2024 23:13:48 +0800 Subject: [PATCH 6/7] Clippy --- plugins/fs/src/commands.rs | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/plugins/fs/src/commands.rs b/plugins/fs/src/commands.rs index 4123698b75..485001781d 100644 --- a/plugins/fs/src/commands.rs +++ b/plugins/fs/src/commands.rs @@ -1205,24 +1205,14 @@ mod test { use super::*; #[test] - fn windows_path() { - assert_eq!( - matches!( - serde_json::from_str::("\"C:/Users\""), - Ok(SafeFilePath::Path(_)) - ), - true - ); - } - - #[test] - fn url() { - assert_eq!( - matches!( - serde_json::from_str::("\"file:///C:/Users\""), - Ok(SafeFilePath::Url(_)) - ), - true - ); + fn safe_file_path_parse() { + assert!(matches!( + serde_json::from_str::("\"C:/Users\""), + Ok(SafeFilePath::Path(_)) + )); + assert!(matches!( + serde_json::from_str::("\"file:///C:/Users\""), + Ok(SafeFilePath::Url(_)) + )); } } From 3c53ac6514ea3bc1151ddd1615f89280af71237a Mon Sep 17 00:00:00 2001 From: Tony Date: Mon, 2 Sep 2024 23:16:42 +0800 Subject: [PATCH 7/7] Unused variable --- plugins/fs/src/commands.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/fs/src/commands.rs b/plugins/fs/src/commands.rs index 485001781d..184d952622 100644 --- a/plugins/fs/src/commands.rs +++ b/plugins/fs/src/commands.rs @@ -1202,10 +1202,10 @@ fn get_stat(metadata: std::fs::Metadata) -> FileInfo { } mod test { - use super::*; - #[test] fn safe_file_path_parse() { + use super::SafeFilePath; + assert!(matches!( serde_json::from_str::("\"C:/Users\""), Ok(SafeFilePath::Path(_))