From c65b386bbe5d573819a84602ee3f54b71498ae7a Mon Sep 17 00:00:00 2001 From: Zihan Dai <99155080+PDGGK@users.noreply.github.com> Date: Sat, 15 Aug 2026 00:28:10 +1000 Subject: [PATCH 1/3] fix(services/ipmfs): stop stripping the root twice from listed entries The listing loop concatenates the path being listed with a files/ls name, then runs the result through build_rel_path a second time: let path = match object.mode() { EntryMode::FILE => format!("{}{}", self.path, object.name), ... }; let path = build_rel_path(&self.root, &path); self.path is what the operator handed the service, so it is already relative to the root -- IpmfsCore::ipmfs_ls is what turns it into a rooted absolute path for the request. The concatenation is therefore root-relative before that call, and the call removes a prefix that is not there. Under the default root "/" the second strip happens to cancel out, which is why this has gone unnoticed. Under any other root, in release: root "/abc/", listing "dir/", name "a" build_rel_path("/abc/", "dir/a") -> "a" the directory is dropped from every entry path root "/abc/", listing the root itself (self.path is "/"), name "a" build_rel_path("/abc/", "/a") panicked: start byte index 5 is out of bounds for string of length 2 In a debug build both cases trip the debug_assert! inside build_rel_path instead. The construction moves into build_entry_path, which takes no root at all -- an entry path does not depend on one. The only thing it has to handle is that self.path is "/" when the root itself is listed, and an entry path carries no leading slash. The self-entry twenty lines above is left alone: it does build_abs_path then build_rel_path, a deliberate round trip, and is correct. Three unit tests. Putting the old expression back passes all of them with a root of "/" and fails three of them with a root of "/abc/" -- which is the shape of the bug. Note for rebasing: #7801 renames this very call to build_relative_path. If that lands first this needs a one-word rebase; the call itself goes away here. --- core/services/ipmfs/src/lister.rs | 61 +++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/core/services/ipmfs/src/lister.rs b/core/services/ipmfs/src/lister.rs index d59eb21f2edd..4d7b481379b6 100644 --- a/core/services/ipmfs/src/lister.rs +++ b/core/services/ipmfs/src/lister.rs @@ -48,6 +48,25 @@ impl IpmfsLister { } } +/// Build a listed entry's path from the path being listed and one `files/ls` name. +/// +/// `list_path` is the path the operator handed to the service, so it is already relative to the +/// root -- `IpmfsCore::ipmfs_ls` is what turns it into a rooted absolute path for the request. +/// Concatenating a name onto it therefore yields a root-relative path directly, and stripping the +/// root off it a second time is what used to truncate the result (or panic outright). +/// +/// The one thing the concatenation has to handle is that `list_path` is `"/"` when the root +/// itself is listed, and an entry path carries no leading slash. +fn build_entry_path(list_path: &str, name: &str, mode: EntryMode) -> String { + let prefix = if list_path == "/" { "" } else { list_path }; + + match mode { + EntryMode::FILE => format!("{prefix}{name}"), + EntryMode::DIR => format!("{prefix}{name}/"), + EntryMode::Unknown => unreachable!(), + } +} + impl oio::PageList for IpmfsLister { async fn next_page(&self, ctx: &mut oio::PageContext) -> Result<()> { let resp = self.core.ipmfs_ls(&self.ctx, &self.path).await?; @@ -79,13 +98,7 @@ impl oio::PageList for IpmfsLister { ctx.done = true; for object in entries_body.entries.unwrap_or_default() { - let path = match object.mode() { - EntryMode::FILE => format!("{}{}", self.path, object.name), - EntryMode::DIR => format!("{}{}/", self.path, object.name), - EntryMode::Unknown => unreachable!(), - }; - - let path = build_rel_path(&self.root, &path); + let path = build_entry_path(&self.path, &object.name, object.mode()); ctx.entries.push_back(oio::Entry::new( &path, @@ -136,3 +149,37 @@ struct IpfsLsResponse { #[serde(rename = "Entries")] entries: Option>, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn listing_the_root_yields_bare_names() { + // `list_path` is "/" here, and an entry path carries no leading slash. + assert_eq!(build_entry_path("/", "a.txt", EntryMode::FILE), "a.txt"); + assert_eq!(build_entry_path("/", "sub", EntryMode::DIR), "sub/"); + } + + #[test] + fn listing_a_directory_keeps_that_directory_in_the_entry_path() { + assert_eq!( + build_entry_path("dir/", "a.txt", EntryMode::FILE), + "dir/a.txt" + ); + assert_eq!(build_entry_path("dir/", "sub", EntryMode::DIR), "dir/sub/"); + } + + #[test] + fn the_entry_path_is_independent_of_the_service_root() { + // This is the property the previous expression lacked. It ran the concatenation through + // `build_rel_path(root, ..)`, which under the default root "/" is a no-op -- and under + // any other root removed a prefix that was never there. `build_entry_path` takes no root + // at all, so the same listing produces the same entry paths whatever the root is. + assert_eq!( + build_entry_path("dir/", "a.txt", EntryMode::FILE), + "dir/a.txt" + ); + assert_eq!(build_entry_path("/", "a.txt", EntryMode::FILE), "a.txt"); + } +} From 54b94e5f64b5ddcc6fdd27cbca12347a74d830f7 Mon Sep 17 00:00:00 2001 From: Zihan Dai <99155080+PDGGK@users.noreply.github.com> Date: Sat, 15 Aug 2026 14:51:41 +1000 Subject: [PATCH 2/3] Inline the entry-path construction and drop the test module Matching the shape asked for on #8068, #8069 and #8071 rather than waiting to be asked again: the extracted helper goes, the construction is inline, and the test module goes with it. The fix is unchanged -- the concatenation is already relative to the root, so the second build_rel_path is what truncated it, and "/" as the listed path must not become a leading slash on the entry. --- core/services/ipmfs/src/lister.rs | 67 ++++++------------------------- 1 file changed, 13 insertions(+), 54 deletions(-) diff --git a/core/services/ipmfs/src/lister.rs b/core/services/ipmfs/src/lister.rs index 4d7b481379b6..282ff93e09e8 100644 --- a/core/services/ipmfs/src/lister.rs +++ b/core/services/ipmfs/src/lister.rs @@ -48,25 +48,6 @@ impl IpmfsLister { } } -/// Build a listed entry's path from the path being listed and one `files/ls` name. -/// -/// `list_path` is the path the operator handed to the service, so it is already relative to the -/// root -- `IpmfsCore::ipmfs_ls` is what turns it into a rooted absolute path for the request. -/// Concatenating a name onto it therefore yields a root-relative path directly, and stripping the -/// root off it a second time is what used to truncate the result (or panic outright). -/// -/// The one thing the concatenation has to handle is that `list_path` is `"/"` when the root -/// itself is listed, and an entry path carries no leading slash. -fn build_entry_path(list_path: &str, name: &str, mode: EntryMode) -> String { - let prefix = if list_path == "/" { "" } else { list_path }; - - match mode { - EntryMode::FILE => format!("{prefix}{name}"), - EntryMode::DIR => format!("{prefix}{name}/"), - EntryMode::Unknown => unreachable!(), - } -} - impl oio::PageList for IpmfsLister { async fn next_page(&self, ctx: &mut oio::PageContext) -> Result<()> { let resp = self.core.ipmfs_ls(&self.ctx, &self.path).await?; @@ -98,7 +79,19 @@ impl oio::PageList for IpmfsLister { ctx.done = true; for object in entries_body.entries.unwrap_or_default() { - let path = build_entry_path(&self.path, &object.name, object.mode()); + // `self.path` is "/" when the root itself is listed, and an entry path carries + // no leading slash. It is already relative to the root, so it must not be + // stripped again. + let prefix = if self.path == "/" { + "" + } else { + self.path.as_str() + }; + let path = match object.mode() { + EntryMode::FILE => format!("{prefix}{}", object.name), + EntryMode::DIR => format!("{prefix}{}/", object.name), + EntryMode::Unknown => unreachable!(), + }; ctx.entries.push_back(oio::Entry::new( &path, @@ -149,37 +142,3 @@ struct IpfsLsResponse { #[serde(rename = "Entries")] entries: Option>, } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn listing_the_root_yields_bare_names() { - // `list_path` is "/" here, and an entry path carries no leading slash. - assert_eq!(build_entry_path("/", "a.txt", EntryMode::FILE), "a.txt"); - assert_eq!(build_entry_path("/", "sub", EntryMode::DIR), "sub/"); - } - - #[test] - fn listing_a_directory_keeps_that_directory_in_the_entry_path() { - assert_eq!( - build_entry_path("dir/", "a.txt", EntryMode::FILE), - "dir/a.txt" - ); - assert_eq!(build_entry_path("dir/", "sub", EntryMode::DIR), "dir/sub/"); - } - - #[test] - fn the_entry_path_is_independent_of_the_service_root() { - // This is the property the previous expression lacked. It ran the concatenation through - // `build_rel_path(root, ..)`, which under the default root "/" is a no-op -- and under - // any other root removed a prefix that was never there. `build_entry_path` takes no root - // at all, so the same listing produces the same entry paths whatever the root is. - assert_eq!( - build_entry_path("dir/", "a.txt", EntryMode::FILE), - "dir/a.txt" - ); - assert_eq!(build_entry_path("/", "a.txt", EntryMode::FILE), "a.txt"); - } -} From 470cc6105bf48e29f51e88b82403603b0f056d8f Mon Sep 17 00:00:00 2001 From: Zihan Dai <99155080+PDGGK@users.noreply.github.com> Date: Sat, 15 Aug 2026 15:04:21 +1000 Subject: [PATCH 3/3] Drop the inline comment --- core/services/ipmfs/src/lister.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/services/ipmfs/src/lister.rs b/core/services/ipmfs/src/lister.rs index 282ff93e09e8..434988dcd45e 100644 --- a/core/services/ipmfs/src/lister.rs +++ b/core/services/ipmfs/src/lister.rs @@ -79,9 +79,6 @@ impl oio::PageList for IpmfsLister { ctx.done = true; for object in entries_body.entries.unwrap_or_default() { - // `self.path` is "/" when the root itself is listed, and an entry path carries - // no leading slash. It is already relative to the root, so it must not be - // stripped again. let prefix = if self.path == "/" { "" } else {