From 253b223e65e8ab8b65218cce8231e8001e86a32b Mon Sep 17 00:00:00 2001
From: Henry Jetmundsen <32406734+henryjetmundsen@users.noreply.github.com>
Date: Fri, 24 Feb 2023 12:39:52 -0800
Subject: [PATCH] pipe binary data to external commands (#8058)

Fixes #7615

# Description

When calling external commands, we create a table from the pipeline data
to handle external commands expecting paginated input. When a binary
value is made into a table, we convert the vector of bytes representing
the binary bytes into a pretty formatted string. This results in the
pretty formatted string being sent to external commands instead of the
actual binary bytes. By checking whether the stdout of the call is being
redirected, we can decide whether to send the raw binary bytes or the
pretty formatted output when creating a table command.

# User-Facing Changes

When passing binary values to external commands, the external command
will receive the actual bytes instead of the pretty printed string. Use
cases that don't involve piping a binary value into an external command
are unchanged.


![new_behavior](https://user-images.githubusercontent.com/32406734/218349172-24cd12f2-d563-4957-bdf1-6aa804b174b2.png)

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

cargo fmt --all -- --check to check standard code formatting (cargo fmt
--all applies these changes)
cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect to check that you're using the standard code
style
    cargo test --workspace to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
---
 crates/nu-cli/src/eval_file.rs                   | 10 ++++------
 crates/nu-command/src/viewers/table.rs           |  8 +++++---
 crates/nu-command/tests/commands/run_external.rs |  9 +++++++++
 src/main.rs                                      |  1 +
 src/test_bins.rs                                 |  9 ++++++++-
 5 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/crates/nu-cli/src/eval_file.rs b/crates/nu-cli/src/eval_file.rs
index 043078a021..433459c6d4 100644
--- a/crates/nu-cli/src/eval_file.rs
+++ b/crates/nu-cli/src/eval_file.rs
@@ -154,12 +154,10 @@ pub(crate) fn print_table_or_error(
         if command.get_block_id().is_some() {
             print_or_exit(pipeline_data, engine_state, config);
         } else {
-            let table = command.run(
-                engine_state,
-                stack,
-                &Call::new(Span::new(0, 0)),
-                pipeline_data,
-            );
+            // The final call on table command, it's ok to set redirect_output to false.
+            let mut call = Call::new(Span::new(0, 0));
+            call.redirect_stdout = false;
+            let table = command.run(engine_state, stack, &call, pipeline_data);
 
             match table {
                 Ok(table) => {
diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs
index 1119617bcb..f8acbee44d 100644
--- a/crates/nu-command/src/viewers/table.rs
+++ b/crates/nu-command/src/viewers/table.rs
@@ -241,12 +241,14 @@ fn handle_table_command(
         PipelineData::ExternalStream { .. } => Ok(input),
         PipelineData::Value(Value::Binary { val, .. }, ..) => Ok(PipelineData::ExternalStream {
             stdout: Some(RawStream::new(
-                Box::new(
+                Box::new(if call.redirect_stdout {
+                    vec![Ok(val)].into_iter()
+                } else {
                     vec![Ok(format!("{}\n", nu_pretty_hex::pretty_hex(&val))
                         .as_bytes()
                         .to_vec())]
-                    .into_iter(),
-                ),
+                    .into_iter()
+                }),
                 ctrlc,
                 call.head,
                 None,
diff --git a/crates/nu-command/tests/commands/run_external.rs b/crates/nu-command/tests/commands/run_external.rs
index 85bd78e6c2..151834c54b 100644
--- a/crates/nu-command/tests/commands/run_external.rs
+++ b/crates/nu-command/tests/commands/run_external.rs
@@ -213,6 +213,15 @@ fn external_command_not_expand_tilde_with_quotes() {
     )
 }
 
+#[test]
+fn external_command_receives_raw_binary_data() {
+    Playground::setup("external command receives raw binary data", |dirs, _| {
+        let actual =
+            nu!(cwd: dirs.test(), pipeline(r#"0x[deadbeef] | nu --testbin input_bytes_length"#));
+        assert_eq!(actual.out, r#"4"#);
+    })
+}
+
 #[cfg(windows)]
 #[test]
 fn failed_command_with_semicolon_will_not_execute_following_cmds_windows() {
diff --git a/src/main.rs b/src/main.rs
index c7f5f33874..e0f0fcef33 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -169,6 +169,7 @@ fn main() -> Result<()> {
             "chop" => test_bins::chop(),
             "repeater" => test_bins::repeater(),
             "nu_repl" => test_bins::nu_repl(),
+            "input_bytes_length" => test_bins::input_bytes_length(),
             _ => std::process::exit(1),
         }
         std::process::exit(0)
diff --git a/src/test_bins.rs b/src/test_bins.rs
index fa5cb0050f..6bbbc29918 100644
--- a/src/test_bins.rs
+++ b/src/test_bins.rs
@@ -1,4 +1,4 @@
-use std::io::{self, BufRead, Write};
+use std::io::{self, BufRead, Read, Write};
 
 use nu_cli::{eval_env_change_hook, eval_hook};
 use nu_command::create_default_context;
@@ -257,6 +257,13 @@ fn did_chop_arguments() -> bool {
     false
 }
 
+pub fn input_bytes_length() {
+    let stdin = io::stdin();
+    let count = stdin.lock().bytes().count();
+
+    println!("{}", count);
+}
+
 fn args() -> Vec<String> {
     // skip (--testbin bin_name args)
     std::env::args().skip(2).collect()