Skip to content

fix(fs): can't use Windows path #1710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 2, 2024
Merged

fix(fs): can't use Windows path #1710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5ff57f0
Fix fs can't use Windows path
Legend-Master Sep 2, 2024
ba83100
Add change file
Legend-Master Sep 2, 2024
2e06cec
Implement `Deserialize` instead
Legend-Master Sep 2, 2024
79c00d7
Rename FilePath's visitor to FilePathVisitor
Legend-Master Sep 2, 2024
1ea593d
Merge branch 'v2' into fs-windows-path
Legend-Master Sep 2, 2024
58748a0
Add todo and test
Legend-Master Sep 2, 2024
a01580d
Clippy
Legend-Master Sep 2, 2024
3c53ac6
Unused variable
Legend-Master Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/fs-windows-path.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fs": patch
---

Fix can't use Windows paths like `C:/Users/UserName/file.txt`
58 changes: 53 additions & 5 deletions plugins/fs/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,44 @@ use std::{

use crate::{scope::Entry, Error, FilePath, FsExt};

#[derive(Debug, serde::Deserialize)]
#[serde(untagged)]
// TODO: Combine this with FilePath
#[derive(Debug)]
pub enum SafeFilePath {
Url(url::Url),
Path(SafePathBuf),
}

impl<'de> serde::Deserialize<'de> for SafeFilePath {
Copy link
Member

@amrbashir amrbashir Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that we have a duplicated enum definition with and now same deserialize implementation, maybe we can combine this logic into one, by using generics? and still export aliases without a generic:

enum FilePathOrUrl<T: FromStr> {
    Url(url::Url),
    Path(T),
}
pub type SafeFilePath = FilePath<SafeFilePath>;
pub type FilePath = FilePath<PathBuf>;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

enum FilePathOrUrl<T: FromStr> {
    Url(url::Url),
    Path(T),
}
pub type SafeFilePath = FilePath<SafeFilePath>;
pub type FilePath = FilePath<PathBuf>;

I'm not sure how that'll work, SafeFilePath doesn't implement FromStr, and that means we either need to do it in tauri or we need a wrapper

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best is to implement FromStr on in tauri. For now, we can just add a TODO then or merge as is.

fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
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<E>(self, s: &str) -> std::result::Result<Self::Value, E>
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)
}
}

impl From<SafeFilePath> for FilePath {
fn from(value: SafeFilePath) -> Self {
match value {
Expand All @@ -43,10 +74,11 @@ impl FromStr for SafeFilePath {
type Err = CommandError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
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())?))
}
}

Expand Down Expand Up @@ -1168,3 +1200,19 @@ fn get_stat(metadata: std::fs::Metadata) -> FileInfo {
blocks: usm!(blocks),
}
}

mod test {
#[test]
fn safe_file_path_parse() {
use super::SafeFilePath;

assert!(matches!(
serde_json::from_str::<SafeFilePath>("\"C:/Users\""),
Ok(SafeFilePath::Path(_))
));
assert!(matches!(
serde_json::from_str::<SafeFilePath>("\"file:///C:/Users\""),
Ok(SafeFilePath::Url(_))
));
}
}
42 changes: 37 additions & 5 deletions plugins/fs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,55 @@ pub use scope::{Event as ScopeEvent, Scope};

type Result<T> = std::result::Result<T, Error>;

// 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, serde::Deserialize)]
#[serde(untagged)]
#[derive(Debug)]
pub enum FilePath {
Url(url::Url),
Path(PathBuf),
}

impl<'de> serde::Deserialize<'de> for FilePath {
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
struct FilePathVisitor;

impl<'de> serde::de::Visitor<'de> for FilePathVisitor {
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<E>(self, s: &str) -> std::result::Result<Self::Value, E>
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(FilePathVisitor)
}
}

impl FromStr for FilePath {
type Err = Infallible;
fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
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)))
}
}

Expand Down