Skip to content

fix(core): reject unsupported list options instead of ignoring them - #8205

Open
jaideeppyne wants to merge 3 commits into
apache:mainfrom
jaideeppyne:fix/list-correctness-check
Open

fix(core): reject unsupported list options instead of ignoring them#8205
jaideeppyne wants to merge 3 commits into
apache:mainfrom
jaideeppyne:fix/list-correctness-check

Conversation

@jaideeppyne

@jaideeppyne jaideeppyne commented Aug 31, 2026

Copy link
Copy Markdown

Which issue does this PR close?

Closes #6676.

Rationale for this change

CorrectnessCheckLayer validates arguments for read, write, stat, delete, copy and compose, but list was forwarded unchecked. So a list option the service cannot honor is dropped silently and the caller gets a full listing back with no signal that anything was ignored.

Reproduced on fs, no network needed:

op.write("d/a", "x").await?;
op.write("d/b", "x").await?;
let got = op.list_with("d/").start_after("d/a").await?;
// expected ["d/b"], got ["d/", "d/a", "d/b"]

That is exactly #6676. versions(true) and deleted(true) fail the same way everywhere: they are only read by the s3, cos, tos, oss and onedrive listers and dropped by every other service. This is not fs-only either, s3-express sets list_with_start_after: false and is affected too.

Two existing tests had encoded the old behavior as expected. bindings/ruby/test/lister_test.rb asserted the full unfiltered listing with the comment "fs backend doesn't support start_after", and the C++ OpenDALOptionsTest.ListOptions set all five options at once against memory and passed only because three of them were dropped. Both are updated here.

The object_store integration already hand-rolls this guard (store.rs checks list_with_start_after and falls back to client-side filtering when it is false), which is a fair sign that callers should not have to discover the limitation themselves.

@Xuanwo, on #6676 you said you were open to this if we can inform users about the limitation clearly. An Unsupported error is the signal the layer already gives for every other operation, so this reuses it rather than inventing a new mechanism.

What changes are included in this PR?

CorrectnessService::list now rejects start_after, versions and deleted when the matching capability is false.

Two options stay ungated on purpose:

  • limit is documented as a backend hint.
  • recursive is emulated by SimulateLayer even when list_with_recursive is false, which is why fs recursion works today. Gating it would break working listings.

Tests: unit tests in correctness_check.rs covering all three options in both directions, plus a separate test pinning that limit and recursive stay ungated. The Ruby test now asserts the error, and the C++ test is split into the two options memory supports plus a new ListOptionsUnsupportedByService for the three it does not.

There is deliberately no behavior-suite test. I tried one and CI showed why it cannot work: CapabilityOverrideLayer is documented to change only the capability reported by OperatorInfo, not what the stack enforces, and it is applied above CorrectnessCheckLayer. The s3 setups use it (OPENDAL_TEST_CAPABILITY_OVERRIDES=...,list_with_versions=false,...) to mask a non-versioned MinIO bucket, so op.info().capability() there reports list_with_versions: false while the check correctly sees the service's real true. A portable test cannot read the capability the check actually uses, so asserting on the advertised one is wrong. Worth knowing separately: that layer can relabel a capability but cannot restrict behavior.

How I found it. My oracle was cross-service agreement: a 71-probe differential harness over list, stat, read, ranges, delete and create_dir, run across every backend I can exercise with no network (fs, memory, dashmap, moka, mini-moka, sled, redb, cacache, persy). start_after being silently ignored on fs was one of the divergences it turned up.

Verified locally: cargo test -p opendal-core --lib (11 correctness_check tests), the behavior suite unchanged at 130 passed on fs and 110 on memory, all 83 C++ tests via cmake/ninja, all 57 Ruby tests via rake test:base, cargo clippy -p opendal-core --all-targets -- -D warnings, cargo fmt --all --check, cargo test -p opendal-core --doc, cargo doc with no new intra-doc warnings, and the integrations/object_store suite including test_list_with_offset.

Are there any user-facing changes?

Yes. Anyone passing one of those three options to a service that does not support it used to get a successful but wrong result and now gets Unsupported. No public API signature changes, so I did not add the breaking-changes label, happy to add it if you would rather flag it in the changelog.

ListOptions and the list futures now document the capability requirement.

AI Usage Statement

AI-assisted. I used Claude Code to build and run the cross-backend differential harness that surfaced the divergence, and to draft the patch; I reviewed the result, chose the scope, and ran the verification above myself.

Assumptions and unknowns worth your review: every service that supports these three options is network-backed, so I could not exercise the positive path against a real backend. It is covered by the mock unit test and by the existing capability-gated behavior tests in CI. I also assumed limit and recursive should stay ungated for the reasons above, and that is the judgement call most worth a second opinion.

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.
@jaideeppyne
jaideeppyne requested a review from Xuanwo as a code owner August 31, 2026 12:40
@jaideeppyne
jaideeppyne requested a review from tisonkun as a code owner August 31, 2026 12:54
@jaideeppyne

Copy link
Copy Markdown
Author

Pushed a follow-up: OpenDALOptionsTest.ListOptions in the C++ binding set all five list options at once against memory, which only passed because the three unsupported ones were being dropped. Split it into the two options memory does support plus a new ListOptionsUnsupportedByService asserting the other three now throw, which also makes it a stronger plumbing test than before. All 83 C++ tests pass locally.

I swept the other bindings for the same pattern; Java, Python, Go and the Rust behavior suite all gate these three on the capability already, so that C++ test was the only unguarded caller.

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.
@jaideeppyne
jaideeppyne requested a review from PsiACE as a code owner August 31, 2026 13:38
@jaideeppyne

Copy link
Copy Markdown
Author

CI found two more things, both now handled, and one of them is worth flagging on its own.

  1. bindings/ruby/test/lister_test.rb had a start_after test asserting the full unfiltered listing, with the comment "fs backend doesn't support start_after". It was pinning the bug. It now asserts the Unsupported error. All 57 Ruby tests pass locally via rake test:base.

  2. I had added a behavior-suite test asserting that an unadvertised option is rejected, and it failed on three of the s3 setups. The cause is not the check: CapabilityOverrideLayer is applied above CorrectnessCheckLayer, and it is documented to change only the capability reported by OperatorInfo, not what the stack enforces. The s3 setups use OPENDAL_TEST_CAPABILITY_OVERRIDES=...,list_with_versions=false,... to mask a non-versioned MinIO bucket, so op.info().capability() reports false there while the check correctly sees s3's real true and lets the call through. A portable test cannot read the capability the check actually uses, so I dropped that test rather than assert on the advertised one. core/tests/behavior/async_list.rs is now byte-identical to main.

The side observation is that CapabilityOverrideLayer can relabel a capability but cannot restrict behavior, so a setup that sets list_with_versions=false to describe a non-versioned bucket still lets versions(true) through to the service. That is pre-existing and out of scope here, but happy to file it separately if it is news.

Also unrelated: the goosefs job failed on a registry timeout pulling goosefs.tencentcloudcr.com, so no OpenDAL code ran there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: python operator.list with start_after does not work for local file system

1 participant