codex: expand native path rendering coverage (#27819)

codex-rs/utils/path-uri/src/native_path_string.rs

@@ -33,9 +33,6 @@ impl PathConvention {
     }
 }
 
-#[cfg(not(any(windows, unix)))]
-compile_error!("PathConvention::native() requires a Windows or Unix target");
-
 impl fmt::Display for PathConvention {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -123,10 +120,15 @@ impl JsonSchema for NativePathString {
 
 fn render_posix_path(path: &PathUri) -> Result<String, NativePathStringError> {
     let url = path.to_url();
+    // POSIX file paths do not have a UNC authority, so `file://server/share`
+    // cannot be represented as `/share` without losing the server identity.
     if url.host_str().is_some() {
         return Err(incompatible_convention(path, PathConvention::Posix));
     }
 
+    // URI segments are already separated with `/` on every host. Decode each
+    // one independently so `file:///a%20dir/file` becomes `/a dir/file` while
+    // an encoded separator cannot silently introduce another path component.
     let mut rendered = String::new();
     for segment in path_segments(&url) {
         rendered.push('/');
@@ -144,6 +146,9 @@ fn render_windows_path(path: &PathUri) -> Result<String, NativePathStringError>
     let mut segments = path_segments(&url);
     let mut rendered = String::new();
     if let Some(host) = url.host_str() {
+        // A URI authority selects the UNC form: `file://server/share/file`
+        // becomes `\\server\share\file`. The first segment is the share name,
+        // which must be present and valid as a Windows path component.
         let Some(share) = segments.next() else {
             return Err(incompatible_convention(path, PathConvention::Windows));
         };
@@ -157,6 +162,9 @@ fn render_windows_path(path: &PathUri) -> Result<String, NativePathStringError>
         rendered.push('\\');
         rendered.push_str(&share);
     } else {
+        // Without an authority, Windows requires a drive root. For example,
+        // `file:///C:/src/main.rs` begins with the `C:` URI segment and renders
+        // as `C:\src\main.rs`; a POSIX URI such as `file:///usr/bin` is rejected.
         let Some(drive) = segments.next() else {
             return Err(incompatible_convention(path, PathConvention::Windows));
         };
@@ -169,6 +177,8 @@ fn render_windows_path(path: &PathUri) -> Result<String, NativePathStringError>
     }
 
     for segment in segments {
+        // URL path separators become Windows separators after each component
+        // has been decoded and checked using Windows filename rules.
         let segment = decode_native_segment(path, segment, PathConvention::Windows)?;
         if !segment.is_empty() {
             validate_windows_component(path, &segment)?;
@@ -176,6 +186,8 @@ fn render_windows_path(path: &PathUri) -> Result<String, NativePathStringError>
         rendered.push('\\');
         rendered.push_str(&segment);
     }
+    // `file:///C:` and `file:///C:/` both identify the drive root, never the
+    // drive-relative path `C:`.
     if rendered.len() == 2 && rendered.as_bytes()[1] == b':' {
         rendered.push('\\');
     }
@@ -192,7 +204,11 @@ fn decode_native_segment(
     segment: &str,
     convention: PathConvention,
 ) -> Result<String, NativePathStringError> {
+    // Decode exactly once. Thus `%20` becomes a space and `%252F` becomes the
+    // literal text `%2F`, rather than being decoded a second time into `/`.
     let bytes = urlencoding::decode_binary(segment.as_bytes());
+    // A separator encoded inside one URI segment would change path structure:
+    // reject `%2F` for both conventions and `%5C` for Windows.
     let contains_separator =
         bytes.contains(&b'/') || (convention == PathConvention::Windows && bytes.contains(&b'\\'));
     if contains_separator {
@@ -212,6 +228,9 @@ fn validate_windows_component(
     path: &PathUri,
     component: &str,
 ) -> Result<(), NativePathStringError> {
+    // Windows forbids control characters and `<>:"/\|?*` in components. It
+    // also aliases trailing spaces and dots, so names such as `report?.txt`,
+    // `trailing.`, and `trailing ` are rejected instead of being rewritten.
     let contains_invalid_character = component
         .chars()
         .any(|character| character <= '\u{1f}' || r#"<>:"/\|?*"#.contains(character));

codex-rs/utils/path-uri/src/native_path_string_tests.rs

@@ -6,11 +6,30 @@ use pretty_assertions::assert_eq;
 fn renders_posix_paths_on_every_host() {
     for (uri, expected) in [
         ("file:///", "/"),
+        ("file:///home/alice/src/main.rs", "/home/alice/src/main.rs"),
         ("file:///home/alice/a%20file.rs", "/home/alice/a file.rs"),
+        ("file:///workspace/src/lib.rs", "/workspace/src/lib.rs"),
+        (
+            "file:///workspace/tests/test.rs",
+            "/workspace/tests/test.rs",
+        ),
+        ("file:///etc", "/etc"),
         ("file:///tmp/", "/tmp/"),
         ("file:///C:/Project", "/C:/Project"),
+        ("file:///C:", "/C:"),
         ("file:///tmp/%E2%98%83", "/tmp/☃"),
         ("file:///tmp/a%5Cb", "/tmp/a\\b"),
+        ("file:///tmp/100%25/file", "/tmp/100%/file"),
+        ("file:///tmp/a%3Fb%23c%25d", "/tmp/a?b#c%d"),
+        ("file:///tmp/a%252Fb", "/tmp/a%2Fb"),
+        (
+            "file:///bad/path/L3RtcC9udWxsLQAt_y1ieXRl",
+            "/bad/path/L3RtcC9udWxsLQAt_y1ieXRl",
+        ),
+        ("FILE:///workspace/src", "/workspace/src"),
+        ("file:/workspace/src", "/workspace/src"),
+        ("file://localhost/workspace/src", "/workspace/src"),
+        ("file://LOCALHOST/workspace/src", "/workspace/src"),
     ] {
         let path = PathUri::parse(uri).expect("valid file URI");
         assert_eq!(
@@ -31,6 +50,8 @@ fn renders_windows_drive_paths_on_every_host() {
         ),
         ("file:///C:/", "C:\\"),
         ("file:///C:", "C:\\"),
+        ("file:///C:/Users", r"C:\Users"),
+        ("file:///C:/Windows", r"C:\Windows"),
         ("file:///d:/snowman/%E2%98%83", r"d:\snowman\☃"),
         ("file:///C:/tmp/", "C:\\tmp\\"),
         ("file:///C:/test%20with%20%25/path", r"C:\test with %\path"),
@@ -66,6 +87,7 @@ fn renders_windows_unc_paths_on_every_host() {
             "file://server/share/src/main.rs",
             r"\\server\share\src\main.rs",
         ),
+        ("file://server/share", r"\\server\share"),
         ("file://server/share/", "\\\\server\\share\\"),
         ("file://shares/files/c%23/p.cs", r"\\shares\files\c#\p.cs"),
         (
@@ -86,7 +108,9 @@ fn renders_windows_unc_paths_on_every_host() {
 #[test]
 fn rejects_paths_incompatible_with_the_convention() {
     for (uri, convention) in [
+        ("file://server/share/file.txt", PathConvention::Posix),
         ("file://server/share/file.rs", PathConvention::Posix),
+        ("file:///usr/local/file.txt", PathConvention::Windows),
         ("file:///home/alice/file.rs", PathConvention::Windows),
         ("file://server/", PathConvention::Windows),
         ("file:///_:/path", PathConvention::Windows),
@@ -117,14 +141,14 @@ fn renders_native_opaque_fallback_paths_lossily() {
     use std::os::unix::ffi::OsStringExt;
 
     let native_path = std::path::PathBuf::from(std::ffi::OsString::from_vec(
-        b"/tmp/null-\0-non-utf8-\xff".to_vec(),
+        b"/tmp/null-\0-\xff-byte".to_vec(),
     ));
     let path = PathUri::from_path(native_path).expect("absolute native path");
 
     assert_eq!(
         NativePathString::from_path_uri(&path, PathConvention::Posix)
             .map(NativePathString::into_string),
-        Ok("/tmp/null-\0-non-utf8-<EFBFBD>".to_string())
+        Ok("/tmp/null-\0-<2D>-byte".to_string())
     );
     assert_eq!(
         NativePathString::from_path_uri(&path, PathConvention::Windows),
@@ -135,6 +159,24 @@ fn renders_native_opaque_fallback_paths_lossily() {
     );
 }
 
+#[cfg(windows)]
+#[test]
+fn renders_windows_namespace_fallback_paths() {
+    for native_path in [
+        r"\\.\COM1",
+        r"\\?\Volume{00000000-0000-0000-0000-000000000000}\file.rs",
+    ] {
+        let path = PathUri::from_path(native_path).expect("absolute Windows namespace path");
+
+        assert_eq!(
+            NativePathString::from_path_uri(&path, PathConvention::Windows)
+                .map(NativePathString::into_string),
+            Ok(native_path.to_string()),
+            "rendering {native_path}"
+        );
+    }
+}
+
 #[cfg(windows)]
 #[test]
 fn renders_native_opaque_fallback_paths_lossily() {