fix(core/layers): handle non-trailing-slash path in native recursive list#7705
Open
tonghuaroot wants to merge 2 commits into
Open
fix(core/layers): handle non-trailing-slash path in native recursive list#7705tonghuaroot wants to merge 2 commits into
tonghuaroot wants to merge 2 commits into
Conversation
…list The native-recursive arm of SimulateAccessor::simulate_list `(_, true, _)` forwarded the list path directly to the backend. For a path without a trailing slash (for example `dir/file`), a backend that natively supports recursive list (such as a WebDAV server honoring `Depth: infinity`) walks the subtree of `dir/file` and returns only that entry, silently dropping prefix-siblings like `dir/file2`. Both the simulated-recursive arm `(true, false, true)` and the non-recursive arm `(false, false, _)` already handle this: for a path without a trailing slash they list the parent directory and wrap the result in a PrefixLister so that entries sharing the path prefix are returned. The native-recursive arm now applies the same handling, making recursive list of a non-trailing-slash path consistent across all backends regardless of native capability. The in-memory service tolerated the bug because its native lister treats the trailing path component as a prefix, so it stayed latent until a real WebDAV backend with native `Depth: infinity` exercised it. A focused regression test using a WebDAV-style native-recursive mock backend is added: it fails before this change (the sibling is dropped) and passes after it. Discovered while investigating apache#4256. Signed-off-by: tonghuaroot <23011166+tonghuaroot@users.noreply.github.com>
dosubot
Bot
added
size:L
2 tasks
Drop step-by-step narration in the prefix-handling arm and the test fixture/regression-test doc comments; keep the non-obvious intent.
erickguan
requested changes
Jun 21, 2026
| self.config.list_recursive, | ||
| ) { | ||
| // Backend supports recursive list, forward directly. | ||
| // Match the non-recursive arm: for a non-trailing-slash path, list |
Member
There was a problem hiding this comment.
minor (non-blocking): If we are breaking out these pattern matching, I would go with some more clear function names. A bunch of boolean matches are really difficult to follow.
Comment on lines
+371
to
+372
| /// Native-recursive backend with WebDAV `Depth: infinity` semantics: a file | ||
| /// path lists only that file and its subtree, not prefix-siblings. |
Member
There was a problem hiding this comment.
Suggested change
| /// Native-recursive backend with WebDAV `Depth: infinity` semantics: a file | |
| /// path lists only that file and its subtree, not prefix-siblings. | |
| /// Native-recursive backend, e.g., WebDAV with `Depth: infinity` semantics, a file | |
| /// path lists only that file and its subtree, not prefix-siblings. |
Did you find tests for non-recursive service tests (perhaps tests for this layer's behavior?
| fn info(&self) -> Arc<AccessorInfo> { | ||
| let info = AccessorInfo::default(); | ||
| info.set_scheme("memory"); | ||
| info.set_native_capability(Capability { |
Member
There was a problem hiding this comment.
We integrated this API into capability() API in Service trait now.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Related to #4256 (does not close it). Fixes a latent correctness bug in the recursive-list path, found while investigating #4256.
Rationale for this change
SimulateAccessor::simulate_list's non-recursive and simulated-recursive arms already prefix-handle a list path without a trailing slash (list the parent, wrap inPrefixLister), so listingdir/filereturns bothdir/fileanddir/file2. The native-recursive arm (_, true, _) didn't — it forwarded the path straight to the backend. On a backend with subtree semantics (WebDAVDepth: infinity) that returns onlydir/file, dropping prefix-siblings. The in-memory service hid this because its native lister is prefix-based.What changes are included in this PR?
PrefixLister, matching the other two arms; trailing-slash paths forward as before.Depth: infinitybackend.