mirror of
https://github.com/nushell/nushell
synced 2025-01-12 21:29:07 +00:00
Handle permission denied error at nu_engine::glob_from
(#14679)
<!-- if this PR closes one or more issues, you can automatically link the PR with them by using one of the [*linking keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), e.g. - this PR should close #xxxx - fixes #xxxx you can also mention related issues, PRs or discussions! --> # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> #14528 mentioned that trying to `open` a file in a directory where you don't have read access results in a "file not found" error. I investigated the error and could find the root issue in the `nu_engine::glob_from` function. It uses `std::path::Path::canonicalize` some layers down and that may return an `std::io::Error`. All these errors were handled as "directory not found" which will be translated to "file not found" in the `open` command. To fix this, I handled the `PermssionDenied` error kind of the io error and passed that down. Now trying to `open` a file from a directory with no permissions returns a "permission denied" error. Before/After: ![image](https://github.com/user-attachments/assets/168cea24-36a6-4c66-98c9-f7ccfa2ea826) # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> That error is fixed, so correct error message. # Tests + Formatting <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> fixes #14528
This commit is contained in:
parent
16e174be7e
commit
dc52a6fec5
2 changed files with 37 additions and 9 deletions
|
@ -91,6 +91,21 @@ impl Command for Open {
|
|||
file: path.item.to_string(),
|
||||
span,
|
||||
},
|
||||
// that particular error in `nu_engine::glob_from` doesn't have a span attached
|
||||
// to it, so let's add it
|
||||
ShellError::GenericError {
|
||||
error,
|
||||
msg,
|
||||
span: _,
|
||||
help,
|
||||
inner,
|
||||
} if error.as_str() == "Permission denied" => ShellError::GenericError {
|
||||
error,
|
||||
msg,
|
||||
span: Some(arg_span),
|
||||
help,
|
||||
inner,
|
||||
},
|
||||
_ => err,
|
||||
})?
|
||||
.1
|
||||
|
|
|
@ -3,6 +3,7 @@ use nu_path::{canonicalize_with, expand_path_with};
|
|||
use nu_protocol::{NuGlob, ShellError, Span, Spanned};
|
||||
use std::{
|
||||
fs,
|
||||
io::ErrorKind,
|
||||
path::{Component, Path, PathBuf},
|
||||
};
|
||||
|
||||
|
@ -70,20 +71,32 @@ pub fn glob_from(
|
|||
if is_symlink {
|
||||
(path.parent().map(|parent| parent.to_path_buf()), path)
|
||||
} else {
|
||||
let path = if let Ok(p) = canonicalize_with(path.clone(), cwd) {
|
||||
if nu_glob::is_glob(p.to_string_lossy().as_ref()) {
|
||||
let path = match canonicalize_with(path.clone(), cwd) {
|
||||
Ok(p) if nu_glob::is_glob(p.to_string_lossy().as_ref()) => {
|
||||
// our path might contain glob metacharacters too.
|
||||
// in such case, we need to escape our path to make
|
||||
// glob work successfully
|
||||
PathBuf::from(nu_glob::Pattern::escape(&p.to_string_lossy()))
|
||||
} else {
|
||||
p
|
||||
}
|
||||
} else {
|
||||
return Err(ShellError::DirectoryNotFound {
|
||||
dir: path.to_string_lossy().to_string(),
|
||||
span: pattern.span,
|
||||
});
|
||||
Ok(p) => p,
|
||||
Err(err) => {
|
||||
return match err.kind() {
|
||||
ErrorKind::PermissionDenied => Err(ShellError::GenericError {
|
||||
error: "Permission denied".into(),
|
||||
msg: err.to_string(),
|
||||
span: None,
|
||||
help: None,
|
||||
inner: vec![],
|
||||
}),
|
||||
// Previously, all these errors were treated as "directory not found."
|
||||
// Now, permission denied errors are handled separately.
|
||||
// TODO: Refine handling of I/O errors for more precise responses.
|
||||
_ => Err(ShellError::DirectoryNotFound {
|
||||
dir: path.to_string_lossy().to_string(),
|
||||
span: pattern.span,
|
||||
}),
|
||||
};
|
||||
}
|
||||
};
|
||||
(path.parent().map(|parent| parent.to_path_buf()), path)
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue