From 5aef1796ccf8efb98aef52cfa5454f211e156c76 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sun, 6 Sep 2026 13:11:35 +0800 Subject: [PATCH] fix(odm): reject ambiguous native source dot segments (#7263) * fix(odm): reject ambiguous native source dot segments * docs(odm): align native provider limitations with implementation --- docs/operations/on-demand-migration.md | 4 +- rustfs/src/on_demand_migration/azure.rs | 12 +++++ rustfs/src/on_demand_migration/gcs.rs | 11 ++++ rustfs/src/on_demand_migration/native_http.rs | 50 ++++++++++++++++++- 4 files changed, 74 insertions(+), 3 deletions(-) diff --git a/docs/operations/on-demand-migration.md b/docs/operations/on-demand-migration.md index 2932ecb3f..9717559f3 100644 --- a/docs/operations/on-demand-migration.md +++ b/docs/operations/on-demand-migration.md @@ -358,8 +358,8 @@ sum by (bucket, reason) (rate(rustfs_on_demand_migration_pull_failures_total[5m] - **Source updates do not propagate.** Once an object is pulled, the local copy is authoritative; a later change on the source is never noticed. Plan the cutover so the source stops taking writes. - **Unversioned buckets re-pull deleted keys.** An unversioned bucket keeps nothing after a delete, so the key looks like an ordinary miss and is migrated again. Only a versioned bucket can shadow the source with a delete marker (`respect_local_delete_marker`). - **SSE-C source objects are not supported.** They are rejected with 424 `unsupported`; migrate them by another route. -- **Anonymous (credential-less) sources are not supported yet.** `source.credentials: null` parses and passes structural validation, but the client builder has no anonymous mode, so the admin `PUT` refuses it and the runtime would treat such a bucket as unavailable. A public source still needs a key pair. -- **Azure Blob is not a supported source** (rustfs/backlog#2166). GCS is supported only through its XML interoperability API with HMAC keys. +- **Native Azure/GCS keys containing a standalone `.` or `..` path segment are unsupported.** The URL transport would remove that segment and address a different object. These keys fail before any source request; ordinary dotted names, repeated slashes and literal percent escapes keep their identity. +- **Anonymous S3 sources are not supported yet.** `source.credentials: null` parses and passes structural validation, but the S3 client builder has no anonymous mode, so the admin `PUT` refuses it and the runtime would treat such a bucket as unavailable. A public S3 source still needs a key pair; native Azure/GCS credentials belong in their provider blocks. - **LIST merges the source only when asked, and only for v2.** With the default `policy.list_through = false` a client that lists before reading will not see un-migrated keys. Turning it on merges `ListObjectsV2` alone; `ListObjects` (v1) and `ListObjectVersions` stay local. - **A merged listing costs up to two local listings and two source listings per page** (one per side, plus a refill when the previous page consumed most of what that side had buffered). Walking N merged keys at `max-keys=K` therefore costs ceil(N/K) requests and between ceil(N/K) and 2*ceil(N/K) source listings. Source listings are capped at 10 per second per bucket (a compile-time constant); a listing that cannot get a slot inside one second is treated like a source failure and follows `policy.source_error`. - **A degraded merged page loses the source keys in its window.** Under `source_error = not_found` the page is answered locally and the source cursor is left where it was, so the keys the source would have contributed between the previous page's last key and this one are not shown again once pagination moves on. The `x-rustfs-on-demand-migration-list: local_only` header marks every page this happened on. diff --git a/rustfs/src/on_demand_migration/azure.rs b/rustfs/src/on_demand_migration/azure.rs index da904c91e..ff2e90cb0 100644 --- a/rustfs/src/on_demand_migration/azure.rs +++ b/rustfs/src/on_demand_migration/azure.rs @@ -917,6 +917,18 @@ mod tests { assert!(head.sse.is_none()); } + #[tokio::test] + async fn dot_segment_keys_fail_before_any_source_request() { + let (endpoint, recorded) = scripted_server(Vec::new()).await; + let backend = backend(&endpoint, Credential::SharedKey(vec![7_u8; 32])); + for key in [".", "..", "dir/./key", "dir/../key", "\u{fffe}/../key"] { + assert!(matches!(backend.head(key).await, Err(SourceError::Unsupported(_))), "HEAD {key:?}"); + assert!(matches!(backend.get(key, None).await, Err(SourceError::Unsupported(_))), "GET {key:?}"); + assert!(matches!(backend.tagging(key).await, Err(SourceError::Unsupported(_))), "tags {key:?}"); + } + assert!(recorded.lock().expect("recorder lock").is_empty()); + } + #[tokio::test] async fn sas_credentials_travel_in_the_query_and_never_sign() { let (endpoint, recorded) = scripted_server(vec![ScriptedResponse::new(200, blob_headers(), String::new())]).await; diff --git a/rustfs/src/on_demand_migration/gcs.rs b/rustfs/src/on_demand_migration/gcs.rs index 5b43ca6ec..e5696748d 100644 --- a/rustfs/src/on_demand_migration/gcs.rs +++ b/rustfs/src/on_demand_migration/gcs.rs @@ -372,6 +372,17 @@ mod tests { ] } + #[tokio::test] + async fn dot_segment_keys_fail_before_any_source_request() { + let (endpoint, recorded) = scripted_server(Vec::new()).await; + let backend = backend(&endpoint); + for key in [".", "..", "dir/./key", "dir/../key", "\u{fffe}/../key"] { + assert!(matches!(backend.head(key).await, Err(SourceError::Unsupported(_))), "HEAD {key:?}"); + assert!(matches!(backend.get(key, None).await, Err(SourceError::Unsupported(_))), "GET {key:?}"); + } + assert!(recorded.lock().expect("recorder lock").is_empty()); + } + #[test] fn objects_list_maps_items_prefixes_and_the_page_token() { let page = parse_objects_list(LIST_PAGE_ONE).expect("page should parse"); diff --git a/rustfs/src/on_demand_migration/native_http.rs b/rustfs/src/on_demand_migration/native_http.rs index 9e1507ce1..519013de1 100644 --- a/rustfs/src/on_demand_migration/native_http.rs +++ b/rustfs/src/on_demand_migration/native_http.rs @@ -116,7 +116,15 @@ impl NativeHttp { .path_segments_mut() .map_err(|_| SourceError::Other("source endpoint cannot carry a path".to_string()))?; path.clear(); - path.extend(segments); + for segment in segments { + // URL normalization drops standalone dot segments. Sending + // that URL could fetch another object and backfill its bytes + // under the originally requested key. + if matches!(segment, "." | "..") { + return Err(SourceError::Unsupported("source path contains an unsupported dot segment".to_string())); + } + path.push(segment); + } } Ok(url) } @@ -431,4 +439,44 @@ mod tests { assert_eq!(url.as_str(), "https://acct.blob.core.windows.net/container/dir/a%20b%3Fc%23d.txt"); assert_eq!(url.query(), None, "a key with '?' must not become a query"); } + + #[test] + fn native_http_refuses_dot_segments_instead_of_addressing_another_object() { + let http = NativeHttp::for_test(Url::parse("https://source.example.com").expect("origin")); + for key in [ + ".", + "..", + "./key", + "../key", + "dir/./key", + "dir/../key", + "dir/.", + "dir/..", + "\u{fffe}/../key", + ] { + let error = http + .url(std::iter::once("bucket").chain(key.split('/'))) + .expect_err("dot segments must not disappear"); + assert!(matches!(error, SourceError::Unsupported(_)), "{key:?}: {error}"); + } + } + + #[test] + fn native_http_preserves_ordinary_dots_empty_segments_and_literal_escapes() { + let http = NativeHttp::for_test(Url::parse("https://source.example.com").expect("origin")); + for (key, path) in [ + ("file.txt", "/bucket/file.txt"), + (".hidden/.../tail.", "/bucket/.hidden/.../tail."), + ("/dir//key/", "/bucket//dir//key/"), + ("%2e/%2E%2E/key", "/bucket/%252e/%252E%252E/key"), + ("a+b &?#", "/bucket/a+b%20&%3F%23"), + ] { + let url = http + .url(std::iter::once("bucket").chain(key.split('/'))) + .expect("representable key"); + assert_eq!(url.path(), path, "{key:?}"); + assert!(url.query().is_none()); + assert!(url.fragment().is_none()); + } + } }