From a64e0956cdf3fdbe3141088116250b83d55f8295 Mon Sep 17 00:00:00 2001
From: Andrew Barnes <barnes333@gmail.com>
Date: Sun, 27 Mar 2022 13:35:59 +1100
Subject: [PATCH] Support binary data to stdin of run-external (#4984)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Add test for passing binary data through externals

This change adds an ignored test to confirm that binary data is passed
correctly between externals to be enabled in a later commit along with
the fix.

To assist in platform agnostic testing of binary data a couple of
additional testbins were added to allow testing on `Value::Binary` inside
`ExternalStream`.

* Support binary data to stdin of run-external

Prior to this change, any pipeline producing binary data (not detected
as string) then feed into an external would be ignored due to
run-external only supporting `Value::String` on stdin.

This change adds binary stdin support for externals allowing something
like this for example:

  〉^cat /dev/urandom | ^head -c 1MiB | ^pv -b | ignore
  1.00MiB

This would previously output `0.00 B [0.00 B/s]` due to the data not
being pushed to stdin at each stage.
---
 crates/nu-command/src/system/run_external.rs | 13 +++++++------
 src/main.rs                                  |  2 ++
 src/test_bins.rs                             | 19 +++++++++++++++++++
 tests/shell/pipeline/commands/external.rs    | 10 ++++++++++
 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs
index 336f345c5..d0ce1ab9d 100644
--- a/crates/nu-command/src/system/run_external.rs
+++ b/crates/nu-command/src/system/run_external.rs
@@ -169,12 +169,13 @@ impl ExternalCommand {
 
                             if let Ok(input) = input {
                                 for value in input.into_iter() {
-                                    if let Value::String { val, span: _ } = value {
-                                        if stdin_write.write(val.as_bytes()).is_err() {
-                                            return Ok(());
-                                        }
-                                    } else {
-                                        return Err(());
+                                    let buf = match value {
+                                        Value::String { val, .. } => val.into_bytes(),
+                                        Value::Binary { val, .. } => val,
+                                        _ => return Err(()),
+                                    };
+                                    if stdin_write.write(&buf).is_err() {
+                                        return Ok(());
                                     }
                                 }
                             }
diff --git a/src/main.rs b/src/main.rs
index d61c72320..63b56412c 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -159,6 +159,8 @@ fn main() -> Result<()> {
                     "echo_env" => test_bins::echo_env(),
                     "cococo" => test_bins::cococo(),
                     "meow" => test_bins::meow(),
+                    "meowb" => test_bins::meowb(),
+                    "relay" => test_bins::relay(),
                     "iecho" => test_bins::iecho(),
                     "fail" => test_bins::fail(),
                     "nonu" => test_bins::nonu(),
diff --git a/src/test_bins.rs b/src/test_bins.rs
index a6b8f9b83..bdd0ac4d4 100644
--- a/src/test_bins.rs
+++ b/src/test_bins.rs
@@ -34,6 +34,25 @@ pub fn meow() {
     }
 }
 
+// A binary version of meow
+pub fn meowb() {
+    let args: Vec<String> = args();
+
+    let stdout = io::stdout();
+    let mut handle = stdout.lock();
+
+    for arg in args.iter().skip(1) {
+        let buf = std::fs::read(arg).expect("Expected a filepath");
+        handle.write_all(&buf).expect("failed to write to stdout");
+    }
+}
+
+// Relays anything received on stdin to stdout
+pub fn relay() {
+    io::copy(&mut io::stdin().lock(), &mut io::stdout().lock())
+        .expect("failed to copy stdin to stdout");
+}
+
 pub fn nonu() {
     args().iter().skip(1).for_each(|arg| print!("{}", arg));
 }
diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs
index 6bb1baf67..2951af458 100644
--- a/tests/shell/pipeline/commands/external.rs
+++ b/tests/shell/pipeline/commands/external.rs
@@ -97,6 +97,16 @@ fn redirects_custom_command_external() {
     assert_eq!(actual.out, "8");
 }
 
+#[test]
+fn passes_binary_data_between_externals() {
+    let actual = nu!(cwd: "tests/fixtures/formats", r#"nu --testbin meowb sample.db | nu --testbin relay | hash sha256"#);
+
+    assert_eq!(
+        actual.out,
+        "2f5050e7eea415c1f3d80b5d93355efd15043ec9157a2bb167a9e73f2ae651f2"
+    )
+}
+
 mod it_evaluation {
     use super::nu;
     use nu_test_support::fs::Stub::{EmptyFile, FileWithContent, FileWithContentToBeTrimmed};