Skip to content

Commit

Permalink
escape redirection path in eden config toml on Windows
Browse files Browse the repository at this point in the history
Summary:
S477425 happened because when we add a new redirection on windows, we didn't escape the backslashes on windows. (Look at the test plan before this change)
The root cause was in this fact that we didn't have any serialization for `redirection` and `redirection_targets` in `CheckoutConfig`
Then the file path save wrong in the toml file.
To fix this issue, we need to consider two things:
1- This diff adds a `serialize_path_map` serialization to both `redirection` and `redirection_targets`
2- Also, when we want to read path from the `config.toml` file, we should consider that all the \\ should convert to \ in addition to / to \
The changes on deserialized function consider these changes.
It is explained in the test plan that the redirect works fine and eden can successfully restart by these changes

Reviewed By: jdelliot

Differential Revision: D67293753

fbshipit-source-id: d27239c4d76d619a29e785bd4378d62b820216d5
  • Loading branch information
kavehahmadi60 authored and facebook-github-bot committed Dec 19, 2024
1 parent 05292cb commit 27d35f1
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 3 deletions.
39 changes: 37 additions & 2 deletions eden/fs/cli_rs/edenfs-client/src/checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use edenfs_utils::path_from_bytes;
#[cfg(windows)]
use edenfs_utils::strip_unc_prefix;
use edenfs_utils::varint::decode_varint;
use serde::ser::SerializeMap;
use serde::Deserialize;
use serde::Deserializer;
use serde::Serialize;
Expand Down Expand Up @@ -311,12 +312,16 @@ struct PredictivePrefetch {
pub struct CheckoutConfig {
repository: Repository,

#[serde(deserialize_with = "deserialize_redirections")]
#[serde(
deserialize_with = "deserialize_redirections",
serialize_with = "serialize_path_map"
)]
redirections: BTreeMap<PathBuf, RedirectionType>,

#[serde(
rename = "redirection-targets",
deserialize_with = "deserialize_redirection_targets",
serialize_with = "serialize_path_map",
default = "default_redirection_targets"
)]
redirection_targets: BTreeMap<PathBuf, PathBuf>,
Expand All @@ -332,6 +337,36 @@ fn default_redirection_targets() -> BTreeMap<PathBuf, PathBuf> {
BTreeMap::new()
}

fn serialize_path_map<V, S>(map: &BTreeMap<PathBuf, V>, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
V: Serialize,
{
let mut map_serializer = serializer.serialize_map(Some(map.len()))?;
for (key, value) in map {
let serialized_key = if cfg!(windows) {
// On Windows, we need to escape backslashes in the path, since
// TOML uses backslashes as an escape character.
// temp_key is used to make sure that the path is not already escaped
// from previous serialization, since we don't want to double-escape it.
let temp_key = key.display().to_string().replace("\\\\", "\\");
temp_key.replace('\\', "\\\\")
} else {
key.to_string_lossy().into_owned()
};
match map_serializer.serialize_entry(&serialized_key, value) {
Ok(_) => continue,
Err(e) => {
return Err(serde::ser::Error::custom(format!(
"Unsupported redirection. Target must be string. Error: {}",
e
)));
}
}
}
map_serializer.end()
}

fn deserialize_redirection_targets<'de, D>(
deserializer: D,
) -> Result<BTreeMap<PathBuf, PathBuf>, D::Error>
Expand All @@ -346,7 +381,7 @@ where
PathBuf::from(
// Convert path separator to backslash on Windows
if cfg!(windows) {
key.replace("/", "\\")
key.replace("\\\\", "\\").replace('/', "\\")
} else {
key
},
Expand Down
2 changes: 1 addition & 1 deletion eden/fs/cli_rs/edenfs-client/src/redirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,7 @@ where
PathBuf::from(
// Convert path separator to backslash on Windows
if cfg!(windows) {
key.replace("/", "\\")
key.replace("\\\\", "\\").replace('/', "\\")
} else {
key
},
Expand Down

0 comments on commit 27d35f1

Please sign in to comment.