From bd3930d00dcc0d75e0a8482f7bf744eebbf006a5 Mon Sep 17 00:00:00 2001
From: marienz <marien.zwart@gmail.com>
Date: Fri, 13 Jun 2025 09:22:37 +1000
Subject: [PATCH] Better error on spawn failure caused by null bytes (#15911)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Description

When attempting to pass a null byte in a commandline argument, Nu
currently fails with:

```
> ^echo (char -i 0)
Error: nu::shell::io::invalid_input

  × I/O error
  ╰─▶   × Could not spawn foreground child

   ╭────
 1 │ crates/nu-command/src/system/run_external.rs:284:17
   · ─────────────────────────┬─────────────────────────
   ·                          ╰── Invalid input parameter
   ╰────
```

This does not explain which input parameter is invalid, or why. Since Nu
does not typically seem to escape null bytes when printing values
containing them, this can make it a bit tricky to track down the
problem.

After this change, it fails with:

```
> ^echo (char -i 0)
Error: nu::shell::io::invalid_input

  × I/O error
  ╰─▶   × Could not spawn foreground child: nul byte found in provided data

   ╭────
 1 │ crates/nu-command/src/system/run_external.rs:282:17
   · ─────────────────────────┬─────────────────────────
   ·                          ╰── Invalid input parameter
   ╰────

```

which is more useful. This could be improved further but this is niche
enough that is probably not necessary.

This might make some other errors unnecessarily verbose but seems like
the better default. I did check that attempting to execute a
non-executable file still has a reasonable error: the error message for
that failure is not affected by this change.

It is still an "internal" error (referencing the Nu code triggering it,
not the user's input) because the `call.head` span available to this
code is for the command, not its arguments. Using it would result in

```
  × I/O error
  ╰─▶   × Could not spawn foreground child: nul byte found in provided data

   ╭─[entry #1:1:2]
 1 │ ^echo (char -i 0)
   ·  ──┬─
   ·    ╰── Invalid input parameter
   ╰────
```

which is actively misleading because "echo" does not contain the nul
byte.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

# 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` to
check that you're using the standard code style
- `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))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the
tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

Haven't tried to write a test yet: it's tricky because the better error
message comes from the Rust stdlib (so a straightforward integration
test checking for the specific message would be brittle)...

# 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-command/src/system/run_external.rs | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs
index 03172d73a2..09d4d6c9f2 100644
--- a/crates/nu-command/src/system/run_external.rs
+++ b/crates/nu-command/src/system/run_external.rs
@@ -275,11 +275,8 @@ impl Command for External {
         );
 
         let mut child = child.map_err(|err| {
-            IoError::new_internal(
-                err,
-                "Could not spawn foreground child",
-                nu_protocol::location!(),
-            )
+            let context = format!("Could not spawn foreground child: {err}");
+            IoError::new_internal(err, context, nu_protocol::location!())
         })?;
 
         if let Some(thread_job) = engine_state.current_thread_job() {