From fea10f6d408ae8f438357cf7fc545140e092c29a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebasti=C3=A1n=20Galkin?= Date: Wed, 10 Dec 2025 18:35:42 -0300 Subject: [PATCH] Port: More fixes for virtual chunk url escapes (#1474) --- icechunk/src/virtual_chunks.rs | 16 ++++++++-------- icechunk/tests/test_virtual_refs.rs | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/icechunk/src/virtual_chunks.rs b/icechunk/src/virtual_chunks.rs index 27e2f412f..6a591cac9 100644 --- a/icechunk/src/virtual_chunks.rs +++ b/icechunk/src/virtual_chunks.rs @@ -295,11 +295,11 @@ impl VirtualChunkResolver { &self, chunk_location: &Url, ) -> Result, VirtualReferenceError> { - let cont = self - .matching_container(chunk_location.to_string().as_str()) - .ok_or_else(|| { - VirtualReferenceErrorKind::NoContainerForUrl(chunk_location.to_string()) - })?; + let location = chunk_location.to_string(); + let location = urlencoding::decode(location.as_str())?; + let cont = self.matching_container(location.as_ref()).ok_or_else(|| { + VirtualReferenceErrorKind::NoContainerForUrl(chunk_location.to_string()) + })?; let cache_key = fetcher_cache_key(cont, chunk_location)?; // TODO: we shouldn't need to clone the container name @@ -416,7 +416,7 @@ impl VirtualChunkResolver { }; let bucket_name = if let Some(host) = chunk_location.host_str() { - host.to_string() + urlencoding::decode(host)?.into_owned() } else { Err(VirtualReferenceErrorKind::CannotParseBucketName( "No bucket name found".to_string(), @@ -475,7 +475,7 @@ impl VirtualChunkResolver { }; let container = if let Some(host) = chunk_location.host_str() { - host.to_string() + urlencoding::decode(host)?.into_owned() } else { Err(VirtualReferenceErrorKind::CannotParseBucketName( "No bucket name found".to_string(), @@ -565,7 +565,7 @@ impl ChunkFetcher for S3Fetcher { checksum: Option<&Checksum>, ) -> Result, VirtualReferenceError> { let bucket_name = if let Some(host) = chunk_location.host_str() { - host.to_string() + urlencoding::decode(host)?.into_owned() } else { Err(VirtualReferenceErrorKind::CannotParseBucketName( "No bucket name found".to_string(), diff --git a/icechunk/tests/test_virtual_refs.rs b/icechunk/tests/test_virtual_refs.rs index 6db877cd0..e0c6c3d9e 100644 --- a/icechunk/tests/test_virtual_refs.rs +++ b/icechunk/tests/test_virtual_refs.rs @@ -210,6 +210,19 @@ async fn create_minio_repository() -> Repository { }), ) .unwrap(), + VirtualChunkContainer::new( + "s3://testbucket/path with spaces/".to_string(), + ObjectStoreConfig::S3Compatible(S3Options { + region: Some(String::from("us-east-1")), + endpoint_url: Some("http://localhost:9000".to_string()), + anonymous: false, + allow_http: true, + force_path_style: true, + network_stream_timeout_seconds: None, + requester_pays: false, + }), + ) + .unwrap(), VirtualChunkContainer::new( "az://testcontainer/".to_string(), ObjectStoreConfig::Azure(HashMap::from([ @@ -230,6 +243,15 @@ async fn create_minio_repository() -> Repository { expires_after: None, }))), ), + ( + "s3://testbucket/path with spaces/".to_string(), + Some(Credentials::S3(S3Credentials::Static(S3StaticCredentials { + access_key_id: "minio123".to_string(), + secret_access_key: "minio123".to_string(), + session_token: None, + expires_after: None, + }))), + ), ( "az://testcontainer/".to_string(), Some(Credentials::Azure(AzureCredentials::FromEnv)),