diff --git a/codex-rs/utils/path-uri/src/absolute_path_normalization.rs b/codex-rs/utils/path-uri/src/absolute_path_normalization.rs new file mode 100644 index 000000000..d391c2e5c --- /dev/null +++ b/codex-rs/utils/path-uri/src/absolute_path_normalization.rs @@ -0,0 +1,46 @@ +use crate::PathConvention; +use crate::PathUri; +use url::Url; + +pub(super) fn path_uri_from_segments<'a>( + convention: PathConvention, + host: Option<&str>, + segments: impl Iterator, +) -> Option { + let mut url = Url::parse("file:///").ok()?; + if let Some(host) = host { + url.set_host(Some(host)).ok()?; + } + let anchor_depth = usize::from(convention == PathConvention::Windows); + let mut depth = 0; + let mut normalized_segments = Vec::new(); + let mut has_trailing_separator = false; + for segment in segments { + match segment { + "" => has_trailing_separator = true, + "." => has_trailing_separator = false, + ".." => { + has_trailing_separator = false; + if depth > anchor_depth { + normalized_segments.pop(); + depth -= 1; + } + } + segment => { + normalized_segments.push(segment); + depth += 1; + has_trailing_separator = false; + } + } + } + if has_trailing_separator + || (convention == PathConvention::Windows && host.is_none() && depth == anchor_depth) + { + normalized_segments.push(""); + } + { + let mut url_segments = url.path_segments_mut().ok()?; + url_segments.clear().extend(normalized_segments); + } + PathUri::try_from(url).ok() +} diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index 7b84beccb..ccc9cfd79 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -18,8 +18,11 @@ use thiserror::Error; use ts_rs::TS; use url::Url; +mod absolute_path_normalization; mod api_path_string; +use absolute_path_normalization::path_uri_from_segments; + pub use api_path_string::LegacyAppPathString; pub use api_path_string::LegacyAppPathStringError; @@ -589,7 +592,7 @@ fn parse_posix_path(path: &str) -> Option { format!("/{path}").as_bytes(), )); } - path_uri_from_segments(/*host*/ None, path.split('/')) + path_uri_from_segments(PathConvention::Posix, /*host*/ None, path.split('/')) } fn parse_windows_path(path: &str) -> Option { @@ -612,6 +615,7 @@ fn parse_windows_path(path: &str) -> Option { if drive.is_ascii_alphabetic() && is_windows_separator_byte(*separator) ) { return path_uri_from_segments( + PathConvention::Windows, /*host*/ None, std::iter::once(&path[..2]).chain(path[3..].split(is_windows_separator_char)), ); @@ -623,31 +627,17 @@ fn parse_windows_path(path: &str) -> Option { let mut components = path[2..].split(is_windows_separator_char); let host = components.next().filter(|host| !host.is_empty())?; let share = components.next().filter(|share| !share.is_empty())?; - return path_uri_from_segments(Some(host), std::iter::once(share).chain(components)) - .or_else(|| Some(windows_opaque_path_uri(path))); + return path_uri_from_segments( + PathConvention::Windows, + Some(host), + std::iter::once(share).chain(components), + ) + .or_else(|| Some(windows_opaque_path_uri(path))); } None } -fn path_uri_from_segments<'a>( - host: Option<&str>, - segments: impl Iterator, -) -> Option { - let mut url = Url::parse("file:///").ok()?; - if let Some(host) = host { - url.set_host(Some(host)).ok()?; - } - { - let mut url_segments = url.path_segments_mut().ok()?; - url_segments.clear(); - for segment in segments { - url_segments.push(segment); - } - } - PathUri::try_from(url).ok() -} - fn windows_opaque_path_uri(path: &str) -> PathUri { let path_bytes = path .encode_utf16() diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index 8954b7ef4..68113c8f6 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -617,6 +617,133 @@ fn join_replaces_posix_absolute_path() { ); } +#[test] +fn join_keeps_canonicalized_posix_double_slash_paths_hierarchical() { + let base = PathUri::parse("file:///workspace").expect("valid base URI"); + let cwd = base + .join("//server/share/project") + .expect("valid absolute path"); + + assert_eq!( + cwd, + PathUri::parse("file:///server/share/project").expect("valid canonical URI") + ); + assert_eq!( + cwd.parent(), + Some(PathUri::parse("file:///server/share").expect("valid parent URI")) + ); + assert_eq!( + cwd.join("AGENTS.md"), + Ok(PathUri::parse("file:///server/share/project/AGENTS.md").expect("valid child URI")) + ); + #[cfg(unix)] + assert_eq!( + cwd.to_abs_path() + .expect("cwd should convert to a native path"), + AbsolutePathBuf::try_from("/server/share/project") + .expect("expected native path should be absolute") + ); +} + +#[test] +fn join_normalizes_absolute_parent_segments() { + for (base, path, expected) in [ + ("file:///workspace", "/tmp/a/../b", "file:///tmp/b"), + ("file:///C:/workspace", r"D:\tmp\a\..\b", "file:///D:/tmp/b"), + ( + "file:///C:/workspace", + r"\\server\share\a\..\b", + "file://server/share/b", + ), + ("file:///workspace", "/tmp//a/../b", "file:///tmp/b"), + ("file:///workspace", "/tmp/a/..//b", "file:///tmp/b"), + ("file:///workspace", "/tmp/a///../b", "file:///tmp/b"), + ( + "file:///C:/workspace", + r"D:\tmp\a\\\..\b", + "file:///D:/tmp/b", + ), + ( + "file:///C:/workspace", + r"\\server\share\a\\\..\b", + "file://server/share/b", + ), + ("file:///workspace", "/tmp/a///b/../..", "file:///tmp"), + ( + "file:///C:/workspace", + r"D:\tmp\a\\\b\..\..", + "file:///D:/tmp", + ), + ( + "file:///C:/workspace", + r"\\server\share\a\\\b\..\..", + "file://server/share", + ), + ] { + let base = PathUri::parse(base).expect("valid base URI"); + let expected = PathUri::parse(expected).expect("valid expected URI"); + assert_eq!(base.join(path), Ok(expected), "joining {path}"); + } +} + +#[test] +fn join_absolute_parent_segments_stop_at_native_path_roots() { + for (base, path, expected) in [ + ("file:///workspace", "/a/..", "file:///"), + ("file:///C:/workspace", r"D:\a\..", "file:///D:/"), + ( + "file:///C:/workspace", + r"\\server\share\a\..", + "file://server/share", + ), + ("file:///workspace", "/../../b", "file:///b"), + ("file:///C:/workspace", r"D:\..\..\b", "file:///D:/b"), + ( + "file:///C:/workspace", + r"\\server\share\..\..\b", + "file://server/share/b", + ), + ( + "file:///C:/workspace", + r"\\server\share\\\..\b", + "file://server/share/b", + ), + ] { + let base = PathUri::parse(base).expect("valid base URI"); + let expected = PathUri::parse(expected).expect("valid expected URI"); + assert_eq!(base.join(path), Ok(expected), "joining {path}"); + } +} + +#[test] +fn join_collapses_redundant_absolute_separators() { + for (base, path, expected) in [ + ("file:///workspace", "/tmp///", "file:///tmp/"), + ("file:///workspace", "///", "file:///"), + ( + "file:///workspace", + "///server/share///", + "file:///server/share/", + ), + ("file:///C:/workspace", r"D:\tmp\\\", "file:///D:/tmp/"), + ("file:///C:/workspace", r"D:\\\", "file:///D:/"), + ( + "file:///C:/workspace", + r"\\server\share\tmp\\\", + "file://server/share/tmp/", + ), + ( + "file:///C:/workspace", + r"\\server\share\\\", + "file://server/share/", + ), + ] { + let base = PathUri::parse(base).expect("valid base URI"); + let expected = PathUri::parse(expected).expect("valid expected URI"); + assert_eq!(base.join(path), Ok(expected), "joining {path}"); + } +} + #[test] fn join_replaces_windows_absolute_path() { let base = PathUri::parse("file:///C:/workspace/src").expect("valid base URI");