From aeffa188f000a2e6a7d5adebf498bdb322425889 Mon Sep 17 00:00:00 2001 From: nibon7 Date: Sun, 24 Dec 2023 23:29:23 +0800 Subject: [PATCH] Fix an infinite loop if the input stream and output stream are the same (#11384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Fixes #11382 # User-Facing Changes * before ```console nushell/test (109f629) [✘?] ❯ open hello.md hello nushell/test (109f629) [✘?] ❯ ls hello.md | get size ╭───┬─────╮ │ 0 │ 6 B │ ╰───┴─────╯ nushell/test (109f629) [✘?] ❯ open --raw hello.md | prepend "world" | save --raw --force hello.md ^C nushell/test (109f629) [✘?] ❯ ls hello.md | get size ╭───┬─────────╮ │ 0 │ 2.8 GiB │ ╰───┴─────────╯ ``` * after ```console nushell/test on  fix_save [✘!?⇡] ❯ open hello.md | prepend "hello" | save --force hello.md nushell/test on  fix_save [✘!?⇡] ❯ open --raw hello.md | prepend "hello" | save --raw --force ../test/hello.md Error: × pipeline input and output are same file ╭─[entry #4:1:1] 1 │ open --raw hello.md | prepend "hello" | save --raw --force ../test/hello.md · ────────┬─────── · ╰── can't save output to '/data/source/nushell/test/hello.md' while it's being reading ╰──── help: you should change output path nushell/test on  fix_save [✘!?⇡] ❯ open hello | prepend "hello" | save --force hello Error: × pipeline input and output are same file ╭─[entry #5:1:1] 1 │ open hello | prepend "hello" | save --force hello · ──┬── · ╰── can't save output to '/data/source/nushell/test/hello' while it's being reading ╰──── help: you should change output path ``` # Tests + Formatting Make sure you've run and fixed any issues with these commands: - [x] add `commands::save::save_same_file_with_extension` - [x] add `commands::save::save_same_file_without_extension` - [x] `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - [x] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - [x] `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)) - [x] `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library # After Submitting --- crates/nu-command/src/debug/metadata.rs | 12 +++++++ crates/nu-command/src/filesystem/open.rs | 10 ++++-- crates/nu-command/src/filesystem/save.rs | 44 ++++++++++++++++++++---- crates/nu-command/tests/commands/save.rs | 44 +++++++++++++++++++++++- crates/nu-protocol/src/pipeline_data.rs | 2 ++ 5 files changed, 101 insertions(+), 11 deletions(-) diff --git a/crates/nu-command/src/debug/metadata.rs b/crates/nu-command/src/debug/metadata.rs index 256b5d80a9..61991cb1d3 100644 --- a/crates/nu-command/src/debug/metadata.rs +++ b/crates/nu-command/src/debug/metadata.rs @@ -86,6 +86,12 @@ impl Command for Metadata { PipelineMetadata { data_source: DataSource::HtmlThemes, } => record.push("source", Value::string("into html --list", head)), + PipelineMetadata { + data_source: DataSource::FilePath(path), + } => record.push( + "source", + Value::string(path.to_string_lossy().to_string(), head), + ), } } @@ -133,6 +139,12 @@ fn build_metadata_record(arg: &Value, metadata: Option<&PipelineMetadata>, head: PipelineMetadata { data_source: DataSource::HtmlThemes, } => record.push("source", Value::string("into html --list", head)), + PipelineMetadata { + data_source: DataSource::FilePath(path), + } => record.push( + "source", + Value::string(path.to_string_lossy().to_string(), head), + ), } } diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs index 3597ae2a2a..114bd3623c 100644 --- a/crates/nu-command/src/filesystem/open.rs +++ b/crates/nu-command/src/filesystem/open.rs @@ -1,10 +1,11 @@ use nu_engine::{current_dir, eval_block, CallExt}; +use nu_path::expand_to_real_path; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::util::BufferedReader; use nu_protocol::{ - Category, Example, IntoInterruptiblePipelineData, PipelineData, RawStream, ShellError, - Signature, Spanned, SyntaxShape, Type, Value, + Category, DataSource, Example, IntoInterruptiblePipelineData, PipelineData, PipelineMetadata, + RawStream, ShellError, Signature, Spanned, SyntaxShape, Type, Value, }; use std::io::BufReader; @@ -157,6 +158,7 @@ impl Command for Open { }; let buf_reader = BufReader::new(file); + let real_path = expand_to_real_path(path); let file_contents = PipelineData::ExternalStream { stdout: Some(RawStream::new( @@ -168,7 +170,9 @@ impl Command for Open { stderr: None, exit_code: None, span: call_span, - metadata: None, + metadata: Some(PipelineMetadata { + data_source: DataSource::FilePath(real_path), + }), trim_end_newline: false, }; let exts_opt: Option> = if raw { diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs index 399b97b130..be87b9b68d 100644 --- a/crates/nu-command/src/filesystem/save.rs +++ b/crates/nu-command/src/filesystem/save.rs @@ -4,8 +4,8 @@ use nu_path::expand_path_with; use nu_protocol::ast::{Call, Expr, Expression}; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Example, PipelineData, RawStream, ShellError, Signature, Span, Spanned, SyntaxShape, - Type, Value, + Category, DataSource, Example, PipelineData, PipelineMetadata, RawStream, ShellError, + Signature, Span, Spanned, SyntaxShape, Type, Value, }; use std::fs::File; use std::io::Write; @@ -149,9 +149,42 @@ impl Command for Save { res } } - PipelineData::ListStream(ls, _) + PipelineData::ListStream(ls, pipeline_metadata) if raw || prepare_path(&path, append, force)?.0.extension().is_none() => { + if let Some(PipelineMetadata { + data_source: DataSource::FilePath(input_path), + }) = pipeline_metadata + { + if path.item == input_path { + return Err(ShellError::GenericError { + error: "pipeline input and output are same file".into(), + msg: format!( + "can't save output to '{}' while it's being reading", + path.item.display() + ), + span: Some(path.span), + help: Some("you should change output path".into()), + inner: vec![], + }); + } + + if let Some(ref err_path) = stderr_path { + if err_path.item == input_path { + return Err(ShellError::GenericError { + error: "pipeline input and stderr are same file".into(), + msg: format!( + "can't save stderr to '{}' while it's being reading", + err_path.item.display() + ), + span: Some(err_path.span), + help: Some("you should change stderr path".into()), + inner: vec![], + }); + } + } + } + let (mut file, _) = get_files( &path, stderr_path.as_ref(), @@ -340,10 +373,7 @@ fn prepare_path( fn open_file(path: &Path, span: Span, append: bool) -> Result { let file = match (append, path.exists()) { - (true, true) => std::fs::OpenOptions::new() - .write(true) - .append(true) - .open(path), + (true, true) => std::fs::OpenOptions::new().append(true).open(path), _ => std::fs::File::create(path), }; diff --git a/crates/nu-command/tests/commands/save.rs b/crates/nu-command/tests/commands/save.rs index 24a5e95aa8..db30faecd9 100644 --- a/crates/nu-command/tests/commands/save.rs +++ b/crates/nu-command/tests/commands/save.rs @@ -1,6 +1,6 @@ use nu_test_support::fs::{file_contents, Stub}; -use nu_test_support::nu; use nu_test_support::playground::Playground; +use nu_test_support::{nu, pipeline}; use std::io::Write; #[test] @@ -325,3 +325,45 @@ fn save_file_correct_relative_path() { assert_eq!(actual, "foo!"); }) } + +#[test] +fn save_same_file_with_extension() { + Playground::setup("save_test_16", |dirs, _sandbox| { + let actual = nu!( + cwd: dirs.test(), pipeline( + " + echo 'world' + | save --raw hello.md; + open --raw hello.md + | prepend 'hello' + | save --raw --force hello.md + " + ) + ); + + assert!(actual + .err + .contains("pipeline input and output are same file")); + }) +} + +#[test] +fn save_same_file_without_extension() { + Playground::setup("save_test_17", |dirs, _sandbox| { + let actual = nu!( + cwd: dirs.test(), pipeline( + " + echo 'world' + | save hello; + open hello + | prepend 'hello' + | save --force hello + " + ) + ); + + assert!(actual + .err + .contains("pipeline input and output are same file")); + }) +} diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 1b58057e74..8dda06dd75 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -4,6 +4,7 @@ use crate::{ format_error, Config, ListStream, RawStream, ShellError, Span, Value, }; use nu_utils::{stderr_write_all_and_flush, stdout_write_all_and_flush}; +use std::path::PathBuf; use std::sync::{atomic::AtomicBool, Arc}; use std::thread; @@ -62,6 +63,7 @@ pub struct PipelineMetadata { pub enum DataSource { Ls, HtmlThemes, + FilePath(PathBuf), } impl PipelineData {