Merge pull request #69 from hotdata-dev/fix/path-resolver-hardening

fix: harden path resolver for whitespace, case, and unicode

Author: zfarrell

Cargo.toml

@@ -22,6 +22,7 @@ arrow = "57"
 parquet = "57"
 object_store = { version = "0.12.4", features = ["aws"] }
 url = "2.5"
+percent-encoding = "2"
 async-trait = "0.1"
 tokio = { version = "1", features = ["full"] }
 futures = "0.3"

src/path_resolver.rs

@@ -5,6 +5,7 @@
 
 use crate::{DuckLakeError, Result};
 use datafusion::datasource::object_store::ObjectStoreUrl;
+use percent_encoding::percent_decode_str;
 use std::sync::Arc;
 
 /// Parses a data path into an ObjectStoreUrl and key path
@@ -37,9 +38,14 @@ use std::sync::Arc;
 /// # Ok::<(), datafusion_ducklake::DuckLakeError>(())
 /// ```
 pub fn parse_object_store_url(data_path: &str) -> Result<(ObjectStoreUrl, String)> {
-    if data_path.starts_with("s3://") {
+    // Trim leading/trailing whitespace (#64)
+    let data_path = data_path.trim();
+
+    // Use case-insensitive scheme matching per RFC 3986 (#61)
+    let lower = data_path.to_ascii_lowercase();
+    if lower.starts_with("s3://") {
         parse_s3_url(data_path)
-    } else if data_path.starts_with("file://") {
+    } else if lower.starts_with("file://") {
         parse_file_url(data_path)
     } else {
         parse_local_path(data_path)
@@ -73,7 +79,20 @@ fn parse_file_url(data_path: &str) -> Result<(ObjectStoreUrl, String)> {
         DuckLakeError::InvalidConfig(format!("Failed to create ObjectStoreUrl: {}", e))
     })?;
 
-    Ok((object_store_url, url.path().to_owned()))
+    // Decode percent-encoded characters to preserve non-ASCII in local filesystem paths (#60).
+    // url::Url::parse() percent-encodes UTF-8 characters, but the local filesystem expects
+    // raw UTF-8 paths.
+    let decoded_path = percent_decode_str(url.path())
+        .decode_utf8()
+        .map_err(|e| {
+            DuckLakeError::InvalidConfig(format!(
+                "Invalid UTF-8 in file path '{}': {}",
+                data_path, e
+            ))
+        })?
+        .to_string();
+
+    Ok((object_store_url, decoded_path))
 }
 
 /// Parse a local filesystem path into ObjectStoreUrl and key path
@@ -674,12 +693,76 @@ mod tests {
 
     #[test]
     fn test_s3_url_uppercase_scheme() {
-        // URL schemes are case-insensitive per RFC 3986
-        // The url::Url crate normalizes to lowercase
-        let result = parse_object_store_url("S3://bucket/path");
-        // This will fail because our code checks for lowercase "s3://"
-        // This documents current behavior - we expect lowercase
-        assert!(result.is_err());
+        // URL schemes are case-insensitive per RFC 3986 (#61)
+        let (url, path) = parse_object_store_url("S3://bucket/path").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("s3://bucket/").unwrap());
+        assert_eq!(path, "/path");
+    }
+
+    #[test]
+    fn test_s3_url_mixed_case_scheme() {
+        let (url, path) = parse_object_store_url("s3://bucket/data").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("s3://bucket/").unwrap());
+        assert_eq!(path, "/data");
+    }
+
+    #[test]
+    fn test_file_url_uppercase_scheme() {
+        let (url, path) = parse_object_store_url("FILE:///tmp/data").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("file:///").unwrap());
+        assert_eq!(path, "/tmp/data");
+    }
+
+    #[test]
+    fn test_file_url_mixed_case_scheme() {
+        let (url, path) = parse_object_store_url("File:///tmp/data").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("file:///").unwrap());
+        assert_eq!(path, "/tmp/data");
+    }
+
+    #[test]
+    fn test_whitespace_trimming_s3() {
+        // Leading/trailing whitespace should be trimmed (#64)
+        let (url, path) = parse_object_store_url("  s3://bucket/data  ").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("s3://bucket/").unwrap());
+        assert_eq!(path, "/data");
+    }
+
+    #[test]
+    fn test_whitespace_trimming_file() {
+        let (url, path) = parse_object_store_url("  file:///tmp/data  ").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("file:///").unwrap());
+        assert_eq!(path, "/tmp/data");
+    }
+
+    #[test]
+    fn test_file_url_non_ascii_preserved() {
+        // Non-ASCII characters in file:// URLs should be preserved (#60)
+        let (url, path) = parse_object_store_url("file:///tmp/données/données").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("file:///").unwrap());
+        assert_eq!(path, "/tmp/données/données");
+    }
+
+    #[test]
+    fn test_file_url_unicode_preserved() {
+        let (url, path) = parse_object_store_url("file:///home/用户/数据").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("file:///").unwrap());
+        assert_eq!(path, "/home/用户/数据");
+    }
+
+    #[test]
+    fn test_file_url_spaces_preserved() {
+        let (url, path) = parse_object_store_url("file:///tmp/my data/file").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("file:///").unwrap());
+        assert_eq!(path, "/tmp/my data/file");
+    }
+
+    #[test]
+    fn test_whitespace_trimming_with_uppercase_scheme() {
+        // Combined: whitespace + uppercase scheme
+        let (url, path) = parse_object_store_url("  S3://bucket/data  ").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("s3://bucket/").unwrap());
+        assert_eq!(path, "/data");
     }
 
     #[test]