Conversation
Implements RFC 7799 for path normalization.
erickguan
changed the title
erickguan
marked this pull request as ready for review
June 21, 2026 12:23
dosubot
Bot
added
size:XL
4 tasks
This was referenced Aug 10, 2026
erickguan
pushed a commit
that referenced
this pull request
Aug 15, 2026
#8070) * 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. * 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. * Drop the inline comment
erickguan
pushed a commit
that referenced
this pull request
Aug 15, 2026
… bodies (#8079) * fix(services/dbfs): root the put path and stop encoding paths in JSON bodies Two defects in how this service addresses paths, both in core.rs. The path in the dbfs/put body is not rooted: let req_body = &json!({ "path": path, Every sibling roots it first -- create_dir at :60, delete at :89, rename at :116, list at :142, get-status at :193 all call build_rooted_abs_path. The value arrives from DbfsBackend::write as the operator-relative path and nothing else applies the root, so with any non-default root a write and the stat, list or delete that follows it address different DBFS paths. And four values are percent-encoded inside JSON request bodies: "path": percent_encode_path(&p), // :65, :94 "source_path": percent_encode_path(&source), // :126 "destination_path": percent_encode_path(&target), // :127 Nothing URL-decodes a JSON string value. The genuine query strings at :149 and :200 are encoded, and the server does decode those -- so the two halves of this file disagree with each other. create_dir("my dir/") creates a directory literally named my%20dir, while the get-status that follows asks for "my dir" and gets NotFound. For delete it is worse than an error: deleter.rs notes the server answers 200 even when the path does not exist, so the call reports success having removed nothing. A repo-wide grep for percent_encode_path inside json! across core/services matches these four lines and nothing else; dbfs is the only service in the tree that does it. Neither change affects an ordinary path. percent_encode_path leaves A-Z a-z 0-9 / - _ . ! ~ * ' ( ) untouched, so a plain key serialises identically; only a path containing a space, %, #, ?, &, +, ,, :, =, @ or a non-ASCII byte changes, and those are the ones broken today. One test, on the only one of the four that is a synchronous request builder. The other three send inside async methods, so pinning them would mean restructuring; their evidence is the in-file inconsistency above. Reverting the rooting fails that test and nothing else. The lister emits root-prefixed entry paths as well -- that is a separate change to a separate file and is not in here. Note for rebasing: #7801 renames build_rooted_abs_path in this file. If it lands first the new call in dbfs_create_file_request needs the new name. * Remove the test module
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
1 participant
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?
Implements RFC 7799's path normalization part.
Also closes #6577
Rationale for this change
What changes are included in this PR?
.trim()fromnormalize_pathto preserve whitespace in path components, aligning with POSIX, URI, and object-store conventions where whitespace is significant content..segment filtering to bothnormalize_pathandnormalize_rootso that./a/./bnormalizes toa/b.build_absolute_path,build_rooted_absolute_path, andbuild_relative_pathas the canonical names; deprecate the oldbuild_abs_path,build_rooted_abs_path,build_rel_pathaliases.Are there any user-facing changes?
No
AI Usage Statement
GPT-5.5 with codex implements some code.