From ada4312a5eb5ab9f9abd259a8dd19a13223b4a9b Mon Sep 17 00:00:00 2001 From: jaideeppyne Date: Mon, 31 Aug 2026 18:09:57 +0530 Subject: [PATCH 1/3] fix(core): reject unsupported list options instead of ignoring them CorrectnessCheckLayer validates arguments for read, write, stat, delete, copy and compose, but forwarded list arguments unchecked. A service that does not support start_after, versions or deleted silently dropped them and returned a full listing. Gate those three on their capability. limit stays ungated because it is documented as a backend hint, and recursive stays ungated because SimulateLayer emulates it when list_with_recursive is false. --- core/core/src/layers/correctness_check.rs | 58 +++++++++++++++++++ .../src/types/operator/operator_futures.rs | 18 ++++++ core/core/src/types/options.rs | 10 +++- core/tests/behavior/async_list.rs | 36 ++++++++++++ 4 files changed, 120 insertions(+), 2 deletions(-) diff --git a/core/core/src/layers/correctness_check.rs b/core/core/src/layers/correctness_check.rs index 4919054b8d18..2571f24787f6 100644 --- a/core/core/src/layers/correctness_check.rs +++ b/core/core/src/layers/correctness_check.rs @@ -160,6 +160,26 @@ impl CorrectnessService { Ok(()) } + + fn check_list_args(&self, args: &OpList) -> Result<()> { + let capability = self.capability(); + let scheme = self.info().scheme(); + if args.start_after().is_some() && !capability.list_with_start_after { + return Err(new_unsupported_error( + scheme, + Operation::List, + "start_after", + )); + } + if args.versions() && !capability.list_with_versions { + return Err(new_unsupported_error(scheme, Operation::List, "versions")); + } + if args.deleted() && !capability.list_with_deleted { + return Err(new_unsupported_error(scheme, Operation::List, "deleted")); + } + + Ok(()) + } } impl Service for CorrectnessService { @@ -419,6 +439,7 @@ impl Service for CorrectnessService { } fn list(&self, ctx: &OperationContext, path: &str, args: OpList) -> Result { + self.check_list_args(&args)?; self.inner.list(ctx, path, args) } @@ -846,6 +867,43 @@ mod tests { assert!(res.is_ok()) } + #[tokio::test] + async fn test_list() { + let op = new_test_operator(Capability { + list: true, + ..Default::default() + }); + let res = op.list_with("path/").start_after("path/key").await; + assert_eq!(res.unwrap_err().kind(), ErrorKind::Unsupported); + let res = op.list_with("path/").versions(true).await; + assert_eq!(res.unwrap_err().kind(), ErrorKind::Unsupported); + let res = op.list_with("path/").deleted(true).await; + assert_eq!(res.unwrap_err().kind(), ErrorKind::Unsupported); + + let op = new_test_operator(Capability { + list: true, + list_with_start_after: true, + list_with_versions: true, + list_with_deleted: true, + ..Default::default() + }); + assert!(op.list_with("path/").start_after("path/key").await.is_ok()); + assert!(op.list_with("path/").versions(true).await.is_ok()); + assert!(op.list_with("path/").deleted(true).await.is_ok()); + } + + /// `limit` is a backend hint and `recursive` is simulated by `SimulateLayer`, + /// so neither is gated on a capability. + #[tokio::test] + async fn test_list_limit_and_recursive_need_no_capability() { + let op = new_test_operator(Capability { + list: true, + ..Default::default() + }); + assert!(op.list_with("path/").limit(1).await.is_ok()); + assert!(op.list_with("path/").recursive(true).await.is_ok()); + } + #[tokio::test] async fn test_compose() { let op = new_test_operator(Capability { diff --git a/core/core/src/types/operator/operator_futures.rs b/core/core/src/types/operator/operator_futures.rs index 924ff88cfbd4..bd708eb698d1 100644 --- a/core/core/src/types/operator/operator_futures.rs +++ b/core/core/src/types/operator/operator_futures.rs @@ -1416,6 +1416,9 @@ impl>>> FutureList { /// The start_after passes to underlying service to specify the specified key /// to start listing from. + /// + /// Requires [`Capability::list_with_start_after`]; otherwise the operation + /// fails with [`ErrorKind::Unsupported`]. pub fn start_after(mut self, v: &str) -> Self { self.args.start_after = Some(v.to_string()); self @@ -1441,6 +1444,9 @@ impl>>> FutureList { /// If `false`, version information will be omitted from the `list` results. /// /// Default to `false` + /// + /// Requires [`Capability::list_with_versions`]; otherwise the operation + /// fails with [`ErrorKind::Unsupported`]. pub fn versions(mut self, v: bool) -> Self { self.args.versions = v; self @@ -1454,6 +1460,9 @@ impl>>> FutureList { /// /// If `true`, subsequent `list` operations will include deleted files or versions. /// If `false`, deleted files or versions will be excluded from the `list` results. + /// + /// Requires [`Capability::list_with_deleted`]; otherwise the operation + /// fails with [`ErrorKind::Unsupported`]. pub fn deleted(mut self, v: bool) -> Self { self.args.deleted = v; self @@ -1477,6 +1486,9 @@ impl>> FutureLister { /// The start_after passes to underlying service to specify the specified key /// to start listing from. + /// + /// Requires [`Capability::list_with_start_after`]; otherwise the operation + /// fails with [`ErrorKind::Unsupported`]. pub fn start_after(mut self, v: &str) -> Self { self.args.start_after = Some(v.to_string()); self @@ -1502,6 +1514,9 @@ impl>> FutureLister { /// If `false`, version information will be omitted from the `list` results. /// /// Default to `false` + /// + /// Requires [`Capability::list_with_versions`]; otherwise the operation + /// fails with [`ErrorKind::Unsupported`]. pub fn versions(mut self, v: bool) -> Self { self.args.versions = v; self @@ -1515,6 +1530,9 @@ impl>> FutureLister { /// /// If `true`, subsequent `list` operations will include deleted files or versions. /// If `false`, deleted files or versions will be excluded from the `list` results. + /// + /// Requires [`Capability::list_with_deleted`]; otherwise the operation + /// fails with [`ErrorKind::Unsupported`]. pub fn deleted(mut self, v: bool) -> Self { self.args.deleted = v; self diff --git a/core/core/src/types/options.rs b/core/core/src/types/options.rs index ec2e91c872dc..48e0c934e1bc 100644 --- a/core/core/src/types/options.rs +++ b/core/core/src/types/options.rs @@ -133,12 +133,18 @@ pub struct ListOptions { pub limit: Option, /// The start_after passes to underlying service to specify the specified key /// to start listing from. + /// + /// Requires [`Capability::list_with_start_after`](crate::Capability::list_with_start_after). pub start_after: Option, /// Whether to list recursively under the prefix; default `false`. pub recursive: bool, - /// Include object versions when supported by the backend; default `false`. + /// Include object versions; default `false`. + /// + /// Requires [`Capability::list_with_versions`](crate::Capability::list_with_versions). pub versions: bool, - /// Include delete markers when supported by version-aware backends; default `false`. + /// Include delete markers; default `false`. + /// + /// Requires [`Capability::list_with_deleted`](crate::Capability::list_with_deleted). pub deleted: bool, } diff --git a/core/tests/behavior/async_list.rs b/core/tests/behavior/async_list.rs index ef25aa22a5cd..155f94abd928 100644 --- a/core/tests/behavior/async_list.rs +++ b/core/tests/behavior/async_list.rs @@ -42,6 +42,7 @@ pub fn tests(op: &Operator, tests: &mut Vec) { test_list_nested_dir, test_list_dir_with_file_path, test_list_with_start_after, + test_list_with_unsupported_options, test_list_non_exist_dir_with_recursive, test_list_dir_with_recursive, test_list_dir_with_recursive_no_trailing_slash, @@ -437,6 +438,41 @@ pub async fn test_list_with_start_after(op: Operator) -> Result<()> { Ok(()) } +/// List options the service cannot honor must fail instead of being ignored. +pub async fn test_list_with_unsupported_options(op: Operator) -> Result<()> { + let cap = op.info().capability(); + let dir = &format!("{}/", uuid::Uuid::new_v4()); + + if !cap.list_with_start_after { + let err = op + .list_with(dir) + .start_after(&format!("{dir}key")) + .await + .expect_err("start_after must be rejected when unsupported"); + assert_eq!(err.kind(), ErrorKind::Unsupported); + } + + if !cap.list_with_versions { + let err = op + .list_with(dir) + .versions(true) + .await + .expect_err("versions must be rejected when unsupported"); + assert_eq!(err.kind(), ErrorKind::Unsupported); + } + + if !cap.list_with_deleted { + let err = op + .list_with(dir) + .deleted(true) + .await + .expect_err("deleted must be rejected when unsupported"); + assert_eq!(err.kind(), ErrorKind::Unsupported); + } + + Ok(()) +} + pub async fn test_list_non_exist_dir_with_recursive(op: Operator) -> Result<()> { let dir = format!("{}/", uuid::Uuid::new_v4()); From afde7ac83794a3b2f9ca29a5829d33516f4cd869 Mon Sep 17 00:00:00 2001 From: jaideeppyne Date: Mon, 31 Aug 2026 18:24:19 +0530 Subject: [PATCH 2/3] test(bindings/cpp): split list options by capability --- bindings/cpp/tests/basic_test.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/bindings/cpp/tests/basic_test.cpp b/bindings/cpp/tests/basic_test.cpp index a19cfe3bf62f..d64071f85714 100644 --- a/bindings/cpp/tests/basic_test.cpp +++ b/bindings/cpp/tests/basic_test.cpp @@ -265,9 +265,6 @@ TEST(OpenDALOptionsTest, ListOptions) { opendal::ListOptions options; options.recursive = true; options.limit = 16; - options.start_after = "list_options/"; - options.versions = true; - options.deleted = true; auto entries = op.List("list_options/", options); std::unordered_set paths; @@ -284,6 +281,23 @@ TEST(OpenDALOptionsTest, ListOptions) { EXPECT_TRUE(paths.find("list_options/nested/file") != paths.end()); } +TEST(OpenDALOptionsTest, ListOptionsUnsupportedByService) { + opendal::Operator op("memory"); + op.Write("list_unsupported/file", "hello"); + + opendal::ListOptions start_after; + start_after.start_after = "list_unsupported/file"; + EXPECT_THROW(op.List("list_unsupported/", start_after), std::exception); + + opendal::ListOptions versions; + versions.versions = true; + EXPECT_THROW(op.List("list_unsupported/", versions), std::exception); + + opendal::ListOptions deleted; + deleted.deleted = true; + EXPECT_THROW(op.List("list_unsupported/", deleted), std::exception); +} + TEST(OpenDALOptionsTest, DeleteOptions) { opendal::Operator op("memory"); op.CreateDir("delete_options/"); From c460194ea12eb6b73655d215aa75525f49632550 Mon Sep 17 00:00:00 2001 From: jaideeppyne Date: Mon, 31 Aug 2026 19:08:25 +0530 Subject: [PATCH 3/3] test: align list option tests with the new capability check The Ruby lister test asserted the unfiltered listing that fs returned while start_after was being dropped; it now asserts the Unsupported error. Drop the behavior-suite test. CapabilityOverrideLayer is documented to change only the capability reported by OperatorInfo, not what the stack enforces, and the s3 test setups use it to mask a non-versioned bucket. A portable test cannot read the capability the check actually uses. --- bindings/ruby/test/lister_test.rb | 8 +++---- core/tests/behavior/async_list.rs | 36 ------------------------------- 2 files changed, 3 insertions(+), 41 deletions(-) diff --git a/bindings/ruby/test/lister_test.rb b/bindings/ruby/test/lister_test.rb index bfeb043f5049..0755c4542ae5 100644 --- a/bindings/ruby/test/lister_test.rb +++ b/bindings/ruby/test/lister_test.rb @@ -67,11 +67,9 @@ class ListerTest < ActiveSupport::TestCase assert_equal ["/", "sample", "sub/"], lists end - test "lists the directory with start_after" do - lister = @op.list("", start_after: "sub/") + test "rejects start_after when the service does not support it" do + error = assert_raises(RuntimeError) { @op.list("", start_after: "sub/") } - lists = lister.map(&:to_h).map { |e| e[:path] }.sort - - assert_equal ["/", "sample", "sub/"], lists # fs backend doesn't support start_after + assert_match(/does not support the operation list with the arguments start_after/, error.message) end end diff --git a/core/tests/behavior/async_list.rs b/core/tests/behavior/async_list.rs index 155f94abd928..ef25aa22a5cd 100644 --- a/core/tests/behavior/async_list.rs +++ b/core/tests/behavior/async_list.rs @@ -42,7 +42,6 @@ pub fn tests(op: &Operator, tests: &mut Vec) { test_list_nested_dir, test_list_dir_with_file_path, test_list_with_start_after, - test_list_with_unsupported_options, test_list_non_exist_dir_with_recursive, test_list_dir_with_recursive, test_list_dir_with_recursive_no_trailing_slash, @@ -438,41 +437,6 @@ pub async fn test_list_with_start_after(op: Operator) -> Result<()> { Ok(()) } -/// List options the service cannot honor must fail instead of being ignored. -pub async fn test_list_with_unsupported_options(op: Operator) -> Result<()> { - let cap = op.info().capability(); - let dir = &format!("{}/", uuid::Uuid::new_v4()); - - if !cap.list_with_start_after { - let err = op - .list_with(dir) - .start_after(&format!("{dir}key")) - .await - .expect_err("start_after must be rejected when unsupported"); - assert_eq!(err.kind(), ErrorKind::Unsupported); - } - - if !cap.list_with_versions { - let err = op - .list_with(dir) - .versions(true) - .await - .expect_err("versions must be rejected when unsupported"); - assert_eq!(err.kind(), ErrorKind::Unsupported); - } - - if !cap.list_with_deleted { - let err = op - .list_with(dir) - .deleted(true) - .await - .expect_err("deleted must be rejected when unsupported"); - assert_eq!(err.kind(), ErrorKind::Unsupported); - } - - Ok(()) -} - pub async fn test_list_non_exist_dir_with_recursive(op: Operator) -> Result<()> { let dir = format!("{}/", uuid::Uuid::new_v4());