From c57eb2f4b911d9e1d98e0b7bae32771e67d92316 Mon Sep 17 00:00:00 2001 From: Riccardo Strina Date: Mon, 22 Dec 2025 23:41:17 +0100 Subject: [PATCH 1/6] Revert quoting all paths on Windows - Add double quotes only to paths that are going to be used by the node proxy as args - Rust functions are already handling the instance where a space is present in the path --- src/debugger.rs | 9 +++------ src/java.rs | 14 ++++++++------ src/jdk.rs | 4 ++-- src/jdtls.rs | 16 ++++++++-------- src/util.rs | 44 +++++++++++++++++--------------------------- 5 files changed, 38 insertions(+), 49 deletions(-) diff --git a/src/debugger.rs b/src/debugger.rs index 67371df..e536aa3 100644 --- a/src/debugger.rs +++ b/src/debugger.rs @@ -12,10 +12,7 @@ use zed_extension_api::{ use crate::{ config::get_java_debug_jar, lsp::LspWrapper, - util::{ - create_path_if_not_exists, get_curr_dir, path_to_quoted_string, - should_use_local_or_download, - }, + util::{create_path_if_not_exists, get_curr_dir, path_to_string, should_use_local_or_download}, }; #[derive(Serialize, Deserialize, Debug)] @@ -155,7 +152,7 @@ impl Debugger { download_file( JAVA_DEBUG_PLUGIN_FORK_URL, - &path_to_quoted_string(jar_path.clone())?, + &path_to_string(jar_path.clone())?, DownloadedFileType::Uncompressed, ) .map_err(|err| { @@ -259,7 +256,7 @@ impl Debugger { download_file( url.as_str(), - &path_to_quoted_string(&jar_path)?, + &path_to_string(&jar_path)?, DownloadedFileType::Uncompressed, ) .map_err(|err| format!("Failed to download {url} {err}"))?; diff --git a/src/java.rs b/src/java.rs index 4a22ee8..87e3b05 100644 --- a/src/java.rs +++ b/src/java.rs @@ -15,7 +15,7 @@ use std::{ use zed_extension_api::{ self as zed, CodeLabel, CodeLabelSpan, DebugAdapterBinary, DebugTaskDefinition, Extension, LanguageServerId, LanguageServerInstallationStatus, StartDebuggingRequestArguments, - StartDebuggingRequestArgumentsRequest, Worktree, + StartDebuggingRequestArgumentsRequest, Worktree, current_platform, lsp::{Completion, CompletionKind}, register_extension, serde_json::{Value, json}, @@ -32,7 +32,7 @@ use crate::{ try_to_fetch_and_install_latest_lombok, }, lsp::LspWrapper, - util::path_to_quoted_string, + util::{path_to_string, quote_path_for_os}, }; const PROXY_FILE: &str = include_str!("proxy.mjs"); @@ -279,17 +279,19 @@ impl Extension for Java { "--input-type=module".to_string(), "-e".to_string(), PROXY_FILE.to_string(), - path_to_quoted_string(current_dir.clone())?, + path_to_string(current_dir.clone())?, ]; // Add lombok as javaagent if settings.java.jdt.ls.lombokSupport.enabled is true let lombok_jvm_arg = if is_lombok_enabled(&configuration) { let lombok_jar_path = self.lombok_jar_path(language_server_id, &configuration, worktree)?; - let canonical_lombok_jar_path = - path_to_quoted_string(current_dir.join(lombok_jar_path))?; + let canonical_lombok_jar_path = path_to_string(current_dir.join(lombok_jar_path))?; - Some(format!("-javaagent:{canonical_lombok_jar_path}")) + Some(format!( + "-javaagent:{}", + quote_path_for_os(canonical_lombok_jar_path, current_platform().0) + )) } else { None }; diff --git a/src/jdk.rs b/src/jdk.rs index aab5f47..0e128c3 100644 --- a/src/jdk.rs +++ b/src/jdk.rs @@ -6,7 +6,7 @@ use zed_extension_api::{ set_language_server_installation_status, }; -use crate::util::{get_curr_dir, path_to_quoted_string, remove_all_files_except}; +use crate::util::{get_curr_dir, path_to_string, remove_all_files_except}; // Errors const JDK_DIR_ERROR: &str = "Failed to read into JDK install directory"; @@ -79,7 +79,7 @@ pub fn try_to_fetch_and_install_latest_jdk( download_file( build_corretto_url(&version, &platform, &arch).as_str(), - path_to_quoted_string(install_path.clone())?.as_str(), + path_to_string(install_path.clone())?.as_str(), match zed::current_platform().0 { Os::Windows => DownloadedFileType::Zip, _ => DownloadedFileType::GzipTar, diff --git a/src/jdtls.rs b/src/jdtls.rs index da915fb..ea8d8b1 100644 --- a/src/jdtls.rs +++ b/src/jdtls.rs @@ -19,7 +19,7 @@ use crate::{ jdk::try_to_fetch_and_install_latest_jdk, util::{ create_path_if_not_exists, get_curr_dir, get_java_exec_name, get_java_executable, - get_java_major_version, get_latest_versions_from_tag, path_to_quoted_string, + get_java_major_version, get_latest_versions_from_tag, path_to_string, quote_path_for_os, remove_all_files_except, should_use_local_or_download, }, }; @@ -65,14 +65,14 @@ pub fn build_jdtls_launch_args( let jdtls_data_path = get_jdtls_data_path(worktree)?; let mut args = vec![ - path_to_quoted_string(java_executable)?, + path_to_string(java_executable)?, "-Declipse.application=org.eclipse.jdt.ls.core.id1".to_string(), "-Dosgi.bundles.defaultStartLevel=4".to_string(), "-Declipse.product=org.eclipse.jdt.ls.core.product".to_string(), "-Dosgi.checkConfiguration=true".to_string(), format!( "-Dosgi.sharedConfiguration.area={}", - path_to_quoted_string(shared_config_path)? + quote_path_for_os(path_to_string(shared_config_path)?, current_platform().0) ), "-Dosgi.sharedConfiguration.area.readOnly=true".to_string(), "-Dosgi.configuration.cascaded=true".to_string(), @@ -86,9 +86,9 @@ pub fn build_jdtls_launch_args( args.extend(jvm_args); args.extend(vec![ "-jar".to_string(), - path_to_quoted_string(jar_path)?, + path_to_string(jar_path)?, "-data".to_string(), - path_to_quoted_string(jdtls_data_path)?, + path_to_string(jdtls_data_path)?, ]); if java_major_version >= 24 { args.push("-Djdk.xml.maxGeneralEntitySizeLimit=0".to_string()); @@ -195,10 +195,10 @@ pub fn try_to_fetch_and_install_latest_jdtls( &format!( "https://www.eclipse.org/downloads/download.php?file=/jdtls/milestones/{latest_version}/{latest_version_build}" ), - path_to_quoted_string(build_path.clone())?.as_str(), + path_to_string(build_path.clone())?.as_str(), DownloadedFileType::GzipTar, )?; - make_file_executable(path_to_quoted_string(binary_path)?.as_str())?; + make_file_executable(path_to_string(binary_path)?.as_str())?; // ...and delete other versions let _ = remove_all_files_except(prefix, build_directory.as_str()); @@ -241,7 +241,7 @@ pub fn try_to_fetch_and_install_latest_lombok( create_path_if_not_exists(prefix)?; download_file( &format!("https://projectlombok.org/downloads/{jar_name}"), - path_to_quoted_string(jar_path.clone())?.as_str(), + path_to_string(jar_path.clone())?.as_str(), DownloadedFileType::Uncompressed, )?; diff --git a/src/util.rs b/src/util.rs index ffb573c..5247df1 100644 --- a/src/util.rs +++ b/src/util.rs @@ -176,8 +176,7 @@ pub fn get_java_exec_name() -> String { /// * [`java_executable`] can't be converted into a String /// * No major version can be determined pub fn get_java_major_version(java_executable: &PathBuf) -> zed::Result { - let program = - path_to_quoted_string(java_executable).map_err(|_| JAVA_EXEC_ERROR.to_string())?; + let program = path_to_string(java_executable).map_err(|_| JAVA_EXEC_ERROR.to_string())?; let output_bytes = Command::new(program).arg("-version").output()?.stderr; let output = String::from_utf8(output_bytes).map_err(|e| e.to_string())?; @@ -250,21 +249,15 @@ fn get_tag_at(github_tags: &Value, index: usize) -> Option<&str> { /// Formats a path string with platform-specific quoting. /// /// On Windows, wraps the path in double quotes for shell mode compatibility. -/// On Unix, returns the path unquoted since spawn() treats quotes as literals. -fn format_path_for_os(path_str: String, os: Os) -> String { - if os == Os::Windows { - format!("\"{}\"", path_str) - } else { - path_str +/// On Unix and Mac return the string as it is +pub fn quote_path_for_os(path_str: String, os: Os) -> String { + match os { + Os::Windows => format!("\"{path_str}\""), + _ => path_str, } } -/// Converts a [`Path`] into a [`String`], with platform-specific quoting. -/// -/// On Windows, the path is wrapped in double quotes (e.g., `"C:\path\to\file"`) -/// for compatibility with shell mode. On Unix-like systems, the path is returned -/// unquoted, as the proxy uses `spawn()` with `shell: false` which treats quotes -/// as literal filename characters, causing "No such file or directory" errors. +/// Converts a [`Path`] into a [`String`]. /// /// # Arguments /// @@ -272,20 +265,17 @@ fn format_path_for_os(path_str: String, os: Os) -> String { /// /// # Returns /// -/// Returns a `String` representing the path, quoted on Windows, unquoted on Unix. +/// Returns a `String` representing the path. /// /// # Errors /// /// This function will return an error when the string conversion fails -pub fn path_to_quoted_string>(path: P) -> zed::Result { - let path_str = path - .as_ref() +pub fn path_to_string>(path: P) -> zed::Result { + path.as_ref() .to_path_buf() .into_os_string() .into_string() - .map_err(|_| PATH_TO_STR_ERROR.to_string())?; - - Ok(format_path_for_os(path_str, current_platform().0)) + .map_err(|_| PATH_TO_STR_ERROR.to_string()) } /// Remove all files or directories that aren't equal to [`filename`]. @@ -372,9 +362,9 @@ mod tests { use super::*; #[test] - fn test_format_path_for_os_windows() { + fn test_quote_path_for_os_windows() { let path = "C:\\Users\\User Name\\Projects\\zed-extension-java".to_string(); - let result = format_path_for_os(path, Os::Windows); + let result = quote_path_for_os(path, Os::Windows); assert_eq!( result, "\"C:\\Users\\User Name\\Projects\\zed-extension-java\"" @@ -382,16 +372,16 @@ mod tests { } #[test] - fn test_format_path_for_os_unix() { + fn test_quote_path_for_os_unix() { let path = "/home/username/Projects/zed extension java".to_string(); - let result = format_path_for_os(path, Os::Mac); + let result = quote_path_for_os(path, Os::Mac); assert_eq!(result, "/home/username/Projects/zed extension java"); } #[test] - fn test_format_path_for_os_linux() { + fn test_quote_path_for_os_linux() { let path = "/home/username/Projects/zed extension java".to_string(); - let result = format_path_for_os(path, Os::Linux); + let result = quote_path_for_os(path, Os::Linux); assert_eq!(result, "/home/username/Projects/zed extension java"); } } From e0b8c091f52f4fc41e3fa7bdbef02a12cfe28182 Mon Sep 17 00:00:00 2001 From: Riccardo Strina Date: Tue, 23 Dec 2025 14:41:46 +0100 Subject: [PATCH 2/6] Update proxy to not spawn child in shell mode - Remove the shell mode from the proxy for Win32 as no longer relying on jdtls.bat - Remove quoting function along with tests --- src/java.rs | 9 +++------ src/jdtls.rs | 4 ++-- src/proxy.mjs | 18 ++++++++---------- src/util.rs | 42 +----------------------------------------- 4 files changed, 14 insertions(+), 59 deletions(-) diff --git a/src/java.rs b/src/java.rs index 87e3b05..2beb223 100644 --- a/src/java.rs +++ b/src/java.rs @@ -15,7 +15,7 @@ use std::{ use zed_extension_api::{ self as zed, CodeLabel, CodeLabelSpan, DebugAdapterBinary, DebugTaskDefinition, Extension, LanguageServerId, LanguageServerInstallationStatus, StartDebuggingRequestArguments, - StartDebuggingRequestArgumentsRequest, Worktree, current_platform, + StartDebuggingRequestArgumentsRequest, Worktree, lsp::{Completion, CompletionKind}, register_extension, serde_json::{Value, json}, @@ -32,7 +32,7 @@ use crate::{ try_to_fetch_and_install_latest_lombok, }, lsp::LspWrapper, - util::{path_to_string, quote_path_for_os}, + util::path_to_string, }; const PROXY_FILE: &str = include_str!("proxy.mjs"); @@ -288,10 +288,7 @@ impl Extension for Java { self.lombok_jar_path(language_server_id, &configuration, worktree)?; let canonical_lombok_jar_path = path_to_string(current_dir.join(lombok_jar_path))?; - Some(format!( - "-javaagent:{}", - quote_path_for_os(canonical_lombok_jar_path, current_platform().0) - )) + Some(format!("-javaagent:{}", canonical_lombok_jar_path)) } else { None }; diff --git a/src/jdtls.rs b/src/jdtls.rs index ea8d8b1..cfcd60a 100644 --- a/src/jdtls.rs +++ b/src/jdtls.rs @@ -19,7 +19,7 @@ use crate::{ jdk::try_to_fetch_and_install_latest_jdk, util::{ create_path_if_not_exists, get_curr_dir, get_java_exec_name, get_java_executable, - get_java_major_version, get_latest_versions_from_tag, path_to_string, quote_path_for_os, + get_java_major_version, get_latest_versions_from_tag, path_to_string, remove_all_files_except, should_use_local_or_download, }, }; @@ -72,7 +72,7 @@ pub fn build_jdtls_launch_args( "-Dosgi.checkConfiguration=true".to_string(), format!( "-Dosgi.sharedConfiguration.area={}", - quote_path_for_os(path_to_string(shared_config_path)?, current_platform().0) + path_to_string(shared_config_path)? ), "-Dosgi.sharedConfiguration.area.readOnly=true".to_string(), "-Dosgi.configuration.cascaded=true".to_string(), diff --git a/src/proxy.mjs b/src/proxy.mjs index 55680cd..57be6ed 100644 --- a/src/proxy.mjs +++ b/src/proxy.mjs @@ -28,11 +28,10 @@ const args = process.argv.slice(3); const PROXY_ID = Buffer.from(process.cwd().replace(/\/+$/, "")).toString("hex"); const PROXY_HTTP_PORT_FILE = join(workdir, "proxy", PROXY_ID); const isWindows = process.platform === "win32"; -const command = isWindows ? `"${bin}"` : bin; -const lsp = spawn(command, args, { - shell: isWindows, - detached: false +const lsp = spawn(bin, args, { + shell: false, + detached: false, }); function cleanup() { @@ -44,19 +43,18 @@ function cleanup() { // Windows: Use taskkill to kill the process tree (cmd.exe + the child) // /T = Tree kill (child processes), /F = Force exec(`taskkill /pid ${lsp.pid} /T /F`); - } - else { - lsp.kill('SIGTERM'); + } else { + lsp.kill("SIGTERM"); setTimeout(() => { if (!lsp.killed && lsp.exitCode === null) { - lsp.kill('SIGKILL'); + lsp.kill("SIGKILL"); } }, 1000); } } // Handle graceful IDE shutdown via stdin close -process.stdin.on('end', () => { +process.stdin.on("end", () => { cleanup(); process.exit(0); }); @@ -71,7 +69,7 @@ setInterval(() => { } catch (e) { // On Windows, checking a process you don't own might throw EPERM. // We only want to kill if the error is ESRCH (No Such Process). - if (e.code === 'ESRCH') { + if (e.code === "ESRCH") { cleanup(); process.exit(0); } diff --git a/src/util.rs b/src/util.rs index 5247df1..17795e9 100644 --- a/src/util.rs +++ b/src/util.rs @@ -246,17 +246,6 @@ fn get_tag_at(github_tags: &Value, index: usize) -> Option<&str> { }) } -/// Formats a path string with platform-specific quoting. -/// -/// On Windows, wraps the path in double quotes for shell mode compatibility. -/// On Unix and Mac return the string as it is -pub fn quote_path_for_os(path_str: String, os: Os) -> String { - match os { - Os::Windows => format!("\"{path_str}\""), - _ => path_str, - } -} - /// Converts a [`Path`] into a [`String`]. /// /// # Arguments @@ -269,7 +258,7 @@ pub fn quote_path_for_os(path_str: String, os: Os) -> String { /// /// # Errors /// -/// This function will return an error when the string conversion fails +/// This function will return an error when the string conversion fails. pub fn path_to_string>(path: P) -> zed::Result { path.as_ref() .to_path_buf() @@ -356,32 +345,3 @@ pub fn should_use_local_or_download( CheckUpdates::Always => Ok(None), } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_quote_path_for_os_windows() { - let path = "C:\\Users\\User Name\\Projects\\zed-extension-java".to_string(); - let result = quote_path_for_os(path, Os::Windows); - assert_eq!( - result, - "\"C:\\Users\\User Name\\Projects\\zed-extension-java\"" - ); - } - - #[test] - fn test_quote_path_for_os_unix() { - let path = "/home/username/Projects/zed extension java".to_string(); - let result = quote_path_for_os(path, Os::Mac); - assert_eq!(result, "/home/username/Projects/zed extension java"); - } - - #[test] - fn test_quote_path_for_os_linux() { - let path = "/home/username/Projects/zed extension java".to_string(); - let result = quote_path_for_os(path, Os::Linux); - assert_eq!(result, "/home/username/Projects/zed extension java"); - } -} From f7344d1ff6f8e4d48a00c1be4841133a80a3c2f5 Mon Sep 17 00:00:00 2001 From: Riccardo Strina Date: Tue, 23 Dec 2025 17:34:31 +0100 Subject: [PATCH 3/6] Cover instance where user provides jdtls.bat --- src/proxy.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proxy.mjs b/src/proxy.mjs index 57be6ed..a5522e9 100644 --- a/src/proxy.mjs +++ b/src/proxy.mjs @@ -30,7 +30,7 @@ const PROXY_HTTP_PORT_FILE = join(workdir, "proxy", PROXY_ID); const isWindows = process.platform === "win32"; const lsp = spawn(bin, args, { - shell: false, + shell: (isWindows && bin.includes(".bat")) ? true : false, detached: false, }); From 039de3daca8ba2fb13715810baacfd299fb8e757 Mon Sep 17 00:00:00 2001 From: Riccardo Strina Date: Tue, 23 Dec 2025 17:39:51 +0100 Subject: [PATCH 4/6] Wrap user provided .bat with quotes --- src/proxy.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/proxy.mjs b/src/proxy.mjs index a5522e9..6944b79 100644 --- a/src/proxy.mjs +++ b/src/proxy.mjs @@ -28,8 +28,9 @@ const args = process.argv.slice(3); const PROXY_ID = Buffer.from(process.cwd().replace(/\/+$/, "")).toString("hex"); const PROXY_HTTP_PORT_FILE = join(workdir, "proxy", PROXY_ID); const isWindows = process.platform === "win32"; +const command = (isWindows && bin.includes(".bat")) ? `"${bin}"` : bin; -const lsp = spawn(bin, args, { +const lsp = spawn(command, args, { shell: (isWindows && bin.includes(".bat")) ? true : false, detached: false, }); From ea551e548f1a90483d987df9511a9bcdf7c068b0 Mon Sep 17 00:00:00 2001 From: Riccardo Strina Date: Tue, 23 Dec 2025 18:20:52 +0100 Subject: [PATCH 5/6] Make proxy's .bat identification more robust --- src/proxy.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/proxy.mjs b/src/proxy.mjs index 6944b79..4b0f2ba 100644 --- a/src/proxy.mjs +++ b/src/proxy.mjs @@ -28,10 +28,10 @@ const args = process.argv.slice(3); const PROXY_ID = Buffer.from(process.cwd().replace(/\/+$/, "")).toString("hex"); const PROXY_HTTP_PORT_FILE = join(workdir, "proxy", PROXY_ID); const isWindows = process.platform === "win32"; -const command = (isWindows && bin.includes(".bat")) ? `"${bin}"` : bin; +const command = (isWindows && bin.endsWith(".bat")) ? `"${bin}"` : bin; const lsp = spawn(command, args, { - shell: (isWindows && bin.includes(".bat")) ? true : false, + shell: (isWindows && bin.endsWith(".bat")) ? true : false, detached: false, }); From 19c5e7bb35509a400604d2cd366cd516d82ac2f1 Mon Sep 17 00:00:00 2001 From: Riccardo Strina Date: Fri, 26 Dec 2025 19:44:33 +0100 Subject: [PATCH 6/6] Remove unnecessary ternary operation --- src/proxy.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proxy.mjs b/src/proxy.mjs index 4b0f2ba..9c6333a 100644 --- a/src/proxy.mjs +++ b/src/proxy.mjs @@ -31,7 +31,7 @@ const isWindows = process.platform === "win32"; const command = (isWindows && bin.endsWith(".bat")) ? `"${bin}"` : bin; const lsp = spawn(command, args, { - shell: (isWindows && bin.endsWith(".bat")) ? true : false, + shell: (isWindows && bin.endsWith(".bat")), detached: false, });