From a0a6b7521b77e0d1e3bbe8f82ea4caaf323e5789 Mon Sep 17 00:00:00 2001 From: jackylee-ch Date: Mon, 21 Sep 2026 14:45:24 +0800 Subject: [PATCH] fix(python): treat a single-letter URL scheme as a filesystem path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Url::parse` accepts a Windows absolute path like `C:\data\file.vortex` as a URL whose scheme is the drive letter, so `resolve_store` handed every absolute Windows path to the object-store registry and failed with an unrecognised scheme. `vx.open`, `vx.io.write` and `vx.dataset` all resolve through this one function. The repository already made this decision twice — `parse_uri_or_path` in vortex-file and `data_source.rs` in vortex-ffi both guard on `url.scheme().len() > 1`, with the rationale written out. vortex-python was never moved onto it. The Windows Python CI job round-trips the relative path "smoke.vortex", which `Url::parse` rejects outright, so the drive-letter arm was never exercised. Signed-off-by: jackylee-ch --- vortex-python/src/object_store/resolve.rs | 26 +++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/vortex-python/src/object_store/resolve.rs b/vortex-python/src/object_store/resolve.rs index 94d48df2bee..ab392c1a038 100644 --- a/vortex-python/src/object_store/resolve.rs +++ b/vortex-python/src/object_store/resolve.rs @@ -35,7 +35,6 @@ pub(crate) fn resolve_store( object_path_from_literal(url_or_path), )), None => { - // If the URL does not parse match Url::parse(url_or_path) { Ok(url) if url.scheme() == "file" => { let path = url @@ -43,14 +42,16 @@ pub(crate) fn resolve_store( .map_err(|_| vortex_err!("invalid file URL: {url_or_path}"))?; Ok(ResolvedStore::Path(path)) } - Ok(url) => { + // `Url::parse` accepts a Windows absolute path like `C:\data` as a URL whose + // scheme is the drive letter. No real URL scheme is one character, so a + // single-letter scheme means this is a filesystem path, not a store. Same rule as + // `vortex_file::parse_uri_or_path`. + Ok(url) if url.scheme().len() > 1 => { let (store, path) = REGISTRY.resolve(&url)?; Ok(ResolvedStore::object_store(store, path)) } - Err(_) => { - // Treat the input string as a local file system path, which may be - Ok(ResolvedStore::Path(PathBuf::from(url_or_path))) - } + // Anything that does not parse as a URL is a local filesystem path. + _ => Ok(ResolvedStore::Path(PathBuf::from(url_or_path))), } } } @@ -162,4 +163,17 @@ mod test { .unwrap_store(); assert_eq!(path.as_ref(), key); } + + /// `Url::parse` reads a Windows drive letter as a scheme, so these must stay filesystem paths. + #[rstest] + #[case::backslashes(r"C:\data\file.vortex")] + #[case::forward_slashes("C:/data/file.vortex")] + #[case::lowercase(r"d:\data\file.vortex")] + #[case::drive_root(r"C:\")] + fn test_single_letter_scheme_is_path(#[case] path: &str) { + assert_eq!( + resolve_store(path, None).unwrap().unwrap_path(), + PathBuf::from(path) + ); + } }