diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs index e8eb175c37..efc42eda03 100644 --- a/crates/nu-command/src/filesystem/save.rs +++ b/crates/nu-command/src/filesystem/save.rs @@ -91,7 +91,8 @@ impl Command for Save { PipelineData::ByteStream(stream, metadata) => { check_saving_to_source_file(metadata.as_ref(), &path, stderr_path.as_ref())?; - let (file, stderr_file) = get_files(&path, stderr_path.as_ref(), append, force)?; + let (file, stderr_file) = + get_files(engine_state, &path, stderr_path.as_ref(), append, force)?; let size = stream.known_size(); let signals = engine_state.signals(); @@ -201,7 +202,8 @@ impl Command for Save { stderr_path.as_ref(), )?; - let (mut file, _) = get_files(&path, stderr_path.as_ref(), append, force)?; + let (mut file, _) = + get_files(engine_state, &path, stderr_path.as_ref(), append, force)?; for val in ls { file.write_all(&value_to_bytes(val)?) .map_err(&from_io_error)?; @@ -226,7 +228,8 @@ impl Command for Save { input_to_bytes(input, Path::new(&path.item), raw, engine_state, stack, span)?; // Only open file after successful conversion - let (mut file, _) = get_files(&path, stderr_path.as_ref(), append, force)?; + let (mut file, _) = + get_files(engine_state, &path, stderr_path.as_ref(), append, force)?; file.write_all(&bytes).map_err(&from_io_error)?; file.flush().map_err(&from_io_error)?; @@ -422,13 +425,14 @@ fn prepare_path( } } -fn open_file(path: &Path, span: Span, append: bool) -> Result { - let file: Result = match (append, path.exists()) - { - (true, true) => std::fs::OpenOptions::new() - .append(true) - .open(path) - .map_err(|err| err.into()), +fn open_file( + engine_state: &EngineState, + path: &Path, + span: Span, + append: bool, +) -> Result { + let file: std::io::Result = match (append, path.exists()) { + (true, true) => std::fs::OpenOptions::new().append(true).open(path), _ => { // This is a temporary solution until `std::fs::File::create` is fixed on Windows (rust-lang/rust#134893) // A TOCTOU problem exists here, which may cause wrong error message to be shown @@ -438,22 +442,51 @@ fn open_file(path: &Path, span: Span, append: bool) -> Result deprecated, reason = "we don't get a IsADirectory error, so we need to provide it" )] - Err(nu_protocol::shell_error::io::ErrorKind::from_std( - std::io::ErrorKind::IsADirectory, - )) + Err(std::io::ErrorKind::IsADirectory.into()) } else { - std::fs::File::create(path).map_err(|err| err.into()) + std::fs::File::create(path) } #[cfg(not(target_os = "windows"))] - std::fs::File::create(path).map_err(|err| err.into()) + std::fs::File::create(path) } }; - file.map_err(|err_kind| ShellError::Io(IoError::new(err_kind, span, PathBuf::from(path)))) + match file { + Ok(file) => Ok(file), + Err(err) => { + // In caase of NotFound, search for the missing parent directory. + // This also presents a TOCTOU (or TOUTOC, technically?) + if err.kind() == std::io::ErrorKind::NotFound { + if let Some(missing_component) = + path.ancestors().skip(1).filter(|dir| !dir.exists()).last() + { + // By looking at the postfix to remove, rather than the prefix + // to keep, we are able to handle relative paths too. + let components_to_remove = path + .strip_prefix(missing_component) + .expect("Stripping ancestor from a path should never fail") + .as_os_str() + .as_encoded_bytes(); + + return Err(ShellError::Io(IoError::new( + ErrorKind::DirectoryNotFound, + engine_state + .span_match_postfix(span, components_to_remove) + .map(|(pre, _post)| pre) + .unwrap_or(span), + PathBuf::from(missing_component), + ))); + } + } + + Err(ShellError::Io(IoError::new(err, span, PathBuf::from(path)))) + } + } } /// Get output file and optional stderr file fn get_files( + engine_state: &EngineState, path: &Spanned, stderr_path: Option<&Spanned>, append: bool, @@ -467,7 +500,7 @@ fn get_files( .transpose()?; // Only if both files can be used open and possibly truncate them - let file = open_file(path, path_span, append)?; + let file = open_file(engine_state, path, path_span, append)?; let stderr_file = stderr_path_and_span .map(|(stderr_path, stderr_path_span)| { @@ -480,7 +513,7 @@ fn get_files( inner: vec![], }) } else { - open_file(stderr_path, stderr_path_span, append) + open_file(engine_state, stderr_path, stderr_path_span, append) } }) .transpose()?; diff --git a/crates/nu-command/tests/commands/save.rs b/crates/nu-command/tests/commands/save.rs index 6253b64d74..2af4f6425f 100644 --- a/crates/nu-command/tests/commands/save.rs +++ b/crates/nu-command/tests/commands/save.rs @@ -536,6 +536,39 @@ fn parent_redirection_doesnt_affect_save() { }) } +#[test] +fn save_missing_parent_dir() { + Playground::setup("save_test_24", |dirs, sandbox| { + sandbox.with_files(&[]); + + let actual = nu!( + cwd: dirs.root(), + r#"'hello' | save save_test_24/foobar/hello.txt"#, + ); + + assert!(actual.err.contains("nu::shell::io::directory_not_found")); + assert!(actual.err.contains("foobar' does not exist")); + }) +} + +#[test] +fn save_missing_ancestor_dir() { + Playground::setup("save_test_24", |dirs, sandbox| { + sandbox.with_files(&[]); + + std::fs::create_dir(dirs.test().join("foo")) + .expect("should have been able to create subdir for test"); + + let actual = nu!( + cwd: dirs.root(), + r#"'hello' | save save_test_24/foo/bar/baz/hello.txt"#, + ); + + assert!(actual.err.contains("nu::shell::io::directory_not_found")); + assert!(actual.err.contains("bar' does not exist")); + }) +} + #[test] fn force_save_to_dir() { let actual = nu!(cwd: "crates/nu-command/tests/commands", r#" diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 9d00ddd62c..5cd204e97d 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -768,6 +768,30 @@ impl EngineState { &[0u8; 0] } + /// If the span's content starts with the given prefix, return two subspans + /// corresponding to this prefix, and the rest of the content. + pub fn span_match_prefix(&self, span: Span, prefix: &[u8]) -> Option<(Span, Span)> { + let contents = self.get_span_contents(span); + + if contents.starts_with(prefix) { + span.split_at(prefix.len()) + } else { + None + } + } + + /// If the span's content ends with the given postfix, return two subspans + /// corresponding to the rest of the content, and this postfix. + pub fn span_match_postfix(&self, span: Span, prefix: &[u8]) -> Option<(Span, Span)> { + let contents = self.get_span_contents(span); + + if contents.ends_with(prefix) { + span.split_at(span.len() - prefix.len()) + } else { + None + } + } + /// Get the global config from the engine state. /// /// Use [`Stack::get_config()`] instead whenever the `Stack` is available, as it takes into diff --git a/crates/nu-protocol/src/span.rs b/crates/nu-protocol/src/span.rs index 7eeaffe04b..6343d7dde0 100644 --- a/crates/nu-protocol/src/span.rs +++ b/crates/nu-protocol/src/span.rs @@ -132,6 +132,45 @@ impl Span { Self::new(self.start - offset, self.end - offset) } + /// Return length of the slice. + pub fn len(&self) -> usize { + self.end - self.start + } + + /// Indicate if slice has length 0. + pub fn is_empty(&self) -> bool { + self.start == self.end + } + + /// Return another span fully inside the [`Span`]. + /// + /// `start` and `end` are relative to `self.start`, and must lie within the `Span`. + /// In other words, both `start` and `end` must be `<= self.len()`. + pub fn subspan(&self, offset_start: usize, offset_end: usize) -> Option { + let len = self.len(); + + if offset_start > len || offset_end > len || offset_start > offset_end { + None + } else { + Some(Self::new( + self.start + offset_start, + self.start + offset_end, + )) + } + } + + /// Return two spans that split the ['Span'] at the given position. + pub fn split_at(&self, offset: usize) -> Option<(Self, Self)> { + if offset < self.len() { + Some(( + Self::new(self.start, self.start + offset), + Self::new(self.start + offset, self.end), + )) + } else { + None + } + } + pub fn contains(&self, pos: usize) -> bool { self.start <= pos && pos < self.end }