Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow filesystem commands to access files with glob metachars in name #10694

Merged
merged 10 commits into from
Oct 18, 2023
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 10 additions & 3 deletions crates/nu-cmd-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,15 @@ version = "0.86.1"

[dependencies]
nu-engine = { path = "../nu-engine", version = "0.86.1" }
nu-glob = { path = "../nu-glob", version = "0.86.1" }
nu-parser = { path = "../nu-parser", version = "0.86.1" }
nu-path = { path = "../nu-path", version = "0.86.1" }
nu-protocol = { version = "0.86.1", path = "../nu-protocol" }
indexmap = { version = "2.0" }
miette = { version = "5.10", features = ["fancy-no-backtrace"] }
nu-protocol = { path = "../nu-protocol", version = "0.86.1" }
nu-utils = { path = "../nu-utils", version = "0.86.1" }

indexmap = "2.0"
miette = "5.10.0"

[dev-dependencies]
nu-test-support = { path = "../nu-test-support", version = "0.86.1" }
rstest = "0.18.2"
207 changes: 207 additions & 0 deletions crates/nu-cmd-base/src/arg_glob.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
// utilities for expanding globs in command arguments

use nu_glob::{glob_with_parent, MatchOptions, Paths};
use nu_protocol::{ShellError, Spanned};
use std::fs;
use std::path::{Path, PathBuf};

// standard glob options to use for filesystem command arguments

const GLOB_PARAMS: MatchOptions = MatchOptions {
case_sensitive: true,
require_literal_separator: false,
require_literal_leading_dot: false,
recursive_match_hidden_dir: true,
};

// handle an argument that could be a literal path or a glob.
// if literal path, return just that (whether user can access it or not).
// if glob, expand into matching paths, using GLOB_PARAMS options.
pub fn arg_glob(
pattern: &Spanned<String>, // alleged path or glob
cwd: &Path, // current working directory
) -> Result<Paths, ShellError> {
arg_glob_opt(pattern, cwd, GLOB_PARAMS)
}

// variant of [arg_glob] that requires literal dot prefix in pattern to match dot-prefixed path.
pub fn arg_glob_leading_dot(pattern: &Spanned<String>, cwd: &Path) -> Result<Paths, ShellError> {
arg_glob_opt(
pattern,
cwd,
MatchOptions {
require_literal_leading_dot: true,
..GLOB_PARAMS
},
)
}

fn arg_glob_opt(
pattern: &Spanned<String>,
cwd: &Path,
options: MatchOptions,
) -> Result<Paths, ShellError> {
// remove ansi coloring (?)
let pattern = {
Spanned {
item: nu_utils::strip_ansi_string_unlikely(pattern.item.clone()),
span: pattern.span,
}
};

// if there's a file with same path as the pattern, just return that.
let pp = cwd.join(&pattern.item);
let md = fs::metadata(pp);
#[allow(clippy::single_match)]
match md {
Ok(_metadata) => {
return Ok(Paths::single(&PathBuf::from(pattern.item), cwd));
}
// file not found, but also "invalid chars in file" (e.g * on Windows). Fall through and glob
Err(_) => {}
}

// user wasn't referring to a specific thing in filesystem, try to glob it.
match glob_with_parent(&pattern.item, options, cwd) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @bobhy
Sorry for a simple question, why are you using glob_with_parent rather than glob_with?

Ok(p) => Ok(p),
Err(pat_err) => {
Err(ShellError::InvalidGlobPattern(
pat_err.msg.into(),
pattern.span, // improve specificity
))
}
}
}

#[cfg(test)]
mod test {
use super::*;
use nu_glob::GlobResult;
use nu_protocol::{Span, Spanned};
use nu_test_support::fs::Stub::EmptyFile;
use nu_test_support::playground::Playground;
use rstest::rstest;

fn spanned_string(str: &str) -> Spanned<String> {
Spanned {
item: str.to_string(),
span: Span::test_data(),
}
}

#[test]
fn does_something() {
let act = arg_glob(&spanned_string("*"), &PathBuf::from("."));
assert!(act.is_ok());
for f in act.expect("checked ok") {
match f {
Ok(p) => {
assert!(!p.to_str().unwrap().is_empty());
}
Err(e) => panic!("unexpected error {:?}", e),
};
}
}

#[test]
fn glob_format_error() {
let act = arg_glob(&spanned_string(r#"ab]c[def"#), &PathBuf::from("."));
assert!(act.is_err());
}

#[rstest]
#[case("*", 4, "no dirs")]
#[case("**/*", 7, "incl dirs")]
fn glob_subdirs(#[case] pat: &str, #[case] exp_count: usize, #[case] case: &str) {
Playground::setup("glob_subdirs", |dirs, sandbox| {
sandbox.with_files(vec![
EmptyFile("yehuda.txt"),
EmptyFile("jttxt"),
EmptyFile("andres.txt"),
]);
sandbox.mkdir(".children");
sandbox.within(".children").with_files(vec![
EmptyFile("timothy.txt"),
EmptyFile("tiffany.txt"),
EmptyFile("trish.txt"),
]);

let p: Vec<GlobResult> = arg_glob(&spanned_string(pat), &dirs.test)
.expect("no error")
.collect();
assert_eq!(
exp_count,
p.iter().filter(|i| i.is_ok()).count(),
" case: {case} ",
);

// expected behavior -- that directories are included in results (if name matches pattern)
let t = p
.iter()
.any(|i| i.as_ref().unwrap().to_string_lossy().contains(".children"));
assert!(t, "check for dir, case {case}");
})
}

#[rstest]
#[case("yehuda.txt", true, 1, "matches literal path")]
#[case("*", false, 3, "matches glob")]
#[case(r#"bad[glob.foo"#, true, 1, "matches literal, would be bad glob pat")]
fn exact_vs_glob(
#[case] pat: &str,
#[case] exp_matches_input: bool,
#[case] exp_count: usize,
#[case] case: &str,
) {
Playground::setup("exact_vs_glob", |dirs, sandbox| {
sandbox.with_files(vec![
EmptyFile("yehuda.txt"),
EmptyFile("jttxt"),
EmptyFile("bad[glob.foo"),
]);

let res = arg_glob(&spanned_string(pat), &dirs.test)
.expect("no error")
.collect::<Vec<GlobResult>>();

eprintln!("res: {:?}", res);
if exp_matches_input {
assert_eq!(
exp_count,
res.len(),
" case {case}: matches input, but count not 1? "
);
assert_eq!(
&res[0].as_ref().unwrap().to_string_lossy(),
pat, // todo: is it OK for glob to return relative paths (not to current cwd, but to arg cwd of arg_glob)?
);
} else {
assert_eq!(exp_count, res.len(), " case: {}: matched glob", case);
}
})
}

#[rstest]
#[case(r#"realbad[glob.foo"#, true, 1, "error, bad glob")]
fn exact_vs_bad_glob(
// if path doesn't exist but pattern is not valid glob, should get error.
#[case] pat: &str,
#[case] _exp_matches_input: bool,
#[case] _exp_count: usize,
#[case] _tag: &str,
) {
Playground::setup("exact_vs_bad_glob", |dirs, sandbox| {
sandbox.with_files(vec![
EmptyFile("yehuda.txt"),
EmptyFile("jttxt"),
EmptyFile("bad[glob.foo"),
]);

let res = arg_glob(&spanned_string(pat), &dirs.test);
assert!(res
.expect_err("expected error")
.to_string()
.contains("Invalid glob pattern"));
})
}
}
3 changes: 3 additions & 0 deletions crates/nu-cmd-base/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
mod arg_glob;
pub mod formats;
pub mod hook;
pub mod input_handler;
pub mod util;
pub use arg_glob::arg_glob;
pub use arg_glob::arg_glob_leading_dot;
29 changes: 4 additions & 25 deletions crates/nu-command/src/filesystem/cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::path::PathBuf;
use std::sync::atomic::AtomicBool;
use std::sync::Arc;

use nu_cmd_base::arg_glob;
use nu_engine::env::current_dir;
use nu_engine::CallExt;
use nu_path::{canonicalize_with, expand_path_with};
Expand All @@ -19,13 +20,6 @@ use super::util::try_interaction;
use crate::filesystem::util::FileStructure;
use crate::progress_bar;

const GLOB_PARAMS: nu_glob::MatchOptions = nu_glob::MatchOptions {
case_sensitive: true,
require_literal_separator: false,
require_literal_leading_dot: false,
recursive_match_hidden_dir: true,
};

#[derive(Clone)]
pub struct Cp;

Expand Down Expand Up @@ -81,12 +75,6 @@ impl Command for Cp {
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
let src: Spanned<String> = call.req(engine_state, stack, 0)?;
let src = {
Spanned {
item: nu_utils::strip_ansi_string_unlikely(src.item),
span: src.span,
}
};
let dst: Spanned<String> = call.req(engine_state, stack, 1)?;
let recursive = call.has_flag("recursive");
let verbose = call.has_flag("verbose");
Expand All @@ -95,7 +83,6 @@ impl Command for Cp {
let update_mode = call.has_flag("update");

let current_dir_path = current_dir(engine_state, stack)?;
let source = current_dir_path.join(src.item.as_str());
let destination = current_dir_path.join(dst.item.as_str());

let path_last_char = destination.as_os_str().to_string_lossy().chars().last();
Expand All @@ -110,7 +97,7 @@ impl Command for Cp {
let span = call.head;

// Get an iterator with all the source files.
let sources: Vec<_> = match nu_glob::glob_with(&source.to_string_lossy(), GLOB_PARAMS) {
let sources: Vec<_> = match arg_glob(&src, &current_dir_path) {
Ok(files) => files.collect(),
Err(e) => {
return Err(ShellError::GenericError(
Expand All @@ -124,13 +111,7 @@ impl Command for Cp {
};

if sources.is_empty() {
return Err(ShellError::GenericError(
"No matches found".into(),
"no matches found".into(),
Some(src.span),
None,
Vec::new(),
));
return Err(ShellError::FileNotFound(src.span));
}

if sources.len() > 1 && !destination.is_dir() {
Expand Down Expand Up @@ -189,9 +170,7 @@ impl Command for Cp {
}

let res = if src == dst {
let message = format!(
"src {source:?} and dst {destination:?} are identical(not copied)"
);
let message = format!("src and dst identical: {:?} (not copied)", src);

return Err(ShellError::GenericError(
"Copy aborted".into(),
Expand Down
26 changes: 4 additions & 22 deletions crates/nu-command/src/filesystem/mv.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::path::{Path, PathBuf};

use super::util::try_interaction;
use nu_cmd_base::arg_glob;
use nu_engine::env::current_dir;
use nu_engine::CallExt;
use nu_protocol::ast::Call;
Expand All @@ -10,13 +11,6 @@ use nu_protocol::{
Spanned, SyntaxShape, Type, Value,
};

const GLOB_PARAMS: nu_glob::MatchOptions = nu_glob::MatchOptions {
case_sensitive: true,
require_literal_separator: false,
require_literal_leading_dot: false,
recursive_match_hidden_dir: true,
};

#[derive(Clone)]
pub struct Mv;

Expand Down Expand Up @@ -70,12 +64,6 @@ impl Command for Mv {
) -> Result<PipelineData, ShellError> {
// TODO: handle invalid directory or insufficient permissions when moving
let spanned_source: Spanned<String> = call.req(engine_state, stack, 0)?;
let spanned_source = {
Spanned {
item: nu_utils::strip_ansi_string_unlikely(spanned_source.item),
span: spanned_source.span,
}
};
let spanned_destination: Spanned<String> = call.req(engine_state, stack, 1)?;
let verbose = call.has_flag("verbose");
let interactive = call.has_flag("interactive");
Expand All @@ -88,17 +76,11 @@ impl Command for Mv {
let source = path.join(spanned_source.item.as_str());
let destination = path.join(spanned_destination.item.as_str());

let mut sources = nu_glob::glob_with(&source.to_string_lossy(), GLOB_PARAMS)
.map_or_else(|_| Vec::new(), Iterator::collect);
let mut sources =
arg_glob(&spanned_source, &path).map_or_else(|_| Vec::new(), Iterator::collect);

if sources.is_empty() {
return Err(ShellError::GenericError(
"File(s) not found".into(),
"could not find any files matching this glob pattern".into(),
Some(spanned_source.span),
None,
Vec::new(),
));
return Err(ShellError::FileNotFound(spanned_source.span));
}

// We have two possibilities.
Expand Down