diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index a788fdc9b4d0..444af1ea28c3 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -9,6 +9,8 @@ use serde::Deserializer; use serde::Serialize; use serde::Serializer; use std::fmt; +use std::io; +use std::path::Path; use std::str::FromStr; use thiserror::Error; use ts_rs::TS; @@ -25,7 +27,7 @@ pub const FILE_SCHEME: &str = "file"; /// /// `file:` paths retain their URI spelling so they can be parsed independently /// of the current host. In particular, `/C:/src` remains ambiguous between a -/// Windows drive path and a valid POSIX path until [`Self::to_native_path`] +/// Windows drive path and a valid POSIX path until [`Self::to_abs_path`] /// applies the current host's rules. A local POSIX `file:` URI can also retain /// percent-encoded non-UTF-8 bytes for lossless native round trips. /// @@ -55,15 +57,27 @@ impl PathUri { /// Converts an absolute path on the current host to a `file:` URI. /// - /// On Unix, [`AbsolutePathBuf`]'s absolute-path invariant is sufficient for - /// `url` to represent the path. On Windows, conversion can still fail for - /// absolute paths whose prefix has no `file:` URI representation, including - /// `\\.\` device paths and generic `\\?\` verbatim namespaces. Those cases - /// return [`PathUriParseError::InvalidFileUriPath`]. - pub fn from_file_path(path: &AbsolutePathBuf) -> Result { - let url = Url::from_file_path(path.as_path()) - .map_err(|()| PathUriParseError::InvalidFileUriPath)?; - Self::try_from(url) + /// On Windows, paths without a URI representation, including `\\.\` device + /// paths and generic `\\?\` verbatim namespaces, are reported as invalid + /// input. + pub fn from_abs_path(path: &AbsolutePathBuf) -> io::Result { + let url = Url::from_file_path(path.as_path()).map_err(|()| { + io::Error::new( + io::ErrorKind::InvalidInput, + PathUriParseError::InvalidFileUriPath, + ) + })?; + Self::try_from(url).map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err)) + } + + /// Converts a path on the current host to a `file:` URI. + /// + /// Relative paths and paths without a URI representation are reported as + /// invalid input. + pub fn from_path(path: impl AsRef) -> io::Result { + let path = AbsolutePathBuf::from_absolute_path_checked(path) + .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; + Self::from_abs_path(&path) } /// Returns the percent-encoded URI path. @@ -149,13 +163,15 @@ impl PathUri { /// on POSIX or a POSIX root on Windows. Because a `file:` URI does not record /// its source operating system, callers should only use this method when the /// URI is known to identify a path on the current host. - pub fn to_native_path(&self) -> Result { - let path = self - .0 - .to_file_path() - .map_err(|()| PathUriParseError::InvalidFileUriPath)?; + pub fn to_abs_path(&self) -> io::Result { + let path = self.0.to_file_path().map_err(|()| { + io::Error::new( + io::ErrorKind::InvalidInput, + PathUriParseError::InvalidFileUriPath, + ) + })?; AbsolutePathBuf::from_absolute_path_checked(path) - .map_err(|_| PathUriParseError::InvalidFileUriPath) + .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err)) } /// Returns a clone of the canonical URL. @@ -216,7 +232,7 @@ impl<'de> Deserialize<'de> for PathUri { .map_or_else(|| path_error.to_string(), |error| error.to_string()), ) })?; - Self::from_file_path(&path).map_err(serde::de::Error::custom) + Self::from_abs_path(&path).map_err(serde::de::Error::custom) } } diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index 6a237a8c1efc..4fb2ba5b452c 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -12,7 +12,7 @@ fn file_uri_round_trips_an_absolute_path() { .expect("current directory") .join("a path/file.rs"); - let uri = PathUri::from_file_path(&path).expect("path should convert to a file URI"); + let uri = PathUri::from_abs_path(&path).expect("path should convert to a file URI"); let uri_string = uri.to_string(); assert!(uri_string.starts_with("file:")); @@ -22,12 +22,26 @@ fn file_uri_round_trips_an_absolute_path() { uri ); assert_eq!( - uri.to_native_path() + uri.to_abs_path() .expect("local file URI should convert to a native path"), path ); } +#[test] +fn non_native_uri_io_conversion_is_invalid_input() { + #[cfg(unix)] + let uri = PathUri::parse("file://server/share/file.txt").expect("valid file URI"); + #[cfg(windows)] + let uri = PathUri::parse("file:///usr/local/file.txt").expect("valid file URI"); + + let error = uri + .to_abs_path() + .expect_err("URI should not be host-native"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); +} + #[test] fn file_uri_parses_a_windows_path_on_any_host() { let uri = PathUri::parse("file:///C:/Users/Alice%20Smith/src/main.rs") @@ -52,8 +66,10 @@ fn file_uri_rejects_windows_prefixes_without_a_uri_representation() { .expect("Windows namespace path should be absolute"); assert_eq!( - PathUri::from_file_path(&path), - Err(PathUriParseError::InvalidFileUriPath), + PathUri::from_abs_path(&path) + .expect_err("Windows namespace path should not convert") + .kind(), + io::ErrorKind::InvalidInput, "converting {native_path}" ); } @@ -85,9 +101,9 @@ fn file_uri_accepts_non_utf8_posix_paths() { let path = PathBuf::from(std::ffi::OsString::from_vec(b"/tmp/non-utf8-\xff".to_vec())); let path = AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute POSIX path"); - let uri = PathUri::from_file_path(&path).expect("non-UTF-8 path should convert to a file URI"); + let uri = PathUri::from_abs_path(&path).expect("non-UTF-8 path should convert to a file URI"); assert_eq!( - uri.to_native_path() + uri.to_abs_path() .expect("URI should convert to native path"), path ); @@ -111,10 +127,10 @@ fn file_uri_round_trips_literal_percent_characters() { fn file_uri_round_trips_windows_unc_paths() { let path = AbsolutePathBuf::from_absolute_path_checked(r"\\server\share\src\main.rs") .expect("absolute UNC path"); - let uri = PathUri::from_file_path(&path).expect("UNC path should convert to a file URI"); + let uri = PathUri::from_abs_path(&path).expect("UNC path should convert to a file URI"); assert_eq!(uri.encoded_path(), "/share/src/main.rs"); - assert_eq!(uri.to_native_path().expect("UNC URI should convert"), path); + assert_eq!(uri.to_abs_path().expect("UNC URI should convert"), path); } #[test] @@ -184,10 +200,17 @@ fn path_uri_deserializes_legacy_absolute_paths() { assert_eq!( uri, - PathUri::from_file_path(&path).expect("expected file URI") + PathUri::from_abs_path(&path).expect("expected file URI") ); } +#[test] +fn path_uri_rejects_relative_native_paths() { + let error = PathUri::from_path("src/lib.rs").expect_err("relative path should be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); +} + #[test] fn path_uri_rejects_legacy_relative_paths_with_absolute_path_guard() { let base = AbsolutePathBuf::current_dir().expect("current directory");