Skip to content

Commit 199ce91

Browse files
Fix 5592: Colon (:) in in object_store::path::{Path} is not handled on Windows (#5830)
* Fix issue #5800: Handle missing files in list_with_delimiter * draft * cargo fmt * Handle leading colon * Add windows CI * Fix CI job * Only run local tests and set target family for failing tests * Run all tests without my changes and removed target os * Restore changes again * Add back newline (removed by mistake) * Fix test after merge with master
1 parent 920a944 commit 199ce91

File tree

2 files changed

+53
-1
lines changed

2 files changed

+53
-1
lines changed

.github/workflows/object_store.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,3 +198,16 @@ jobs:
198198
run: cargo build --target wasm32-unknown-unknown
199199
- name: Build wasm32-wasi
200200
run: cargo build --target wasm32-wasi
201+
202+
windows:
203+
name: cargo test LocalFileSystem (win64)
204+
runs-on: windows-latest
205+
defaults:
206+
run:
207+
working-directory: object_store
208+
steps:
209+
- uses: actions/checkout@v4
210+
with:
211+
submodules: true
212+
- name: Run LocalFileSystem tests
213+
run: cargo test local::tests

object_store/src/local.rs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,22 @@ impl LocalFileSystem {
297297
path: location.as_ref()
298298
}
299299
);
300-
self.config.prefix_to_filesystem(location)
300+
let path = self.config.prefix_to_filesystem(location)?;
301+
302+
#[cfg(target_os = "windows")]
303+
let path = {
304+
let path = path.to_string_lossy();
305+
306+
// Assume the first char is the drive letter and the next is a colon.
307+
let mut out = String::new();
308+
let drive = &path[..2]; // The drive letter and colon (e.g., "C:")
309+
let filepath = &path[2..].replace(':', "%3A"); // Replace subsequent colons
310+
out.push_str(drive);
311+
out.push_str(filepath);
312+
PathBuf::from(out)
313+
};
314+
315+
Ok(path)
301316
}
302317

303318
/// Enable automatic cleanup of empty directories when deleting files
@@ -1053,6 +1068,7 @@ mod tests {
10531068
use super::*;
10541069

10551070
#[tokio::test]
1071+
#[cfg(target_family = "unix")]
10561072
async fn file_test() {
10571073
let root = TempDir::new().unwrap();
10581074
let integration = LocalFileSystem::new_with_prefix(root.path()).unwrap();
@@ -1069,6 +1085,7 @@ mod tests {
10691085
}
10701086

10711087
#[test]
1088+
#[cfg(target_family = "unix")]
10721089
fn test_non_tokio() {
10731090
let root = TempDir::new().unwrap();
10741091
let integration = LocalFileSystem::new_with_prefix(root.path()).unwrap();
@@ -1481,6 +1498,28 @@ mod tests {
14811498
assert_eq!(list, vec![c, a]);
14821499
}
14831500

1501+
#[tokio::test]
1502+
#[cfg(target_os = "windows")]
1503+
async fn filesystem_filename_with_colon() {
1504+
let root = TempDir::new().unwrap();
1505+
let integration = LocalFileSystem::new_with_prefix(root.path()).unwrap();
1506+
let path = Path::parse("file%3Aname.parquet").unwrap();
1507+
let location = Path::parse("file:name.parquet").unwrap();
1508+
1509+
integration.put(&location, "test".into()).await.unwrap();
1510+
let list = flatten_list_stream(&integration, None).await.unwrap();
1511+
assert_eq!(list, vec![path.clone()]);
1512+
1513+
let result = integration
1514+
.get(&location)
1515+
.await
1516+
.unwrap()
1517+
.bytes()
1518+
.await
1519+
.unwrap();
1520+
assert_eq!(result, Bytes::from("test"));
1521+
}
1522+
14841523
#[tokio::test]
14851524
async fn delete_dirs_automatically() {
14861525
let root = TempDir::new().unwrap();

0 commit comments

Comments
 (0)