Fix an infinite loop if the input stream and output stream are the same (#11384)

# 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
This commit is contained in:
nibon7 2023-12-24 23:29:23 +08:00 committed by GitHub
parent 543a25599c
commit aeffa188f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 101 additions and 11 deletions

View File

@ -86,6 +86,12 @@ impl Command for Metadata {
PipelineMetadata { PipelineMetadata {
data_source: DataSource::HtmlThemes, data_source: DataSource::HtmlThemes,
} => record.push("source", Value::string("into html --list", head)), } => 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 { PipelineMetadata {
data_source: DataSource::HtmlThemes, data_source: DataSource::HtmlThemes,
} => record.push("source", Value::string("into html --list", head)), } => 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),
),
} }
} }

View File

@ -1,10 +1,11 @@
use nu_engine::{current_dir, eval_block, CallExt}; use nu_engine::{current_dir, eval_block, CallExt};
use nu_path::expand_to_real_path;
use nu_protocol::ast::Call; use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::util::BufferedReader; use nu_protocol::util::BufferedReader;
use nu_protocol::{ use nu_protocol::{
Category, Example, IntoInterruptiblePipelineData, PipelineData, RawStream, ShellError, Category, DataSource, Example, IntoInterruptiblePipelineData, PipelineData, PipelineMetadata,
Signature, Spanned, SyntaxShape, Type, Value, RawStream, ShellError, Signature, Spanned, SyntaxShape, Type, Value,
}; };
use std::io::BufReader; use std::io::BufReader;
@ -157,6 +158,7 @@ impl Command for Open {
}; };
let buf_reader = BufReader::new(file); let buf_reader = BufReader::new(file);
let real_path = expand_to_real_path(path);
let file_contents = PipelineData::ExternalStream { let file_contents = PipelineData::ExternalStream {
stdout: Some(RawStream::new( stdout: Some(RawStream::new(
@ -168,7 +170,9 @@ impl Command for Open {
stderr: None, stderr: None,
exit_code: None, exit_code: None,
span: call_span, span: call_span,
metadata: None, metadata: Some(PipelineMetadata {
data_source: DataSource::FilePath(real_path),
}),
trim_end_newline: false, trim_end_newline: false,
}; };
let exts_opt: Option<Vec<String>> = if raw { let exts_opt: Option<Vec<String>> = if raw {

View File

@ -4,8 +4,8 @@ use nu_path::expand_path_with;
use nu_protocol::ast::{Call, Expr, Expression}; use nu_protocol::ast::{Call, Expr, Expression};
use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{ use nu_protocol::{
Category, Example, PipelineData, RawStream, ShellError, Signature, Span, Spanned, SyntaxShape, Category, DataSource, Example, PipelineData, PipelineMetadata, RawStream, ShellError,
Type, Value, Signature, Span, Spanned, SyntaxShape, Type, Value,
}; };
use std::fs::File; use std::fs::File;
use std::io::Write; use std::io::Write;
@ -149,9 +149,42 @@ impl Command for Save {
res res
} }
} }
PipelineData::ListStream(ls, _) PipelineData::ListStream(ls, pipeline_metadata)
if raw || prepare_path(&path, append, force)?.0.extension().is_none() => 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( let (mut file, _) = get_files(
&path, &path,
stderr_path.as_ref(), stderr_path.as_ref(),
@ -340,10 +373,7 @@ fn prepare_path(
fn open_file(path: &Path, span: Span, append: bool) -> Result<File, ShellError> { fn open_file(path: &Path, span: Span, append: bool) -> Result<File, ShellError> {
let file = match (append, path.exists()) { let file = match (append, path.exists()) {
(true, true) => std::fs::OpenOptions::new() (true, true) => std::fs::OpenOptions::new().append(true).open(path),
.write(true)
.append(true)
.open(path),
_ => std::fs::File::create(path), _ => std::fs::File::create(path),
}; };

View File

@ -1,6 +1,6 @@
use nu_test_support::fs::{file_contents, Stub}; use nu_test_support::fs::{file_contents, Stub};
use nu_test_support::nu;
use nu_test_support::playground::Playground; use nu_test_support::playground::Playground;
use nu_test_support::{nu, pipeline};
use std::io::Write; use std::io::Write;
#[test] #[test]
@ -325,3 +325,45 @@ fn save_file_correct_relative_path() {
assert_eq!(actual, "foo!"); 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"));
})
}

View File

@ -4,6 +4,7 @@ use crate::{
format_error, Config, ListStream, RawStream, ShellError, Span, Value, format_error, Config, ListStream, RawStream, ShellError, Span, Value,
}; };
use nu_utils::{stderr_write_all_and_flush, stdout_write_all_and_flush}; 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::sync::{atomic::AtomicBool, Arc};
use std::thread; use std::thread;
@ -62,6 +63,7 @@ pub struct PipelineMetadata {
pub enum DataSource { pub enum DataSource {
Ls, Ls,
HtmlThemes, HtmlThemes,
FilePath(PathBuf),
} }
impl PipelineData { impl PipelineData {