diff --git a/Cargo.lock b/Cargo.lock index 074d8c9c..92230b00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -99,6 +99,7 @@ dependencies = [ "assert_cmd", "atty", "clap", + "clircle", "console", "content_inspector", "dirs", @@ -117,6 +118,7 @@ dependencies = [ "syntect", "tempdir", "unicode-width", + "wait-timeout", "wild", ] @@ -251,6 +253,18 @@ dependencies = [ "vec_map", ] +[[package]] +name = "clircle" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e27a01e782190a8314e65cc94274d9bbcd52e05a9d15b437fe2b31259b854b0d" +dependencies = [ + "cfg-if 1.0.0", + "nix", + "serde", + "winapi", +] + [[package]] name = "console" version = "0.14.0" @@ -691,6 +705,18 @@ dependencies = [ "autocfg", ] +[[package]] +name = "nix" +version = "0.19.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2ccba0cfe4fdf15982d1674c69b1fd80bad427d293849982668dfe454bd61f2" +dependencies = [ + "bitflags", + "cc", + "cfg-if 1.0.0", + "libc", +] + [[package]] name = "normalize-line-endings" version = "0.3.0" diff --git a/Cargo.toml b/Cargo.toml index 70db2509..ab9fdc2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ serde = { version = "1.0", features = ["derive"] } serde_yaml = "0.8" semver = "0.11" path_abs = { version = "0.5", default-features = false } +clircle = "0.2.0" [dependencies.git2] version = "0.13" @@ -76,6 +77,7 @@ tempdir = "0.3" assert_cmd = "1.0.2" serial_test = "0.5.1" predicates = "1.0.6" +wait-timeout = "0.2.0" [build-dependencies] clap = { version = "2.33", optional = true } diff --git a/src/controller.rs b/src/controller.rs index 8dd73986..a1e3f119 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -1,3 +1,4 @@ +use std::convert::TryFrom; use std::io::{self, Write}; use crate::assets::HighlightingAssets; @@ -66,6 +67,14 @@ impl<'b> Controller<'b> { } let attached_to_pager = output_type.is_pager(); + let rw_cycle_detected = !attached_to_pager && { + let identifiers: Vec<_> = inputs + .iter() + .flat_map(clircle::Identifier::try_from) + .collect(); + clircle::stdout_among_inputs(&identifiers) + }; + let writer = output_type.handle()?; let mut no_errors: bool = true; @@ -78,6 +87,11 @@ impl<'b> Controller<'b> { } }; + if rw_cycle_detected { + print_error(&"The output file is also an input!".into(), writer); + return Ok(false); + } + for (index, input) in inputs.into_iter().enumerate() { match input.open(io::stdin().lock()) { Err(error) => { diff --git a/src/input.rs b/src/input.rs index 5d8493c0..5b6a4e67 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,6 +1,8 @@ +use std::convert::TryFrom; use std::ffi::{OsStr, OsString}; use std::fs::File; use std::io::{self, BufRead, BufReader, Read}; +use std::path::Path; use content_inspector::{self, ContentType}; @@ -191,6 +193,18 @@ impl<'a> Input<'a> { } } +impl TryFrom<&'_ Input<'_>> for clircle::Identifier { + type Error = (); + + fn try_from(input: &Input) -> std::result::Result { + match input.kind { + InputKind::OrdinaryFile(ref path) => Self::try_from(Path::new(path)).map_err(|_| ()), + InputKind::StdIn => Self::try_from(clircle::Stdio::Stdin).map_err(|_| ()), + InputKind::CustomReader(_) => Err(()), + } + } +} + pub(crate) struct InputReader<'a> { inner: Box, pub(crate) first_line: Vec, diff --git a/tests/examples/cycle.txt b/tests/examples/cycle.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index a85f0fd4..c79f4d90 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -1,13 +1,19 @@ -use assert_cmd::Command; +use assert_cmd::assert::OutputAssertExt; +use assert_cmd::cargo::CommandCargoExt; use predicates::{prelude::predicate, str::PredicateStrExt}; use serial_test::serial; use std::env; +use std::fs::File; use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; use std::str::from_utf8; +use std::time::Duration; const EXAMPLES_DIR: &str = "tests/examples"; +const SAFE_CHILD_PROCESS_CREATION_TIME: Duration = Duration::from_millis(100); +const CHILD_WAIT_TIMEOUT: Duration = Duration::from_secs(15); -fn bat_with_config() -> Command { +fn bat_raw_command() -> Command { let mut cmd = Command::cargo_bin("bat").unwrap(); cmd.current_dir("tests/examples"); cmd.env_remove("PAGER"); @@ -19,7 +25,11 @@ fn bat_with_config() -> Command { cmd } -fn bat() -> Command { +fn bat_with_config() -> assert_cmd::Command { + assert_cmd::Command::from_std(bat_raw_command()) +} + +fn bat() -> assert_cmd::Command { let mut cmd = bat_with_config(); cmd.arg("--no-config"); cmd @@ -258,6 +268,80 @@ fn line_range_multiple() { .stdout("line 1\nline 2\nline 4\n"); } +#[test] +fn basic_io_cycle() { + let file_out = Stdio::from(File::open("tests/examples/cycle.txt").unwrap()); + bat_raw_command() + .arg("test.txt") + .arg("cycle.txt") + .stdout(file_out) + .assert() + .failure(); +} + +#[test] +fn stdin_to_stdout_cycle() { + let file_out = Stdio::from(File::open("tests/examples/cycle.txt").unwrap()); + let file_in = Stdio::from(File::open("tests/examples/cycle.txt").unwrap()); + bat_raw_command() + .stdin(file_in) + .arg("test.txt") + .arg("-") + .stdout(file_out) + .assert() + .failure(); +} + +#[cfg(unix)] +#[test] +fn no_args_doesnt_break() { + use std::io::Write; + use std::os::unix::io::FromRawFd; + use std::thread; + + use clircle::nix::pty::{openpty, OpenptyResult}; + use wait_timeout::ChildExt; + + // To simulate bat getting started from the shell, a process is created with stdin and stdout + // as the slave end of a pseudo terminal. Although both point to the same "file", bat should + // not exit, because in this case it is safe to read and write to the same fd, which is why + // this test exists. + let OpenptyResult { master, slave } = openpty(None, None).expect("Couldn't open pty."); + let mut master = unsafe { File::from_raw_fd(master) }; + let stdin = unsafe { Stdio::from_raw_fd(slave) }; + let stdout = unsafe { Stdio::from_raw_fd(slave) }; + + let mut child = bat_raw_command() + .stdin(stdin) + .stdout(stdout) + .spawn() + .expect("Failed to start."); + + // Some time for the child process to start and to make sure, that we can poll the exit status. + // Although this waiting period is not necessary, it is best to keep it in and be absolutely + // sure, that the try_wait does not error later. + thread::sleep(SAFE_CHILD_PROCESS_CREATION_TIME); + + // The child process should be running and waiting for input, + // therefore no exit status should be available. + let exit_status = child + .try_wait() + .expect("Error polling exit status, this should never happen."); + assert!(exit_status.is_none()); + + // Write Ctrl-D (end of transmission) to the pty. + master + .write_all(&[0x04]) + .expect("Couldn't write EOT character to master end."); + + let exit_status = child + .wait_timeout(CHILD_WAIT_TIMEOUT) + .expect("Error polling exit status, this should never happen.") + .expect("Exit status not set, but the child should have exited already."); + + assert!(exit_status.success()); +} + #[test] fn tabs_numbers() { bat()