diff --git a/crates/ide-core/Cargo.toml b/crates/ide-core/Cargo.toml index a2a26d945..81e06570d 100644 --- a/crates/ide-core/Cargo.toml +++ b/crates/ide-core/Cargo.toml @@ -19,4 +19,4 @@ tokio-util.workspace = true tempfile.workspace = true [target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.61", features = ["Win32_Globalization", "Win32_System_Registry", "Win32_Foundation"] } +windows-sys = { version = "0.61", features = ["Win32_Globalization", "Win32_Security", "Win32_System_Registry", "Win32_Foundation"] } diff --git a/crates/ide-core/src/process.rs b/crates/ide-core/src/process.rs index 85ee20f5d..9aaaff28f 100644 --- a/crates/ide-core/src/process.rs +++ b/crates/ide-core/src/process.rs @@ -245,10 +245,7 @@ pub fn find_executable(name: &str, search: &OsStr) -> Option #[cfg(windows)] pub fn user_path() -> Option { use windows_sys::Win32::Foundation::ERROR_SUCCESS; - use windows_sys::Win32::System::Registry::{ - HKEY, HKEY_CURRENT_USER, KEY_READ, RRF_RT_REG_EXPAND_SZ, RRF_RT_REG_SZ, RegCloseKey, RegGetValueW, - RegOpenKeyExW, - }; + use windows_sys::Win32::System::Registry::{HKEY, HKEY_CURRENT_USER, KEY_READ, RegCloseKey, RegOpenKeyExW}; fn wide(s: &str) -> Vec { s.encode_utf16().chain(std::iter::once(0)).collect() @@ -259,45 +256,53 @@ pub fn user_path() -> Option { if RegOpenKeyExW(HKEY_CURRENT_USER, wide("Environment").as_ptr(), 0, KEY_READ, &mut key) != ERROR_SUCCESS { return None; } - let read = || { - // Only `Path`; `REG_EXPAND_SZ` is accepted so `%USERPROFILE%\…` entries expand. - let mut kind = 0u32; - let mut len = 0u32; - let name = wide("Path"); - let status = RegGetValueW( - key, - std::ptr::null(), - name.as_ptr(), - RRF_RT_REG_SZ | RRF_RT_REG_EXPAND_SZ, - &mut kind, - std::ptr::null_mut(), - &mut len, - ); - if status != ERROR_SUCCESS || len == 0 { - return None; - } - let mut buf = vec![0u16; (len as usize).div_ceil(2)]; - let status = RegGetValueW( - key, - std::ptr::null(), - name.as_ptr(), - RRF_RT_REG_SZ | RRF_RT_REG_EXPAND_SZ, - &mut kind, - buf.as_mut_ptr() as *mut _, - &mut len, - ); - if status != ERROR_SUCCESS { - return None; - } - let text = String::from_utf16_lossy(&buf); - Some(text.trim_end_matches('\0').to_owned()) - }; - let value = read(); + let value = read_registry_string(key, &wide("Path")); RegCloseKey(key); value.filter(|v| !v.trim().is_empty()).map(OsString::from) } } +/// The string of value `name` under open `key`: `REG_SZ` as-is, `REG_EXPAND_SZ` expanded +/// against the current environment. `None` when the value is missing or unreadable. +/// +/// A `REG_EXPAND_SZ` value's first size query reports the unexpanded size, so a value +/// that grows when expanded is re-read at the size the failed call reports. And the +/// terminating NUL can leak into the read buffer with a stray UTF-16 unit behind it — +/// seen with a `REG_EXPAND_SZ` user `Path`, where it made `env::set_var("PATH", …)` +/// panic with "strings passed to WinAPI cannot contain NULs" — so the text is cut at +/// the first NUL, not just stripped of trailing ones. +#[cfg(windows)] +fn read_registry_string(key: windows_sys::Win32::System::Registry::HKEY, name: &[u16]) -> Option { + use windows_sys::Win32::Foundation::{ERROR_MORE_DATA, ERROR_SUCCESS}; + use windows_sys::Win32::System::Registry::{RRF_RT_REG_EXPAND_SZ, RRF_RT_REG_SZ, RegGetValueW}; + + const FLAGS: u32 = RRF_RT_REG_SZ | RRF_RT_REG_EXPAND_SZ; + let mut kind = 0u32; + let mut len = 0u32; + // SAFETY: `name` is NUL-terminated and the size query writes only `kind` and `len`. + let status = + unsafe { RegGetValueW(key, std::ptr::null(), name.as_ptr(), FLAGS, &mut kind, std::ptr::null_mut(), &mut len) }; + if status != ERROR_SUCCESS || len == 0 { + return None; + } + let buf = loop { + let mut buf = vec![0u16; len as usize / 2 + 1]; + // SAFETY: as above; the buffer holds at least the `len` bytes the call may write. + let status = unsafe { + RegGetValueW(key, std::ptr::null(), name.as_ptr(), FLAGS, &mut kind, buf.as_mut_ptr() as *mut _, &mut len) + }; + if status == ERROR_SUCCESS { + break buf; + } + if status != ERROR_MORE_DATA || len == 0 { + return None; + } + }; + let text = String::from_utf16_lossy(&buf); + let end = text.find('\0').unwrap_or(text.len()); + Some(text[..end].to_owned()) +} + /// The user's own `PATH` entries; `None` outside Windows (see the Windows impl). #[cfg(not(windows))] pub fn user_path() -> Option { @@ -334,6 +339,96 @@ mod tests { } } + #[cfg(windows)] + #[test] + fn registry_strings_expand_and_stay_terminator_free() { + use windows_sys::Win32::Foundation::ERROR_SUCCESS; + use windows_sys::Win32::System::Registry::{ + HKEY, HKEY_CURRENT_USER, KEY_READ, KEY_WRITE, REG_EXPAND_SZ, REG_SZ, RegCloseKey, RegCreateKeyExW, + RegDeleteTreeW, RegSetValueExW, + }; + + fn wide(s: &str) -> Vec { + s.encode_utf16().chain(std::iter::once(0)).collect() + } + + struct TestState { + key: HKEY, + name: String, + variable: String, + previous: Option, + } + impl Drop for TestState { + fn drop(&mut self) { + // SAFETY: the key belongs to this test; Windows environment mutation is safe. + unsafe { + RegCloseKey(self.key); + RegDeleteTreeW(HKEY_CURRENT_USER, wide(&self.name).as_ptr()); + match &self.previous { + Some(value) => std::env::set_var(&self.variable, value), + None => std::env::remove_var(&self.variable), + } + } + } + } + let name = format!("Software\\ai.l0ng.openidea.registry-read-test-{}", std::process::id()); + let variable = format!("OPENIDEA_TEST_REGVAR_{}", std::process::id()); + let long = "x".repeat(600); + let mut key: HKEY = std::ptr::null_mut(); + let opened = unsafe { + RegCreateKeyExW( + HKEY_CURRENT_USER, + wide(&name).as_ptr(), + 0, + std::ptr::null(), + 0, + KEY_READ | KEY_WRITE, + std::ptr::null(), + &mut key, + std::ptr::null_mut(), + ) + }; + assert_eq!(opened, ERROR_SUCCESS, "the test key must be creatable under HKCU\\Software"); + let _cleanup = TestState { key, name, previous: std::env::var_os(&variable), variable: variable.clone() }; + // SAFETY: Windows permits environment mutation. No other test uses this process-specific name. + unsafe { std::env::set_var(&variable, &long) }; + let set = |value: &str, kind: u32, data: &[u16]| { + // SAFETY: `key` is open for writing and `data` holds `data.len() * 2` bytes. + assert_eq!( + unsafe { + RegSetValueExW( + key, + wide(value).as_ptr(), + 0, + kind, + data.as_ptr() as *const _, + (data.len() * 2) as u32, + ) + }, + ERROR_SUCCESS + ); + }; + let read = |value: &str| read_registry_string(key, &wide(value)); + set("Plain", REG_SZ, &wide("plain")); + set("Expand", REG_EXPAND_SZ, &wide(&format!("%{variable}%\\bin"))); + set("ExpandLong", REG_EXPAND_SZ, &wide(&format!("%{variable}%"))); + // Reproduce the reported interior terminator followed by a stale UTF-16 unit. + set("TrailingGarbage", REG_SZ, &wide("C:\\Zed\\bin\0n")); + set("ExpandTrailingGarbage", REG_EXPAND_SZ, &wide(&format!("%{variable}%\\bin\0n"))); + set("Empty", REG_SZ, &wide("")); + let plain = read("Plain"); + let expand = read("Expand"); + let expand_long = read("ExpandLong"); + assert_eq!(plain.as_deref(), Some("plain")); + assert_eq!(expand.as_deref(), Some(format!("{long}\\bin")).as_deref()); + // Longer than its unexpanded form: needs the re-read at the expanded size. + assert_eq!(expand_long.as_deref(), Some(long.as_str())); + assert_eq!(read("TrailingGarbage").as_deref(), Some("C:\\Zed\\bin")); + assert_eq!(read("ExpandTrailingGarbage").as_deref(), Some(format!("{long}\\bin")).as_deref()); + assert_eq!(read("Empty").as_deref(), Some("")); + assert_eq!(read("Missing"), None); + } + #[test] fn append_path_entries_keeps_the_order_and_adds_what_is_missing() { let join = |dirs: &[&str]| std::env::join_paths(dirs).unwrap(); diff --git a/src/features/settings/__tests__/inspections.test.ts b/src/features/settings/__tests__/inspections.test.ts index c20644a00..32a134972 100644 --- a/src/features/settings/__tests__/inspections.test.ts +++ b/src/features/settings/__tests__/inspections.test.ts @@ -45,6 +45,7 @@ describe("inspections tree", () => { it("groups servers by language with seen and configured codes, built-ins by category", () => { expect(tree.map((g) => g.name)).toEqual([ + "Cargo", "General", "JavaScript", "Other",