remote: Fix missing path escaping in SFTP upload (#62239)
# Objective - Fixes #62238 - Properly escapes paths in the sftp PUT line ## Solution - 10-line wrapper function that escapes paths as sftp expects. Namely: paths in quotes with `\\` and `"` escaped. ## Testing - Did you test these changes? If so, how? `cargo check` - Are there any parts that need more testing? Up to you, this is a simple change and I dont think sftp with default install paths ever worked on MacOS - How can other people (reviewers) test your changes? Is there anything specific they need to know? Just connect to a remote on OSX and see if the logs had an sftp upload failure in there - If relevant, what platforms did you test these changes on, and are there any important ones you can't test? ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fix remote uploads over sftp where the paths contain spaces --------- Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>
This commit is contained in:
1 parent
7d34522515
commit
44612e574b
1 file changed
+74
-1
@@ -112,6 +112,27 @@ fn bracket_ipv6(host: &str) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
// Quote paths for sftp batch parsing and double backslashes for POSIX glob();
|
||||
// Win32-OpenSSH accepts the same encoding.
|
||||
fn escape_sftp_path(path: &str) -> String {
|
||||
let mut escaped = String::with_capacity(path.len() + 2);
|
||||
escaped.push('"');
|
||||
for character in path.chars() {
|
||||
if character == '"' || character == '\\' {
|
||||
escaped.push('\\');
|
||||
}
|
||||
escaped.push(character);
|
||||
}
|
||||
escaped.push('"');
|
||||
escaped
|
||||
}
|
||||
|
||||
fn sftp_put_command(source_path: &str, destination_path: &str) -> String {
|
||||
let source = escape_sftp_path(source_path);
|
||||
let destination = escape_sftp_path(destination_path);
|
||||
format!("put {source} {destination}\n")
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
|
||||
pub struct SshConnectionOptions {
|
||||
pub host: SshConnectionHost,
|
||||
@@ -1230,7 +1251,7 @@ impl SshRemoteConnection {
|
||||
if Self::is_sftp_available().await {
|
||||
log::debug!("using SFTP for file upload");
|
||||
let mut command = self.build_sftp_command();
|
||||
let sftp_batch = format!("put {src_path_display} {dest_path_str}\n");
|
||||
let sftp_batch = sftp_put_command(&src_path_display, &dest_path_str);
|
||||
|
||||
let mut child = command.spawn()?;
|
||||
if let Some(mut stdin) = child.stdin.take() {
|
||||
@@ -2175,6 +2196,58 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sftp_put_command_quotes_paths() {
|
||||
assert_eq!(
|
||||
sftp_put_command(
|
||||
"/tmp/Zed Repro/remote_server",
|
||||
".zed_server/downloaded server",
|
||||
),
|
||||
"put \"/tmp/Zed Repro/remote_server\" \".zed_server/downloaded server\"\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sftp_put_command_escapes_quotes_in_paths() {
|
||||
assert_eq!(
|
||||
sftp_put_command(
|
||||
r#"/tmp/Zed "Nightly"/remote_server"#,
|
||||
".zed_server/remote_server",
|
||||
),
|
||||
"put \"/tmp/Zed \\\"Nightly\\\"/remote_server\" \".zed_server/remote_server\"\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sftp_put_command_doubles_trailing_destination_backslash_before_closing_quote() {
|
||||
assert_eq!(
|
||||
sftp_put_command("/tmp/remote_server", r"C:\zed server\"),
|
||||
"put \"/tmp/remote_server\" \"C:\\\\zed server\\\\\"\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sftp_put_command_doubles_source_backslashes_for_posix_glob() {
|
||||
assert_eq!(
|
||||
sftp_put_command(
|
||||
r"/tmp/zed\server/remote_server",
|
||||
".zed_server/remote_server",
|
||||
),
|
||||
"put \"/tmp/zed\\\\server/remote_server\" \".zed_server/remote_server\"\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sftp_put_command_doubles_windows_source_backslashes_on_all_platforms() {
|
||||
assert_eq!(
|
||||
sftp_put_command(
|
||||
r"C:\Users\Smit\Zed Repro\remote_server",
|
||||
".zed_server/remote_server",
|
||||
),
|
||||
"put \"C:\\\\Users\\\\Smit\\\\Zed Repro\\\\remote_server\" \".zed_server/remote_server\"\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn scp_args_exclude_port_forward_flags() {
|
||||
let options = SshConnectionOptions {
|
||||
|
||||
Reference in new issue
Block a user