fix(services/upyun): do not panic when creating the service root - #8050
Open
PDGGK wants to merge 1 commit into
Open
fix(services/upyun): do not panic when creating the service root#8050PDGGK wants to merge 1 commit into
PDGGK wants to merge 1 commit into
Conversation
create_dir sliced the last byte off the absolute path to drop its
trailing slash:
let path = build_abs_path(&self.root, path);
let path = path[..path.len() - 1].to_string();
build_abs_path returns an empty string when the root is / and the path
is /, so path.len() - 1 underflows and the call panics with "attempt to
subtract with overflow" rather than returning a Result.
The root is reachable from the public API: Operator::create_dir only
rejects a path that does not end with /, and / does end with /, so
op.create_dir("/") on a default-rooted upyun operator panics inside the
library.
This was the only `len() - 1]` slice left in core/services; the sibling
services already use trim_end_matches('/') for this, which is total and
identical for a normalized path since normalize_path leaves exactly one
trailing slash.
Two unit tests. The root one panics against the previous expression;
the trailing-slash one passes either way, so it is a control rather
than a second copy of the same assertion.
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
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?
None filed — reporting and fixing together.
Rationale of this change
UpyunCore::create_dirdropped the trailing slash by slicing off the last byte:build_abs_pathreturns an empty string when the root is/and the path is/:so
path.len() - 1underflows and the call panics withattempt to subtract with overflowinstead of returning aResult.The root is reachable from the public API.
Operator::create_dironly rejects a path that does not end with/:and
"/"does end with/, soop.create_dir("/").awaiton a default-rooted upyun operator panics inside the library rather than returning an error.This was the only remaining
len() - 1]slice undercore/services. The sibling services already usetrim_end_matches('/')for exactly this (azdls/src/core.rsuses it in five places), which is total and produces an identical result for a normalized path, sincenormalize_pathleaves exactly one trailing slash.Are there any user-facing changes?
A panic becomes ordinary behaviour: the request is built against the bucket root and the service's own response decides the outcome. No change for any non-root path.
Tests
Two unit tests on the extracted
folder_pathhelper. The root case panics against the previous expression:The trailing-slash case passes both before and after, so it serves as the control showing the first is not trivially red.
With the fix: 4/4 pass.
cargo fmt --checkexits 0,cargo clippy --all-targetsis clean.