Fix an infinite loop if the input stream and output stream are the same#11384
Merged
WindSoilder merged 4 commits intonushell:mainfrom Dec 24, 2023
Merged
Fix an infinite loop if the input stream and output stream are the same#11384WindSoilder merged 4 commits intonushell:mainfrom
WindSoilder merged 4 commits intonushell:mainfrom
Conversation
Contributor
Contributor
Author
|
It is a problem. Maybe we could detect whether input and output refer to the same file and return an error. diff --git a/crates/nu-command/src/debug/metadata.rs b/crates/nu-command/src/debug/metadata.rs
index 256b5d80a..61991cb1d 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 3597ae2a2..114bd3623 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<Vec<String>> = if raw {
diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs
index f61d0f286..11b2be074 100644
--- a/crates/nu-command/src/filesystem/save.rs
+++ b/crates/nu-command/src/filesystem/save.rs
@@ -1,11 +1,11 @@
use nu_engine::current_dir;
use nu_engine::CallExt;
-use nu_path::expand_path_with;
+use nu_path::{expand_path_with, expand_to_real_path};
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,12 +149,35 @@ impl Command for Save {
res
}
}
- PipelineData::ListStream(ls, _)
+ PipelineData::ListStream(ls, pipeline_metadata)
if raw || prepare_path(&path, append, force)?.0.extension().is_none() =>
{
- // We need to consume the input stream here to prevent an infinite
- // loop if the input stream and output stream are the same.
- let values: Vec<Value> = ls.collect();
+ if let Some(PipelineMetadata {
+ data_source: DataSource::FilePath(input_path),
+ }) = pipeline_metadata
+ {
+ if expand_to_real_path(&path.item) == input_path {
+ return Err(ShellError::GenericError {
+ error: "pipeline input and output are same file".into(),
+ msg: "can't save output".into(),
+ span: Some(path.span),
+ help: Some("you should change output path".into()),
+ inner: vec![],
+ });
+ }
+
+ if let Some(ref err_path) = stderr_path {
+ if expand_to_real_path(&err_path.item) == input_path {
+ return Err(ShellError::GenericError {
+ error: "pipeline input and stderr are same file".into(),
+ msg: "can't save stderr".into(),
+ span: Some(err_path.span),
+ help: Some("you should change stderr path".into()),
+ inner: vec![],
+ });
+ }
+ }
+ }
let (mut file, _) = get_files(
&path,
@@ -164,7 +187,7 @@ impl Command for Save {
err_append,
force,
)?;
- for val in values.into_iter() {
+ for val in ls {
file.write_all(&value_to_bytes(val)?)
.map_err(|err| ShellError::IOError {
msg: err.to_string(),
diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs
index 1b58057e7..8dda06dd7 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 {
❯ open --raw a.md | prepend "hello" | save --force --raw ../test/a.md
Error: × pipeline input and output are same file
╭─[entry #2:1:1]
1 │ open --raw a.md | prepend "hello" | save --force --raw ../test/a.md
· ──────┬─────
· ╰── can't save output
╰────
help: you should change output path
nushell/test on fix_save [✘?⇡] |
Contributor
|
I think DataSource::FilePath is interesting. Making this draft for now. |
Contributor
Author
|
@WindSoilder any thoughts? |
Contributor
|
Thanks! I think it's a good idea too :-) |
WindSoilder
approved these changes
Dec 24, 2023
Contributor
WindSoilder
left a comment
There was a problem hiding this comment.
Really thanks for a great improvement!
It gives me one more reason to use open rather than cat :-D
dmatos2012
pushed a commit
to dmatos2012/nushell
that referenced
this pull request
Feb 20, 2024
…me (nushell#11384) # Description Fixes nushell#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 nushell#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 nushell#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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #11382
User-Facing Changes
Tests + Formatting
Make sure you've run and fixed any issues with these commands:
commands::save::save_same_file_with_extensioncommands::save::save_same_file_without_extensioncargo fmt --all -- --checkto check standard code formatting (cargo fmt --allapplies these changes)cargo clippy --workspace -- -D warnings -D clippy::unwrap_usedto check that you're using the standard code stylecargo test --workspaceto check that all tests pass (on Windows make sure to enable developer mode)cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"to run the tests for the standard libraryAfter Submitting