Skip to content

Commit

Permalink
Fix an infinite loop if the input stream and output stream are the sa…
Browse files Browse the repository at this point in the history
…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 #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
  • Loading branch information
nibon7 committed Dec 24, 2023
1 parent 543a255 commit aeffa18
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 11 deletions.
12 changes: 12 additions & 0 deletions crates/nu-command/src/debug/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
),
}
}

Expand Down Expand Up @@ -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),
),
}
}

Expand Down
10 changes: 7 additions & 3 deletions crates/nu-command/src/filesystem/open.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand Down
44 changes: 37 additions & 7 deletions crates/nu-command/src/filesystem/save.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -340,10 +373,7 @@ fn prepare_path(

fn open_file(path: &Path, span: Span, append: bool) -> Result<File, ShellError> {
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),
};

Expand Down
44 changes: 43 additions & 1 deletion crates/nu-command/tests/commands/save.rs
Original file line number Diff line number Diff line change
@@ -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]
Expand Down Expand Up @@ -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"));
})
}
2 changes: 2 additions & 0 deletions crates/nu-protocol/src/pipeline_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -62,6 +63,7 @@ pub struct PipelineMetadata {
pub enum DataSource {
Ls,
HtmlThemes,
FilePath(PathBuf),
}

impl PipelineData {
Expand Down

0 comments on commit aeffa18

Please sign in to comment.