Repository navigation
fix(realpath): reject non-directory path components and forcing them to ENOTDIR - #16153
Martell0x1 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Merging this PR will not alter performance1 warning, 1 note
Comparing Footnotes
|
2f1fa3d to
3cc5e18
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
@codspeedbot explain why performance improved |
| OwningComponent::CurDir => {} | ||
| OwningComponent::ParentDir => { | ||
| // checking for current component | ||
| if !result.as_os_str().is_empty() { |
There was a problem hiding this comment.
this also runs with MissingHandling::Missing, so realpath -m missing/.. and -m file/.. fail now, GNU accepts both
please skip the check in that mode and add a -m test
There was a problem hiding this comment.
ignored the check for MissingHandler::Missing
| Ok(meta) if meta.is_dir() => {} | ||
| Ok(_) => { | ||
| return Err(Error::new(ErrorKind::NotADirectory, "Not a directory")); | ||
| } |
There was a problem hiding this comment.
why rebuild a NotFound error? Err(e) => return Err(e) is enough
| OwningComponent::CurDir => {} | ||
| OwningComponent::ParentDir => { | ||
| // checking for current component | ||
| if !result.as_os_str().is_empty() { |
There was a problem hiding this comment.
could you please reuse ensure_is_directory(&result)? here? it already gives ENOTDIR, without the hardcoded strings
|
|
||
| // spell-checker:ignore nusr | ||
|
|
||
| use uucore::util_name; |
There was a problem hiding this comment.
util_name is already imported from uutests, does this compile?
There was a problem hiding this comment.
actually it did , i guess as rust allows importing the same macro's name but from different paths
| .ucmd() | ||
| .arg("regular_file/..") | ||
| .fails() | ||
| .code_is(1) |
There was a problem hiding this comment.
the issue also mentions -L, -e and readlink -f, please add tests for them
-L still goes through normalize_path, so it isn't fixed yet
There was a problem hiding this comment.
yes i totaly forgot to add Logical path support .
|
|
||
| at.mkdir("directory"); | ||
|
|
||
| scene.ucmd().arg("directory/..").succeeds(); |
There was a problem hiding this comment.
please check the stdout too, not only the exit code
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
e7c8a6b to
8fbaeb1
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Problem
uutils
realpathincorrectly accepts paths containing a regular file followed by...For example,
/tmp/example/file/..succeeds in uutils, whereas GNUrealpathfails withENOTDIR(Not a directory).The implementation removes the preceding component when encountering
ParentDirwithout verifying that the component is a directory.Root Cause
In
src/uucore/src/lib/features/fs.rs, theParentDirbranch performs:This removes the preceding component without validating whether it can be traversed as a directory.
Fix
Validate path components before resolving
ParentDir, rejecting non-directory and nonexistent intermediate components where required by the selected resolution mode.Tests
cargo test --no-default-features --features realpathRequires #16161
Closes #16117